Harden CI and make its lint step actually lint

.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)
@@ -540,8 +540,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 +776,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/testutil/fakeupstream.go

@@ -91,7 +91,7 @@ func NewUpstream(t *testing.T, scripts ...Script) *Upstream {
 	t.Helper()
 	u := &Upstream{scripts: scripts}
 	u.Server = httptest.NewServer(http.HandlerFunc(u.serve))
-	t.Cleanup(u.Server.Close)
+	t.Cleanup(u.Close)
 	return u
 }
 
@@ -164,7 +164,7 @@ func (u *Upstream) serveStream(w http.ResponseWriter, s Script) {
 	if err != nil {
 		panic(err)
 	}
-	defer conn.Close()
+	defer func() { _ = conn.Close() }()
 
 	var head bytes.Buffer
 	fmt.Fprintf(&head, "HTTP/1.1 %d %s\r\n", statusOr(s.Status), http.StatusText(statusOr(s.Status)))
@@ -204,7 +204,7 @@ func (u *Upstream) serveStream(w http.ResponseWriter, s Script) {
 		// The final frame is where a truncation has to happen, so it is
 		// written as a partial chunk rather than a complete one.
 		if s.Ending == EndTruncateMidFrame && i == len(frames)-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()
 			resetOrClose(conn, false)
 			return
@@ -266,7 +266,7 @@ func splitFrames(payload string) []string {
 }
 
 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()
 }