Blame the client only when it left with bytes still owed to it

.github/dependabot.yml

@@ -0,0 +1,28 @@
+version: 2
+
+updates:
+  # The workflow pins actions to commit SHAs, which is the point — a mutable
+  # tag is a stranger's push away from running in CI. Pinning without this
+  # file just trades a supply-chain hole for a stale one, so let Dependabot
+  # move the pins; it rewrites the SHA and the trailing version comment
+  # together.
+  - package-ecosystem: github-actions
+    directory: /
+    schedule:
+      interval: weekly
+      day: monday
+    # Two first-party actions. Separate PRs for them is noise, not signal.
+    groups:
+      actions:
+        patterns: ["*"]
+
+  - package-ecosystem: gomod
+    directory: /
+    schedule:
+      interval: weekly
+      day: monday
+    groups:
+      # Patch and minor bumps land as one PR that CI either passes or does
+      # not. Majors stay separate, because those are the ones worth reading.
+      go-minor:
+        update-types: [minor, patch]

.github/workflows/ci.yml

@@ -11,32 +11,70 @@ on:
     - cron: "0 6 * * 1"
   workflow_dispatch:
 
+# go test runs arbitrary code from the PR, so its token gets read and nothing
+# else. Also set in repo settings; this copy is the one that shows up in a diff.
+permissions:
+  contents: read
+
+# Cancel a superseded PR run on force-push. Not main or the weekly sweep:
+# each of those runs is the record for one commit.
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
 jobs:
   check:
     runs-on: ubuntu-latest
+    # The default is 6 hours. The race suite takes a couple of minutes.
+    timeout-minutes: 15
+    env:
+      # The runner image ships no golangci-lint, and `make lint` used to fall
+      # back to `go vet` when it was missing, so this step passed for months
+      # without linting. Pinned like gitleaks, and to the version developers
+      # run locally, so CI and a laptop disagree only when the code differs.
+      GOLANGCI_VERSION: 2.13.2
+      GOLANGCI_SHA256: 2277d43b98ec0054280f2ac26b53268bae97682444678a59a657dd565da021d6
     steps:
-      - uses: actions/checkout@v7
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          # Without this the job's token is left behind in .git/config, where
+          # the pull request's own test code can read it.
+          persist-credentials: false
 
-      - uses: actions/setup-go@v7
+      - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
         with:
           go-version-file: go.mod
 
-      # fmt + vet + lint + the full suite under -race.
+      - name: Install golangci-lint
+        run: |
+          set -euo pipefail
+          tarball="golangci-lint-${GOLANGCI_VERSION}-linux-amd64.tar.gz"
+          curl -sSfL -o "$tarball" \
+            "https://github.com/golangci/golangci-lint/releases/download/v${GOLANGCI_VERSION}/${tarball}"
+          echo "${GOLANGCI_SHA256}  ${tarball}" | sha256sum -c -
+          tar -xzf "$tarball" --strip-components=1 \
+            "golangci-lint-${GOLANGCI_VERSION}-linux-amd64/golangci-lint"
+          sudo install golangci-lint /usr/local/bin/golangci-lint
+
+      # fmt + vet + lint + the full suite under -race. CI is set by Actions, so
+      # a missing linter fails here instead of quietly downgrading to go vet.
       - run: make check
 
   secrets:
     runs-on: ubuntu-latest
+    timeout-minutes: 10
     env:
       # Pinned and checksummed. An unpinned scanner is a supply-chain hole in
       # the one job whose purpose is supply-chain hygiene.
       GITLEAKS_VERSION: 8.30.1
       GITLEAKS_SHA256: 551f6fc83ea457d62a0d98237cbad105af8d557003051f41f3e7ca7b3f2470eb
     steps:
-      - uses: actions/checkout@v7
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
         with:
           # gitleaks walks real history; the default shallow clone gives it one
           # commit and nothing to walk.
           fetch-depth: 0
+          persist-credentials: false
 
       - name: Install gitleaks
         run: |

.golangci.yml

@@ -0,0 +1,10 @@
+version: "2"
+
+issues:
+  # The defaults stop at 3 of a kind and 50 per linter, which quietly turns a
+  # full run into a sample. A gate has to report everything it found.
+  max-issues-per-linter: 0
+  max-same-issues: 0
+  # One issue per line by default, so a second linter's finding on an already
+  # flagged line is dropped. That hid an SA1019 deprecation behind a govet hint.
+  uniq-by-line: false

