Skip to content

test: ✅ Parity body tests #986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
evaline-ju merged 9 commits into from
Sep 14, 2026
Merged

test: ✅ Parity body tests #986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f8cbcf5
:white_check_mark: Parity tests that check body
evaline-ju Sep 11, 2026
05e4411
:art: Update interfaces
evaline-ju Sep 14, 2026
d605f45
Merge branch 'main' into parity-body-tests
evaline-ju Sep 14, 2026
48b108e
:art: Address review comments
evaline-ju Sep 14, 2026
3ddfb2f
:white_check_mark: Update tests based on review
evaline-ju Sep 14, 2026
d3d7138
:white_check_mark: Address review comments
evaline-ju Sep 14, 2026
6936673
Merge branch 'main' into parity-body-tests
evaline-ju Sep 14, 2026
ff9e2ed
:bug: Update tls bridge client test usage
evaline-ju Sep 14, 2026
41b63b3
:bulb: Update test comments
evaline-ju Sep 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions authbridge/authlib/listener/forwardproxy/tlsbridge_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func TestTransparentBridge(t *testing.T) {
t.Fatalf("NewEphemeralSource: %v", err)
}
minter := tlsbridge.NewMinter(src, tlsbridge.MinterOpts{})
up, err := tlsbridge.NewUpstreamClient(originCAPEM)
up, err := tlsbridge.NewUpstreamClient(originCAPEM, false)

