Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ export type FocusManifestGuidance = {

const MAX_LIST_ITEMS = 200;
const MAX_ITEM_LENGTH = 300;
const MAX_GLOBSTAR_SLASH_ALTERNATIVES = 128;
export const MAX_FOCUS_MANIFEST_BYTES = 64 * 1024;

const EMPTY_GATE_CONFIG: FocusManifestGateConfig = {
Expand Down Expand Up @@ -913,14 +914,36 @@ function linearGlobMatcher(pattern: string): (path: string) => boolean {
* Compiling once lets a caller test many paths against one pattern without recompiling per path — see
* {@link matchedPatterns}. An empty/blank pattern never matches.
*/
function expandGlobstarSlash(pattern: string): string[] {
const alternatives = [""];
for (let idx = 0; idx < pattern.length; ) {
if (pattern.startsWith("**/", idx)) {
const count = alternatives.length;
const canKeepRootAlternatives = count * 2 <= MAX_GLOBSTAR_SLASH_ALTERNATIVES;
for (let altIdx = count - 1; altIdx >= 0; altIdx -= 1) {
const prefix = alternatives[altIdx]!;
alternatives[altIdx] = `${prefix}*/`;
if (canKeepRootAlternatives) alternatives.push(prefix);
}
idx += 3;
continue;
}
for (let altIdx = 0; altIdx < alternatives.length; altIdx += 1) alternatives[altIdx] += pattern[idx]!;
idx += 1;
}
return alternatives;
}

function compileManifestPathMatcher(pattern: string): (normalizedPath: string) => boolean {
const normalizedPattern = normalizePathForMatch(pattern);
if (!normalizedPattern) return () => false;
if (normalizedPattern.includes("*")) {
// A double-star-then-slash run collapses the mandatory separator into the wildcard so the glob matches
// zero-depth/root too (e.g. a leading double-star glob matches a root-level file). Then run the linear matcher.
const globbed = normalizedPattern.replace(/\*\*\//g, "*");
return linearGlobMatcher(globbed);
// `**/` means zero or more whole path segments. Keep the slash in the non-root alternative so
// basename globs (e.g. `**/safe.ts`) do not degrade into suffix globs that match `unsafe.ts`.
const matchers = expandGlobstarSlash(normalizedPattern).map((globbed) =>
globbed.includes("*") ? linearGlobMatcher(globbed) : (normalizedPath: string) => normalizedPath === globbed,
);
return (normalizedPath) => matchers.some((matcher) => matcher(normalizedPath));
}
const dirPattern = normalizedPattern.endsWith("/") ? normalizedPattern : `${normalizedPattern}/`;
return (normalizedPath) => normalizedPath === normalizedPattern || normalizedPath.startsWith(dirPattern);
Expand Down
25 changes: 24 additions & 1 deletion test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,15 @@ describe("matchesManifestPath", () => {
expect(matchesManifestPath("a/b/c.ts", "**/*.ts")).toBe(true);
});

it("keeps **/ on path-segment boundaries instead of broad suffix matching (#review-audit)", () => {
expect(matchesManifestPath("safe.ts", "**/safe.ts")).toBe(true);
expect(matchesManifestPath("dir/safe.ts", "**/safe.ts")).toBe(true);
expect(matchesManifestPath("unsafe.ts", "**/safe.ts")).toBe(false);
expect(matchesManifestPath("src/safe.ts", "src/**/safe.ts")).toBe(true);
expect(matchesManifestPath("src/dir/safe.ts", "src/**/safe.ts")).toBe(true);
expect(matchesManifestPath("src/unsafe.ts", "src/**/safe.ts")).toBe(false);
});

it("multi-wildcard matching is correct (ordered substrings, suffix cannot overlap)", () => {
expect(matchesManifestPath("xayybzzc", "*a*b*c")).toBe(true);
expect(matchesManifestPath("aXbXc", "a*b*c")).toBe(true);
Expand All @@ -212,6 +221,15 @@ describe("matchesManifestPath", () => {
expect(result).toBe(false);
expect(elapsed).toBeLessThan(100); // the old per-star regex did not return within 30s on this input
});

it("bounds repeated **/ expansion while retaining linear matching (#review-audit)", () => {
const globstarRun = "**/".repeat(20) + "safe.ts";
const start = performance.now();
const result = matchesManifestPath("a/b/c/safe.ts", globstarRun);
const elapsed = performance.now() - start;
expect(result).toBe(false);
expect(elapsed).toBeLessThan(100);
});
});

// Regression tests for the three compileManifestPathMatcher branches: exact,
Expand Down Expand Up @@ -1204,11 +1222,16 @@ describe("review.exclude_paths (#review-exclude-paths)", () => {

it("excludeReviewPaths filters matching files; empty globs return the same array (byte-identical)", () => {
const files = [{ path: "src/a.ts" }, { path: "pnpm-lock.yaml" }, { path: "dist/bundle.js" }];
// `*` collapses to `.*` (crosses slashes), so `*.yaml` matches a top-level lockfile; `dist/**` matches under dist/.
// `*` crosses slashes, so `*.yaml` matches a top-level lockfile; `dist/**` matches under dist/.
expect(excludeReviewPaths(files, ["*.yaml", "dist/**"])).toEqual([{ path: "src/a.ts" }]);
expect(excludeReviewPaths(files, ["docs/**"])).toEqual(files); // no match → unchanged
expect(excludeReviewPaths(files, [])).toBe(files); // empty → same reference (no-op)
});

it("does not exclude attacker-named suffix collisions for **/ basename globs (#review-audit)", () => {
const files = [{ path: "unsafe.ts" }, { path: "dir/safe.ts" }, { path: "feature.ts" }];
expect(excludeReviewPaths(files, ["**/safe.ts"])).toEqual([{ path: "unsafe.ts" }, { path: "feature.ts" }]);
});
});

describe("review.pre_merge_checks (#review-pre-merge-checks)", () => {
Expand Down
Loading