refactor: unify boundary hardening for file reads

This commit is contained in:
Peter Steinberger
2026-02-26 13:04:33 +01:00
parent cf4853e2b8
commit eac86c2081
11 changed files with 455 additions and 56 deletions

View File

@@ -137,6 +137,7 @@ function makeFileStat(params?: {
mtimeMs?: number;
dev?: number;
ino?: number;
nlink?: number;
}): import("node:fs").Stats {
return {
isFile: () => true,
@@ -145,6 +146,7 @@ function makeFileStat(params?: {
mtimeMs: params?.mtimeMs ?? 1234,
dev: params?.dev ?? 1,
ino: params?.ino ?? 1,
nlink: params?.nlink ?? 1,
} as unknown as import("node:fs").Stats;
}
@@ -649,4 +651,66 @@ describe("agents.files.get/set symlink safety", () => {
undefined,
);
});
it("rejects agents.files.get when allowlisted file is a hardlinked alias", async () => {
const workspace = "/workspace/test-agent";
const candidate = path.resolve(workspace, "AGENTS.md");
mocks.fsRealpath.mockImplementation(async (p: string) => {
if (p === workspace) {
return workspace;
}
return p;
});
mocks.fsLstat.mockImplementation(async (...args: unknown[]) => {
const p = typeof args[0] === "string" ? args[0] : "";
if (p === candidate) {
return makeFileStat({ nlink: 2 });
}
throw createEnoentError();
});
const { respond, promise } = makeCall("agents.files.get", {
agentId: "main",
name: "AGENTS.md",
});
await promise;
expect(respond).toHaveBeenCalledWith(
false,
undefined,
expect.objectContaining({ message: expect.stringContaining("unsafe workspace file") }),
);
});
it("rejects agents.files.set when allowlisted file is a hardlinked alias", async () => {
const workspace = "/workspace/test-agent";
const candidate = path.resolve(workspace, "AGENTS.md");
mocks.fsRealpath.mockImplementation(async (p: string) => {
if (p === workspace) {
return workspace;
}
return p;
});
mocks.fsLstat.mockImplementation(async (...args: unknown[]) => {
const p = typeof args[0] === "string" ? args[0] : "";
if (p === candidate) {
return makeFileStat({ nlink: 2 });
}
throw createEnoentError();
});
const { respond, promise } = makeCall("agents.files.set", {
agentId: "main",
name: "AGENTS.md",
content: "x",
});
await promise;
expect(respond).toHaveBeenCalledWith(
false,
undefined,
expect.objectContaining({ message: expect.stringContaining("unsafe workspace file") }),
);
expect(mocks.fsOpen).not.toHaveBeenCalled();
});
});

View File

@@ -181,6 +181,9 @@ async function resolveAgentWorkspaceFilePath(params: {
if (!targetStat.isFile()) {
return { kind: "invalid", requestPath, reason: "symlink target is not a file" };
}
if (targetStat.nlink > 1) {
return { kind: "invalid", requestPath, reason: "hardlinked file target not allowed" };
}
} catch (err) {
if (isNotFoundPathError(err) && params.allowMissing) {
return { kind: "missing", requestPath, ioPath: targetReal, workspaceReal };
@@ -193,6 +196,9 @@ async function resolveAgentWorkspaceFilePath(params: {
if (!candidateLstat.isFile()) {
return { kind: "invalid", requestPath, reason: "path is not a regular file" };
}
if (candidateLstat.nlink > 1) {
return { kind: "invalid", requestPath, reason: "hardlinked file path not allowed" };
}
const candidateReal = await fs.realpath(candidatePath).catch(() => candidatePath);
if (!isPathInside(workspaceReal, candidateReal)) {
@@ -207,6 +213,9 @@ async function statFileSafely(filePath: string): Promise<FileMeta | null> {
if (lstat.isSymbolicLink() || !stat.isFile()) {
return null;
}
if (stat.nlink > 1) {
return null;
}
if (!sameFileIdentity(stat, lstat)) {
return null;
}
@@ -226,6 +235,9 @@ async function writeFileSafely(filePath: string, content: string): Promise<void>
if (lstat.isSymbolicLink() || !stat.isFile()) {
throw new Error("unsafe file path");
}
if (stat.nlink > 1) {
throw new Error("hardlinked file path is not allowed");
}
if (!sameFileIdentity(stat, lstat)) {
throw new Error("path changed during write");
}