fix: reject inverted TrafficProtectionPolicy paranoia levels at admission#251
Merged
Conversation
A TrafficProtectionPolicy whose blocking paranoia level exceeds its detection paranoia level is an illegal OWASP CRS configuration. CRS rule 901500 fails closed and denies every request at phase 1 with HTTP 500 before any attack evaluation, so a misconfigured policy silently 500s 100% of traffic to the protected route. Because blocking and detection were validated independently (each 1..4, default 1), setting blocking: 2 while leaving detection at its default 1 was admissible. Add a CEL admission rule on ParanoiaLevels requiring detection >= blocking so the illegal combination is rejected at write time. Key changes: - add XValidation rule "self.detection >= self.blocking" on ParanoiaLevels - regenerate CRD and API reference docs - add e2e regression asserting admission rejects blocking > detection
scotwells
approved these changes
Jul 11, 2026
ecv
marked this pull request as ready for review
July 11, 2026 01:43
This was referenced Jul 12, 2026
ecv
added a commit
that referenced
this pull request
Jul 12, 2026
Add an envtest suite that installs the generated CRDs against a real apiserver and creates a defaulted TrafficProtectionPolicy. The apiserver validates a schema node's structural default against that node's own CEL rules at CRD registration, so an invalid default is rejected during envtest start — the fake client the rest of the suite uses never sees schema, defaults, or CEL, which is why the #257 regression went undetected. The suite skips when KUBEBUILDER_ASSETS is unset (plain go test) and runs under `make test`, which provisions the assets. Key checks: - TPP CRD installs and a policy created with only its required targetRef defaults paranoiaLevels to a pair satisfying detection>=blocking. - An inverted user-supplied pair is still rejected, so the default fix did not loosen the #251 rule it depends on. Part of #257.
ecv
added a commit
that referenced
this pull request
Jul 13, 2026
The ParanoiaLevels struct gained an XValidation rule (self.detection >= self.blocking) in #251, but the field kept an empty default. The apiserver validates a schema node's structural default against its own CEL rules, and the default {} has no detection/blocking keys — so CRD registration fails with "no such key: detection" and kubectl apply of the CRD is rejected, breaking every branch's e2e "Prepare e2e" step. Set the field default to {blocking:1,detection:1} so it satisfies the rule (1 >= 1). Per-field defaults are unchanged and #251's rule still rejects inverted user-supplied values. CRD manifest regenerated. Closes #257
This was referenced Jul 13, 2026
ecv
added a commit
that referenced
this pull request
Jul 13, 2026
Add a quarantined, expected-red e2e case asserting the controller withholds an already-persisted inverted-paranoia TrafficProtectionPolicy from the coraza edge data plane, so benign traffic still serves 200 instead of the CRS 901500 fail-closed 500. The test injects a grandfathered inverted policy by bypassing the #251 CEL admission rule (patch the paranoiaLevels validation off the CRD, create the policy, restore the rule), then asserts a benign GET returns 200. It fails until #252's skip-attachment is enforced on the downstream-gateway path, so it lives under test/e2e-nonblocking and is excluded from the gating test-e2e run via the separate test-e2e-nonblocking target. The controller-side fix that makes this pass is to follow on this branch.
This was referenced Jul 14, 2026
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
2 participants
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.
Summary
Fixes #250 via option 1 (CEL admission validation) from the issue's root-cause comment.
A
TrafficProtectionPolicywhose OWASP CRS blocking paranoia level exceeds its detection paranoia level is an illegal CRS configuration. CRS rule 901500 fails closed and denies every request at phase 1 with HTTP 500 before any attack evaluation, so a misconfigured policy silently 500s 100% of traffic to the protected route. Becauseblockinganddetectionwere validated independently (each1..4, default1), settingblocking: 2while leavingdetectionat its default1was admissible.Change
XValidationrule onParanoiaLevelsrequiringself.detection >= self.blocking, so the illegal combination is rejected at write time (fail-closed, no runtime surprise).Test
test/e2e/trafficprotectionpolicy-paranoia-validation/asserts admission:blocking: 2 / detection: 1blocking: 2withdetectiondefaulteddetection >= blockingCEL not live-tested locally (kind clusters were stopped); chainsaw test guards CI.
Companion PR
This is the preferred fail-closed fix. Companion #252 implements option 3 (controller status condition) as defense-in-depth for already-persisted policies CEL cannot re-validate. The two are complementary and can merge independently.