Skip to content

fix: clone RouteBaseDirectives before per-policy append (#246)#249

Merged
ecv merged 2 commits into
mainfrom
fix/246-clone-route-base-directives
Jul 10, 2026
Merged

fix: clone RouteBaseDirectives before per-policy append (#246)#249
ecv merged 2 commits into
mainfrom
fix/246-clone-route-base-directives

Conversation

@ecv

@ecv ecv commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What

getCorazaDirectivesForTrafficProtectionPolicy seeded its per-policy directive list by aliasing the shared RouteBaseDirectives slice, then appended to it. With spare capacity (cap > len) the append writes into the shared backing array — per-policy directives leak into the base seen by other policies.

Fix

directives := slices.Clone(r.Config.Gateway.Coraza.RouteBaseDirectives)

Defensive copy before appending, matching the existing safe precedent at internal/extensionserver/cache/index.go:248 (make+copy).

Impact

Latent/dormant today — the builder is reached only under IsEPPEmissionEnabled(), off by default (Coraza injected via the extension server's PostTranslateModify, not EnvoyPatchPolicies). Fixes the corruption pre-emptively so enabling EPP emission doesn't ship a cross-policy config bug: SecRuleEngine On/DetectionOnly/Off, score thresholds, and rule removals could bleed across policies (order- and capacity-dependent).

Tests

TestGetCorazaDirectivesDoesNotAliasRouteBaseDirectives — seeds the shared base slice with spare capacity, builds an Enforce policy, then builds a second Observe policy and asserts the first's SecRuleEngine On was not overwritten to DetectionOnly.

Confirmed to fail on the aliased code (the leak reproduces exactly) and pass with slices.Clone:

--- FAIL: TestGetCorazaDirectivesDoesNotAliasRouteBaseDirectives
    ... does not contain "SecRuleEngine On"
    second policy leaked into the first via a shared RouteBaseDirectives backing array

Verification

  • go build ./internal/controller/... — clean
  • go vet ./internal/controller/... — clean
  • go test ./internal/controller/ -run TestGetCorazaDirectives — ok

Fixes #246. Related: #242.

ecv added 2 commits July 10, 2026 15:33
getCorazaDirectivesForTrafficProtectionPolicy seeded its per-policy
directive list by aliasing the operator's shared RouteBaseDirectives
slice, then appended to it. When the shared slice had spare capacity
(cap > len), the append wrote into the shared backing array, leaking
per-policy directives (SecRuleEngine mode, score thresholds, rule
removals) into the base seen by other policies.

Clone the base slice before appending, matching the safe precedent in
internal/extensionserver/cache/index.go. Latent today: the builder is
reached only under IsEPPEmissionEnabled(), off by default.

Fixes #246
Add TestGetCorazaDirectivesDoesNotAliasRouteBaseDirectives: seed the
shared base slice with spare capacity, build an Enforce policy, then
build a second Observe policy and assert the first's SecRuleEngine
token was not overwritten. Fails on the aliased code, passes with the
slices.Clone fix.
@ecv ecv marked this pull request as ready for review July 10, 2026 19:35
@ecv ecv requested review from kevwilliams and scotwells July 10, 2026 19:35
@ecv ecv enabled auto-merge July 10, 2026 19:36
@ecv ecv merged commit e159acf into main Jul 10, 2026
8 of 9 checks passed
@ecv ecv deleted the fix/246-clone-route-base-directives branch July 10, 2026 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants