Refuse an orphan path the storage layer would rewrite

The orphan check compared the path it was given with the paths file rows
hold, but Flysystem rewrites a path before it touches storage: "./a/b",
"a/./b", "a//b", "/a/b", "a\b" and "a/x/../b" all become "a/b". Any of
them made a file somebody owns look like an orphan, so deleting it
removed their bytes without delete_others_files, and importing it put a
second row on them. The same spellings walked past the exclusion of
derived-artifact folders.

isOrphan(), which import and delete both go through, now refuses a path
the normalizer would change or rejects outright. The scan only offers
paths as storage lists them, so nothing it sends is affected.

GHSA-pv88-7863-5hwq
This commit is contained in:
ignacionelson
2026-10-05 01:16:52 -03:00
parent c5a547ffc9
commit 1e34773ad3
+29
View File
@@ -11,6 +11,8 @@ use App\Modules\Files\Uploads\UploadExtensionPolicy;
use App\Modules\Platform\Settings\ExternalStorageConfigApplier;
use App\Modules\Platform\Settings\ExternalStorageSettings;
use Illuminate\Support\Facades\Storage;
use League\Flysystem\WhitespacePathNormalizer;
use Throwable;
/**
* Finds files sitting on disk with no corresponding File row — v1
@@ -112,6 +114,10 @@ class OrphanFileScanner
return false;
}
if (! $this->isCanonical($path)) {
return false;
}
if ($this->isExcluded($path) || ! Storage::disk($disk)->exists($path)) {
return false;
}
@@ -119,6 +125,29 @@ class OrphanFileScanner
return ! in_array($path, $this->knownPaths($disk), true);
}
/**
* Whether the storage layer would act on exactly this spelling.
*
* Flysystem rewrites a path before it touches storage: "./a/b.txt",
* "a/./b.txt", "a//b.txt", "/a/b.txt", "a\b.txt" and "a/x/../b.txt"
* all become "a/b.txt". The checks here compare strings, so any of
* those made a file somebody owns look like an orphan, and deleting or
* adopting it then reached the real file (GHSA-pv88-7863-5hwq). The
* scan only offers paths as storage lists them, already canonical, so
* a path that would be rewritten did not come from the scan and is
* refused rather than repaired.
*/
private function isCanonical(string $path): bool
{
try {
return (new WhitespacePathNormalizer)->normalizePath($path) === $path;
} catch (Throwable) {
// A path climbing out of the root, or carrying control
// characters, is refused by the normalizer itself.
return false;
}
}
public function isAllowedFor(User $viewer, string $path): bool
{
return $this->extensionPolicy->isAllowed($viewer, basename($path));