fix: flag inverted TrafficProtectionPolicy paranoia levels via status condition#252
Merged
Conversation
ecv
marked this pull request as ready for review
July 11, 2026 14:13
ecv
requested review from
aflor024,
kevwilliams,
privateip and
scotwells
and removed request for
yahyafakhroji
July 13, 2026 03:24
A TrafficProtectionPolicy whose OWASP CRS detection paranoia level is below its blocking paranoia level is an illegal configuration: CRS rule 901500 fails closed and denies every request at phase 1 with HTTP 500 before any attack evaluation. Such a policy silently 500s 100% of traffic to the protected route with no signal to the tenant about why. Reject the policy during reconciliation: set Accepted=False with reason Invalid on the affected ancestor and skip attachment so the broken directives are never emitted to the data plane. This also surfaces already-persisted policies that carry the inverted configuration. Key changes: - add paranoiaLevelsResolveError guard in the TPP reconciler - set Accepted=False/Invalid and skip attach for HTTPRoute and Gateway targets when detection < blocking - add unit coverage for the guard and both process paths
ecv
force-pushed
the
fix/250-paranoia-status-condition
branch
from
July 13, 2026 16:32
d17ce22 to
1b06e9c
Compare
ecv
enabled auto-merge
July 13, 2026 16:37
kevwilliams
approved these changes
Jul 13, 2026
This was referenced Jul 13, 2026
ecv
added a commit
that referenced
this pull request
Jul 13, 2026
Add a gated e2e case on the downstream WAF data plane exercising a valid Enforce TrafficProtectionPolicy end to end: a benign GET returns 200 with the backend body intact, and a CRS-tripping attack payload returns 403 with no backend leakage. Assertions check body integrity, not status alone, per datum-cloud/infra#3321. Neutralization of already-persisted inverted policies is tracked separately as follow-up to #252, where the controller-side fix and its test belong.
This was referenced Jul 13, 2026
ecv
added a commit
that referenced
this pull request
Jul 13, 2026
An inverted-paranoia TrafficProtectionPolicy (OWASP CRS detection below blocking) makes CRS rule 901500 fail closed and 500 every request. #252 set Accepted=False/Invalid and skipped attachment on the controller's EnvoyPatchPolicy path, but the live edge enforces coraza through the extension server, which builds per-route directives in a separate path (internal/extensionserver/cache) that #252 never touched. A grandfathered inverted policy therefore still 500'd benign traffic on the downstream gateway. Gate the extension server's directive emission on the same inverted-level check the controller uses, so an inverted policy produces no directives and the mutation layer withholds it from the data plane. A benign request is then unaffected instead of 500'ing. Key changes: - add shared TrafficProtectionPolicySpec.InvertedParanoiaLevels predicate and route both the controller resolve-error guard and the extension server emission through it - return nil directives for an inverted policy in computeCorazaDirectives so ApplyTPPRouteConfig skips the route - add API and emission-layer unit tests: inverted not emitted, valid and equal levels emitted - promote the neutralization e2e into the gating test/e2e suite and drop the now-empty test-e2e-nonblocking target Fixes #273. Refs #252, #242, #268.
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.
ecv
added a commit
that referenced
this pull request
Jul 13, 2026
An inverted-paranoia TrafficProtectionPolicy (OWASP CRS detection below blocking) makes CRS rule 901500 fail closed and 500 every request. #252 set Accepted=False/Invalid and skipped attachment on the controller's EnvoyPatchPolicy path, but the live edge enforces coraza through the extension server, which builds per-route directives in a separate path (internal/extensionserver/cache) that #252 never touched. A grandfathered inverted policy therefore still 500'd benign traffic on the downstream gateway. Gate the extension server's directive emission on the same inverted-level check the controller uses, so an inverted policy produces no directives and the mutation layer withholds it from the data plane. A benign request is then unaffected instead of 500'ing. Key changes: - add shared TrafficProtectionPolicySpec.InvertedParanoiaLevels predicate and route both the controller resolve-error guard and the extension server emission through it - return nil directives for an inverted policy in computeCorazaDirectives so ApplyTPPRouteConfig skips the route - add API and emission-layer unit tests: inverted not emitted, valid and equal levels emitted - promote the neutralization e2e into the gating test/e2e suite and drop the now-empty test-e2e-nonblocking target Fixes #273. Refs #252, #242, #268.
ecv
added a commit
that referenced
this pull request
Jul 13, 2026
…edge fix: neutralize persisted inverted TPP on the coraza edge path (#252 follow-up)
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
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
Addresses #250 via option 3 (controller status condition, defense-in-depth) from the issue's root-cause comment.
A
TrafficProtectionPolicywhose OWASP CRS detection paranoia level is below its blocking paranoia level is illegal: CRS rule 901500 fails closed and denies every request at phase 1 with HTTP 500 before any attack evaluation. Such a policy silently 500s 100% of traffic with no signal to the tenant about why.Change
Reject the policy during reconciliation:
Accepted=Falsewith reasonInvalidon the affected ancestor.Test
TestParanoiaLevelsResolveError— guard, 4 cases (equal / higher detection / inverted / defaulted inverted).TestProcessTrafficProtectionPolicyForHTTPRouteandTestProcessTrafficProtectionPolicyForGateway, assertingAccepted=False/Invalidand no attachment.Companion PR
Companion to #251, which implements option 1 (CEL admission validation, the preferred fail-closed fix). CEL only validates on write; this guard catches policies already stored with the bad config and gives tenants an actionable
Accepted=Falsereason. The two are complementary and can merge independently.