Skip to content

Commit 19d59bb

Browse files
Merge pull request #1366 from corbitsdev/cl-10240-fixsearch-allow-recursive-globs-pinned-to-a-literal-dir
fix(search): allow recursive globs pinned to a literal dir
2 parents ac2667d + 7dfc196 commit 19d59bb

2 files changed

Lines changed: 186 additions & 4 deletions

File tree

‎src/plugins/search-files-fail-closed.test.ts‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,74 @@ describe("CL-9469 search_files fails closed on unbounded root walks", () => {
173173
}
174174
});
175175
});
176+
177+
test("recursive glob pinned to a literal dir at the root is allowed", async () => {
178+
await withFixture(async ({ cwd }) => {
179+
for (const spawn of [undefined, rgMissingSpawn()] as const) {
180+
const tools = createPosixTools({
181+
cwd,
182+
plugins: [ripgrepPlugin(cwd, {}, spawn)],
183+
});
184+
const result = await tools.run(
185+
{
186+
id: "1",
187+
name: "search_files",
188+
arguments: { pattern: "sub/**/*.ts" },
189+
},
190+
new AbortController().signal,
191+
);
192+
expect(result.isError !== true).toBe(true);
193+
expect(String(result.content)).toContain("keep.ts");
194+
expect(String(result.content)).toContain("deep.ts");
195+
}
196+
});
197+
});
198+
199+
test("root dot-relative recursive glob is refused, not walked", async () => {
200+
await withFixture(async ({ cwd }) => {
201+
const spawnCalls: string[][] = [];
202+
const tools = createPosixTools({
203+
cwd,
204+
plugins: [ripgrepPlugin(cwd, {}, throwingSpawn(spawnCalls))],
205+
});
206+
const started = Date.now();
207+
const result = await tools.run(
208+
{ id: "1", name: "search_files", arguments: { pattern: "./**/*.ts" } },
209+
new AbortController().signal,
210+
);
211+
expect(Date.now() - started).toBeLessThan(5_000);
212+
expect(spawnCalls.length).toBe(0);
213+
expect(result.isError).toBe(true);
214+
expect(String(result.content)).toContain("narrow `path`");
215+
expect(String(result.content)).toContain("glob");
216+
expect(String(result.content)).not.toContain(TIMEOUT_PREFIX);
217+
});
218+
});
219+
220+
test("interior-dotdot collapse to the root is refused, not walked", async () => {
221+
await withFixture(async ({ cwd }) => {
222+
const spawnCalls: string[][] = [];
223+
const tools = createPosixTools({
224+
cwd,
225+
plugins: [ripgrepPlugin(cwd, {}, throwingSpawn(spawnCalls))],
226+
});
227+
const started = Date.now();
228+
const result = await tools.run(
229+
{
230+
id: "1",
231+
name: "search_files",
232+
arguments: { pattern: "a/../**/*.ts" },
233+
},
234+
new AbortController().signal,
235+
);
236+
expect(Date.now() - started).toBeLessThan(5_000);
237+
expect(spawnCalls.length).toBe(0);
238+
expect(result.isError).toBe(true);
239+
expect(String(result.content)).toContain("narrow `path`");
240+
expect(String(result.content)).toContain("glob");
241+
expect(String(result.content)).not.toContain(TIMEOUT_PREFIX);
242+
});
243+
});
176244
});
177245