Makefile

@@ -64,11 +64,15 @@ bench:
 vet:
 	go vet ./...
 
-## lint: golangci-lint when installed, otherwise go vet
+## lint: golangci-lint when installed, otherwise go vet (an error under CI)
 .PHONY: lint
 lint:
 ifdef GOLANGCI
 	golangci-lint run ./...
+else ifdef CI
+	@echo "golangci-lint is not installed; the go vet fallback would report a" >&2
+	@echo "green lint step that linted almost nothing. Install it in the workflow." >&2
+	@exit 1
 else
 	@echo "golangci-lint not installed; falling back to go vet (make tools to install)"
 	@go vet ./...

cmd/demo/main.go

@@ -252,14 +252,14 @@ func streamHandler(n int, end ending, finish string) http.HandlerFunc {
 		if err != nil {
 			return
 		}
-		defer conn.Close()
+		defer func() { _ = conn.Close() }()
 
 		writeHead(rw)
 		for i := 0; i < n; i++ {
 			frame := fmt.Sprintf(
 				`data: {"id":"chatcmpl-demo","model":"demo-model","choices":[{"index":0,"delta":{"content":"tok%d"}}]}`+"\n\n", i)
 			if end == endTruncate && i == n-1 {
-				fmt.Fprintf(rw, "%x\r\n%s", len(frame), frame[:len(frame)/2])
+				_, _ = fmt.Fprintf(rw, "%x\r\n%s", len(frame), frame[:len(frame)/2])
 				_ = rw.Flush()
 				return
 			}
@@ -291,7 +291,7 @@ func slowFirstToken(think time.Duration) http.HandlerFunc {
 		if err != nil {
 			return
 		}
-		defer conn.Close()
+		defer func() { _ = conn.Close() }()
 
 		writeHead(rw)
 		time.Sleep(think)
@@ -311,7 +311,7 @@ func errorMidStream() http.HandlerFunc {
 		if err != nil {
 			return
 		}
-		defer conn.Close()
+		defer func() { _ = conn.Close() }()
 		writeHead(rw)
 		writeChunk(rw, `data: {"choices":[{"index":0,"delta":{"content":"partial answer"}}]}`+"\n\n")
 		writeChunk(rw, `data: {"error":{"message":"the model backend crashed","type":"server_error"}}`+"\n\n")
@@ -358,7 +358,7 @@ func writeHead(rw *bufio.ReadWriter) {
 }
 
 func writeChunk(rw *bufio.ReadWriter, data string) {
-	fmt.Fprintf(rw, "%x\r\n%s\r\n", len(data), data)
+	_, _ = fmt.Fprintf(rw, "%x\r\n%s\r\n", len(data), data)
 	_ = rw.Flush()
 }
 
@@ -370,7 +370,7 @@ func hangUpMidStream(addr string) {
 		return
 	}
 	body := `{"model":"demo-model","stream":true,"messages":[{"role":"user","content":"hello"}]}`
-	fmt.Fprintf(conn, "POST /demo/v1/chat/completions HTTP/1.1\r\nHost: demo\r\n"+
+	_, _ = fmt.Fprintf(conn, "POST /demo/v1/chat/completions HTTP/1.1\r\nHost: demo\r\n"+
 		"Content-Type: application/json\r\nContent-Length: %d\r\n\r\n%s", len(body), body)
 
 	buf := make([]byte, 4096)

cmd/llm-proxy/main.go

@@ -57,7 +57,7 @@ func run() error {
 	fs.BoolVar(&f.check, "check", false, "validate the config, print the route table and exit")
 	fs.BoolVar(&f.showVersion, "version", false, "print the version and exit")
 	fs.Usage = func() {
-		fmt.Fprintf(fs.Output(), "llm-proxy — an OpenAI-compatible wire-level debugging proxy\n\nusage: llm-proxy [flags]\n\n")
+		_, _ = fmt.Fprintf(fs.Output(), "llm-proxy — an OpenAI-compatible wire-level debugging proxy\n\nusage: llm-proxy [flags]\n\n")
 		fs.PrintDefaults()
 	}
 	if err := fs.Parse(os.Args[1:]); err != nil {
@@ -75,15 +75,15 @@ func run() error {
 			"       see llm-proxy.example.yaml for a starting point")
 	}
 
-	cfg, warnings, err := config.Load(path)
+	cfg, _, err := config.Load(path)
 	if err != nil {
 		return fmt.Errorf("config %s:\n%w", path, err)
 	}
 	applyFlags(cfg, &f)
 	// Re-validate after the flags, and keep *these* warnings: -listen can move
 	// the proxy onto a public address, and the warning about an unguarded
 	// listener has to reflect where it will actually listen.
-	warnings, err = cfg.Validate()
+	warnings, err := cfg.Validate()
 	if err != nil {
 		return fmt.Errorf("config %s:\n%w", path, err)
 	}
@@ -187,7 +187,7 @@ func printRouteTable(c *config.Config, path string, warnings []config.Warning) {
 	}
 
 	for _, r := range c.Routes {
-		auth := "none"
+		var auth string
 		switch {
 		case r.APIKeyEnv == "":
 			auth = "client-supplied only"

internal/config/main_test.go

@@ -10,6 +10,6 @@ import (
 // llm-proxy is likely to have it exported — and without this every test that
 // assumes an unguarded proxy would fail on their machine and pass in CI.
 func TestMain(m *testing.M) {
-	os.Unsetenv("LLM_PROXY_TOKENS")
+	_ = os.Unsetenv("LLM_PROXY_TOKENS")
 	os.Exit(m.Run())
 }

internal/obs/logging.go

@@ -60,7 +60,7 @@ func NewLogger(o Options) *Logger {
 
 	var consoleHandler slog.Handler
 	if pretty {
-		consoleHandler = tint.NewHandler(out, &tint.Options{
+		consoleHandler = tint.NewTextHandler(out, &tint.Options{
 			Level:      level,
 			TimeFormat: "15:04:05.000",
 			NoColor:    !color,

internal/proxy/auth_test.go

@@ -32,7 +32,7 @@ func TestUnauthenticatedRequestIsRejected(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 
 	if resp.StatusCode != http.StatusUnauthorized {
 		t.Fatalf("Status = %d, want 401", resp.StatusCode)
@@ -154,7 +154,7 @@ func TestWrongTokenIsRejected(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 
 	if resp.StatusCode != http.StatusUnauthorized {
 		t.Errorf("Status = %d, want 401", resp.StatusCode)

internal/proxy/handler.go

@@ -360,7 +360,7 @@ func (h *routeHandler) deliver(
 	cancelUp context.CancelCauseFunc,
 	prefix []byte,
 ) {
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 
 	streaming := isEventStream(resp.Header)
 	analyzer := analyze.New(h.srv.now)
@@ -518,7 +518,16 @@ func (h *routeHandler) postmortem(rec *record.Request, status int, a *record.Att
 	// consequence of that departure, not an independent vendor failure. Left
 	// alone it would blame the vendor for whatever its socket did in response
 	// to a cancelled read.
-	if f != nil && f.Side == fault.SideUpstream && rec.ClientGone() {
+	//
+	// "Had already gone" has to mean gone *before the response was finished*,
+	// which is why this asks whether bytes were left undelivered rather than
+	// whether the client is gone now. The watcher stays armed until the handler
+	// returns — well past this point — so a client that took every byte the
+	// upstream sent and then closed, which is what any client without
+	// keep-alives does on success, would otherwise reach in and rewrite a
+	// vendor truncation as its own fault, depending purely on whether its FIN
+	// landed before this line ran.
+	if f != nil && f.Side == fault.SideUpstream && rec.ClientLeftMidResponse() {
 		f = fault.AsInduced(f)
 	}
 
@@ -540,8 +549,8 @@ func (h *routeHandler) postmortem(rec *record.Request, status int, a *record.Att
 
 	if status >= 400 && rec.Fault() == nil {
 		env := oaierr.Parse(errBody)
-		switch {
-		case status == http.StatusTooManyRequests:
+		switch status {
+		case http.StatusTooManyRequests:
 			rec.SetFault(statusFault(fault.KindRateLimited, status, env))
 		default:
 			if which, ok := h.srv.ctxMatchers.Match(env, errBody); ok {
@@ -776,7 +785,7 @@ func decodeForDisplay(body []byte, encoding string) []byte {
 	if err != nil {
 		return body
 	}
-	defer zr.Close()
+	defer func() { _ = zr.Close() }()
 	// A capped peek is usually a partial gzip stream, so a read error still
 	// leaves whatever decoded successfully.
 	out, _ := io.ReadAll(io.LimitReader(zr, maxErrorBody))

internal/proxy/main_test.go

@@ -8,6 +8,6 @@ import (
 // See the note in internal/config: the suite must not inherit the operator's
 // LLM_PROXY_TOKENS, or every test would get a 401 from its own proxy.
 func TestMain(m *testing.M) {
-	os.Unsetenv("LLM_PROXY_TOKENS")
+	_ = os.Unsetenv("LLM_PROXY_TOKENS")
 	os.Exit(m.Run())
 }

internal/proxy/routing_test.go

@@ -147,7 +147,7 @@ func TestUnknownRouteReturnsParseableError(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 
 	if resp.StatusCode != http.StatusNotFound {
 		t.Errorf("Status = %d, want 404", resp.StatusCode)
@@ -189,7 +189,7 @@ func TestHealthAndRoutesEndpoints(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 
 	var routes struct {
 		Routes []struct {

internal/proxy/statuses_test.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/peterhoneder/llm-proxy/internal/fault"
 	"github.com/peterhoneder/llm-proxy/internal/record"
@@ -149,6 +150,37 @@ func TestNonStreamingTruncatedBody(t *testing.T) {
 	requireKind(t, snap, fault.KindTruncatedBody)
 }
 
+// The same truncation, but with the client hanging up the instant it has the
+// body — which is what every client without keep-alives does on every request.
+//
+// The client watcher stays armed until the handler returns, so its stamp can
+// land while the verdict is still being drawn. It must not change the answer:
+// the client took every byte the upstream sent, so it interrupted nothing, and
+// the vendor's body still ended mid-JSON. Attribution here is decided by
+// undelivered bytes, not by which goroutine got there first.
+func TestClientHangUpAfterTruncatedBodyStillBlamesUpstream(t *testing.T) {
+	t.Parallel()
+	upstream := newRawUpstream(t, func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = io.WriteString(w, `{"id":"cmpl-9","choices":[{"index":0,"message":{"content":"cut o`)
+	})
+	h := newHarnessWithUpstream(t, upstream)
+
+	c := testutil.Dial(t, h.addr)
+	c.Send("POST", "/vendor/v1/chat/completions", "localhost", `{"model":"m","messages":[]}`)
+	c.ReadStatusLine()
+	c.ReadHeaders()
+	c.ReadSome(time.Second)
+	c.HangUp()
+
+	snap := h.waitForRecord(t, 3*time.Second)
+	requireSide(t, snap, fault.SideUpstream)
+	requireKind(t, snap, fault.KindTruncatedBody)
+	if snap.Fault.Induced {
+		t.Error("a client that received the whole body cannot have induced the truncation")
+	}
+}
+
 func TestNonStreamingCompleteResponse(t *testing.T) {
 	t.Parallel()
 	h := newHarness(t, testutil.Script{

internal/record/record.go

@@ -389,6 +389,21 @@ func (r *Request) ClientGone() bool {
 	return !r.ClientGoneAt.IsZero()
 }
 
+// ClientLeftMidResponse reports whether the client went away while the proxy
+// still had bytes to hand it.
+//
+// The bare fact that a client is gone says nothing about causation. A client
+// with keep-alives disabled closes the connection on every successful request,
+// once it has everything it asked for, and the watcher is still armed while the
+// verdict is being drawn. Pairing the departure with undelivered bytes is what
+// separates a client that interrupted the response from one that was simply
+// finished with it.
+func (r *Request) ClientLeftMidResponse() bool {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	return !r.ClientGoneAt.IsZero() && r.BytesToClient < r.BytesFromUpstream
+}
+
 // AddRead records bytes read from the upstream.
 func (r *Request) AddRead(n int, at time.Time) {
 	r.lastUpstreamByte.Store(at.UnixNano())