fix: make error-page body Envoy-safe via operator allowlist + startup validation (#243)#244
Open
ecv wants to merge 2 commits into
Open
fix: make error-page body Envoy-safe via operator allowlist + startup validation (#243)#244ecv wants to merge 2 commits into
ecv wants to merge 2 commits into
Conversation
The extension server injects the branded error page into Envoy's
local_reply_config as a SubstitutionFormatString (TextFormatSource), so
Envoy parses the body as a format template. Any bare '%' in the HTML is
read as the start of a command operator; the embedded default page's CSS
(e.g. "height: 100%") is not escaped, so Envoy rejects the whole listener
("Couldn't find valid command at position 330"). On the failOpen:false
downstream hook that NACKs the xDS update fleet-wide, taking every route
with a TrafficProtectionPolicy in Enforce mode offline.
Escape bare '%' to '%%' at inject time while preserving already-escaped
'%%' and valid '%COMMAND%' operators (such as the intended
%RESPONSE_CODE%). The transform is idempotent and also protects any
mounted override page.
Fixes #243
Key changes:
- add escapeEnvoyFormatLiterals and apply it to the local reply body
- unit tests for the escaper and a regression guard asserting the
embedded default page is Envoy-safe after escaping
The percent escaper preserved any command-shaped token (%NAME%, %REQ(...)%), but only %RESPONSE_CODE% is ever templated into the body. A command-shaped literal in an override page (e.g. "%COMPLETE%") is not a valid Envoy operator and, passed through unescaped, re-triggers the exact listener rejection this escaper exists to prevent (issue #243). Replace the grammar match with an allowlist of operators we actually emit, so every other %...% token is treated as a literal and escaped. Add a startup validation that round-trips the assembled local_reply_config and asserts the injected body carries no bare '%', catching a malformed body in the operator process instead of NACKing the xDS update fleet-wide on the failOpen:false downstream hook. A failed override falls back to the embedded default; a failed default disables branding rather than stalling the data plane. Key changes: - replace envoyCommandToken regex with envoyBodyAllowedCommands allowlist - add assertEnvoyFormatSafe and ValidateLocalReplyConfig - validate at extension-server startup with fail-safe fallback - update escaper tests for allowlist behavior; add validation tests
ecv
changed the title
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.
What
Fixes #243. A
TrafficProtectionPolicyinEnforcemode takes the whole gateway offline (100% of traffic fails) whenever the branded error page reaches Envoy with a%it can't parse.Root cause
The extension server injects the branded page into Envoy's
local_reply_configas aSubstitutionFormatString(TextFormatSource), so Envoy parses the body as a format template. Any bare%is read as the start of a command operator. The embedded default page's CSS (height: 100%, gradient stops,border-radius: 50%) is unescaped, so Envoy rejects the listener:With the downstream EG hook on
failOpen: false, the rejected update is NACKed fleet-wide — Envoy ends up with notcp-80listener and every request gets an empty reply.Fix
Two layers, matching both remediations called out in #243:
Escape at inject time, against an allowlist. Bare
%→%%while preserving already-escaped%%and the operators we actually template. The allowlist (envoyBodyAllowedCommands, currently just%RESPONSE_CODE%) is deliberately not a grammar match: any other command-shaped token (e.g. a literal%COMPLETE%in a mounted override page) is not a valid Envoy operator, so it is treated as a literal and escaped rather than passed through to re-trigger the same listener rejection. Idempotent; protects the embedded default and any override page.Validate at startup.
ValidateLocalReplyConfiground-trips the assembledlocal_reply_configand asserts the injected body carries no bare%, so a malformed body is caught in the operator process instead of NACKing the xDS update fleet-wide in the data plane. It is fail-safe: a failing override falls back to the embedded default, and a failing default disables branding rather than stalling the data plane — the extension server never fails startup on a content problem.Adds unit tests for the escaper (allowlist preserve/escape, idempotency), a regression guard asserting the embedded default page is Envoy-safe after escaping, and tests for the startup validator.
Proof (Chainsaw e2e on a local WAF-enabled kind harness)
The e2e test is in #245. Same fresh-fleet harness, only the operator image differs:
HTTP 000empty reply, test FAILS → https://gist.github.com/ecv/e47733b657475887c4c29cced339512aHTTP 200, test PASSES → https://gist.github.com/ecv/4c601d3d96fd5b317c5f3b814bbdd510Manual confirmation on the same stack: benign
GET /→ 200 with backend body; an XSS request → 403 (WAF still enforces).