From eaf78d8b9cd77bdeb73d290ac0229a10c7bfb3f3 Mon Sep 17 00:00:00 2001 From: Bernard Badenhorst <58713832+bernard-jigx@users.noreply.github.com> Date: Wed, 15 Apr 2026 10:57:07 -0700 Subject: [PATCH 1/3] fix(security): resolve path traversal bypass in evaluateFilePath MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read deny patterns were matched only against the raw input string, so absolute-path globs like Read(/Users/you/.ssh/**) could be bypassed by passing a relative path with `..` segments that resolved into the same location (e.g. ../../.ssh/id_rsa from a project nested under $HOME). evaluateFilePath now accepts an optional projectRoot and matches deny globs against both the raw input and the fully-resolved absolute path. checkFilePathDenyPolicy in server.ts passes CLAUDE_PROJECT_DIR (falling back to process.cwd()) so the resolved form is checked for ctx_execute_file and any other tool routed through the policy. Backwards compatible: callers that don't pass projectRoot see identical behavior — covered by a regression-guard test alongside the new bypass test. --- src/security.ts | 24 ++++++++++++++++++++---- src/server.ts | 10 ++++++++-- tests/security.test.ts | 33 +++++++++++++++++++++++++++++++-- 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/src/security.ts b/src/security.ts index 12e0019e..6cba3019 100644 --- a/src/security.ts +++ b/src/security.ts @@ -429,19 +429,35 @@ export function evaluateCommandDenyOnly( * * Normalizes backslashes to forward slashes before matching so that * Windows paths work with Unix-style glob patterns. + * + * When `projectRoot` is supplied, the path is also matched in its + * resolved absolute form — preventing `..` traversal from bypassing + * absolute-path deny rules like `Read(/Users/.../.ssh/**)`. */ export function evaluateFilePath( filePath: string, denyGlobs: string[][], caseInsensitive: boolean = process.platform === "win32", + projectRoot?: string, ): { denied: boolean; matchedPattern?: string } { - // Normalize backslashes to forward slashes for cross-platform matching - const normalized = filePath.replace(/\\/g, "/"); + const toForward = (path: string): string => path.replace(/\\/g, "/"); + + // Match against both the raw input and (when projectRoot is supplied) + // the fully-resolved absolute path. Deduplicated so tests and callers + // that pass an already-absolute path don't pay the matching cost twice. + const candidates: string[] = [toForward(filePath)]; + if (projectRoot) { + const absolute = toForward(resolve(projectRoot, filePath)); + if (absolute !== candidates[0]) candidates.push(absolute); + } for (const globs of denyGlobs) { for (const glob of globs) { - if (fileGlobToRegex(glob, caseInsensitive).test(normalized)) { - return { denied: true, matchedPattern: glob }; + const regex = fileGlobToRegex(glob, caseInsensitive); + for (const candidate of candidates) { + if (regex.test(candidate)) { + return { denied: true, matchedPattern: glob }; + } } } } diff --git a/src/server.ts b/src/server.ts index 93249a91..8ddeb61f 100644 --- a/src/server.ts +++ b/src/server.ts @@ -371,8 +371,14 @@ function checkFilePathDenyPolicy( toolName: string, ): ToolResult | null { try { - const denyGlobs = readToolDenyPatterns("Read", process.env.CLAUDE_PROJECT_DIR); - const result = evaluateFilePath(filePath, denyGlobs); + const projectDir = process.env.CLAUDE_PROJECT_DIR ?? process.cwd(); + const denyGlobs = readToolDenyPatterns("Read", projectDir); + const result = evaluateFilePath( + filePath, + denyGlobs, + process.platform === "win32", + projectDir, + ); if (result.denied) { return trackResponse(toolName, { content: [{ diff --git a/tests/security.test.ts b/tests/security.test.ts index 9796c3a4..5bbff272 100644 --- a/tests/security.test.ts +++ b/tests/security.test.ts @@ -8,8 +8,8 @@ import { describe, test, beforeAll, afterAll } from "vitest"; import { strict as assert } from "node:assert"; import { writeFileSync, mkdirSync, rmSync } from "node:fs"; -import { join } from "node:path"; -import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { tmpdir, homedir } from "node:os"; import { parseBashPattern, @@ -474,6 +474,35 @@ describe("evaluateFilePath", () => { assert.equal(result.denied, true); assert.equal(result.matchedPattern, "**/.env"); }); + + test("evaluateFilePath: traversal does not bypass absolute deny glob when projectRoot is supplied", () => { + // An absolute deny rule for ~/.ssh/** should still match when the caller + // passes a ../-traversal relative path that resolves into ~/.ssh. + const home = homedir(); + const projectRoot = resolve(home, "some/project"); + const denyGlobs = [[`${home}/.ssh/**`]]; + + const result = evaluateFilePath( + "../../.ssh/id_rsa", + denyGlobs, + false, + projectRoot, + ); + assert.equal(result.denied, true); + assert.equal(result.matchedPattern, `${home}/.ssh/**`); + }); + + test("evaluateFilePath: without projectRoot, absolute deny glob is still bypassable (regression guard)", () => { + // Documents the pre-fix behavior: without projectRoot, `..` is not + // resolved, so the raw string doesn't match the absolute glob. + // This test exists so any change in behavior is intentional. + const result = evaluateFilePath( + "../../.ssh/id_rsa", + [["/Users/someone/.ssh/**"]], + false, + ); + assert.equal(result.denied, false); + }); }); describe("Shell-Escape Scanner", () => { From d9d766f5649b010ad8e205a0d2def9f14e386b4b Mon Sep 17 00:00:00 2001 From: Bernard Badenhorst <58713832+bernard-jigx@users.noreply.github.com> Date: Fri, 17 Apr 2026 08:03:42 -0700 Subject: [PATCH 2/3] fix(security): cover symlink escapes and normalize Windows test glob --- src/security.ts | 37 ++++++++++++++++++++++++++++--------- tests/security.test.ts | 13 +++++++------ 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/security.ts b/src/security.ts index 6cba3019..65bcfad9 100644 --- a/src/security.ts +++ b/src/security.ts @@ -1,4 +1,4 @@ -import { readFileSync } from "node:fs"; +import { readFileSync, realpathSync } from "node:fs"; import { resolve } from "node:path"; import { homedir } from "node:os"; @@ -431,8 +431,20 @@ export function evaluateCommandDenyOnly( * Windows paths work with Unix-style glob patterns. * * When `projectRoot` is supplied, the path is also matched in its - * resolved absolute form — preventing `..` traversal from bypassing - * absolute-path deny rules like `Read(/Users/.../.ssh/**)`. + * fully-resolved absolute form **and** — when the file exists — in + * its canonical form (`fs.realpathSync`). This prevents two classes + * of bypass: + * + * 1. `..` traversal: a relative path like `../../.ssh/id_rsa` no + * longer evades absolute-path deny rules. + * 2. Symlink escape: a project-local path whose realpath points + * outside the project (e.g. `safe.log -> ~/.ssh/id_rsa`) no + * longer evades absolute-path deny rules. + * + * realpath is best-effort: if the file does not exist yet (ENOENT) + * or the syscall fails for any reason, the lexical resolved form is + * still checked. This keeps the function usable for paths that will + * be created during execution. */ export function evaluateFilePath( filePath: string, @@ -442,13 +454,20 @@ export function evaluateFilePath( ): { denied: boolean; matchedPattern?: string } { const toForward = (path: string): string => path.replace(/\\/g, "/"); - // Match against both the raw input and (when projectRoot is supplied) - // the fully-resolved absolute path. Deduplicated so tests and callers - // that pass an already-absolute path don't pay the matching cost twice. - const candidates: string[] = [toForward(filePath)]; + // Match against the raw input, the lexically-resolved absolute path, + // and the canonical (symlink-resolved) path when the file exists. + // Deduplicated so absolute inputs and paths that don't cross symlinks + // don't pay the matching cost multiple times. + const candidates = new Set(); + candidates.add(toForward(filePath)); if (projectRoot) { - const absolute = toForward(resolve(projectRoot, filePath)); - if (absolute !== candidates[0]) candidates.push(absolute); + const lexical = resolve(projectRoot, filePath); + candidates.add(toForward(lexical)); + try { + candidates.add(toForward(realpathSync(lexical))); + } catch { + // File does not exist yet, or realpath failed — rely on lexical form. + } } for (const globs of denyGlobs) { diff --git a/tests/security.test.ts b/tests/security.test.ts index 5bbff272..eeabc0e6 100644 --- a/tests/security.test.ts +++ b/tests/security.test.ts @@ -480,26 +480,27 @@ describe("evaluateFilePath", () => { // passes a ../-traversal relative path that resolves into ~/.ssh. const home = homedir(); const projectRoot = resolve(home, "some/project"); - const denyGlobs = [[`${home}/.ssh/**`]]; + const denyGlob = resolve(home, ".ssh").replace(/\\/g, "/") + "/**"; const result = evaluateFilePath( "../../.ssh/id_rsa", - denyGlobs, - false, + [[denyGlob]], + process.platform === "win32", projectRoot, ); assert.equal(result.denied, true); - assert.equal(result.matchedPattern, `${home}/.ssh/**`); + assert.equal(result.matchedPattern, denyGlob); }); test("evaluateFilePath: without projectRoot, absolute deny glob is still bypassable (regression guard)", () => { // Documents the pre-fix behavior: without projectRoot, `..` is not // resolved, so the raw string doesn't match the absolute glob. // This test exists so any change in behavior is intentional. + const absoluteSshGlob = resolve(homedir(), ".ssh").replace(/\\/g, "/") + "/**"; const result = evaluateFilePath( "../../.ssh/id_rsa", - [["/Users/someone/.ssh/**"]], - false, + [[absoluteSshGlob]], + process.platform === "win32", ); assert.equal(result.denied, false); }); From c18f9a41a674d899b045c618599251cf019f04b1 Mon Sep 17 00:00:00 2001 From: Bernard Badenhorst <58713832+bernard-jigx@users.noreply.github.com> Date: Fri, 17 Apr 2026 08:14:09 -0700 Subject: [PATCH 3/3] test(hooks): align MCP sentinel setup with readiness guard --- tests/hooks/tool-naming.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/hooks/tool-naming.test.ts b/tests/hooks/tool-naming.test.ts index 34afb550..0ce6bdcb 100644 --- a/tests/hooks/tool-naming.test.ts +++ b/tests/hooks/tool-naming.test.ts @@ -53,7 +53,7 @@ const mcpSentinel = resolve(tmpdir(), `context-mode-mcp-ready-${process.ppid}`); beforeEach(() => { if (typeof resetGuidanceThrottle === "function") resetGuidanceThrottle(); - writeFileSync(mcpSentinel, String(process.pid)); + writeFileSync(mcpSentinel, String(process.ppid)); }); afterEach(() => {