fix(fs): preserve prune and trash fallback behavior

This commit is contained in:
Peter Steinberger 2026-05-06 23:05:13 +01:00
parent feb21f0be6
commit 261ca3cbc0
No known key found for this signature in database
6 changed files with 69 additions and 14 deletions

View File

@ -9,6 +9,7 @@
- Centralize safe path segment validation, directory identity guards, and guarded mutation wrappers so future filesystem helpers reuse the same race-resistant checks.
- Route archive ZIP staging, temp workspace sync reads, secret-file commits, and atomic move/replace fallbacks through shared pinned-read or guarded-write primitives without applying private-directory modes to public paths.
- Close guarded fallback write handles without following path names if post-write directory verification fails, avoiding descriptor leaks and unsafe cleanup in symlink-swap races.
- Preserve empty-directory pruning and broken-symlink trash moves across guarded fallback paths.
### Tests

View File

@ -84,6 +84,8 @@ export async function pruneExpiredStoreEntries(params: {
}
if (shouldDescend && (await pruneDir(fullPath, relativePath, depth + 1))) {
await assertRootGuard();
// Keep empty-dir pruning on the same root-bounded remove path as files;
// the Root fallback handles empty directories without recursive delete.
await scopedRoot.remove(relativePath).catch(() => undefined);
}
continue;

View File

@ -1379,7 +1379,7 @@ async function removePathFallback(resolved: { resolved: string }): Promise<void>
const guard = await createAsyncDirectoryGuard(path.dirname(resolved.resolved));
await getFsSafeTestHooks()?.beforeRootFallbackMutation?.("remove", resolved.resolved);
await assertAsyncDirectoryGuard(guard);
await fs.rm(resolved.resolved);
await ((await fs.lstat(resolved.resolved)).isDirectory() ? fs.rmdir(resolved.resolved) : fs.rm(resolved.resolved));
await assertAsyncDirectoryGuard(guard).catch(() => undefined);
}

View File

@ -30,11 +30,14 @@ function isSameOrChildPath(candidate: string, parent: string): boolean {
}
function resolveAllowedTrashRoots(allowedRoots?: Iterable<string>): string[] {
const roots = [...(allowedRoots ?? [os.homedir(), os.tmpdir()])].map((root) => {
const roots = [...(allowedRoots ?? [os.homedir(), os.tmpdir()])].flatMap((root) => {
const lexicalRoot = path.resolve(root);
try {
return path.resolve(fs.realpathSync.native(root));
// Keep both spellings: broken symlink targets cannot be realpathed and
// may only compare equal to the caller's lexical allowed root.
return [path.resolve(fs.realpathSync.native(root)), lexicalRoot];
} catch {
return path.resolve(root);
return [lexicalRoot];
}
});
return [...new Set(roots)];
@ -43,27 +46,39 @@ function resolveAllowedTrashRoots(allowedRoots?: Iterable<string>): string[] {
type TrashTargetGuard = {
path: string;
realPath: string;
realPathResolved: boolean;
stat: fs.Stats;
};
function resolveTrashTargetPath(targetPath: string): { path: string; resolved: boolean } {
try {
return { path: path.resolve(fs.realpathSync.native(targetPath)), resolved: true };
} catch {
// Broken symlinks are valid trash targets. Fall back to the lexical path,
// then rely on lstat identity so the move renames the symlink itself.
return { path: path.resolve(targetPath), resolved: false };
}
}
function assertAllowedTrashTarget(
targetPath: string,
allowedRoots?: Iterable<string>,
): TrashTargetGuard {
let resolvedTargetPath = path.resolve(targetPath);
const stat = fs.lstatSync(resolvedTargetPath);
try {
resolvedTargetPath = path.resolve(fs.realpathSync.native(targetPath));
} catch {
// The subsequent move will surface missing or inaccessible targets.
}
const stat = fs.lstatSync(path.resolve(targetPath));
const resolvedTarget = resolveTrashTargetPath(targetPath);
const resolvedTargetPath = resolvedTarget.path;
const isAllowed = resolveAllowedTrashRoots(allowedRoots).some(
(root) => resolvedTargetPath !== root && isSameOrChildPath(resolvedTargetPath, root),
);
if (!isAllowed) {
throw new Error(`Refusing to trash path outside allowed roots: ${targetPath}`);
}
return { path: path.resolve(targetPath), realPath: resolvedTargetPath, stat };
return {
path: path.resolve(targetPath),
realPath: resolvedTargetPath,
realPathResolved: resolvedTarget.resolved,
stat,
};
}
function assertTrashTargetGuard(guard: TrashTargetGuard): void {
@ -71,8 +86,11 @@ function assertTrashTargetGuard(guard: TrashTargetGuard): void {
if (!sameFileIdentity(stat, guard.stat)) {
throw new Error(`Refusing to trash path after it changed: ${guard.path}`);
}
const realPath = path.resolve(fs.realpathSync.native(guard.path));
if (realPath !== guard.realPath) {
const current = resolveTrashTargetPath(guard.path);
if (guard.realPathResolved && (!current.resolved || current.path !== guard.realPath)) {
throw new Error(`Refusing to trash path after it changed: ${guard.path}`);
}
if (!guard.realPathResolved && current.resolved) {
throw new Error(`Refusing to trash path after it changed: ${guard.path}`);
}
}

View File

@ -229,4 +229,21 @@ describe("trash edge paths", () => {
await fs.rm(path.dirname(copiedDest), { recursive: true, force: true });
}
});
it.runIf(process.platform !== "win32")("moves broken symlinks to trash", async () => {
const root = await tempRoot("fs-safe-trash-broken-link-");
const linkPath = path.join(root, "broken-link");
const missingTarget = path.join(root, "missing-target");
await fs.symlink(missingTarget, linkPath);
const dest = await movePathToTrash(linkPath, { allowedRoots: [root] });
try {
// Broken links cannot be realpathed; the guard keeps lstat identity and
// renames the link itself instead of requiring the target to exist.
await expect(fs.readlink(dest)).resolves.toBe(missingTarget);
await expect(fs.lstat(linkPath)).rejects.toMatchObject({ code: "ENOENT" });
} finally {
await fs.rm(path.dirname(dest), { recursive: true, force: true });
}
});
});

View File

@ -72,4 +72,21 @@ describe("platform fallback coverage", () => {
code: "ENOENT",
});
});
it("prunes empty directories through the Windows remove fallback", async () => {
await importRootForPlatform("win32");
const { fileStore } = await import("../src/file-store.js");
const rootDir = await tempRoot("fs-safe-win-prune-");
const store = fileStore({ rootDir });
const stalePath = path.join(rootDir, "old", "stale.txt");
await fs.mkdir(path.dirname(stalePath), { recursive: true });
await fs.writeFile(stalePath, "stale", "utf8");
await fs.utimes(stalePath, new Date(0), new Date(0));
await store.pruneExpired({ ttlMs: 1, recursive: true, pruneEmptyDirs: true });
// Root.remove's Node fallback must use rmdir for empty directories; fs.rm
// without recursive rejects dirs and would silently leave pruneEmptyDirs work.
await expect(fs.stat(path.join(rootDir, "old"))).rejects.toMatchObject({ code: "ENOENT" });
});
});