178246
describe("CL-9469 unbounded-root predicate", () => {
@@ -209,6 +277,91 @@ describe("CL-9469 unbounded-root predicate", () => {
209277
true,
210278
);
211279
});
280+
test("recursive glob with a leading literal dir is bounded", () => {
281+
for (const pattern of [
282+
"src/**/*.ts",
283+
"packages/a/src/**/*.ts",
284+
"docs/**",
285+
"sub/**",
286+
"src.v2/**",
287+
]) {
288+
expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe(
289+
false,
290+
);
291+
}
292+
});
293+
test("recursive descent with no leading literal dir stays unbounded", () => {
294+
for (const pattern of ["**", "**/*.ts", "**/*", "/**/*.ts"]) {
295+
expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe(
296+
true,
297+
);
298+
}
299+
});
300+
// A leading `.`/`..` is relative notation, not a literal pin.
301+
test("leading dot or dotdot is not a literal pin", () => {
302+
for (const pattern of [
303+
"./**",
304+
"./**/*.ts",
305+
"./src/**",
306+
"../**",
307+
"../src/**",
308+
]) {
309+
expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe(
310+
true,
311+
);
312+
}
313+
});
314+
// Interior `..` collapses first, so the pin must survive the fold.
315+
test("interior dotdot collapse is normalized, so the pin must survive", () => {
316+
for (const pattern of [
317+
"a/../**",
318+
"src/../../**",
319+
"./a/../**",
320+
"a/../**/*.ts",
321+
]) {
322+
expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe(
323+
true,
324+
);
325+
}
326+
for (const pattern of [
327+
"a/./**",
328+
"a//**",
329+
"src/../packages/**",
330+
"*/src/**",
331+
]) {
332+
expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe(
333+
false,
334+
);
335+
}
336+
});
337+
// A `..` after the first `**` is never collapsed: hasLiteralBefore only folds
338+
// the prefix *before* the first `**`, so a `..` in unanchored recursive
339+
// descent does not pop the pin. And because rg is cwd-anchored, `a/**/../**`
340+
// descends from `a` and steps back up a relative level — it cannot escape the
341+
// workspace root (there is no `..` above the anchor), so it stays a bounded
342+
// subtree walk of `a`.
343+
test("interior `..` after the pin is a bounded cwd-anchored descent", () => {
344+
for (const pattern of [
345+
"a/**/../**",
346+
"src/**/../**",
347+
"sub/**/../**/*.ts",
348+
"docs/**/../**",
349+
"a/src/**/../**",
350+
"a/**/../src/**",
351+
]) {
352+
expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe(
353+
false,
354+
);
355+
}
356+
// An all-wildcard pin followed by `..` leaves no name to anchor, so it is
357+
// still an unbounded root walk; a `../../` that folds the prefix to the
358+
// root is refused (already covered as "collapse to the root").
359+
for (const pattern of ["*/**/../**", "a/src/../../**"]) {
360+
expect(isUnboundedRootSearch({ path: undefined, pattern, cwd })).toBe(
361+
true,
362+
);
363+
}
364+
});
212365
test("refusal message carries scope guidance and never reads as a timeout", () => {
213366
const message = formatUnboundedSearchMessage("search_files", "**/*.ts");
214367
expect(message).toContain("narrow `path`");

‎src/plugins/tool-time-budget.ts‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,40 @@ export function formatSearchTimeoutMessage(
2727
return `${trimmed}\n\n${notice}`;
2828
}
2929

30-
// A search_files pattern that walks the whole tree: recursive descent or a
31-
// bare star with no literal constraint. Tighter globs (`*.ts`, `*config*`)
32-
// still match a bounded name space and stay allowed at the root.
30+
// A search_files pattern that has no bound on where a match can begin is a
31+
// whole-tree walk: bare stars, or a recursive descent that starts at the root
32+
// with nothing literal before it. A literal path segment before the first `**`
33+
// (`src/**/*.ts`) pins the walk to that subtree, so it is bounded even though
34+
// it recurses. Tighter globs (`*.ts`, `*config*`) still match a bounded name
35+
// space and stay allowed at the root.
3336
export function isUnboundedSearchGlob(pattern: string): boolean {
34-
return pattern === "*" || pattern.includes("**");
37+
if (pattern === "*") return true;
38+
const firstRecursive = pattern.indexOf("**");
39+
if (firstRecursive === -1) return false;
40+
// Everything before the first `**` is only wildcard descent and separators,
41+
// so the walk still reaches every file from the root.
42+
return !hasLiteralBefore(pattern.slice(0, firstRecursive));
43+
}
44+
45+
// True when the pre-`**` prefix names a real directory that anchors the walk
46+
// away from the root. Leading root-relative notation (`.`/`..`/empty) does not
47+
// pin; interior `.`/`..`/empty segments collapse first, so only a surviving
48+
// name segment counts.
49+
function hasLiteralBefore(prefix: string): boolean {
50+
const segments = prefix.split("/");
51+
if (segments[0] === "" || segments[0] === "." || segments[0] === "..") {
52+
return false;
53+
}
54+
const names: string[] = [];
55+
for (const segment of segments) {
56+
if (segment === "" || segment === ".") continue;
57+
if (segment === "..") {
58+
names.pop();
59+
continue;
60+
}
61+
names.push(segment);
62+
}
63+
return names.some((segment) => [...segment].some((char) => char !== "*"));
3564
}
3665

3766
// The raw path argument resolved against the session root: omitted, empty,

0 commit comments

Comments
 (0)