feat(proxy): inject session ID and sequence number headers on matching requests#198
Open
SasSwart wants to merge 10 commits into
Open
feat(proxy): inject session ID and sequence number headers on matching requests#198SasSwart wants to merge 10 commits into
SasSwart wants to merge 10 commits into
Conversation
This was referenced Apr 30, 2026
SasSwart
force-pushed
the
sasswart/feat-config-session-correlation-header-injection
branch
from
c24a0fd to
c08f291
Compare
SasSwart
force-pushed
the
sasswart/feat-config-session-correlation-header-injection
branch
from
c08f291 to
3ea7428
Compare
SasSwart
force-pushed
the
sasswart/feat-proxy-inject-session-headers
branch
from
6e61348 to
d8635fb
Compare
Base automatically changed from
sasswart/feat-config-session-correlation-header-injection
to
main
May 7, 2026 14:04
…g requests When session correlation is enabled and the outgoing request matches a configured inject target, set X-Coder-Agent-Firewall-Session-Id and X-Coder-Agent-Firewall-Sequence-Number headers on the forwarded request. Any values the jailed client may have set are overwritten so the upstream always sees boundary's authoritative session ID and sequence number. The sequence number is pre-allocated before the audit event so both the audit log and the injected header carry the same value. audit.Request gains a SequenceNumber pointer field; when non-nil the socket auditor uses it instead of calling its own counter. New proxy.Config fields: SessionCorrelation, SessionID, SequenceCounter. New Server method: shouldInjectHeaders (domain + optional path glob matching). Tests cover matched domain, unmatched domain, disabled injection, client-supplied header overwrite, path glob matching, and sequence number incrementing.
Updated the SequenceNumber field in the audit.Request struct and related handling in the SocketAuditor and proxy components to use int32 instead of uint64. This change ensures consistency in data types across the application. Additionally, minor adjustments were made to the session correlation tests to reflect the updated header names and improve clarity in assertions.
SasSwart
force-pushed
the
sasswart/feat-proxy-inject-session-headers
branch
from
d8635fb to
79d6485
Compare
…ber handling Eliminated the SequenceCounter from the SocketAuditor and related components, simplifying the sequence number management. The SequenceNumber field in the audit.Request struct is now a non-pointer int32, ensuring consistent handling across the application. Adjusted tests to reflect these changes and maintain functionality.
…comments Updated the host port stripping logic in the shouldInjectHeaders function to utilize net.SplitHostPort for improved clarity and reliability. Removed outdated comments regarding session correlation header injection to enhance code readability.
…nd transport configuration Added a new test to verify that the sequence number increments correctly across different request types (HTTP and HTTPS) in the proxy. Introduced a `WithForwardTransport` option to allow tests to specify a custom HTTP transport for handling self-signed certificates. Updated the proxy server configuration to support this transport, ensuring proper handling of requests with session correlation and sequence number injection.
Contributor
Author
|
I've self-reviewed this agent generated PR. Apart from double-checking |
SasSwart
added a commit
that referenced
this pull request
May 7, 2026
…er agreement Add integration tests that verify the core invariants of session correlation across the proxy, auditor, and forwarded request headers working together. These tests fill the gap identified during review of the session correlation PR stack (#196, #197, #198) where unit tests verified each component in isolation but did not verify them in concert. New test file: proxy/proxy_session_correlation_integration_test.go Tests added: - LLMRequestAuditAndHeadersAgree: audit sequence number matches the forwarded header value on inject-target requests. - NonLLMRequestAuditedWithoutHeaders: allowed non-inject-target requests are audited but carry no correlation headers. - DeniedRequestAuditedNeverForwarded: denied requests consume a sequence number but are never forwarded. - MixedRequestsSequenceOrdering: interleaved LLM, non-LLM, and denied requests all advance the counter monotonically. - SequenceGapRevealsAgenticLoop: gap between two LLM sequence numbers precisely equals intermediate tool-use requests. - SpoofedHeadersOverwrittenWithCorrectSequence: client-supplied headers are replaced and the audit event still agrees. - DisabledCorrelationNoHeadersNoPreallocatedSequence: disabled correlation means no headers and no pre-allocated sequence. - ConcurrentRequestsUniqueSequenceNumbers: concurrent requests each get a unique, dense sequence number.
johnstcn
reviewed
May 8, 2026
Member
johnstcn
left a comment
johnstcn
left a comment
There was a problem hiding this comment.
This is my first read of a boundary PR, I don't see anything immediately blocking but I would recommend a second pair of eyes more familiar with this codebase.
Comment on lines
+20
to
+21
| mu sync.Mutex | ||
| headers http.Header |
Member
There was a problem hiding this comment.
Suggestion, non-blocking: This only allows you to check one request at a time. A chan http.Header would let you fire off multiple requests and check them in order of arrival.
Contributor
Author
|
Thanks @johnstcn! I'll have a look at your feedback! Susana has been reviewing but she's out today. Hence knocking on your door. |
Replace the custom shouldInjectHeaders domain/path matching with the existing rulesengine engine. Inject targets now use the same "domain=... path=..." syntax and identical matching semantics as --allow rules. Changes: - config: Remove InjectTarget struct and ParseInjectTarget. Change SessionCorrelationConfig.InjectTargets from []InjectTarget to []string (raw rule specs). DefaultInjectTargetFromEnv returns a string. ValidateSessionCorrelation delegates parsing to rulesengine.ParseAllowSpecs. - config: Simplify buildSessionCorrelation to pass raw strings. - proxy: Replace sessionCorrelation field with injectEngine (*rulesengine.Engine). shouldInjectHeaders now delegates to injectEngine.Evaluate instead of hand-rolled matching. - proxy: Add InjectEngine to proxy.Config, built from SessionCorrelation.InjectTargets at construction time. - tests: Update all session correlation tests to use raw rule strings. This resolves the behavioral differences between inject targets and allow rules: case sensitivity, wildcard/subdomain support, path glob semantics, trailing-slash normalization, and input validation are now identical.
The behavior is already documented on the Config.ForwardTransport field. Addresses review feedback from johnstcn.
…get-matching refactor(proxy): unify inject target matching with rulesengine
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
3 participants
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.
PR Map
RFC: Bridge ↔ Boundaries Correlation
When session correlation is enabled and the outgoing request matches a configured inject target, set
X-Coder-Agent-Firewall-Session-IdandX-Coder-Agent-Firewall-Sequence-Numberheaders on the forwarded request. Any values the jailed client may have set are overwritten so the upstream always sees boundary's authoritative session ID and sequence number.Depends on #197.
Changes
audit/request.go: AddSequenceNumber int32field toRequest. The proxy assigns this value before auditing so the audit log and the injected HTTP header carry the same sequence number.audit/socket_auditor.go: Remove theseq *SequenceCounterfield. The auditor now readsreq.SequenceNumberdirectly instead of generating its own value.NewSocketAuditorno longer accepts a*SequenceCounterparameter.audit/multi_auditor.go: RemoveSequenceCountercreation inSetupAuditor; callNewSocketAuditorwith three arguments (logger, socket path, session ID).audit/socket_auditor_test.go: Removeseqfield from test auditor structs. Tests that exercise sequence numbers now setSequenceNumberon theRequestdirectly.cli/cli.go: Minor punctuation fix in the--session-id-inject-targetflag description.proxy/proxy.go:sessionCorrelation,sessionID,seqCounter,forwardTransportfields toServer.SessionCorrelation,SessionID,ForwardTransporttoConfig(the sequence counter is an internal value-type field onServer, not user-configurable).shouldInjectHeaders(host, path)method: matches request host (case-insensitive, port-stripped) and optional path glob against configuredInjectTargets.processHTTPRequestallocates a sequence number fromp.seqCounterand passes it to bothAuditRequestandforwardRequest.forwardRequeststamps both headers on matching requests after copying client headers (overwriting any client-supplied values). UsesForwardTransportwhen set.proxy/proxy_audit_test.go: New testTestSequenceNumberIncrementsAcrossRequestTypes— verifies sequence numbers increment correctly across plain HTTP, implicit-CONNECT HTTPS, and explicit-CONNECT tunnel requests.proxy/proxy_framework_test.go: AddWithSessionCorrelation,WithSessionID,WithForwardTransporttest options; wire them into the proxyConfig.proxy/proxy_session_correlation_test.go: New test file with seven tests:TestSessionCorrelation_MatchedDomain: headers injected on matching domain.TestSessionCorrelation_MatchedDomainWithPort: headers injected when inject target domain includes a port.TestSessionCorrelation_UnmatchedDomain: headers absent on non-matching domain.TestSessionCorrelation_Disabled: headers absent when correlation is disabled.TestSessionCorrelation_OverwritesClientValue: proxy overwrites spoofed client headers.TestSessionCorrelation_PathMatching: headers injected only when path glob matches.TestSessionCorrelation_SequenceNumberIncrements: sequence numbers increment across requests.Implementation notes
processHTTPRequestviap.seqCounter.Next(), then threaded through to both the auditor and the header injection site. This guarantees the audit event and the HTTP header always agree on the sequence number for a given request.shouldInjectHeadersstrips the port from both the request host and the configured target domain usingnet.SplitHostPortbefore comparison, and usespath.Matchfor glob matching, consistent with the RFC'spath=/api/v2/aibridge/*syntax.SequenceCounteronServeris a value type (audit.SequenceCounter) backed by anatomic.Int32. It is always present and starts at 0; sequence numbers are allocated for every request regardless of whether session correlation is enabled.Note
This PR was authored by Coder Agents.