From f438ca6abdfe16999c188b355c55fdf2246d5c1f Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Thu, 29 Oct 2026 21:45:05 -0400 Subject: [PATCH 1/2] Fix: Fold a redirected bridged CONNECT into its inner request in agentop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isBridgedInner matched the inner request's Host against the tunnel's CONNECT target, so a request the inference-router redirected to another server never folded: the tunnel-open stood as its own blank-method row naming the original server, with no response row (the inner request carries it), roughly doubling the timeline and reading as if traffic still went to the host the client named. A redirected inner row carries the client's host in RequestedHost — the CONNECT's target — so accept that as the second match arm. RequestedHost is present only on a redirected request, so an unredirected request still cannot fold onto someone else's tunnel. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/agentop/tui/events_pane.go | 21 +++++++++---- cmd/agentop/tui/events_pane_test.go | 46 +++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/cmd/agentop/tui/events_pane.go b/cmd/agentop/tui/events_pane.go index fd2aebfeb..cb9f1e433 100644 --- a/cmd/agentop/tui/events_pane.go +++ b/cmd/agentop/tui/events_pane.go @@ -654,6 +654,10 @@ func closedTunnels(events []pipeline.SessionEvent) map[string]bool { // host-part (port-insensitive) handles both the standard-443 case (inner host // = "example.com") and the non-standard case (inner host = "example.com:8443"). // +// A redirected inner request also folds: a plugin (inference-router) may have +// moved it to another host after decryption, so inner.Host is the dial target +// and the host the client named — the CONNECT's target — is RequestedHost. +// // Two guards keep unrelated events from folding: // - inner must be another outbound REQUEST, never a response, so a plain // request→response exchange isn't mistaken for a bridged pair. @@ -661,12 +665,19 @@ func closedTunnels(events []pipeline.SessionEvent) map[string]bool { // passthrough CONNECTs to the same host (common with connection pooling) // would otherwise fold — hiding one real message and mislabeling the other. func isBridgedInner(tunnel, inner *pipeline.SessionEvent) bool { + if inner.Direction != pipeline.Outbound || + inner.Phase != pipeline.SessionRequest || + isTunnelOpen(inner) { + return false + } host := hostOnly(inner.Host) - return inner.Direction == pipeline.Outbound && - inner.Phase == pipeline.SessionRequest && - !isTunnelOpen(inner) && - host != "" && - host == hostOnly(tunnel.Host) + if host != "" && host == hostOnly(tunnel.Host) { + return true + } + // RequestedHost is present only when a redirect moved the request, so this + // arm cannot fold an unredirected request onto someone else's tunnel. + return inner.RequestedHost != "" && + hostOnly(inner.RequestedHost) == hostOnly(tunnel.Host) } // hostOnly returns the host portion of a "host:port" string, or the input diff --git a/cmd/agentop/tui/events_pane_test.go b/cmd/agentop/tui/events_pane_test.go index ea51008c3..516821c2b 100644 --- a/cmd/agentop/tui/events_pane_test.go +++ b/cmd/agentop/tui/events_pane_test.go @@ -377,6 +377,52 @@ func TestBuildEventRows_CollapsesBridgedConnect(t *testing.T) { } } +// TestBuildEventRows_CollapsesBridgedConnectRedirected locks the fold under a +// plugin redirect: the CONNECT named the host the client asked for, the inner +// request went elsewhere, and RequestedHost carries the client's host. Without +// the RequestedHost match arm the open stood as its own blank-method row and +// the timeline read as if traffic still went to the original server. +func TestBuildEventRows_CollapsesBridgedConnectRedirected(t *testing.T) { + events := []pipeline.SessionEvent{ + // CONNECT tunnel-open to the client's configured inference server. + { + Direction: pipeline.Outbound, + Phase: pipeline.SessionRequest, + Host: "ete-litellm.example.com:443", + Tunnel: true, + }, + // Inner request the inference-router redirected to another server: + // Host is the dial target, RequestedHost the host the client named. + { + Direction: pipeline.Outbound, + Phase: pipeline.SessionRequest, + Host: "litellm.cb.example.com", + RequestedHost: "ete-litellm.example.com:443", + Inference: &pipeline.InferenceExtension{Model: "glm-4.6"}, + }, + { + Direction: pipeline.Outbound, + Phase: pipeline.SessionResponse, + Host: "litellm.cb.example.com", + Inference: &pipeline.InferenceExtension{Model: "glm-4.6", TotalTokens: 500}, + StatusCode: 200, + }, + } + rows := buildEventRows(events) + if len(rows) != 2 { + t.Fatalf("redirected bridged call produced %d rows, want 2 (collapsed req + resp)", len(rows)) + } + if rows[0].event.Host != "litellm.cb.example.com" { + t.Errorf("collapsed row host = %q, want the redirected host", rows[0].event.Host) + } + if rows[0].tunnel == nil { + t.Fatalf("row 0 should carry the folded CONNECT tunnel") + } + if rows[0].tunnel.Host != "ete-litellm.example.com:443" { + t.Errorf("folded tunnel host = %q, want the client's original server", rows[0].tunnel.Host) + } +} + // TestBuildEventRows_PassthroughTunnelStandsAlone covers every shape in which a // non-bridged CONNECT tunnel-open must remain its own row rather than fold: // a lone trailing CONNECT, two back-to-back CONNECTs to the SAME host From 387aee21924e7271df4f9e9bcb811102d2531f8e Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Thu, 29 Oct 2026 21:56:36 -0400 Subject: [PATCH 2/2] Fix: Fold a denied bridged CONNECT into its denial row in agentop Fixes review: isBridgedInner only folded SessionRequest, so a bridged tunnel whose first decrypted row was a denial (a pipeline plugin Reject, recorded before any request row) kept its orphan tunnel-open row. Files: cmd/agentop/tui/events_pane.go, cmd/agentop/tui/events_pane_test.go Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- cmd/agentop/tui/events_pane.go | 16 +++++++---- cmd/agentop/tui/events_pane_test.go | 44 +++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/cmd/agentop/tui/events_pane.go b/cmd/agentop/tui/events_pane.go index cb9f1e433..e2e9e9283 100644 --- a/cmd/agentop/tui/events_pane.go +++ b/cmd/agentop/tui/events_pane.go @@ -584,9 +584,10 @@ func (m *model) selectedEventRow() (eventRow, bool) { // buildEventRows turns the chronological event slice into display rows, one // per network message. The only folding is the TLS-bridge CONNECT pair: a // tunnel-open event (opaque, host:port) immediately followed by the decrypted -// inner request for the same host is rendered as a single row keyed on the -// inner request, with the tunnel attached for the detail pane. A non-bridged -// (passthrough) tunnel has no inner request following it, so it stands as its +// inner row for the same host — a request, or a denial when a plugin rejected +// it before any request row — is rendered as a single row keyed on the +// inner row, with the tunnel attached for the detail pane. A non-bridged +// (passthrough) tunnel has no inner row following it, so it stands as its // own row — it IS the whole message. func buildEventRows(events []pipeline.SessionEvent) []eventRow { closed := closedTunnels(events) @@ -659,14 +660,17 @@ func closedTunnels(events []pipeline.SessionEvent) map[string]bool { // and the host the client named — the CONNECT's target — is RequestedHost. // // Two guards keep unrelated events from folding: -// - inner must be another outbound REQUEST, never a response, so a plain -// request→response exchange isn't mistaken for a bridged pair. +// - inner must be another outbound REQUEST or DENIAL, never a response, so a +// plain request→response exchange isn't mistaken for a bridged pair. A +// denial folds too: the proxy defers a bridged open and records it with +// whichever row comes first, and a pipeline plugin's Reject can land +// first. // - inner must NOT itself be a tunnel-open (Tunnel marker). Two back-to-back // passthrough CONNECTs to the same host (common with connection pooling) // would otherwise fold — hiding one real message and mislabeling the other. func isBridgedInner(tunnel, inner *pipeline.SessionEvent) bool { if inner.Direction != pipeline.Outbound || - inner.Phase != pipeline.SessionRequest || + (inner.Phase != pipeline.SessionRequest && inner.Phase != pipeline.SessionDenied) || isTunnelOpen(inner) { return false } diff --git a/cmd/agentop/tui/events_pane_test.go b/cmd/agentop/tui/events_pane_test.go index 516821c2b..73378e067 100644 --- a/cmd/agentop/tui/events_pane_test.go +++ b/cmd/agentop/tui/events_pane_test.go @@ -423,6 +423,50 @@ func TestBuildEventRows_CollapsesBridgedConnectRedirected(t *testing.T) { } } +// TestBuildEventRows_CollapsesBridgedConnectDenied locks the fold when the +// tunnel's first decrypted row is a DENIAL, not a request: the proxy defers a +// bridged open and records it with whichever row comes first, and a pipeline +// plugin's Reject — recorded before any request row — can be that first row. +// Each subtest covers one match arm: unredirected denial (Host) and redirected +// denial (RequestedHost). +func TestBuildEventRows_CollapsesBridgedConnectDenied(t *testing.T) { + for name, inner := range map[string]pipeline.SessionEvent{ + "unredirected": { + Direction: pipeline.Outbound, + Phase: pipeline.SessionDenied, + Host: "ete-litellm.example.com", + StatusCode: 403, + }, + "redirected": { + Direction: pipeline.Outbound, + Phase: pipeline.SessionDenied, + Host: "litellm.cb.example.com", + RequestedHost: "ete-litellm.example.com:443", + StatusCode: 403, + }, + } { + events := []pipeline.SessionEvent{ + { + Direction: pipeline.Outbound, + Phase: pipeline.SessionRequest, + Host: "ete-litellm.example.com:443", + Tunnel: true, + }, + inner, + } + rows := buildEventRows(events) + if len(rows) != 1 { + t.Fatalf("%s: denied bridged call produced %d rows, want 1 (the denial carrying the tunnel)", name, len(rows)) + } + if rows[0].tunnel == nil { + t.Fatalf("%s: denial row should carry the folded CONNECT tunnel", name) + } + if rows[0].event.Phase != pipeline.SessionDenied { + t.Errorf("%s: folded row phase = %q, want the denial kept", name, rows[0].event.Phase) + } + } +} + // TestBuildEventRows_PassthroughTunnelStandsAlone covers every shape in which a // non-bridged CONNECT tunnel-open must remain its own row rather than fold: // a lone trailing CONNECT, two back-to-back CONNECTs to the SAME host