diff --git a/CHANGELOG.md b/CHANGELOG.md index dd81da3..30ffa92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ ### Security and Correctness +- Create `append`, `openWritable`, and fallback `copyIn` parents through guarded per-component walks and continue I/O through the resolved in-root parent, preventing symlink-swap races from creating directories outside the root while preserving valid in-root symlink parents. Thanks @yetval for reporting this. - Add pinned-destination hardlink rejection and bounded original-content restoration to `replaceFileAtomic()` and its sync variant, including typed `restored` / `restore-failed` receipts for torn copy-fallback writes. - Add sibling staging to `writeExternalFileWithinRoot()`: external producers can write a randomized file in the target directory for fsynced same-filesystem atomic replacement, while private workspace staging remains the cross-device-tolerant default; staged and final basenames share portable C0/C1 and Windows-invalid-character sanitization on every host. - Enforce `movePathWithCopyFallback({ sourceHardlinks: "reject" })` with a streaming, entry-capped recursive preflight before mutation, closing a shipped 0.4.x gap where the common same-filesystem rename bypassed the policy; approved trees commit through a fresh staged copy with open-time and post-copy link-count fences so a scan/rename race cannot publish a hardlinked inode. diff --git a/src/root-impl.ts b/src/root-impl.ts index 0b90341..65807b4 100644 --- a/src/root-impl.ts +++ b/src/root-impl.ts @@ -731,6 +731,16 @@ function rootWriteQueueKey(root: RootContext, relativePath: string): string { type PinnedWriteTarget = { rootReal: string; targetPath: string; relativeParentPath: string; basename: string; mode: number }; +async function prepareRootWriteTarget(rootReal: string, targetPath: string): Promise { + const parentPath = await mkdirPathComponentsWithGuards({ + rootReal, + targetPath: path.dirname(targetPath), + }); + // Continue through the guarded walk's real parent instead of re-entering + // the original path through a symlinked component. + return path.join(parentPath, path.basename(targetPath)); +} + async function writeTempFileForAtomicReplace(params: { tempPath: string; data: string | Buffer; @@ -793,16 +803,11 @@ async function openWritableFileInRoot( } catch (err) { throw new FsSafeError("path-alias", "path alias escape blocked", { cause: err }); } - if (params.mkdir !== false) { - const parentGuard = await createNearestExistingDirectoryGuard(rootReal, path.dirname(resolved)); - await withAsyncDirectoryGuards([parentGuard], async () => { - await fs.mkdir(path.dirname(resolved), { recursive: true }); - }); - } - - let ioPath = resolved; + let ioPath = params.mkdir === false + ? resolved + : await prepareRootWriteTarget(rootReal, resolved); try { - const resolvedRealPath = await fs.realpath(resolved); + const resolvedRealPath = await fs.realpath(ioPath); if (!isPathInside(rootWithSep, resolvedRealPath)) { throw new FsSafeError("outside-workspace", "file is outside workspace root"); } @@ -1559,16 +1564,16 @@ async function writeMissingFileFallback( } catch (err) { throw new FsSafeError("path-alias", "path alias escape blocked", { cause: err }); } - if (params.mkdir !== false) { - await fs.mkdir(path.dirname(resolved), { recursive: true }); - } - const parentGuard = await createAsyncDirectoryGuard(path.dirname(resolved)); + const targetPath = params.mkdir === false + ? resolved + : await prepareRootWriteTarget(rootReal, resolved); + const parentGuard = await createAsyncDirectoryGuard(path.dirname(targetPath)); let created = false; try { const { handle, writtenStat } = await withAsyncDirectoryGuards( [parentGuard], async () => { - const handle = await fs.open(resolved, OPEN_WRITE_CREATE_FLAGS, params.mode ?? 0o600); + const handle = await fs.open(targetPath, OPEN_WRITE_CREATE_FLAGS, params.mode ?? 0o600); created = true; try { if (typeof params.data === "string") { @@ -1592,7 +1597,7 @@ async function writeMissingFileFallback( await handle.close(); await verifyAtomicWriteResult({ root, - targetPath: resolved, + targetPath, expectedIdentity: writtenStat, }); created = false; @@ -1605,7 +1610,7 @@ async function writeMissingFileFallback( throw err; } finally { if (created) { - await fs.rm(resolved, { force: true }).catch(() => undefined); + await fs.rm(targetPath, { force: true }).catch(() => undefined); } } } diff --git a/test/guarded-mkdir-symlink-parent.test.ts b/test/guarded-mkdir-symlink-parent.test.ts index 0c2dcc9..e29544f 100644 --- a/test/guarded-mkdir-symlink-parent.test.ts +++ b/test/guarded-mkdir-symlink-parent.test.ts @@ -143,3 +143,29 @@ it.runIf(process.platform !== "win32")( expect(written).toBe("# new content\n"); }, ); + +it.runIf(process.platform !== "win32")( + "appends and opens writable files through an in-root symlinked parent", + async () => { + const rootDir = await tempRoot("fs-safe-root-writable-symlink-"); + const realDir = path.join(rootDir, "real-parent"); + await fs.mkdir(realDir); + await fs.symlink(realDir, path.join(rootDir, "alias"), "dir"); + const r = await root(rootDir); + + await r.append("alias/logs/app.log", "entry\n"); + const opened = await r.openWritable("alias/output/result.txt"); + try { + await opened.handle.writeFile("result\n"); + } finally { + await opened.handle.close(); + } + + await expect(fs.readFile(path.join(realDir, "logs/app.log"), "utf8")).resolves.toBe( + "entry\n", + ); + await expect(fs.readFile(path.join(realDir, "output/result.txt"), "utf8")).resolves.toBe( + "result\n", + ); + }, +); diff --git a/test/write-boundary-bypass.test.ts b/test/write-boundary-bypass.test.ts index 785a33b..231e8c7 100644 --- a/test/write-boundary-bypass.test.ts +++ b/test/write-boundary-bypass.test.ts @@ -257,4 +257,89 @@ describe("write, move, and delete boundary bypass attempts", () => { }); await expectNoOutsideWrite(layout); }); + + it.runIf(process.platform !== "win32")("does not create directories outside the root when append races a parent symlink swap", async () => { + const layout = await makeTempLayout("fs-safe-append-mkdir-swap"); + const safeRoot = await openRoot(layout.root); + const swapPath = path.join(layout.root, "data"); + let stopSwapping = false; + const swapper = (async () => { + while (!stopSwapping) { + await fsp.mkdir(swapPath).catch(() => {}); + await fsp.rm(swapPath, { force: true, recursive: true }).catch(() => {}); + await fsp.symlink(layout.outside, swapPath, "dir").catch(() => {}); + await fsp.rm(swapPath, { force: true }).catch(() => {}); + } + })(); + + try { + for (let attempt = 0; attempt < 400; attempt++) { + await safeRoot.append("data/logs/2026/07/app.log", "entry\n").catch(() => {}); + expect(await fsp.readdir(layout.outside)).toEqual(["secret.txt"]); + } + } finally { + stopSwapping = true; + await swapper; + await fsp.rm(swapPath, { force: true, recursive: true }).catch(() => {}); + } + await expectNoOutsideWrite(layout); + }, 30000); + + it.runIf(process.platform !== "win32")("does not create directories outside the root when openWritable races a parent symlink swap", async () => { + const layout = await makeTempLayout("fs-safe-open-writable-mkdir-swap"); + const safeRoot = await openRoot(layout.root); + const swapPath = path.join(layout.root, "data"); + let stopSwapping = false; + const swapper = (async () => { + while (!stopSwapping) { + await fsp.mkdir(swapPath).catch(() => {}); + await fsp.rm(swapPath, { force: true, recursive: true }).catch(() => {}); + await fsp.symlink(layout.outside, swapPath, "dir").catch(() => {}); + await fsp.rm(swapPath, { force: true }).catch(() => {}); + } + })(); + + try { + for (let attempt = 0; attempt < 400; attempt++) { + const opened = await safeRoot.openWritable("data/logs/2026/07/app.log").catch(() => undefined); + await opened?.handle.close().catch(() => {}); + expect(await fsp.readdir(layout.outside)).toEqual(["secret.txt"]); + } + } finally { + stopSwapping = true; + await swapper; + await fsp.rm(swapPath, { force: true, recursive: true }).catch(() => {}); + } + await expectNoOutsideWrite(layout); + }, 30000); + + it.runIf(process.platform !== "win32")("does not create directories outside the root when copyIn fallback races a parent symlink swap", async () => { + configureFsSafeNative({ mode: "off" }); + const layout = await makeTempLayout("fs-safe-copy-in-mkdir-swap"); + const safeRoot = await openRoot(layout.root); + const source = path.join(layout.root, "source.txt"); + await fsp.writeFile(source, "source"); + const swapPath = path.join(layout.root, "data"); + let stopSwapping = false; + const swapper = (async () => { + while (!stopSwapping) { + await fsp.mkdir(swapPath).catch(() => {}); + await fsp.rm(swapPath, { force: true, recursive: true }).catch(() => {}); + await fsp.symlink(layout.outside, swapPath, "dir").catch(() => {}); + await fsp.rm(swapPath, { force: true }).catch(() => {}); + } + })(); + + try { + for (let attempt = 0; attempt < 400; attempt++) { + await safeRoot.copyIn("data/logs/2026/07/app.log", source).catch(() => {}); + expect(await fsp.readdir(layout.outside)).toEqual(["secret.txt"]); + } + } finally { + stopSwapping = true; + await swapper; + await fsp.rm(swapPath, { force: true, recursive: true }).catch(() => {}); + } + await expectNoOutsideWrite(layout); + }, 60000); });