Skip to content

fix(engine): include inputTokenPattern in deny-hook ruleSignature identity - #8127

Closed
JSONbored wants to merge 1 commit into
mainfrom
fix/deny-hook-rule-signature-input-token-pattern
Closed

fix(engine): include inputTokenPattern in deny-hook ruleSignature identity#8127
JSONbored wants to merge 1 commit into
mainfrom
fix/deny-hook-rule-signature-input-token-pattern

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • ruleSignature (packages/loopover-engine/src/miner/deny-hook-synthesis.ts) is the sole identity function resolveEffectiveDenyRules (merge built-in + approved custom rules) and synthesizeDenyRuleProposals (dedup newly-derived proposals) use to decide "is this the same rule." It included 4 of DenyRule's 5 matching fields, omitting inputTokenPattern — added specifically because a plain inputIncludesAll substring test false-positives on unrelated longer flags (-f vs --follow-tags).
  • A maintainer-approved custom rule narrowing an existing one with the same matcher/pathPattern/inputIncludesAll/reason but a different (or newly-added) inputTokenPattern would be treated as an exact duplicate and silently dropped from the merged set.
  • Fix adds inputTokenPattern to the signature, nullish-coalesced like the other optional fields — but via .toString() rather than the field itself, since RegExp doesn't survive JSON.stringify (it serializes to {}}); a naive rule.inputTokenPattern ?? null addition would still collapse two different patterns onto the same signature. .toString() captures both source and flags (e.g. "/^-[a-z]*f[a-z]*$/i").

Closes #8013.

Test plan

  • New regression test in test/unit/miner-deny-hook-synthesis.test.ts: two approved rules identical except for inputTokenPattern both survive resolveEffectiveDenyRules (not deduped), and each one's own pattern is still the one enforced.
  • Verified the test actually catches the bug: reverted the source fix locally, confirmed the new test fails (11 merged rules instead of the expected 12), then restored the fix and confirmed green.
  • npm run build --workspace @loopover/engine && npm run build:miner (rebuilt so the test, which imports the compiled miner-lib wrapper, reflects the source change)
  • npx tsc --noEmit
  • npm run test --workspace @loopover/engine (648/648 passing)
  • npm run engine-parity:drift-check — clean, not one of the 5 hand-duplicated twin-pair files

…ntity

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.
@JSONbored JSONbored self-assigned this Jul 22, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored JSONbored closed this Jul 22, 2026
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 22, 2026
@JSONbored
JSONbored deleted the fix/deny-hook-rule-signature-input-token-pattern branch July 22, 2026 23:47
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.02%. Comparing base (c02a277) to head (509ba5e).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8127      +/-   ##
==========================================
- Coverage   92.02%   92.02%   -0.01%     
==========================================
  Files         760      760              
  Lines       77394    77395       +1     
  Branches    23392    23393       +1     
==========================================
- Hits        71225    71222       -3     
  Misses       5061     5061              
- Partials     1108     1112       +4     
Flag Coverage Δ
shard-1 57.10% <100.00%> (+<0.01%) ⬆️
shard-2 54.33% <0.00%> (-0.01%) ⬇️
shard-3 50.28% <0.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...s/loopover-engine/src/miner/deny-hook-synthesis.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ruleSignature's deny-hook identity computation omits inputTokenPattern, so dedup can silently drop a rule that differs only by that field

1 participant