Repository navigation
Strip every X-Saturn-* header and trailer before forwarding upstream (Q-R8STRIP2) - #58
Merged
Merged
Conversation
…(Q-R8STRIP2) Hugo's ruling Q-R8STRIP2 (2026-10-04): no X-Saturn-* header reaches the upstream (vLLM / Dynamo). Traefik is the primary strip (saturn-k8s); phoebe's #57 strip stays as the non-bypassable backstop and is widened from "outside the trusted set and edge contract" to the whole X-Saturn-* namespace, case-insensitive, in headers and trailers, on every route. - identity: StripUntrustedSaturnHeaders -> StripSaturnHeaders (prefix rule); delete edgeContractHeaders / ForwardableSaturnHeaders and their oracle tests. The trusted-header registry now governs only what FromRequest reads. - proxy: the strip runs right after FromRequest; everything after uses the parsed identity. The R8 legacy-quota log diagnostic read raw headers after the strip (it had silently stopped firing since #57, and its test panicked on a nil Debug logger); its presence is now recorded before the strip. The per-request Debug count line is dropped since the strip now removes headers on every request. - tests: every route (dedicated, shared, gateway with forged routing headers + decoy upstream, shared via wake incl. probes) sees zero X-Saturn-* headers, reaches the right upstream and meters the right identity; over-the-wire chunked trailers carry no X-Saturn-* trailer; look-alikes (X-Saturnine, X-SaturnX, X-Saturn) are kept. Claude-Session: https://claude.ai/code/session_01JR59LERE7vBqUT9cga5Kvq
Go keeps '_' in header names and WSGI/CGI-style upstreams fold '_' into '-', so an X_Saturn_* header would read upstream as a real X-Saturn-* header. isSaturnHeader now treats '_' as '-' in the prefix; look-alikes (X_Saturnine, X-SaturnX) still survive. Adds a test that every Header* constant FromRequest reads is in the stripped namespace. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… cannot race on r.Trailer The Transport can Close the request body on one goroutine while another reads it to EOF; both stripped r.Trailer unsynchronized, risking Go's fatal concurrent-map-write error. A mutex now guards the strip only (never the inner Read, so Close can still interrupt a blocked Read). TestTrailerStripBodyConcurrentReadAndCloseDoNotRace fails under -race without the lock. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gacy check without an admitter - proxy: before the strip, log at Debug how many X-Saturn-* names are neither trusted nor edge-contract identity headers (the edge strip should have removed them). Only the count is logged. Tests: TestUnexpectedSaturnHeaderAtEntryLogsDebugCount (proxy), TestCountUnexpectedSaturnHeadersCountsOnlyNamesTheEdgeShouldHaveStripped (identity). The edge-contract list is restored for this count only. - proxy: legacyQuotaHeadersPresent runs only when an admitter is set, the only path where its diagnostic can fire. TestLegacyOnlyQuotaHeadersLogPreR8ProducerMarker still proves the marker fires after the strip (its logger fixture gains a Debug writer). - tests: document that isSaturnName is a deliberately independent oracle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…railer test TestNoSaturnTrailersReachUpstreamOverWire now also drives the shared header-routed route and the shared wake path (cold probe, re-probe, metered forward; all three upstream requests are checked). Control trailers are required on every upstream request that carries a body. The empty-body case is renamed dedicated/empty-body-served-not-502 and skips the trailer loop, since a zero-length body cannot carry trailers upstream. With the strip disabled, every non-empty case fails. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
Review battery — Tier 2 — run wf_f6e1f4cc-969 — terminal status: ESCALATE after 1 roundRound 1 (sonnet): 15 raw, 15 fresh, 4 refuted, 11 confirmed, 8 persona; fixes applied. Gate green at Fixes pushed:
Escalations and decisions (author decisions, inside Hugo's Q-R8STRIP2 ruling):
Caveat: orchestrated by the authoring session; finder, verifier and fix agents ran with fresh context. |
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.
Summary
Implements Hugo's ruling Q-R8STRIP2 (2026-10-04): no
X-Saturn-*header reaches the upstream (vLLM / Dynamo). This widens phoebe's #57 strip, which kept the trusted set and the eight edge-contract headers, into a strip of the whole namespace.identity.StripUntrustedSaturnHeadersis nowidentity.StripSaturnHeaders. It removes every header whose name starts withX-Saturn-(case-insensitive), including trusted, edge-contract, retired and unregistered names. Names that only look similar (X-Saturnine,X-SaturnX, bareX-Saturn) are kept.edgeContractHeaders,ForwardableSaturnHeadersand their oracle tests are deleted.PHOEBE_TRUSTED_HEADERSnow controls only whatFromRequestreads. It no longer affects what is forwarded.handleProxyright afteridentity.FromRequest, on every route. The trailer handling from Strip untrusted X-Saturn-* request headers before forwarding upstream (Q-R8STRIP b) #57 is unchanged and now strips the full namespace: the strip at body EOF and on Close, plus the Director-side strip innewUpstreamProxy.X-Saturn-*header from the raw request, with one exception, which this PR fixes. The R8 legacy-quota log diagnostic (legacyQuotaHeadersPresent(r.Header)) read raw headers after the strip. Since Strip untrusted X-Saturn-* request headers before forwarding upstream (Q-R8STRIP b) #57 it had silently stopped firing, because the strip had already removed those headers. On the currentrelease-2026.08.01its test,TestLegacyOnlyQuotaHeadersLogPreR8ProducerMarker, also panics: its logger has no Debug field and the strip's Debug log line dereferences it. The diagnostic now records whether the headers were present before the strip. The per-request Debug count line is removed, because the strip now removes headers on every request. Phoebe adds noX-Saturn-*header to the outbound request.Contracts
X-Saturn-*request header or trailer on any route: dedicated header-routed, shared header-routed, gateway, and the shared wake path including its probes. This includesX-Saturn-Upstream,-Resource-Id,-Auth-Id,-Org-Id,-Serving-Modeand the rate-limit envelope. Any engine-side code that read those headers would stop seeing them. None is known.PHOEBE_TRUSTED_HEADERSname and format are unchanged.https://claude.ai/code/session_01JR59LERE7vBqUT9cga5Kvq