diff --git a/packages/core/src/environment/tools/read-file.ts b/packages/core/src/environment/tools/read-file.ts index e1b7c09..745e270 100644 --- a/packages/core/src/environment/tools/read-file.ts +++ b/packages/core/src/environment/tools/read-file.ts @@ -28,7 +28,7 @@ * Docs: /docs/tools § "File tools". */ import path from "node:path"; -import { open, stat } from "node:fs/promises"; +import { open, realpath, stat } from "node:fs/promises"; import { partialToolCallOutput } from "../../omnimessage/index.js"; import type { OmniMessage } from "../../omnimessage/index.js"; import type { ToolDefinitionConfig } from "../../interfaces.js"; @@ -65,6 +65,29 @@ const DEFAULT_OUTPUT_BUDGET = 64000; */ const SECRET_BASENAMES = new Set([".vault.toml", ".project_config.toml"]); +/** + * The secret-store name a path hits, or null. Checks the LOWERCASED basename of both the + * lexical path and its realpath: a case-insensitive filesystem (macOS/Windows) opens + * ".VAULT.TOML" as .vault.toml, and a symlink's own name says nothing about what it + * dereferences to — either would slip past a plain case-sensitive basename check. On a + * case-sensitive filesystem the lowercase compare can refuse a differently-cased sibling + * that is NOT the secret store; for a guard, that false positive is the right trade. + */ +async function secretStoreHit(resolved: string): Promise { + let target = resolved; + try { + target = await realpath(resolved); + } catch { + // Missing file / permission error: the lexical name is still checked below, and the + // read path reports the real error afterwards. + } + for (const name of [path.basename(resolved), path.basename(target)]) { + const lower = name.toLowerCase(); + if (SECRET_BASENAMES.has(lower)) return lower; + } + return null; +} + /** Renders one `cat -n` style line: 6-column right-aligned line number, tab, content. */ export function numberedLine(lineNo: number, content: string): string { const capped = @@ -251,9 +274,13 @@ export function createReadFileTool(definition: ToolDefinitionConfig): BuiltinToo const resolved = path.resolve(ctx.workspaceDir, filePath); // Secret stores are refused by name: read_file is auto-approved under read-only // approval, so it needs its own guard (aligned with the system prompt's ban). - if (SECRET_BASENAMES.has(path.basename(resolved))) { + // secretStoreHit resolves symlinks and compares case-insensitively — the lexical + // basename alone is bypassable via ".VAULT.TOML" (case-insensitive filesystems) or + // a symlink pointing at the store. + const secretHit = await secretStoreHit(resolved); + if (secretHit !== null) { yield delta( - `Refusing to read "${filePath}": ${path.basename(resolved)} holds the user's secrets and must never enter the conversation.`, + `Refusing to read "${filePath}": ${secretHit} holds the user's secrets and must never enter the conversation.`, ); return { stopReason: "failed" }; } diff --git a/packages/core/test/file-tools.test.ts b/packages/core/test/file-tools.test.ts index fecf5cf..f15445a 100644 --- a/packages/core/test/file-tools.test.ts +++ b/packages/core/test/file-tools.test.ts @@ -6,7 +6,17 @@ * read-image.test.ts); Environment-side framing is covered by environment.test.ts. */ import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { chmod, mkdir, mkdtemp, readdir, readFile, rm, stat, writeFile } from "node:fs/promises"; +import { + chmod, + mkdir, + mkdtemp, + readdir, + readFile, + rm, + stat, + symlink, + writeFile, +} from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { @@ -384,6 +394,32 @@ describe("read_file — argument coercion, CRLF, secret guard", () => { expect(cfg.result?.stopReason).toBe("failed"); expect(cfg.text).toContain("Refusing to read"); }); + + it("refuses case-variant names and symlinks that dereference to a secret store", async () => { + await writeFile(path.join(tmp, ".vault.toml"), "SECRET=1\n"); + // Case variant: a case-insensitive filesystem (macOS/Windows) opens ".VAULT.TOML" as + // the store itself; on a case-sensitive one refusing the name is a harmless false positive. + const upper = await run(tool(), { file_path: ".VAULT.TOML" }, tmp); + expect(upper.result?.stopReason).toBe("failed"); + expect(upper.text).toContain("Refusing to read"); + // Symlink: the link's own basename says nothing about what it dereferences to. + await symlink(path.join(tmp, ".vault.toml"), path.join(tmp, "notes.txt")); + const linked = await run(tool(), { file_path: "notes.txt" }, tmp); + expect(linked.result?.stopReason).toBe("failed"); + expect(linked.text).toContain("Refusing to read"); + expect(linked.text).not.toContain("SECRET=1"); + // A symlink NAMED like a store but pointing elsewhere is refused on the lexical name — + // an acceptable false positive for a guard. + await writeFile(path.join(tmp, "plain.txt"), "hello\n"); + await symlink(path.join(tmp, "plain.txt"), path.join(tmp, ".project_config.toml")); + const named = await run(tool(), { file_path: ".project_config.toml" }, tmp); + expect(named.result?.stopReason).toBe("failed"); + // An ordinary symlink to an ordinary file still reads. + await symlink(path.join(tmp, "plain.txt"), path.join(tmp, "alias.txt")); + const ok = await run(tool(), { file_path: "alias.txt" }, tmp); + expect(ok.result?.stopReason).toBeUndefined(); + expect(ok.text).toContain("hello"); + }); }); describe("read_file — bounded scan and output budget", () => {