test(proxy): integration tests for session correlation audit and header agreement by SasSwart · Pull Request #199 · coder/boundary

SasSwart · 2026-05-01T12:36:26Z

PR Map

feat(audit): session ID generation, sequence counter, and BoundaryLog wiring #196
feat(config): session correlation header injection configuration #197
feat(proxy): inject session ID and sequence number headers on matching requests #198
👉🏼 test(proxy): integration tests for session correlation audit and header agreement #199

Integration tests for session correlation, requested during review of #196. The existing unit tests in #198 verify header injection and audit capture independently; these tests verify both sides together across realistic multi-request scenarios.

Depends on #201.

Changes

proxy/proxy_session_correlation_integration_test.go: New test file with 8 integration tests and supporting helpers.

Test scenarios

Test	What it verifies
`LLMRequestAuditAndHeadersAgree`	Audit sequence number == 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 advance the counter monotonically
`SequenceGapRevealsAgenticLoop`	Gap between two LLM sequence numbers equals the count of intermediate tool-use requests
`SpoofedHeadersOverwrittenWithCorrectSequence`	Client-supplied headers are replaced; audit event still agrees with header
`DisabledCorrelationNoHeadersNoPreallocatedSequence`	Disabled correlation means no headers and no pre-allocated sequence number
`ConcurrentRequestsUniqueSequenceNumbers`	10 concurrent requests each get a unique, dense sequence number

Test helpers

multiRequestCapturingBackend: like the existing headerCapturingBackend but records headers from every request (not just the last), needed for multi-request scenarios.
sessionCorrelationIntegrationSetup: shared setup that wires a proxy with two httptest backends (LLM inject target + non-LLM), a capturingAuditor, and a shared SequenceCounter.

Note

This PR was authored by Coder Agents.

SasSwart

A few things to address before I can request external review.

SasSwart · 2026-05-13T12:04:11Z

+			InjectTargets: []config.InjectTarget{{Domain: llmURL.Hostname()}},
+		}),
+		WithSessionID("should-not-appear"),
+		// Explicitly do NOT set WithSequenceCounter; seqCounter is nil.


Don't we still set seqCounter even when correlation is disabled? This might not be a valid way to test.

…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.

…ames and sequence number type Modified integration tests to reflect changes in session correlation header names and updated the sequence number type from uint64 to int32. Adjusted assertions in tests to ensure consistency with the new data types and header configurations, enhancing clarity and correctness in the test suite.

- Remove '// ---------- Integration Tests ----------' section separator - Rename 'hdr'/'Hdr' variables to 'header'/'Header' for clarity - Rename 'llm'/'llmBackend' to 'injectBackend'/'inject'/'backend' to reflect the actual concept (inject target) rather than a specific use case (LLM) - Update comments to match the new naming Generated by Coder Agents

…n integration tests - Enhanced comments for clarity regarding the purpose of `injectBackend` and `otherBackend`. - Removed unnecessary comments to streamline the test code. - Adjusted formatting for consistency and readability in the `sessionCorrelationIntegrationSetup` struct. These changes aim to improve the maintainability and understanding of the integration tests related to session correlation.

- Rename sessionCorrelationIntegrationSetup to correlationTestEnv and newSessionCorrelationIntegrationSetup to newCorrelationTestEnv for brevity and clarity. - Rename TestIntegration_DisabledCorrelationNoHeadersNoPreallocatedSequence to TestIntegration_DisabledCorrelationNoHeaders. The sequence counter is a value type on the proxy server and always increments regardless of the correlation setting, so the previous name and assertions about 'no pre-allocated sequence number' were misleading. The test now focuses on what actually differs: no headers are injected. - Remove misleading 'auditor falls back to its own counter' comment. Generated by Coder Agents

PR #201 was merged to main, replacing config.InjectTarget struct with []string rule specs (rulesengine syntax). Update the integration test file to use the []string format, and sync config/, proxy/proxy.go, and other test files from main to fix the build. Generated by Coder Agents

dannykopping · 2026-05-18T06:58:32Z

+// verifies that when a jailed client sets its own correlation headers,
+// the proxy replaces them with the real session ID and the real
+// sequence number, and the audit event still agrees with the header.
+func TestIntegration_SpoofedHeadersOverwrittenWithCorrectSequence(t *testing.T) {


Nice! We should probably add an audit log for this; useful to know when some{one,thing}'s trying to do this.

We should probably add an audit log for this; useful to know when some{one,thing}'s trying to do this

Happy to do this as a follow-up. Will create an issue for it and link it here.

I've created the issue

dannykopping · 2026-05-18T07:05:54Z

@@ -0,0 +1,560 @@
+package proxy


All your HTTP requests should have a context set, please.

Addressed in 7339ad5

Thanks. AFAICS there are no timeouts set; did I miss something?

😅 Yeah. I added the contexts and then found this:

boundary/proxy/proxy_framework_test.go

Lines 210 to 235 in 3e1e57b

// Create HTTP client for direct proxy requests

pt.client = &http.Client{

Transport: &http.Transport{

TLSClientConfig: &tls.Config{

InsecureSkipVerify: true, // Skip cert verification for testing

},

},

Timeout: 5 * time.Second,

}

// Create HTTP client for proxy transport (implicit CONNECT)

proxyURL, err := url.Parse("http://localhost:" + strconv.Itoa(pt.port))

require.NoError(pt.t, err, "Failed to parse proxy URL")

pt.proxyClient = &http.Client{

Transport: &http.Transport{

Proxy: http.ProxyURL(proxyURL),

TLSClientConfig: &tls.Config{

InsecureSkipVerify: true, // Skip cert verification for testing

},

},

Timeout: 10 * time.Second,

}

return pt

}

