Skip to content
Closed
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
10 changes: 7 additions & 3 deletions src/signals/change-guardrail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,18 @@ function canonicalize(value: string): string {
// matchesAny below) so every caller — present or future, direct or indirect, including
// content-lane/spec-resolver.ts's config-driven globs — is protected automatically. A caller with a STRICTER
// fail direction (e.g. matchesAny's fail-toward-guarding for a security guardrail) overrides at its own layer.
const MAX_GLOB_WILDCARD_GROUPS = 2;
// Exported so any OTHER glob-safety check in this codebase (e.g. focus-manifest.ts's parse-time
// normalizeOptionalGlob, which rejects an over-complex contentLane.*Glob before it ever reaches globToRegExp)
// shares this exact threshold instead of drifting with its own re-derived number.
export const MAX_GLOB_WILDCARD_GROUPS = 2;

/** Count `*` GROUPS in `glob` — a `**` pair is ONE group (it compiles to a single `.*`, see globToRegExp), not
* two. Mirrors globToRegExp's own tokenization exactly (including consuming a `**`'s trailing `/`) so the count
* reflects the actual number of backtracking-capable groups the compiled RegExp will contain, not raw `*`
* character count (which would double-count every globstar and reject legitimate globs like
* "public/**\/*.json" — 2 real groups — as if they were 3-groups-dangerous). */
function countWildcardGroups(glob: string): number {
* "public/**\/*.json" — 2 real groups — as if they were 3-groups-dangerous). Exported for reuse by any other
* glob-safety check in this codebase — see MAX_GLOB_WILDCARD_GROUPS above. */
export function countWildcardGroups(glob: string): number {
let count = 0;
for (let i = 0; i < glob.length; i += 1) {
if (glob.charAt(i) !== "*") continue;
Expand Down
28 changes: 12 additions & 16 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { GatePolicyPack, GateRuleMode, JsonValue, RepositorySettings } from
import { normalizeAutonomyPolicy, normalizeAutoMaintainPolicy } from "../settings/autonomy";
import { normalizeCommandAuthorizationPolicy } from "../settings/command-authorization";
import { mergeContributorBlacklists, normalizeContributorBlacklist } from "../settings/contributor-blacklist";
import { countWildcardGroups, MAX_GLOB_WILDCARD_GROUPS } from "./change-guardrail";
import { PUBLIC_LOCAL_PATH_INLINE } from "./redaction";

export type FocusManifestSource = "repo_file" | "api_record" | "none";
Expand Down Expand Up @@ -618,30 +619,25 @@ function normalizeOptionalPositiveInteger(value: JsonValue | undefined, field: s
return null;
}

// A glob compiled to RegExp (review/content-lane/spec-resolver.ts's globToRegExp reuse of the guardrail-path
// compiler) chains a `[^/]*` per `*` — MULTIPLE chained wildcards separated by literal characters can
// catastrophically backtrack on an adversarial near-miss input (verified empirically: 5 chained wildcards against
// a maximally-adversarial 300-char input took ~19 SECONDS; 3 stays under 5ms even at that same length). No
// legitimate single-purpose entry-file glob for this feature needs more than a couple of wildcards, so this caps
// wildcard count at parse time — well before the string ever reaches RegExp compilation — rather than trying to
// make the compiled pattern itself provably safe.
const MAX_GLOB_WILDCARDS = 3;

/** Normalize + bound a maintainer-supplied glob string: trims/length-caps like any other string field, AND caps
* the number of `*` wildcard characters (see MAX_GLOB_WILDCARDS) so it can never compile into a
* catastrophically-backtracking RegExp downstream. A glob over the cap is REJECTED (warns, returns null) rather
* than truncated — silently cutting wildcards out of a maintainer's pattern would silently change its meaning,
* which is worse than making them fix an over-complex glob. */
* the number of wildcard GROUPS (see change-guardrail.ts's MAX_GLOB_WILDCARD_GROUPS / countWildcardGroups —
* the SAME group-based count globToRegExp itself enforces, reused here rather than re-derived, so this file
* can never drift from the actual safe boundary) so it can never compile into a catastrophically-backtracking
* RegExp downstream. A `**` pair is ONE group, not two raw `*` characters — counting raw characters would both
* wrongly reject a legitimate 2-group glob like "public/**\/*.json" (3 characters, 2 groups) and wrongly admit
* a genuinely dangerous 3-single-star glob at the same raw-character count. A glob over the cap is REJECTED
* (warns, returns null) rather than truncated — silently cutting wildcards out of a maintainer's pattern would
* silently change its meaning, which is worse than making them fix an over-complex glob. */
function normalizeOptionalGlob(value: JsonValue | undefined, field: string, warnings: string[]): string | null {
const normalized = normalizeOptionalString(value, field, warnings);
if (normalized === null) return null;
if (normalized.length > MAX_ITEM_LENGTH) {
warnings.push(`Manifest field "${field}" truncated an over-long glob.`);
}
const bounded = normalized.slice(0, MAX_ITEM_LENGTH);
const wildcardCount = (bounded.match(/\*/g) ?? []).length;
if (wildcardCount > MAX_GLOB_WILDCARDS) {
warnings.push(`Manifest field "${field}" has too many wildcards (${wildcardCount} > ${MAX_GLOB_WILDCARDS}); ignoring it.`);
const wildcardGroupCount = countWildcardGroups(bounded);
if (wildcardGroupCount > MAX_GLOB_WILDCARD_GROUPS) {
warnings.push(`Manifest field "${field}" has too many wildcards (${wildcardGroupCount} > ${MAX_GLOB_WILDCARD_GROUPS}); ignoring it.`);
return null;
}
return bounded;
Expand Down
24 changes: 16 additions & 8 deletions test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1090,26 +1090,34 @@ describe("parseFocusManifest gate config", () => {
expect(m.warnings.some((w) => /contentLane\.entryFileGlob.*truncated/.test(w))).toBe(true);
});

it("SECURITY (ReDoS): a glob with too many wildcards is REJECTED at parse time rather than ever reaching RegExp compilation", () => {
// 5 chained single-segment wildcards is empirically catastrophic against an adversarial input (verified
// ~19s in manual testing) — must never survive parsing to reach globToRegExp at all.
it("SECURITY (ReDoS): a glob with too many wildcard GROUPS is REJECTED at parse time rather than ever reaching RegExp compilation", () => {
// Shares change-guardrail.ts's countWildcardGroups/MAX_GLOB_WILDCARD_GROUPS — see that file for the
// benchmark proving 3+ chained wildcard groups risk catastrophic backtracking. 5 single-segment groups here
// is well over the cap; must never survive parsing to reach globToRegExp at all.
const pathological = "registry/*-*-*-*-*-final.json";
const m = parseFocusManifest({ contentLane: { entryFileGlob: pathological, collectionField: "items" } });
expect(m.contentLane.entryFileGlob).toBeNull();
expect(m.contentLane.present).toBe(false); // entryFileGlob is REQUIRED — a rejected glob degrades to absent
expect(m.warnings.some((w) => /contentLane\.entryFileGlob.*too many wildcards/.test(w))).toBe(true);
// A glob AT the cap (3 wildcards) is accepted; the optional providerFileGlob/artifactGlob fields are dropped
// individually (with a warning) without invalidating the whole block, since only entryFileGlob/collectionField
// are required.
// A glob AT the cap (2 wildcard groups) is accepted; the optional providerFileGlob/artifactGlob fields are
// dropped individually (with a warning) without invalidating the whole block, since only
// entryFileGlob/collectionField are required.
const atCap = parseFocusManifest({
contentLane: { entryFileGlob: "registry/*/*/*.json", providerFileGlob: "providers/*-*-*-*-*.json", collectionField: "items" },
contentLane: { entryFileGlob: "registry/*/*.json", providerFileGlob: "providers/*-*-*-*-*.json", collectionField: "items" },
});
expect(atCap.contentLane.present).toBe(true);
expect(atCap.contentLane.entryFileGlob).toBe("registry/*/*/*.json");
expect(atCap.contentLane.entryFileGlob).toBe("registry/*/*.json");
expect(atCap.contentLane.providerFileGlob).toBeNull();
expect(atCap.warnings.some((w) => /contentLane\.providerFileGlob.*too many wildcards/.test(w))).toBe(true);
});

it("REGRESSION: a `**` globstar plus a single `*` — 3 raw star CHARACTERS but only 2 wildcard GROUPS — is accepted, not rejected (counting raw characters instead of groups would wrongly reject this legitimate shape)", () => {
const m = parseFocusManifest({ contentLane: { entryFileGlob: "public/**/*.json", collectionField: "items" } });
expect(m.contentLane.present).toBe(true);
expect(m.contentLane.entryFileGlob).toBe("public/**/*.json");
expect(m.warnings.some((w) => /entryFileGlob.*too many wildcards/.test(w))).toBe(false);
});

it("contentLaneConfigToJson returns null for an absent config, and omits unset optional fields", () => {
expect(contentLaneConfigToJson(parseFocusManifest(null).contentLane)).toBeNull();
const m = parseFocusManifest({ contentLane: { entryFileGlob: "registry/*.json", collectionField: "items" } });
Expand Down