@huang195 huang195 Sep 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note (not a defect — please just make sure this doesn't get lost)

This line and its five siblings are the fix for a currently-broken main, not test cleanup incidental to the parity work.

main tip bafeceb0 is red: Go CI (authlib) → Lint fails with

vet: listener/forwardproxy/tlsbridge_integration_test.go:117:52: not enough arguments in call to tlsbridge.NewUpstreamClient

#677 (5d56f48f) changed the signature to NewUpstreamClient(extraRootsPEM []byte, insecure bool) and updated only tlsbridge/upstream_test.go and cmd/authbridge-proxy/main.go. It missed the five call sites here plus one in tunnelreason_integration_test.go. No build tag on these files, so it's a straight compile failure in the forwardproxy test package. main has been red since 69b48de6.

The false values are right — they preserve the pre-#677 verifying behavior at every site, including TestBridge_UnverifiableUpstream_FallsOpenToTunnel, which needs verification enabled for its premise to hold.

Two asks:

  1. Consider cherry-picking ff9e2ed into its own fix: PR so main goes green immediately instead of waiting on review of the parity work. If you'd rather keep it here, that's fine too — but then this PR should be treated as merge-blocking-urgent.
  2. Either way, mention it in the PR body. Right now the description is purely about parity body coverage, so a reader has no idea this PR carries a build fix.

@evaline-ju evaline-ju Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 PR body mention updated

if err != nil {
t.Fatalf("NewUpstreamClient: %v", err)
}
Expand Down Expand Up @@ -268,7 +268,7 @@ func TestTransparentBridge_CustomPort(t *testing.T) {
t.Fatalf("NewEphemeralSource: %v", err)
}
minter := tlsbridge.NewMinter(src, tlsbridge.MinterOpts{})
up, err := tlsbridge.NewUpstreamClient(originCAPEM)
up, err := tlsbridge.NewUpstreamClient(originCAPEM, false)
if err != nil {
t.Fatalf("NewUpstreamClient: %v", err)
}
Expand Down Expand Up @@ -382,7 +382,7 @@ func TestConnectBridge(t *testing.T) {
t.Fatalf("NewEphemeralSource: %v", err)
}
minter := tlsbridge.NewMinter(src, tlsbridge.MinterOpts{})
up, err := tlsbridge.NewUpstreamClient(originCAPEM)
up, err := tlsbridge.NewUpstreamClient(originCAPEM, false)
if err != nil {
t.Fatalf("NewUpstreamClient: %v", err)
}
Expand Down Expand Up @@ -563,15 +563,15 @@ func TestBridge_UnverifiableUpstream_FallsOpenToTunnel(t *testing.T) {
originHostPort := originURL.Host // "127.0.0.1:port"

// Build the bridge Engine. The Upstream client trusts ONLY the system roots
// (NewUpstreamClient(nil)) — it does NOT trust the httptest origin's
// (NewUpstreamClient(nil, false)) — it does NOT trust the httptest origin's
// self-signed CA, so bridgeServe's upstream-verify HEAD fails and the branch
// falls open to a plain tunnel.
src, err := tlsbridge.NewEphemeralSource()
if err != nil {
t.Fatalf("NewEphemeralSource: %v", err)
}
minter := tlsbridge.NewMinter(src, tlsbridge.MinterOpts{})
up, err := tlsbridge.NewUpstreamClient(nil)
up, err := tlsbridge.NewUpstreamClient(nil, false)
Comment thread
evaline-ju marked this conversation as resolved.
if err != nil {
t.Fatalf("NewUpstreamClient: %v", err)
}
Expand Down Expand Up @@ -699,15 +699,15 @@ func TestBridge_PinnedClient_AutoSkipsThenTunnels(t *testing.T) {
}
originHostPort := originURL.Host // "127.0.0.1:port"

// Upstream trusts the origin (NewUpstreamClient(originCAPEM)) so upstream-verify
// Upstream trusts the origin (NewUpstreamClient(originCAPEM, false)) so upstream-verify
// PASSES and bridgeServe reaches the Terminate step where the pinned agent
// rejects the minted leaf.
src, err := tlsbridge.NewEphemeralSource()
if err != nil {
t.Fatalf("NewEphemeralSource: %v", err)
}
minter := tlsbridge.NewMinter(src, tlsbridge.MinterOpts{})
up, err := tlsbridge.NewUpstreamClient(originCAPEM)
up, err := tlsbridge.NewUpstreamClient(originCAPEM, false)
if err != nil {
t.Fatalf("NewUpstreamClient: %v", err)
}
Expand Down Expand Up @@ -869,7 +869,7 @@ func TestBridge_NonTLS_Passthrough(t *testing.T) {
t.Fatalf("NewEphemeralSource: %v", err)
}
minter := tlsbridge.NewMinter(src, tlsbridge.MinterOpts{})
up, err := tlsbridge.NewUpstreamClient(nil)
up, err := tlsbridge.NewUpstreamClient(nil, false)
if err != nil {
t.Fatalf("NewUpstreamClient: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion authbridge/authlib/listener/forwardproxy/tunnelreason_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func bridgeForRejectTest(t *testing.T) (*Server, *session.Store, string) {
if err != nil {
t.Fatalf("NewEphemeralSource: %v", err)
}
up, err := tlsbridge.NewUpstreamClient(originCAPEM)
up, err := tlsbridge.NewUpstreamClient(originCAPEM, false)
if err != nil {
t.Fatalf("NewUpstreamClient: %v", err)
}
Expand Down
92 changes: 80 additions & 12 deletions authbridge/authlib/listener/parity/drivers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,40 @@ type fixture struct {
path string
reqBody []byte

// upstreamStatus / upstreamBody are what the httptest backend serves
// on the proxy path, and what the parity harness synthesizes into the
// extproc ResponseHeaders/ResponseBody messages. Zero status skips
// the response phase entirely (deny-at-request scenarios).
upstreamStatus int
upstreamBody []byte
// upstreamStatus / upstreamBody / upstreamContentType are what the
// httptest backend serves on the proxy path, and what the parity
// harness synthesizes into the extproc ResponseHeaders/ResponseBody
// messages. Zero status skips the response phase entirely (deny-at-
// request scenarios). Empty content-type defaults to application/json.
upstreamStatus int
upstreamBody []byte
upstreamContentType string

// pipelineRefusedPreRun asserts the listener refused before the
// pipeline (e.g. body overflow). Default false: every listener must
// record. Combine with expectedWireStatus to pin the wire code.
pipelineRefusedPreRun bool

// expectedWireStatus, when non-zero, is asserted against every
// listener's wire status. Only meaningful together with
// pipelineRefusedPreRun — on the success path extproc has no HTTP
// transport and reports 0.
expectedWireStatus int
Comment thread
evaline-ju marked this conversation as resolved.

// expectedPluginEvents anchors correctness — maps each expected
// SessionEvent.Plugins key to its exact JSON. Empty means "don't
// assert content beyond the pairwise diff." Fixtures that want
// bug-catching (not just drift-catching) fill this in.
expectedPluginEvents map[string]string
}

// contentType returns the fixture's response content-type or a sensible
// default. Kept as a helper so both driver paths stay compact.
func (f fixture) contentType() string {
if f.upstreamContentType != "" {
return f.upstreamContentType
}
return "application/json"
}

// buildSpyPipeline routes construction through plugins.BuildWithDeps
Expand All @@ -63,6 +91,12 @@ func spyEntry(name string, cfg spyConfig) config.PluginEntry {
// (Host casing, timestamps, RequestID, Duration, TLS, Identity) are
// excluded — expanding coverage there is a follow-up fixture pass.
type observation struct {
// PipelineRan is false when the listener rejected the request before
// the pipeline (e.g. request body too large). Overflow fixtures then
// assert wire status only and skip session-event comparisons.
PipelineRan bool
WireStatus int // captured from the transport, not from the session event

Phase string
StatusCode int
Error *errorSummary
Expand Down Expand Up @@ -251,7 +285,7 @@ func runExtproc(t *testing.T, f fixture, wantPhase pipeline.SessionPhase) *obser
ResponseHeaders: &extprocv3.HttpHeaders{
Headers: makeHeaders(
":status", fmt.Sprintf("%d", f.upstreamStatus),
"content-type", "application/json",
"content-type", f.contentType(),
"content-length", fmt.Sprintf("%d", len(f.upstreamBody)),
),
},
Expand Down Expand Up @@ -284,7 +318,41 @@ func runExtproc(t *testing.T, f fixture, wantPhase pipeline.SessionPhase) *obser
}
}

return observe(t, store, f.direction, wantPhase)
return finalizeObservation(t, f, observe(t, store, f.direction, wantPhase), extprocWireStatus(stream))
}

// extprocWireStatus reads the HTTP status from an ImmediateResponse if
// one was sent, else 0. Pipeline-ran is inferred from observe(): a
// session event exists iff the pipeline reached the recording site.
func extprocWireStatus(stream *mockStream) int {
for _, r := range stream.responses {
if imm := r.GetImmediateResponse(); imm != nil && imm.Status != nil {
return int(imm.Status.Code)
}
}
return 0
}

// finalizeObservation stamps PipelineRan + WireStatus onto an
// observation. pipelineRefusedPreRun is a strict expectation: the
// listener MUST refuse before the pipeline. A missing event when the
// fixture didn't opt in is a bug; an event present when it did is
// also a bug (the listener silently stopped enforcing the cap).
func finalizeObservation(t *testing.T, f fixture, obs *observation, wireStatus int) *observation {
t.Helper()
if obs == nil {
if !f.pipelineRefusedPreRun {

@huang195 huang195 Sep 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must-fix — pipelineRefusedPreRun only permits a missing session event; nothing requires one.

So TestParity_InboundRequestBodyOverflow passes if both inbound listeners stop enforcing the 1 MiB cap: the pipeline runs, both listeners record, finalizeObservation stamps PipelineRan: true on both, and observationDiff falls through to the ordinary session-event comparison and agrees. A test named for overflow rejection would then be green with an unbounded body flowing through.

That is exactly the shared-drop failure mode expectedPluginEvents was added to close (see the "correctness anchors" comment in parity_test.go), but the mechanism doesn't reach this fixture — it has no expected events, and WireStatus is only ever compared pairwise, never against 413.

Two small changes close it:

  1. Here, assert the fixture's expectation rather than merely permitting it:
obs.PipelineRan = true
if f.pipelineRefusedPreRun {
    t.Errorf("fixture %q expected the listener to refuse before the pipeline, but an event was recorded", f.name)
}
  1. Add an expectedWireStatus int to fixture and check it in assertParity, so 413 is pinned rather than merely agreed on.

t.Errorf("no session event recorded for fixture %q; set pipelineRefusedPreRun=true if expected", f.name)
return nil
}
return &observation{PipelineRan: false, WireStatus: wireStatus}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
if f.pipelineRefusedPreRun {
t.Errorf("fixture %q expected the listener to refuse before the pipeline, but an event was recorded (wireStatus=%d)", f.name, wireStatus)
}
obs.PipelineRan = true
obs.WireStatus = wireStatus
return obs
}

// --- reverseproxy driver -------------------------------------------------
Expand All @@ -306,7 +374,7 @@ func runReverseProxy(t *testing.T, f fixture, wantPhase pipeline.SessionPhase) *
w.WriteHeader(http.StatusInternalServerError)
return
}
w.Header().Set("Content-Type", "application/json")
w.Header().Set("Content-Type", f.contentType())
w.WriteHeader(f.upstreamStatus)
_, _ = w.Write(f.upstreamBody)
}))
Expand Down Expand Up @@ -352,7 +420,7 @@ func runReverseProxy(t *testing.T, f fixture, wantPhase pipeline.SessionPhase) *
t.Errorf("reverseproxy: fixture %q asked for deny but upstream was reached", f.name)
}

return observe(t, store, pipeline.Inbound, wantPhase)
return finalizeObservation(t, f, observe(t, store, pipeline.Inbound, wantPhase), resp.StatusCode)
}

// --- forwardproxy driver -------------------------------------------------
Expand All @@ -374,7 +442,7 @@ func runForwardProxy(t *testing.T, f fixture, wantPhase pipeline.SessionPhase) *
w.WriteHeader(http.StatusInternalServerError)
return
}
w.Header().Set("Content-Type", "application/json")
w.Header().Set("Content-Type", f.contentType())
w.WriteHeader(f.upstreamStatus)
_, _ = w.Write(f.upstreamBody)
}))
Expand Down Expand Up @@ -425,7 +493,7 @@ func runForwardProxy(t *testing.T, f fixture, wantPhase pipeline.SessionPhase) *
t.Errorf("forwardproxy: fixture %q asked for deny but upstream was reached", f.name)
}

return observe(t, store, pipeline.Outbound, wantPhase)
return finalizeObservation(t, f, observe(t, store, pipeline.Outbound, wantPhase), resp.StatusCode)
}

// --- construction-only helpers -------------------------------------------
Expand Down
Loading
Loading