From ceacee2a48238f9b5fcf6ca3d7eba6a0a8535bc8 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 8 Oct 2026 16:25:50 -0700 Subject: [PATCH 1/5] fix(search): allow recursive globs pinned to a literal dir --- src/plugins/search-files-fail-closed.test.ts | 42 ++++++++++++++++++++ src/plugins/tool-time-budget.ts | 26 ++++++++++-- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/src/plugins/search-files-fail-closed.test.ts b/src/plugins/search-files-fail-closed.test.ts index 8ed160dc8..e75af95e1 100644 --- a/src/plugins/search-files-fail-closed.test.ts +++ b/src/plugins/search-files-fail-closed.test.ts @@ -173,6 +173,28 @@ describe("CL-9469 search_files fails closed on unbounded root walks", () => { } }); }); + + test("recursive glob pinned to a literal dir at the root is allowed", async () => { + await withFixture(async ({ cwd }) => { + for (const spawn of [undefined, rgMissingSpawn()] as const) { + const tools = createPosixTools({ + cwd, + plugins: [ripgrepPlugin(cwd, {}, spawn)], + }); + const result = await tools.run( + { + id: "1", + name: "search_files", + arguments: { pattern: "sub/**/*.ts" }, + }, + new AbortController().signal, + ); + expect(result.isError !== true).toBe(true); + expect(String(result.content)).toContain("keep.ts"); + expect(String(result.content)).toContain("deep.ts"); + } + }); + }); }); describe("CL-9469 unbounded-root predicate", () => { @@ -209,6 +231,26 @@ describe("CL-9469 unbounded-root predicate", () => { true, ); }); + // A recursive descent is only a whole-tree walk when nothing literal pins it + // down; a leading literal dir keeps it bounded even at the workspace root. + test("recursive glob with a leading literal dir is bounded", () => { + for (const pattern of [ + "src/**/*.ts", + "packages/a/src/**/*.ts", + "docs/**", + ]) { + expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe( + false, + ); + } + }); + test("recursive descent with no leading literal dir stays unbounded", () => { + for (const pattern of ["**", "**/*.ts", "**/*", "/**/*.ts"]) { + expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe( + true, + ); + } + }); test("refusal message carries scope guidance and never reads as a timeout", () => { const message = formatUnboundedSearchMessage("search_files", "**/*.ts"); expect(message).toContain("narrow `path`"); diff --git a/src/plugins/tool-time-budget.ts b/src/plugins/tool-time-budget.ts index ae6f7dfad..4597e1c80 100644 --- a/src/plugins/tool-time-budget.ts +++ b/src/plugins/tool-time-budget.ts @@ -27,11 +27,29 @@ export function formatSearchTimeoutMessage( return `${trimmed}\n\n${notice}`; } -// A search_files pattern that walks the whole tree: recursive descent or a -// bare star with no literal constraint. Tighter globs (`*.ts`, `*config*`) -// still match a bounded name space and stay allowed at the root. +// A search_files pattern that has no bound on where a match can begin is a +// whole-tree walk: bare stars, or a recursive descent that starts at the root +// with nothing literal before it. A literal path segment before the first `**` +// (`src/**/*.ts`) pins the walk to that subtree, so it is bounded even though +// it recurses. Tighter globs (`*.ts`, `*config*`) still match a bounded name +// space and stay allowed at the root. export function isUnboundedSearchGlob(pattern: string): boolean { - return pattern === "*" || pattern.includes("**"); + if (pattern === "*") return true; + const firstRecursive = pattern.indexOf("**"); + if (firstRecursive === -1) return false; + // Everything before the first `**` is only wildcard descent and separators, + // so the walk still reaches every file from the root. + return !hasLiteralBefore(pattern.slice(0, firstRecursive)); +} + +// True when the given glob prefix names any concrete path text (a character +// that is neither a `*` wildcard nor a `/` separator), i.e. a segment that +// anchors the walk to a real location. +function hasLiteralBefore(prefix: string): boolean { + for (const char of prefix) { + if (char !== "*" && char !== "/") return true; + } + return false; } // The raw path argument resolved against the session root: omitted, empty, From ab155ccfc368fa0e645d02a0eb3d55284dd86677 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 8 Oct 2026 16:29:05 -0700 Subject: [PATCH 2/5] fix(search): refuse dot-prefixed recursive root globs --- src/plugins/search-files-fail-closed.test.ts | 38 ++++++++++++++++++++ src/plugins/tool-time-budget.ts | 20 +++++++---- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/plugins/search-files-fail-closed.test.ts b/src/plugins/search-files-fail-closed.test.ts index e75af95e1..f7cf1f9de 100644 --- a/src/plugins/search-files-fail-closed.test.ts +++ b/src/plugins/search-files-fail-closed.test.ts @@ -195,6 +195,27 @@ describe("CL-9469 search_files fails closed on unbounded root walks", () => { } }); }); + + test("root dot-relative recursive glob is refused, not walked", async () => { + await withFixture(async ({ cwd }) => { + const spawnCalls: string[][] = []; + const tools = createPosixTools({ + cwd, + plugins: [ripgrepPlugin(cwd, {}, throwingSpawn(spawnCalls))], + }); + const started = Date.now(); + const result = await tools.run( + { id: "1", name: "search_files", arguments: { pattern: "./**/*.ts" } }, + new AbortController().signal, + ); + expect(Date.now() - started).toBeLessThan(5_000); + expect(spawnCalls.length).toBe(0); + expect(result.isError).toBe(true); + expect(String(result.content)).toContain("narrow `path`"); + expect(String(result.content)).toContain("glob"); + expect(String(result.content)).not.toContain(TIMEOUT_PREFIX); + }); + }); }); describe("CL-9469 unbounded-root predicate", () => { @@ -238,6 +259,8 @@ describe("CL-9469 unbounded-root predicate", () => { "src/**/*.ts", "packages/a/src/**/*.ts", "docs/**", + "sub/**", + "src.v2/**", ]) { expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe( false, @@ -251,6 +274,21 @@ describe("CL-9469 unbounded-root predicate", () => { ); } }); + // F1 soundness hole: `.`/`..` are relative-notation, not literal pins, so a + // leading dot segment must not whitelist a root walk (`./**`, `./**/*.ts`) + // and `..` must not be readable as an anchored workspace-escape. + test("leading dot or dotdot is not a literal pin", () => { + for (const pattern of [ + "./**", + "./**/*.ts", + "./src/**", + "../**", + ]) { + expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe( + true, + ); + } + }); test("refusal message carries scope guidance and never reads as a timeout", () => { const message = formatUnboundedSearchMessage("search_files", "**/*.ts"); expect(message).toContain("narrow `path`"); diff --git a/src/plugins/tool-time-budget.ts b/src/plugins/tool-time-budget.ts index 4597e1c80..22f0a2472 100644 --- a/src/plugins/tool-time-budget.ts +++ b/src/plugins/tool-time-budget.ts @@ -42,14 +42,22 @@ export function isUnboundedSearchGlob(pattern: string): boolean { return !hasLiteralBefore(pattern.slice(0, firstRecursive)); } -// True when the given glob prefix names any concrete path text (a character -// that is neither a `*` wildcard nor a `/` separator), i.e. a segment that -// anchors the walk to a real location. +// True when the leading path component of the glob is a literal pin: a real, +// name-bearing directory segment that anchors the walk away from the workspace +// root. A `.`/`..` step or a separator-only prefix is relative-notation that +// does not pin to a concrete boundary (``./src/**`` is still an unbounded +// root-relative walk), and a bare wildcard run is never a literal name. function hasLiteralBefore(prefix: string): boolean { - for (const char of prefix) { - if (char !== "*" && char !== "/") return true; + for (const segment of prefix.split("/")) { + if (segment.length === 0) continue; // leading/trailing/duplicate separator + if (segment === "." || segment === "..") return false; // relative step + for (const char of segment) { + if (char !== "*") return true; // genuine name character anchors + } + // The segment is all wildcards; a later segment may still name a real + // directory (`*/src/**`), so keep scanning. } - return false; + return false; // nothing but separators and/or wildcard runs } // The raw path argument resolved against the session root: omitted, empty, From 253ff9c0236021f0a9d6c98770c996f1961fa813 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 8 Oct 2026 16:32:38 -0700 Subject: [PATCH 3/5] fix(search): normalize interior dotdot in glob pin detection --- src/plugins/search-files-fail-closed.test.ts | 53 ++++++++++++++++++++ src/plugins/tool-time-budget.ts | 46 ++++++++++++----- 2 files changed, 87 insertions(+), 12 deletions(-) diff --git a/src/plugins/search-files-fail-closed.test.ts b/src/plugins/search-files-fail-closed.test.ts index f7cf1f9de..94dc337b8 100644 --- a/src/plugins/search-files-fail-closed.test.ts +++ b/src/plugins/search-files-fail-closed.test.ts @@ -216,6 +216,31 @@ describe("CL-9469 search_files fails closed on unbounded root walks", () => { expect(String(result.content)).not.toContain(TIMEOUT_PREFIX); }); }); + + test("interior-dotdot collapse to the root is refused, not walked", async () => { + await withFixture(async ({ cwd }) => { + const spawnCalls: string[][] = []; + const tools = createPosixTools({ + cwd, + plugins: [ripgrepPlugin(cwd, {}, throwingSpawn(spawnCalls))], + }); + const started = Date.now(); + const result = await tools.run( + { + id: "1", + name: "search_files", + arguments: { pattern: "a/../**/*.ts" }, + }, + new AbortController().signal, + ); + expect(Date.now() - started).toBeLessThan(5_000); + expect(spawnCalls.length).toBe(0); + expect(result.isError).toBe(true); + expect(String(result.content)).toContain("narrow `path`"); + expect(String(result.content)).toContain("glob"); + expect(String(result.content)).not.toContain(TIMEOUT_PREFIX); + }); + }); }); describe("CL-9469 unbounded-root predicate", () => { @@ -283,11 +308,39 @@ describe("CL-9469 unbounded-root predicate", () => { "./**/*.ts", "./src/**", "../**", + "../src/**", + ]) { + expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe( + true, + ); + } + }); + // F3 soundness hole: interior `..` collapse must be normalized, not read + // verbatim. `a/../**` collapses to `./**` (root-wide walk), `src/../../**` + // pops above the workspace root (escape), and `./a/../**` is still root + // relative. Only a prefix that collapses to a surviving name-bearing pin + // (`src/../packages/**` -> `packages/**`) stays bounded. + test("interior dotdot collapse is normalized, so the pin must survive", () => { + for (const pattern of [ + "a/../**", + "src/../../**", + "./a/../**", + "a/../**/*.ts", ]) { expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe( true, ); } + for (const pattern of [ + "a/./**", + "a//**", + "src/../packages/**", + "*/src/**", + ]) { + expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe( + false, + ); + } }); test("refusal message carries scope guidance and never reads as a timeout", () => { const message = formatUnboundedSearchMessage("search_files", "**/*.ts"); diff --git a/src/plugins/tool-time-budget.ts b/src/plugins/tool-time-budget.ts index 22f0a2472..9b7cd1dca 100644 --- a/src/plugins/tool-time-budget.ts +++ b/src/plugins/tool-time-budget.ts @@ -42,22 +42,44 @@ export function isUnboundedSearchGlob(pattern: string): boolean { return !hasLiteralBefore(pattern.slice(0, firstRecursive)); } -// True when the leading path component of the glob is a literal pin: a real, -// name-bearing directory segment that anchors the walk away from the workspace -// root. A `.`/`..` step or a separator-only prefix is relative-notation that -// does not pin to a concrete boundary (``./src/**`` is still an unbounded -// root-relative walk), and a bare wildcard run is never a literal name. +// True when the pre-`**` prefix collapses to a real, name-bearing directory +// that anchors the walk away from the workspace root. The prefix is split on +// `/` and collapsed left-to-right on a stack: `.` and empty segments are +// skipped and `..` pops the preceding segment. A segment of only `*` wildcards +// is pushed as a non-name pin (it occupies a slot but cannot anchor the walk); +// the first surviving name-bearing segment is the pin. A prefix whose collapse +// runs the stack empty (the pattern reaches the root, e.g. `a/../**`) or where +// a `..` pops above the root (`src/../../**` escapes the workspace) leaves no +// literal pin, so the recursion is a whole-tree walk. A leading separator or +// dot-relative step (`/`, `./src/**`) likewise never pins to a real boundary. function hasLiteralBefore(prefix: string): boolean { - for (const segment of prefix.split("/")) { - if (segment.length === 0) continue; // leading/trailing/duplicate separator - if (segment === "." || segment === "..") return false; // relative step + const segments = prefix.split("/"); + // A leading separator, dot, or dotdot step is root-relative notation that + // does not pin to a concrete boundary, even when a name follows (`./src/**` + // still walks from the root). `*/src/**` is unaffected: `*` is not a `..`. + if (segments[0] === "" || segments[0] === "." || segments[0] === "..") { + return false; + } + // Stack of surviving segments (true = name-bearing pin). `.`/empty are + // skipped; `..` pops the top; all-wildcard runs push a non-name slot. + const stack: boolean[] = []; + for (const segment of segments) { + if (segment.length === 0 || segment === ".") continue; + if (segment === "..") { + if (stack.length === 0) return false; // pop above the workspace root + stack.pop(); + continue; + } + let isName = false; for (const char of segment) { - if (char !== "*") return true; // genuine name character anchors + if (char !== "*") { + isName = true; + break; + } } - // The segment is all wildcards; a later segment may still name a real - // directory (`*/src/**`), so keep scanning. + stack.push(isName); } - return false; // nothing but separators and/or wildcard runs + return stack.some(Boolean); // at least one name-bearing pin survives } // The raw path argument resolved against the session root: omitted, empty, From 131bec72a31f8e7787bceebf313d5ad8fcd1c5df Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 8 Oct 2026 20:20:59 -0700 Subject: [PATCH 4/5] fix(search): simplify literal-dir pin detection Replace the stack-collapse hasLiteralBefore with a terse fold over the collapsed prefix segments, and trim over-verbose test comments. --- src/plugins/search-files-fail-closed.test.ts | 12 ++----- src/plugins/tool-time-budget.ts | 37 +++++--------------- 2 files changed, 11 insertions(+), 38 deletions(-) diff --git a/src/plugins/search-files-fail-closed.test.ts b/src/plugins/search-files-fail-closed.test.ts index 94dc337b8..2f8a4d683 100644 --- a/src/plugins/search-files-fail-closed.test.ts +++ b/src/plugins/search-files-fail-closed.test.ts @@ -277,8 +277,6 @@ describe("CL-9469 unbounded-root predicate", () => { true, ); }); - // A recursive descent is only a whole-tree walk when nothing literal pins it - // down; a leading literal dir keeps it bounded even at the workspace root. test("recursive glob with a leading literal dir is bounded", () => { for (const pattern of [ "src/**/*.ts", @@ -299,9 +297,7 @@ describe("CL-9469 unbounded-root predicate", () => { ); } }); - // F1 soundness hole: `.`/`..` are relative-notation, not literal pins, so a - // leading dot segment must not whitelist a root walk (`./**`, `./**/*.ts`) - // and `..` must not be readable as an anchored workspace-escape. + // A leading `.`/`..` is relative notation, not a literal pin. test("leading dot or dotdot is not a literal pin", () => { for (const pattern of [ "./**", @@ -315,11 +311,7 @@ describe("CL-9469 unbounded-root predicate", () => { ); } }); - // F3 soundness hole: interior `..` collapse must be normalized, not read - // verbatim. `a/../**` collapses to `./**` (root-wide walk), `src/../../**` - // pops above the workspace root (escape), and `./a/../**` is still root - // relative. Only a prefix that collapses to a surviving name-bearing pin - // (`src/../packages/**` -> `packages/**`) stays bounded. + // Interior `..` collapses first, so the pin must survive the fold. test("interior dotdot collapse is normalized, so the pin must survive", () => { for (const pattern of [ "a/../**", diff --git a/src/plugins/tool-time-budget.ts b/src/plugins/tool-time-budget.ts index 9b7cd1dca..bb503cd8f 100644 --- a/src/plugins/tool-time-budget.ts +++ b/src/plugins/tool-time-budget.ts @@ -42,44 +42,25 @@ export function isUnboundedSearchGlob(pattern: string): boolean { return !hasLiteralBefore(pattern.slice(0, firstRecursive)); } -// True when the pre-`**` prefix collapses to a real, name-bearing directory -// that anchors the walk away from the workspace root. The prefix is split on -// `/` and collapsed left-to-right on a stack: `.` and empty segments are -// skipped and `..` pops the preceding segment. A segment of only `*` wildcards -// is pushed as a non-name pin (it occupies a slot but cannot anchor the walk); -// the first surviving name-bearing segment is the pin. A prefix whose collapse -// runs the stack empty (the pattern reaches the root, e.g. `a/../**`) or where -// a `..` pops above the root (`src/../../**` escapes the workspace) leaves no -// literal pin, so the recursion is a whole-tree walk. A leading separator or -// dot-relative step (`/`, `./src/**`) likewise never pins to a real boundary. +// True when the pre-`**` prefix names a real directory that anchors the walk +// away from the root. Leading root-relative notation (`.`/`..`/empty) does not +// pin; interior `.`/`..`/empty segments collapse first, so only a surviving +// name segment counts. function hasLiteralBefore(prefix: string): boolean { const segments = prefix.split("/"); - // A leading separator, dot, or dotdot step is root-relative notation that - // does not pin to a concrete boundary, even when a name follows (`./src/**` - // still walks from the root). `*/src/**` is unaffected: `*` is not a `..`. if (segments[0] === "" || segments[0] === "." || segments[0] === "..") { return false; } - // Stack of surviving segments (true = name-bearing pin). `.`/empty are - // skipped; `..` pops the top; all-wildcard runs push a non-name slot. - const stack: boolean[] = []; + const names: string[] = []; for (const segment of segments) { - if (segment.length === 0 || segment === ".") continue; + if (segment === "" || segment === ".") continue; if (segment === "..") { - if (stack.length === 0) return false; // pop above the workspace root - stack.pop(); + names.pop(); continue; } - let isName = false; - for (const char of segment) { - if (char !== "*") { - isName = true; - break; - } - } - stack.push(isName); + names.push(segment); } - return stack.some(Boolean); // at least one name-bearing pin survives + return names.some((segment) => [...segment].some((char) => char !== "*")); } // The raw path argument resolved against the session root: omitted, empty, From 7dfc196544555a6f1680f9c4875e5dc8235ea4a4 Mon Sep 17 00:00:00 2001 From: "user.email" Date: Thu, 8 Oct 2026 23:08:58 -0700 Subject: [PATCH 5/5] test(search): pin interior-dotdot-after-pin glob edge case --- src/plugins/search-files-fail-closed.test.ts | 28 ++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/plugins/search-files-fail-closed.test.ts b/src/plugins/search-files-fail-closed.test.ts index 2f8a4d683..1720d5d9f 100644 --- a/src/plugins/search-files-fail-closed.test.ts +++ b/src/plugins/search-files-fail-closed.test.ts @@ -334,6 +334,34 @@ describe("CL-9469 unbounded-root predicate", () => { ); } }); + // A `..` after the first `**` is never collapsed: hasLiteralBefore only folds + // the prefix *before* the first `**`, so a `..` in unanchored recursive + // descent does not pop the pin. And because rg is cwd-anchored, `a/**/../**` + // descends from `a` and steps back up a relative level — it cannot escape the + // workspace root (there is no `..` above the anchor), so it stays a bounded + // subtree walk of `a`. + test("interior `..` after the pin is a bounded cwd-anchored descent", () => { + for (const pattern of [ + "a/**/../**", + "src/**/../**", + "sub/**/../**/*.ts", + "docs/**/../**", + "a/src/**/../**", + "a/**/../src/**", + ]) { + expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe( + false, + ); + } + // An all-wildcard pin followed by `..` leaves no name to anchor, so it is + // still an unbounded root walk; a `../../` that folds the prefix to the + // root is refused (already covered as "collapse to the root"). + for (const pattern of ["*/**/../**", "a/src/../../**"]) { + expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe( + true, + ); + } + }); test("refusal message carries scope guidance and never reads as a timeout", () => { const message = formatUnboundedSearchMessage("search_files", "**/*.ts"); expect(message).toContain("narrow `path`");