Skip to content

fix: neutralize persisted inverted TPP on the coraza edge path (#252 follow-up)#274

Merged
ecv merged 3 commits into
mainfrom
fix/273-neutralize-inverted-edge
Jul 13, 2026
Merged

fix: neutralize persisted inverted TPP on the coraza edge path (#252 follow-up)#274
ecv merged 3 commits into
mainfrom
fix/273-neutralize-inverted-edge

Conversation

@ecv

@ecv ecv commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

What

Follow-up to #252. Makes the controller genuinely withhold an already-persisted inverted-paranoia TrafficProtectionPolicy (blocking > detection) from the coraza edge data plane, so benign traffic serves 200 instead of the CRS 901500 fail-closed 500. Also promotes the neutralization e2e (split out of #272) into the gating suite.

Root cause

#252 set Accepted=False/Invalid and skipped attachment — but only on the controller's EnvoyPatchPolicy path (internal/controller/trafficprotectionpolicy_controller.go). The live datum-downstream-gateway enforces coraza through the extension server, which builds per-route directives in a completely separate path (internal/extensionserver/cache/index.gocomputeCorazaDirectives) that #252 never touched. So a grandfathered inverted policy still emitted its coraza SecRules to the edge and 500'd benign traffic — confirmed in prod (kev-httpbin ~53k 500s/24h, pcep-lol-hnqr6o ~17k/24h). See #273.

Fix

Tests

  • Go unit tests (table-driven, matching existing style):
    • api/v1alpha: InvertedParanoiaLevels — inverted reported, equal/higher-detection/no-ruleset not.
    • internal/extensionserver/cache: emission-layer — inverted → no directives (index build + computeCorazaDirectives), valid and equal levels → emitted.
  • go test ./... (non-e2e), go vet, make generate (no diff), make lint (0 issues) all green locally.

e2e

Promoted trafficprotectionpolicy-neutralize-inverted into the gating test/e2e/ suite (dropped the now-empty test-e2e-nonblocking target) so this PR's own E2E check validates the fix end-to-end in the downstream-WAF kind env (#268). The benign GET / → 200 assertion now describes neutralized behavior rather than an expected-red defect. Kind e2e can only be confirmed by CI — if the E2E check reds, the move is trivially revertible back to quarantine.

Notes

Fixes #273. Refs #252, #242, #268, datum-cloud/infra#3408.

ecv added 2 commits July 13, 2026 19:19
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.
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
ecv force-pushed the fix/273-neutralize-inverted-edge branch from 8b2a047 to d0ca80a Compare July 13, 2026 23:19
The benign-GET probe returned HTTP 000 (connection refused), not the
neutralized 200, because two harness defects fired before the WAF path was
ever exercised:

- the probe script had no timeout, so chainsaw's ~5s default killed the
  40-attempt retry loop after 2 attempts;
- nothing waited for the freshly-created waf-gw to reach Programmed=True, so
  the probe hit the cold-start window.

Add a 180s script timeout and gate the probe on Gateway Programmed=True (3m,
matching the enforce test), so the loop retries into the ready window. The
controller fix under test is unchanged.
@ecv
ecv marked this pull request as ready for review July 13, 2026 23:41
@ecv
ecv requested review from kevwilliams and scotwells July 13, 2026 23:41
@ecv
ecv merged commit b967515 into main Jul 13, 2026
11 checks passed
@ecv
ecv deleted the fix/273-neutralize-inverted-edge branch July 13, 2026 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

#252 skip-attachment does not neutralize persisted inverted TPP on the coraza edge path

2 participants