Skip to content

fix(cache): set secure 0755 permissions mode when creating directory in FileVarExportHandler #10555

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gr8man wants to merge 2 commits into
base: develop
Choose a base branch
from
+33 −1
Open

fix(cache): set secure 0755 permissions mode when creating directory in FileVarExportHandler #10555

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
23ab8d3
fix: correct cache directory permissions mode to 0755 in FileVarExpor…
gr8man Sep 12, 2026
3b9452e
fix: handle false return value from glob in FactoriesCacheFileVarExpo…
gr8man Sep 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion system/Cache/FactoriesCache/FileVarExportHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public function save(string $key, mixed $val): void

// Two processes may try to create the directory at the same time.
// is_dir() confirms it exists, so suppressing the warning is safe.
if (! is_dir($this->path) && ! @mkdir($this->path, 0777, true) && ! is_dir($this->path)) {
if (! is_dir($this->path) && ! @mkdir($this->path, 0755, true) && ! is_dir($this->path)) {
log_message('error', 'FactoriesCache: cannot create cache directory: ' . $this->path);

return;
Expand Down
32 changes: 32 additions & 0 deletions tests/system/Cache/FactoriesCacheFileVarExportHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

use CodeIgniter\Cache\FactoriesCache\FileVarExportHandler;
use PHPUnit\Framework\Attributes\Group;
use ReflectionProperty;

/**
* @internal
Expand All @@ -27,4 +28,35 @@ protected function createFactoriesCache(): void
$this->handler = new FileVarExportHandler();
$this->cache = new FactoriesCache($this->handler);
}

public function testSaveCreatesDirectoryWithCorrectPermissions(): void
{
$dir = WRITEPATH . 'cache_test_dir_' . uniqid('', true);

try {
$handler = new FileVarExportHandler();
$ref = new ReflectionProperty(FileVarExportHandler::class, 'path');
$ref->setValue($handler, $dir);

$handler->save('test_key', ['data']);

$this->assertDirectoryExists($dir);

if (! is_windows()) {
$perms = fileperms($dir) & 0777;
$expected = 0755 & ~umask();
$this->assertSame($expected, $perms);
}
} finally {
if (is_dir($dir)) {
$files = glob("{$dir}/*");

if ($files !== false) {
array_map(unlink(...), $files);
}

rmdir($dir);
}
}
}

@michalsn michalsn Sep 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test also passes without the fix. With the usual umask of 0022, mkdir with 0777 and with 0755 both give you a 0755 directory, so the assertion can't tell the two apart. Try umask(0000) before the save (restore the old mask in finally) and then assert 0755. That's the permissive-umask case, and it will actually fail if someone reverts the change.

Also, please use ReflectionHelper instead.

}
Loading