From 509ba5eb15a2157954088b75e705b44df59a256f Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:46:05 -0700 Subject: [PATCH] fix(engine): include inputTokenPattern in deny-hook ruleSignature identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../src/miner/deny-hook-synthesis.ts | 5 ++++ test/unit/miner-deny-hook-synthesis.test.ts | 25 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/packages/loopover-engine/src/miner/deny-hook-synthesis.ts b/packages/loopover-engine/src/miner/deny-hook-synthesis.ts index 7b811d0cf4..e4dfeeb400 100644 --- a/packages/loopover-engine/src/miner/deny-hook-synthesis.ts +++ b/packages/loopover-engine/src/miner/deny-hook-synthesis.ts @@ -134,6 +134,11 @@ function ruleSignature(rule: DenyRule): string { matcher: rule.matcher, pathPattern: rule.pathPattern ?? null, inputIncludesAll: rule.inputIncludesAll ?? null, + // RegExp doesn't survive JSON.stringify (it serializes to `{}`), so two rules differing only by + // inputTokenPattern would otherwise collapse to the same signature. .toString() (e.g. "/^-[a-z]*f[a-z]*$/i") + // captures both source and flags, matching this function's own "identical = same effective match behavior" + // contract for the other optional fields. + inputTokenPattern: rule.inputTokenPattern ? rule.inputTokenPattern.toString() : null, reason: rule.reason, }); } diff --git a/test/unit/miner-deny-hook-synthesis.test.ts b/test/unit/miner-deny-hook-synthesis.test.ts index 8c47978af4..462a4ee788 100644 --- a/test/unit/miner-deny-hook-synthesis.test.ts +++ b/test/unit/miner-deny-hook-synthesis.test.ts @@ -128,6 +128,31 @@ describe("resolveEffectiveDenyRules() (#4522)", () => { expect(verdict.allowed).toBe(false); expect(verdict.blockedBy?.pathPattern).toBe("**/changelog.md"); }); + + // #8013: ruleSignature (the identity function resolveEffectiveDenyRules dedupes with) used to omit + // inputTokenPattern entirely, so two rules identical in every OTHER field but a different inputTokenPattern + // collapsed onto the same signature and the second was silently dropped as a "duplicate". + it("does NOT dedupe two approved rules that differ only by inputTokenPattern (#8013)", () => { + const now = new Date(0).toISOString(); + const baseProposal = (inputTokenPattern: RegExp, id: string) => ({ + id, + status: "approved" as const, + rule: { matcher: "*", inputIncludesAll: ["push"], inputTokenPattern, reason: "custom force-push guard" }, + audit: { kind: "manual", synthesizedAt: now }, + }); + const approved = [ + baseProposal(/^-[a-z]*f[a-z]*$/i, "custom:1"), + baseProposal(/^--force$/i, "custom:2"), + ]; + const effective = resolveEffectiveDenyRules({ approvedProposals: approved }); + // Both survive: a real bug here collapses the second one away, leaving only DEFAULT_DENY_RULES.length + 1. + expect(effective.length).toBe(DEFAULT_DENY_RULES.length + 2); + // And each one's OWN pattern is still the one that's actually enforced (not silently replaced by the other). + const longFlagVerdict = evaluateDenyHooks({ name: "Bash", input: { command: "git push --force" } }, effective); + expect(longFlagVerdict.allowed).toBe(false); + const shortFlagVerdict = evaluateDenyHooks({ name: "Bash", input: { command: "git push -f" } }, effective); + expect(shortFlagVerdict.allowed).toBe(false); + }); }); describe("initDenyHookSynthesisStore() (#4522)", () => {