Skip to content

Commit 509ba5e

Browse files
committed
fix(engine): include inputTokenPattern in deny-hook ruleSignature identity
ruleSignature (the identity function resolveEffectiveDenyRules and synthesizeDenyRuleProposals use to decide "is this the same rule") omitted DenyRule's inputTokenPattern field, so a maintainer-approved rule differing from an existing one only by that field would collapse onto the same signature and silently get dropped as a duplicate. RegExp doesn't survive JSON.stringify (it serializes to `{}`), so a naive `rule.inputTokenPattern ?? null` addition wouldn't distinguish two different patterns either — using .toString() instead captures both source and flags. Closes #8013.
1 parent c02a277 commit 509ba5e

2 files changed

Lines changed: 30 additions & 0 deletions

File tree

packages/loopover-engine/src/miner/deny-hook-synthesis.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,11 @@ function ruleSignature(rule: DenyRule): string {
134134
matcher: rule.matcher,
135135
pathPattern: rule.pathPattern ?? null,
136136
inputIncludesAll: rule.inputIncludesAll ?? null,
137+
// RegExp doesn't survive JSON.stringify (it serializes to `{}`), so two rules differing only by
138+
// inputTokenPattern would otherwise collapse to the same signature. .toString() (e.g. "/^-[a-z]*f[a-z]*$/i")
139+
// captures both source and flags, matching this function's own "identical = same effective match behavior"
140+
// contract for the other optional fields.
141+
inputTokenPattern: rule.inputTokenPattern ? rule.inputTokenPattern.toString() : null,
137142
reason: rule.reason,
138143
});
139144
}

test/unit/miner-deny-hook-synthesis.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,31 @@ describe("resolveEffectiveDenyRules() (#4522)", () => {
128128
expect(verdict.allowed).toBe(false);
129129
expect(verdict.blockedBy?.pathPattern).toBe("**/changelog.md");
130130
});
131+
132+
// #8013: ruleSignature (the identity function resolveEffectiveDenyRules dedupes with) used to omit
133+
// inputTokenPattern entirely, so two rules identical in every OTHER field but a different inputTokenPattern
134+
// collapsed onto the same signature and the second was silently dropped as a "duplicate".
135+
it("does NOT dedupe two approved rules that differ only by inputTokenPattern (#8013)", () => {
136+
const now = new Date(0).toISOString();
137+
const baseProposal = (inputTokenPattern: RegExp, id: string) => ({
138+
id,
139+
status: "approved" as const,
140+
rule: { matcher: "*", inputIncludesAll: ["push"], inputTokenPattern, reason: "custom force-push guard" },
141+
audit: { kind: "manual", synthesizedAt: now },
142+
});
143+
const approved = [
144+
baseProposal(/^-[a-z]*f[a-z]*$/i, "custom:1"),
145+
baseProposal(/^--force$/i, "custom:2"),
146+
];
147+
const effective = resolveEffectiveDenyRules({ approvedProposals: approved });
148+
// Both survive: a real bug here collapses the second one away, leaving only DEFAULT_DENY_RULES.length + 1.
149+
expect(effective.length).toBe(DEFAULT_DENY_RULES.length + 2);
150+
// And each one's OWN pattern is still the one that's actually enforced (not silently replaced by the other).
151+
const longFlagVerdict = evaluateDenyHooks({ name: "Bash", input: { command: "git push --force" } }, effective);
152+
expect(longFlagVerdict.allowed).toBe(false);
153+
const shortFlagVerdict = evaluateDenyHooks({ name: "Bash", input: { command: "git push -f" } }, effective);
154+
expect(shortFlagVerdict.allowed).toBe(false);
155+
});
131156
});
132157

133158
describe("initDenyHookSynthesisStore() (#4522)", () => {

0 commit comments

Comments
 (0)