Skip to content

Commit fbcba1e

Browse files
authored
fix(engine): include inputTokenPattern in ruleSignature's deny-hook identity (#8126)
ruleSignature previously omitted inputTokenPattern from the identity it computes for resolveEffectiveDenyRules/synthesizeDenyRuleProposals, so a maintainer-approved custom rule that narrowed an existing rule only by adding/changing inputTokenPattern would be silently deduped away as an exact duplicate. Serializes .source + .flags rather than the RegExp object itself, since JSON.stringify collapses every RegExp instance to "{}" regardless of its actual pattern. Closes #8013
1 parent c02a277 commit fbcba1e

2 files changed

Lines changed: 33 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
@@ -130,10 +130,15 @@ export function changedPathToDenyGlob(path: string): string | null {
130130
}
131131

132132
function ruleSignature(rule: DenyRule): string {
133+
// `inputTokenPattern` is a RegExp: JSON.stringify serializes every RegExp instance as "{}" regardless of its
134+
// actual source/flags (no enumerable own properties, no toJSON), so two rules with genuinely different
135+
// patterns would still collide here if the RegExp object itself were included directly. Serialize `.source`
136+
// + `.flags` instead so distinct patterns produce distinct signatures.
133137
return JSON.stringify({
134138
matcher: rule.matcher,
135139
pathPattern: rule.pathPattern ?? null,
136140
inputIncludesAll: rule.inputIncludesAll ?? null,
141+
inputTokenPattern: rule.inputTokenPattern ? { source: rule.inputTokenPattern.source, flags: rule.inputTokenPattern.flags } : null,
137142
reason: rule.reason,
138143
});
139144
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
setProposalStatuses,
1818
synthesizeDenyRuleProposals,
1919
} from "../../packages/loopover-miner/lib/deny-hook-synthesis.js";
20+
import type { DenyRuleProposal } from "../../packages/loopover-engine/src/miner/deny-hook-synthesis";
2021
// #7525: normalizeRepoFullName is defined in the engine and re-exported unchanged by the miner-lib module
2122
// above; import it from the engine source directly so the guard's src branches are the ones exercised.
2223
import { normalizeRepoFullName } from "../../packages/loopover-engine/src/miner/deny-hook-synthesis";
@@ -128,6 +129,33 @@ describe("resolveEffectiveDenyRules() (#4522)", () => {
128129
expect(verdict.allowed).toBe(false);
129130
expect(verdict.blockedBy?.pathPattern).toBe("**/changelog.md");
130131
});
132+
133+
it("#8013: keeps two rules distinct when they differ only by inputTokenPattern, including two different patterns (not just presence vs. absence)", () => {
134+
const proposal = (id: string, inputTokenPattern?: RegExp): DenyRuleProposal => ({
135+
id,
136+
status: "approved",
137+
rule: { matcher: "Bash", inputIncludesAll: ["git"], reason: "test", ...(inputTokenPattern ? { inputTokenPattern } : {}) },
138+
audit: { kind: "manual", synthesizedAt: "2026-01-01T00:00:00.000Z" },
139+
});
140+
const noPattern = proposal("a");
141+
const withFollowTags = proposal("b", /^--follow-tags$/);
142+
const withF = proposal("c", /^-f$/);
143+
144+
const effective = resolveEffectiveDenyRules({ includeDefaults: false, approvedProposals: [noPattern, withFollowTags, withF] });
145+
// All three must survive -- none of them are "the same rule" as either of the others.
146+
expect(effective).toHaveLength(3);
147+
});
148+
149+
it("#8013: still dedupes two rules whose inputTokenPattern has the identical source+flags", () => {
150+
const proposal = (id: string): DenyRuleProposal => ({
151+
id,
152+
status: "approved",
153+
rule: { matcher: "Bash", inputIncludesAll: ["git"], reason: "test", inputTokenPattern: /^-f$/ },
154+
audit: { kind: "manual", synthesizedAt: "2026-01-01T00:00:00.000Z" },
155+
});
156+
const effective = resolveEffectiveDenyRules({ includeDefaults: false, approvedProposals: [proposal("a"), proposal("b")] });
157+
expect(effective).toHaveLength(1);
158+
});
131159
});
132160

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

0 commit comments

Comments
 (0)