fix(fs): close guarded fallback handles on post-check failure
This commit is contained in:
parent
5218746972
commit
70fdf86fde
@ -8,6 +8,7 @@
|
||||
- Harden Root fallback mutators, archive merges, private store reads/writes, durable queue ids, JSON fallback writes, sibling temp writes, temp filename sanitization, and trash moves against symlink-swap and path traversal edge cases.
|
||||
- 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 if post-write directory verification fails, avoiding descriptor leaks in symlink-swap races.
|
||||
|
||||
### Tests
|
||||
|
||||
|
||||
@ -15,15 +15,29 @@ import {
|
||||
export async function withAsyncDirectoryGuards<T>(
|
||||
guards: readonly AsyncDirectoryGuard[],
|
||||
mutate: () => Promise<T>,
|
||||
options: { verifyAfter?: boolean } = {},
|
||||
options: {
|
||||
verifyAfter?: boolean;
|
||||
onPostGuardFailure?: (result: T, error: unknown) => Promise<void> | void;
|
||||
} = {},
|
||||
): Promise<T> {
|
||||
for (const guard of guards) {
|
||||
await assertAsyncDirectoryGuard(guard);
|
||||
}
|
||||
const result = await mutate();
|
||||
if (options.verifyAfter !== false) {
|
||||
for (const guard of guards) {
|
||||
await assertAsyncDirectoryGuard(guard);
|
||||
try {
|
||||
for (const guard of guards) {
|
||||
await assertAsyncDirectoryGuard(guard);
|
||||
}
|
||||
} catch (error) {
|
||||
if (options.onPostGuardFailure) {
|
||||
try {
|
||||
await options.onPostGuardFailure(result, error);
|
||||
} catch {
|
||||
// Preserve the boundary failure. Cleanup is best-effort.
|
||||
}
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
return result;
|
||||
|
||||
@ -207,12 +207,20 @@ async function runPinnedWriteFallback(params: {
|
||||
}
|
||||
const targetPath = path.join(parentPath, params.basename);
|
||||
if (params.overwrite === false) {
|
||||
let handle = await withAsyncDirectoryGuards([parentGuard], async () =>
|
||||
await fs.open(
|
||||
targetPath,
|
||||
fsSync.constants.O_WRONLY | fsSync.constants.O_CREAT | fsSync.constants.O_EXCL,
|
||||
params.mode,
|
||||
)
|
||||
let handle = await withAsyncDirectoryGuards(
|
||||
[parentGuard],
|
||||
async () =>
|
||||
await fs.open(
|
||||
targetPath,
|
||||
fsSync.constants.O_WRONLY | fsSync.constants.O_CREAT | fsSync.constants.O_EXCL,
|
||||
params.mode,
|
||||
),
|
||||
{
|
||||
onPostGuardFailure: async (openedHandle) => {
|
||||
await openedHandle.close().catch(() => undefined);
|
||||
await fs.rm(targetPath, { force: true }).catch(() => undefined);
|
||||
},
|
||||
},
|
||||
);
|
||||
let created = true;
|
||||
try {
|
||||
|
||||
@ -1609,21 +1609,29 @@ async function writeMissingFileFallback(
|
||||
|
||||
let created = false;
|
||||
try {
|
||||
const { handle, writtenStat } = await withAsyncDirectoryGuards([parentGuard], async () => {
|
||||
const handle = await fs.open(resolved, OPEN_WRITE_CREATE_FLAGS, params.mode ?? 0o600);
|
||||
created = true;
|
||||
try {
|
||||
if (typeof params.data === "string") {
|
||||
await handle.writeFile(params.data, params.encoding ?? "utf8");
|
||||
} else {
|
||||
await handle.writeFile(params.data);
|
||||
const { handle, writtenStat } = await withAsyncDirectoryGuards(
|
||||
[parentGuard],
|
||||
async () => {
|
||||
const handle = await fs.open(resolved, OPEN_WRITE_CREATE_FLAGS, params.mode ?? 0o600);
|
||||
created = true;
|
||||
try {
|
||||
if (typeof params.data === "string") {
|
||||
await handle.writeFile(params.data, params.encoding ?? "utf8");
|
||||
} else {
|
||||
await handle.writeFile(params.data);
|
||||
}
|
||||
return { handle, writtenStat: await handle.stat() };
|
||||
} catch (error) {
|
||||
await handle.close().catch(() => undefined);
|
||||
throw error;
|
||||
}
|
||||
return { handle, writtenStat: await handle.stat() };
|
||||
} catch (error) {
|
||||
await handle.close().catch(() => undefined);
|
||||
throw error;
|
||||
}
|
||||
});
|
||||
},
|
||||
{
|
||||
onPostGuardFailure: async ({ handle }) => {
|
||||
await handle.close().catch(() => undefined);
|
||||
},
|
||||
},
|
||||
);
|
||||
await handle.close();
|
||||
await verifyAtomicWriteResult({
|
||||
root,
|
||||
|
||||
95
test/guarded-write-cleanup.test.ts
Normal file
95
test/guarded-write-cleanup.test.ts
Normal file
@ -0,0 +1,95 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { configureFsSafePython } from "../src/pinned-python-config.js";
|
||||
import { runPinnedWriteHelper } from "../src/pinned-write.js";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
async function tempRoot(prefix: string): Promise<string> {
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), prefix));
|
||||
tempDirs.push(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
async function replaceParentAfterOpen(params: {
|
||||
targetPath: string;
|
||||
parentPath: string;
|
||||
movedParentPath: string;
|
||||
}): Promise<() => void> {
|
||||
const originalOpen = fs.open;
|
||||
let closeSpy: ReturnType<typeof vi.spyOn> | undefined;
|
||||
const openSpy = vi.spyOn(fs, "open").mockImplementation(async (...args) => {
|
||||
const handle = await originalOpen(...args);
|
||||
if (String(args[0]) === params.targetPath) {
|
||||
closeSpy = vi.spyOn(handle, "close");
|
||||
await fs.rename(params.parentPath, params.movedParentPath);
|
||||
await fs.mkdir(params.parentPath);
|
||||
}
|
||||
return handle;
|
||||
});
|
||||
return () => {
|
||||
openSpy.mockRestore();
|
||||
expect(closeSpy).toHaveBeenCalledTimes(1);
|
||||
};
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
vi.restoreAllMocks();
|
||||
configureFsSafePython({ mode: "auto", pythonPath: undefined });
|
||||
Object.defineProperty(process, "platform", originalPlatformDescriptor);
|
||||
await Promise.all(tempDirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true })));
|
||||
});
|
||||
|
||||
const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, "platform")!;
|
||||
|
||||
describe("guarded fallback write cleanup", () => {
|
||||
it.runIf(process.platform !== "win32")("closes pinned no-overwrite handles when post guards fail", async () => {
|
||||
configureFsSafePython({ mode: "off" });
|
||||
const base = await tempRoot("fs-safe-pinned-post-guard-");
|
||||
const parentPath = path.join(base, "nested");
|
||||
const movedParentPath = path.join(base, "nested-real");
|
||||
const targetPath = path.join(parentPath, "created.txt");
|
||||
await fs.mkdir(parentPath);
|
||||
const assertClosed = await replaceParentAfterOpen({
|
||||
targetPath,
|
||||
parentPath,
|
||||
movedParentPath,
|
||||
});
|
||||
|
||||
await expect(
|
||||
runPinnedWriteHelper({
|
||||
rootPath: base,
|
||||
relativeParentPath: "nested",
|
||||
basename: "created.txt",
|
||||
mkdir: false,
|
||||
mode: 0o600,
|
||||
overwrite: false,
|
||||
input: { kind: "buffer", data: "payload" },
|
||||
}),
|
||||
).rejects.toBeTruthy();
|
||||
|
||||
assertClosed();
|
||||
});
|
||||
|
||||
it.runIf(process.platform !== "win32")("closes root no-overwrite handles when post guards fail", async () => {
|
||||
Object.defineProperty(process, "platform", { configurable: true, value: "win32" });
|
||||
const { root: openRoot } = await import("../src/index.js");
|
||||
const base = await tempRoot("fs-safe-root-post-guard-");
|
||||
const parentPath = path.join(base, "nested");
|
||||
const movedParentPath = path.join(base, "nested-real");
|
||||
await fs.mkdir(parentPath);
|
||||
const targetPath = path.join(await fs.realpath(parentPath), "created.txt");
|
||||
const assertClosed = await replaceParentAfterOpen({
|
||||
targetPath,
|
||||
parentPath,
|
||||
movedParentPath,
|
||||
});
|
||||
const scoped = await openRoot(base);
|
||||
|
||||
await expect(scoped.create("nested/created.txt", "payload")).rejects.toBeTruthy();
|
||||
|
||||
assertClosed();
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user