From 5cedfde61a03bc63a44259d98d4749de9940e205 Mon Sep 17 00:00:00 2001 From: saif <11242541+saifsmailbox98@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:59:42 +0530 Subject: [PATCH 1/3] fix(agent-proxy): stop logging client cancellations as proxy errors An agent hanging up mid-request, which happens on every interrupted prompt and every exit with calls in flight, cancels the request context and fails the round trip. That was classified as a proxy error, logged at ERROR, and answered with a 502 nobody was left to receive. Since the agent's TUI shares the terminal the proxy logs to, the line landed on top of the user's input. Cancellations now record their own decision at debug level, and report no status rather than an invented one. --- packages/agentproxy/activitylog_test.go | 1 + packages/agentproxy/forward_test.go | 56 +++++++++++++++++++++++++ packages/agentproxy/proxy.go | 21 ++++++++-- 3 files changed, 74 insertions(+), 4 deletions(-) diff --git a/packages/agentproxy/activitylog_test.go b/packages/agentproxy/activitylog_test.go index b250dae0..33138ed4 100644 --- a/packages/agentproxy/activitylog_test.go +++ b/packages/agentproxy/activitylog_test.go @@ -43,6 +43,7 @@ func TestLevelFor(t *testing.T) { decisionPassthrough: zerolog.DebugLevel, decisionBlocked: zerolog.WarnLevel, decisionError: zerolog.ErrorLevel, + decisionCanceled: zerolog.DebugLevel, } for decision, want := range cases { if got := levelFor(decision); got != want { diff --git a/packages/agentproxy/forward_test.go b/packages/agentproxy/forward_test.go index 47ae78ba..08ac5693 100644 --- a/packages/agentproxy/forward_test.go +++ b/packages/agentproxy/forward_test.go @@ -2,6 +2,7 @@ package agentproxy import ( "bufio" + "bytes" "encoding/base64" "fmt" "io" @@ -12,6 +13,9 @@ import ( "strings" "testing" "time" + + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" ) func proxyAuthHeader(projectID, environment, secretPath, jwt string) string { @@ -186,3 +190,55 @@ func TestPlainForwardBlocksUnmatchedHost(t *testing.T) { t.Fatalf("expected 403 in block mode for an unmatched host, got %d", resp.StatusCode) } } + +// An agent that hangs up mid-request (an interrupted prompt, or the agent exiting) must not put an +// error on the terminal its TUI is drawing to. +func TestClientAbortIsNotLoggedAsError(t *testing.T) { + var buf bytes.Buffer + previous := log.Logger + log.Logger = zerolog.New(&buf) + t.Cleanup(func() { log.Logger = previous }) + + release := make(chan struct{}) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-release + })) + t.Cleanup(func() { close(release); upstream.Close() }) + + u, err := url.Parse(upstream.URL) + if err != nil { + t.Fatal(err) + } + + jwt := "test.jwt.token" + scope := agentScope{projectID: "proj", environment: "dev", secretPath: "/"} + client := newTestProxy(t, UnmatchedAllow, jwt, scope, nil) + + if _, err := fmt.Fprintf(client, "POST http://%s/v1/messages HTTP/1.1\r\nHost: %s\r\nProxy-Authorization: %s\r\nContent-Length: 0\r\n\r\n", + u.Host, u.Host, proxyAuthHeader("proj", "dev", "/", jwt)); err != nil { + t.Fatal(err) + } + // Give the proxy time to reach the (hanging) upstream, then go away like an interrupted agent. + time.Sleep(200 * time.Millisecond) + _ = client.Close() + + var line string + for deadline := time.Now().Add(3 * time.Second); time.Now().Before(deadline); { + if line = buf.String(); strings.Contains(line, activityEventName) { + break + } + time.Sleep(20 * time.Millisecond) + } + if !strings.Contains(line, activityEventName) { + t.Fatalf("expected an activity record for the aborted request, got %q", line) + } + if !strings.Contains(line, `"decision":"`+decisionCanceled+`"`) { + t.Errorf("expected decision %q, got %q", decisionCanceled, line) + } + if strings.Contains(line, `"level":"error"`) { + t.Errorf("client abort logged at error level: %q", line) + } + if strings.Contains(line, `"status":`) { + t.Errorf("a canceled request has no response status to report: %q", line) + } +} diff --git a/packages/agentproxy/proxy.go b/packages/agentproxy/proxy.go index 1b6016ce..8af3df9a 100644 --- a/packages/agentproxy/proxy.go +++ b/packages/agentproxy/proxy.go @@ -71,6 +71,7 @@ const ( decisionPassthrough = "passthrough" decisionBlocked = "blocked" decisionError = "error" + decisionCanceled = "canceled" activityEventName = "agent-proxy.request" @@ -286,7 +287,7 @@ func Start(opts Options) error { return fmt.Errorf("failed to listen on port %d: %w", opts.Port, err) } log.Info().Msgf("Infisical agent proxy listening on :%d", opts.Port) - log.Info().Msg("per-request activity logging on: brokered=info, blocked=warn, error=error, passthrough=debug (use --log-level to filter)") + log.Info().Msg("per-request activity logging on: brokered=info, blocked=warn, error=error, passthrough=debug, canceled=debug (use --log-level to filter)") sigCh := make(chan os.Signal, 1) signal.Notify(sigCh, os.Interrupt, syscall.SIGTERM) @@ -480,6 +481,15 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem resp, outcome, err := ps.forward(r, scheme, hostname, port, jwt, scope) + // The agent hanging up mid-request (an interrupted prompt, or the agent exiting) cancels the request + // context and fails the round trip. Nothing went wrong, and no response can be delivered to a client + // that is already gone, so it is recorded as its own decision rather than a proxy error: an agent's + // TUI shares this terminal, and an ERROR line with an invented status would land on top of it. + if err != nil && r.Context().Err() != nil { + ps.emitActivity(method, reqPath, hostname, port, decisionCanceled, 0, scope, outcome, r.Context().Err()) + return + } + status := http.StatusOK decision := decisionPassthrough switch { @@ -524,7 +534,7 @@ func levelFor(decision string) zerolog.Level { return zerolog.WarnLevel case decisionError: return zerolog.ErrorLevel - case decisionPassthrough: + case decisionPassthrough, decisionCanceled: return zerolog.DebugLevel default: return zerolog.InfoLevel @@ -544,8 +554,11 @@ func (ps *proxyServer) emitActivity(method, reqPath, hostname, port, decision st Str("method", method). Str("host", hostname). Int("port", portNum). - Str("path", reqPath). - Int("status", status) + Str("path", reqPath) + // A canceled request never got a response, so it reports no status rather than an invented one. + if status != 0 { + ev = ev.Int("status", status) + } if outcome.agentName != "" { ev = ev.Str("agentName", outcome.agentName) } From 0658643ae5fc8d7dfe320a7fc001b22daaa5fcae Mon Sep 17 00:00:00 2001 From: saif <11242541+saifsmailbox98@users.noreply.github.com> Date: Tue, 4 Aug 2026 22:37:35 +0530 Subject: [PATCH 2/3] fix(agent-proxy): keep brokered and blocked records when the client hangs up A cancellation must not be able to erase what already happened. A credential applied to the request stays brokered at info level, and a refused host stays blocked at warn, so an agent cannot keep either out of the activity log by disconnecting at the right moment. --- packages/agentproxy/forward_test.go | 59 +++++++++++++++++++++++++++++ packages/agentproxy/proxy.go | 23 +++++++---- 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/packages/agentproxy/forward_test.go b/packages/agentproxy/forward_test.go index 08ac5693..f40748ca 100644 --- a/packages/agentproxy/forward_test.go +++ b/packages/agentproxy/forward_test.go @@ -242,3 +242,62 @@ func TestClientAbortIsNotLoggedAsError(t *testing.T) { t.Errorf("a canceled request has no response status to report: %q", line) } } + +// A credential already applied to the request must stay on the record at its normal level, so hanging +// up mid-request cannot keep a brokered call out of the activity log. +func TestClientAbortStillRecordsBrokeredCredential(t *testing.T) { + var buf bytes.Buffer + previous := log.Logger + log.Logger = zerolog.New(&buf) + t.Cleanup(func() { log.Logger = previous }) + + release := make(chan struct{}) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-release + })) + t.Cleanup(func() { close(release); upstream.Close() }) + + u, err := url.Parse(upstream.URL) + if err != nil { + t.Fatal(err) + } + + jwt := "test.jwt.token" + scope := agentScope{projectID: "proj", environment: "dev", secretPath: "/"} + services := []*resolvedService{{ + name: "internal", + hostPatterns: parseHostPatterns(u.Hostname()), + isEnabled: true, + credentials: []resolvedCredential{ + {secretKey: "GITHUB_PAT", role: roleHeaderRewrite, headerName: "Authorization", headerPrefix: "Bearer", value: "real_secret"}, + }, + }} + client := newTestProxy(t, UnmatchedAllow, jwt, scope, services) + + if _, err := fmt.Fprintf(client, "POST http://%s/issues HTTP/1.1\r\nHost: %s\r\nProxy-Authorization: %s\r\nContent-Length: 0\r\n\r\n", + u.Host, u.Host, proxyAuthHeader("proj", "dev", "/", jwt)); err != nil { + t.Fatal(err) + } + time.Sleep(200 * time.Millisecond) + _ = client.Close() + + var line string + for deadline := time.Now().Add(3 * time.Second); time.Now().Before(deadline); { + if line = buf.String(); strings.Contains(line, activityEventName) { + break + } + time.Sleep(20 * time.Millisecond) + } + if !strings.Contains(line, `"decision":"`+decisionBrokered+`"`) { + t.Errorf("a credential was applied, so the record must stay %q: %q", decisionBrokered, line) + } + if !strings.Contains(line, `"level":"info"`) { + t.Errorf("brokered records must stay at info so they survive the default filter: %q", line) + } + if !strings.Contains(line, "GITHUB_PAT") { + t.Errorf("the applied credential must still be named on the record: %q", line) + } + if strings.Contains(line, "real_secret") { + t.Errorf("the record must never contain the secret value: %q", line) + } +} diff --git a/packages/agentproxy/proxy.go b/packages/agentproxy/proxy.go index 8af3df9a..cf6534e7 100644 --- a/packages/agentproxy/proxy.go +++ b/packages/agentproxy/proxy.go @@ -482,19 +482,23 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem resp, outcome, err := ps.forward(r, scheme, hostname, port, jwt, scope) // The agent hanging up mid-request (an interrupted prompt, or the agent exiting) cancels the request - // context and fails the round trip. Nothing went wrong, and no response can be delivered to a client - // that is already gone, so it is recorded as its own decision rather than a proxy error: an agent's - // TUI shares this terminal, and an ERROR line with an invented status would land on top of it. - if err != nil && r.Context().Err() != nil { - ps.emitActivity(method, reqPath, hostname, port, decisionCanceled, 0, scope, outcome, r.Context().Err()) - return - } + // context and fails the round trip. Nothing went wrong, and no response can reach a client that is + // already gone, so it is not an error: an agent's TUI shares this terminal, and an ERROR line with an + // invented status would land on top of it. What already happened still counts, though. A credential + // applied to the request stays `brokered` and a refused host stays `blocked`, so hanging up at the + // right moment cannot drop either from the activity log. + canceled := err != nil && r.Context().Err() != nil status := http.StatusOK decision := decisionPassthrough switch { case errors.Is(err, errHostBlocked): decision, status = decisionBlocked, http.StatusForbidden + case canceled && outcome.service != nil: + decision, status = decisionBrokered, 0 + ps.recordUsage(outcome.service.id) + case canceled: + decision, status = decisionCanceled, 0 case err != nil: decision, status = decisionError, http.StatusBadGateway case outcome.service != nil: @@ -506,7 +510,10 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem ps.emitActivity(method, reqPath, hostname, port, decision, status, scope, outcome, err) if err != nil { - http.Error(w, err.Error(), status) + // A canceled request has no one left to answer, so the error response is skipped. + if !canceled { + http.Error(w, err.Error(), status) + } return } defer resp.Body.Close() From e1863511dfecce2ec70a2537986f212b9182b891 Mon Sep 17 00:00:00 2001 From: saif <11242541+saifsmailbox98@users.noreply.github.com> Date: Tue, 4 Aug 2026 22:41:30 +0530 Subject: [PATCH 3/3] chore(agent-proxy): drop the added code comments --- packages/agentproxy/forward_test.go | 5 ----- packages/agentproxy/proxy.go | 9 +-------- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/packages/agentproxy/forward_test.go b/packages/agentproxy/forward_test.go index f40748ca..a687fa13 100644 --- a/packages/agentproxy/forward_test.go +++ b/packages/agentproxy/forward_test.go @@ -191,8 +191,6 @@ func TestPlainForwardBlocksUnmatchedHost(t *testing.T) { } } -// An agent that hangs up mid-request (an interrupted prompt, or the agent exiting) must not put an -// error on the terminal its TUI is drawing to. func TestClientAbortIsNotLoggedAsError(t *testing.T) { var buf bytes.Buffer previous := log.Logger @@ -218,7 +216,6 @@ func TestClientAbortIsNotLoggedAsError(t *testing.T) { u.Host, u.Host, proxyAuthHeader("proj", "dev", "/", jwt)); err != nil { t.Fatal(err) } - // Give the proxy time to reach the (hanging) upstream, then go away like an interrupted agent. time.Sleep(200 * time.Millisecond) _ = client.Close() @@ -243,8 +240,6 @@ func TestClientAbortIsNotLoggedAsError(t *testing.T) { } } -// A credential already applied to the request must stay on the record at its normal level, so hanging -// up mid-request cannot keep a brokered call out of the activity log. func TestClientAbortStillRecordsBrokeredCredential(t *testing.T) { var buf bytes.Buffer previous := log.Logger diff --git a/packages/agentproxy/proxy.go b/packages/agentproxy/proxy.go index cf6534e7..aa7df7c9 100644 --- a/packages/agentproxy/proxy.go +++ b/packages/agentproxy/proxy.go @@ -481,16 +481,11 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem resp, outcome, err := ps.forward(r, scheme, hostname, port, jwt, scope) - // The agent hanging up mid-request (an interrupted prompt, or the agent exiting) cancels the request - // context and fails the round trip. Nothing went wrong, and no response can reach a client that is - // already gone, so it is not an error: an agent's TUI shares this terminal, and an ERROR line with an - // invented status would land on top of it. What already happened still counts, though. A credential - // applied to the request stays `brokered` and a refused host stays `blocked`, so hanging up at the - // right moment cannot drop either from the activity log. canceled := err != nil && r.Context().Err() != nil status := http.StatusOK decision := decisionPassthrough + // Blocked and brokered are checked before canceled, so hanging up cannot drop them from the log. switch { case errors.Is(err, errHostBlocked): decision, status = decisionBlocked, http.StatusForbidden @@ -510,7 +505,6 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem ps.emitActivity(method, reqPath, hostname, port, decision, status, scope, outcome, err) if err != nil { - // A canceled request has no one left to answer, so the error response is skipped. if !canceled { http.Error(w, err.Error(), status) } @@ -562,7 +556,6 @@ func (ps *proxyServer) emitActivity(method, reqPath, hostname, port, decision st Str("host", hostname). Int("port", portNum). Str("path", reqPath) - // A canceled request never got a response, so it reports no status rather than an invented one. if status != 0 { ev = ev.Int("status", status) }