fix(waf): an advisory match must not mask custom enforcement (TIGER-82) - #2
Merged
Conversation
inspect() returned on the FIRST shipped match. A soft category is capped at 'log', and waf.action itself defaults to 'log', while custom admin rules were only evaluated when nothing shipped had matched at all. So a request matching BOTH a shipped heuristic AND an administrator's custom block rule was ALLOWED: the observe-only verdict shadowed the policy that said block, and the firewall plugin correctly let a 'log' through. The precedence gap is the bug, not the ordering: an advisory rule may inform, never mask. Everything is now evaluated and the STRONGEST action wins (log < captcha < block), keeping the label of whichever rule produced it. It short-circuits only on a block, which nothing can outrank. Deliberately unchanged: learn/off mode and the outage fail-open both live downstream in the firewall plugin, which still never enforces a 'log'. Also adds a test harness -- this module shipped with none, and it is a security module whose enforcement logic just changed. 7 tests, mutation-verified: restoring first-match-wins fails both masking tests. The suite includes the control that matters in the other direction too, since "always return block" would satisfy the masking tests while turning a heuristic into a site-breaking rule: an advisory match ALONE must still be advisory, and a log-only custom rule must not soften a shipped block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L8p9pLJ3DFstG3xZuh2QgZ
The suite added alongside the TIGER-82 fix would otherwise never run: this repo had no PR workflow at all, so a security module's enforcement tests would sit in the tree guarding nothing. Unit-only, so no database service is needed -- the WAF service reads config from the registry and rules from a file. Dependencies come from a sibling tiger-core checkout, the same pattern TigerStripe and TigerShop use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L8p9pLJ3DFstG3xZuh2QgZ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
inspect()returned on the first shipped match. A soft category is capped atlog, andwaf.actionitself defaults tolog, while custom admin rules were only evaluated when nothing shipped had matched at all.So a request matching both a shipped heuristic and an administrator's custom block rule was allowed — the observe-only verdict shadowed the policy that said block, and the firewall plugin correctly let a
logthrough.The fix
The precedence gap is the bug, not the ordering. An advisory rule may inform, never mask. Everything is now evaluated and the strongest action wins (
log<captcha<block), keeping the label of whichever rule produced it. It short-circuits only on ablock, which nothing can outrank.Deliberately unchanged: learn/off mode and the outage fail-open both live downstream in the firewall plugin, which still never enforces a
log.Tests — this module had none
TigerShield shipped with no test suite, and this is a security module whose enforcement logic just changed, so I stood up a minimal harness (bootstrap resolving tiger-core's autoloader, same pattern as TigerStripe).
7 tests, mutation-verified: restoring first-match-wins fails both masking tests.
The suite deliberately guards the other direction too, because "always return block" would satisfy the masking tests while turning a heuristic into a site-breaking rule:
log)blockwaf.actionis still honoured for high-tier categories🤖 Generated with Claude Code
https://claude.ai/code/session_01L8p9pLJ3DFstG3xZuh2QgZ