fix(apply): stop normalization widening an exact-path DENY to the whole LB - #3
Merged
Conversation
…le LB
`normalize_service_policy_spec` nested-merges each matcher against `_RULE_DEFAULTS` so a partial
value keeps its required sub-keys. For `path` that also injected the default
`prefix_values: ["/"]` into any rule that named only `exact_values` / `regex_values` /
`suffix_values` — and XC ORs the value lists, so a surgical one-endpoint DENY denied every path
on the load balancer.
Verified live on vpcopilot-lab (snapshot + verified rollback, policy deleted after):
before normalized DENY path {"prefix_values": ["/"], "exact_values": ["/__widening-probe__"]}
GET /__widening-probe__ -> 403 GET / -> 403 GET /api/pay -> 403
after normalized DENY path {"exact_values": ["/__widening-probe__"]}
GET /__widening-probe__ -> 403 GET / -> 307 GET /api/pay -> 405 (both baseline)
12 of the 31 generated rules on disk are exact-only and were being widened. Not an outage — the
safety spine catches it, because the legit probe is blocked too, so validation fails and rolls
back. The cost is effectiveness: an exact-path band-aid could not pass validation on the first
attempt, burning refine attempts or being reported as a failure when the policy was correct and
normalization broke it.
The nested merge exists to supply required STRUCTURAL sub-keys, not a matching value, so the fix
drops only the inherited catch-all when the rule named a path itself. A rule naming NO path still
gets `prefix_values: ["/"]` — that is what makes `generate`'s trailing allow-all work against
XC's default-deny.
Tests pin both directions, written before the fix: exact/regex/suffix-only lose the catch-all;
an explicit prefix, prefix+exact together, a no-path rule, the trailing allow-all, the structural
sub-keys and every other nested matcher are unchanged. Diffed old vs new normalization over all
15 service-policy artifacts on disk: 12 changed, 3 identical, and every changed rule differed
ONLY in `path` with all other fields byte-identical. 200 pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
A band-aid meant to block one endpoint was denying every path on the load balancer.
Cause
normalize_service_policy_specnested-merges each matcher against_RULE_DEFAULTSso a partialvalue keeps its required sub-keys. For
path, that also injected the defaultprefix_values: ["/"]into any rule naming onlyexact_values/regex_values/suffix_values.XC ORs the path value lists, so the injected catch-all matched everything.
12 of the 31 generated rules on disk are exact-only and were affected —
/api/pay,/api/login,/admin/users,/users/v1/register, and eight more.Verified live on
vpcopilot-labSnapshot + verified rollback, probe policy deleted after; baseline was
/→ 307,/api/pay→ 405./__widening-probe__//api/pay{"prefix_values": ["/"], "exact_values": ["/__widening-probe__"]}{"exact_values": ["/__widening-probe__"]}Impact
Not an outage. The safety spine catches it — the legit probe is blocked too, so
legit_okisfalse, validation fails and the policy rolls back. What it cost is effectiveness: an exact-path
band-aid could never pass validation on the first attempt, so it burned refine attempts recovering
or was reported as a failure when the policy was correct and normalization broke it.
The fix
The nested merge exists to supply required structural sub-keys, not a matching value. It now
drops only the inherited catch-all, and only when the rule named a path itself. A rule naming no
path still gets
prefix_values: ["/"]— that is what makesgenerate's trailing allow-all workagainst XC's default-deny.
Tests, written before the fix
Both directions, since this is on every apply path for all seven controls:
prefix_values; prefix + exact together; a rule withno path at all; an empty path object; the trailing allow-all; the structural sub-keys
(
transformers,invert_matcher); every other nested matcher and unrelated required default.Plus an old-vs-new diff over all 15 service-policy artifacts on disk: 12 changed, 3 identical,
and every changed rule differed only in
path, all other fields byte-identical.ruffclean · 200 pass · coverage 73%.🤖 Generated with Claude Code