That sets timeouts on all of the requests made by this test suite.

dannykopping

Overall tests look solid and coverage is good 👍

A few relatively minor points to address

…n tests - Remove redundant require.NotNil assertions on int32 SequenceNumber fields (value types cannot be nil) - Assert audit events are empty before the request in LLMRequestAuditAndHeadersAgree to prove causality - Add missing HTTP status code checks in SequenceGapRevealsAgenticLoop - Use context-aware HTTP requests throughout via doGet helper and http.NewRequestWithContext with t.Context() Co-authored-by: Coder Agents <coder-agents@coder.com>

- Introduced ExpectGetViaProxy method to streamline context-bound GET requests through the proxy, enhancing test reliability. - Updated integration tests to utilize ExpectGetViaProxy, improving readability and consistency in handling HTTP requests. - Removed redundant doGet helper function as its functionality is now encapsulated in ExpectGetViaProxy. These changes aim to improve the maintainability and clarity of the proxy integration tests.

…tCapturingBackend - Updated headersAt method to return an error when the index is out of range, improving robustness. - Adjusted integration tests to handle the new error return, ensuring proper assertions and error checks. - This change enhances the reliability of session correlation tests by preventing potential panics from invalid index access.

- Added nil checks for the proxy target before stopping it to prevent potential nil pointer dereference errors. - This change enhances the stability of the correlation test environment by ensuring safe resource cleanup.

…e naming - Renamed header variables to headers for clarity and consistency across multiple integration tests. - Added nil checks for headers to ensure they are not nil before assertions, enhancing test robustness. - These changes improve the readability and reliability of the session correlation integration tests.

SasSwart

Review feedback tended to.

dannykopping

Good to go once (or if) we have timeouts set to prevent requests hanging indefinitely.

dannykopping · 2026-05-21T13:01:19Z

@@ -0,0 +1,560 @@
+package proxy


Thanks. AFAICS there are no timeouts set; did I miss something?

SasSwart force-pushed the sasswart/feat-proxy-inject-session-headers branch 2 times, most recently from d8635fb to 79d6485 Compare May 7, 2026 14:33

SasSwart force-pushed the sasswart/test-session-correlation-integration branch from ae01776 to 4a1655f Compare May 7, 2026 17:21

This was referenced May 12, 2026

refactor(proxy): unify inject target matching with rulesengine #201

Merged

feat(proxy): inject session ID and sequence number headers on matching requests #198

Merged

SasSwart changed the base branch from sasswart/feat-proxy-inject-session-headers to sasswart/refactor-unify-inject-target-matching May 12, 2026 10:11

Base automatically changed from sasswart/refactor-unify-inject-target-matching to sasswart/feat-proxy-inject-session-headers May 12, 2026 12:24

Base automatically changed from sasswart/feat-proxy-inject-session-headers to main May 13, 2026 08:56

SasSwart marked this pull request as draft May 13, 2026 09:39

SasSwart commented May 13, 2026

View reviewed changes

SasSwart marked this pull request as ready for review May 13, 2026 12:14

SasSwart requested a review from dannykopping May 14, 2026 07:39

SasSwart added 7 commits May 14, 2026 10:25

make fmt

f578460

SasSwart force-pushed the sasswart/test-session-correlation-integration branch from 11b11c0 to f578460 Compare May 14, 2026 10:29

test(proxy): use Given/When/Then style comments in integration tests

a9d59ec

dannykopping reviewed May 18, 2026

View reviewed changes

doc-check[bot] and others added 5 commits May 18, 2026 09:00

fix(proxy): improve stop method in correlation test environment

525ef13

- Added nil checks for the proxy target before stopping it to prevent potential nil pointer dereference errors. - This change enhances the stability of the correlation test environment by ensuring safe resource cleanup.

fix(proxy): update session ID in integration test for consistency

6952ad0

SasSwart added 2 commits May 21, 2026 12:39

refactor(proxy): rename and enhance sequence gap integration test

845c5ce

SasSwart commented May 21, 2026

View reviewed changes

SasSwart requested a review from dannykopping May 21, 2026 12:58

dannykopping approved these changes May 21, 2026

View reviewed changes

SasSwart merged commit d00bd87 into main May 21, 2026
5 checks passed

SasSwart deleted the sasswart/test-session-correlation-integration branch May 21, 2026 13:38

	// Create HTTP client for direct proxy requests
	pt.client = &http.Client{
	Transport: &http.Transport{
	TLSClientConfig: &tls.Config{
	InsecureSkipVerify: true, // Skip cert verification for testing
	},
	},
	Timeout: 5 * time.Second,
	}

	// Create HTTP client for proxy transport (implicit CONNECT)
	proxyURL, err := url.Parse("http://localhost:" + strconv.Itoa(pt.port))
	require.NoError(pt.t, err, "Failed to parse proxy URL")

	pt.proxyClient = &http.Client{
	Transport: &http.Transport{
	Proxy: http.ProxyURL(proxyURL),
	TLSClientConfig: &tls.Config{
	InsecureSkipVerify: true, // Skip cert verification for testing
	},
	},
	Timeout: 10 * time.Second,
	}

	return pt
	}

Conversation

SasSwart commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Map

Changes

Test scenarios

Test helpers

Uh oh!

SasSwart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dannykopping left a comment

Choose a reason for hiding this comment

Uh oh!

SasSwart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dannykopping left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SasSwart commented May 1, 2026 •

edited

Loading