ci: get the lint job running again on golangci-lint v2

.github/workflows/ci.yml

@@ -43,6 +43,6 @@ jobs:
         with:
           go-version: '1.25'
           check-latest: true
-      - uses: golangci/golangci-lint-action@v6
+      - uses: golangci/golangci-lint-action@v9
         with:
-          version: latest
+          version: v2.13.1

.golangci.yml

@@ -1,23 +1,65 @@
 # Lint config for selfsight. Boring, mainstream linters only (DESIGN.md,
 # principle 5). Kept small so contributors aren't fighting the linter.
-run:
-  timeout: 3m
+#
+# Format version 2 (golangci-lint v2+). Note that errcheck, govet, ineffassign,
+# staticcheck and unused are on by default in v2, so they are not listed here;
+# gofmt and goimports moved to their own `formatters` section.
+version: "2"
 
 linters:
   enable:
-    - errcheck
-    - govet
-    - ineffassign
-    - staticcheck
-    - unused
-    - gofmt
-    - goimports
-    - misspell
     - gosec
+    - misspell
+  settings:
+    gosec:
+      # gosec's taint-analysis rules are off. Two reasons, in order:
+      #
+      # 1. They are not deterministic in v2.13.1. On identical source with a
+      #    cold cache, G703 reported three findings in internal/api/history.go
+      #    on roughly one run in three and none on the others. A check that
+      #    fails at random is worse than no check — it teaches people to rerun
+      #    CI until it goes green.
+      # 2. What they look for is already enforced, and tested. Every path they
+      #    flagged is built from a device name that has passed
+      #    core.ValidateDeviceName, which admits a single safe path element
+      #    (letters, digits, dot, dash, underscore — no separators, no ".."),
+      #    and config loading refuses anything else. See the traversal cases in
+      #    internal/core/config_test.go.
+      #
+      # Worth revisiting when a later gosec makes these stable.
+      excludes:
+        - G703 # path traversal via taint analysis
+        - G706 # log injection via taint analysis
+  exclusions:
+    generated: lax
+    # These four are golangci-lint's own default exclusion sets, which v1
+    # applied implicitly. Listed explicitly because v2 does not.
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    rules:
+      # testdata holds sanitized device transcripts, not Go code.
+      - path: testdata/
+        linters: [all]
+      # Tests stand up throwaway HTTP servers and hand them deliberately
+      # simple cookies and forms. gosec's hardening rules are aimed at
+      # production handlers and only add noise here.
+      - path: _test\.go
+        linters: [gosec]
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
 
-issues:
-  exclude-rules:
-    # testdata holds sanitized device transcripts, not Go code.
-    - path: testdata/
-      linters:
-        - all
+formatters:
+  enable:
+    - gofmt
+    - goimports
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$

internal/api/auth.go

@@ -230,6 +230,9 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
 	s.auth.sessions[token] = now.Add(sessionTTL)
 	s.auth.mu.Unlock()
 
+	//nolint:gosec // G124: HttpOnly and SameSite are set; Secure is deliberately
+	// conditional. selfsight is commonly reached over plain HTTP on a LAN, where
+	// an unconditional Secure would stop the browser returning the cookie at all.
 	http.SetCookie(w, &http.Cookie{
 		Name: sessionCookie, Value: token, Path: "/",
 		HttpOnly: true, SameSite: http.SameSiteLaxMode,
@@ -312,6 +315,7 @@ func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) {
 		delete(s.auth.sessions, ck.Value)
 		s.auth.mu.Unlock()
 	}
+	//nolint:gosec // G124: see the note on the login cookie above.
 	http.SetCookie(w, &http.Cookie{
 		Name: sessionCookie, Value: "", Path: "/",
 		HttpOnly: true, SameSite: http.SameSiteLaxMode,

internal/api/firmware.go

@@ -52,20 +52,13 @@ func (s *Server) upgradeFor(name string) *upgradeState {
 	return u
 }
 
-// upgradeInProgress reports whether ANY device is mid-upgrade. Firmware
-// upgrades run strictly one AP at a time across the whole fleet — flashing two
-// APs at once risks taking down the network that carries the upgrade itself.
+// runningUpgradeLocked names the device currently upgrading. Caller holds s.mu.
 //
-// This is for reporting only (the progress endpoint). Deciding whether an
-// upgrade may start must go through claimUpgrade: a separate check and act
+// Firmware upgrades run strictly one AP at a time across the whole fleet —
+// flashing two APs at once risks taking down the network that carries the
+// upgrade itself. Deciding whether an upgrade may start must go through
+// claimUpgrade rather than calling this directly: a separate check and act
 // leaves a window in which two requests both see an idle fleet.
-func (s *Server) upgradeInProgress() (string, bool) {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-	return s.runningUpgradeLocked()
-}
-
-// runningUpgradeLocked names the device currently upgrading. Caller holds s.mu.
 func (s *Server) runningUpgradeLocked() (string, bool) {
 	for name, u := range s.upgrades {
 		u.mu.Lock()
@@ -254,6 +247,8 @@ func (s *Server) handleFirmwareUpgrade(w http.ResponseWriter, r *http.Request) {
 
 	dm := s.managerFor(dev)
 	backupDir := s.deviceDataDir(dev.Name)
+	//nolint:gosec // G118: detaching from the request context is the point —
+	// see the comment below.
 	go func() {
 		// Detached from the HTTP request: the upgrade outlives it by minutes.
 		ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute)

internal/capture/sanitize.go

@@ -92,6 +92,8 @@ Then eyeball every output file before committing (git history is permanent).
 			return fmt.Errorf("read %s: %w", e.Name(), err)
 		}
 		clean := s.scrub(raw)
+		//nolint:gosec // G306: sanitized fixtures are committed to the repo and
+		// meant to be world-readable; they contain no secrets by construction.
 		if err := os.WriteFile(filepath.Join(*outDir, e.Name()), clean, 0o644); err != nil {
 			return fmt.Errorf("write %s: %w", e.Name(), err)
 		}

internal/driver/wax/client.go

@@ -73,6 +73,14 @@ func New(host string, opts ...Option) *Client {
 	// Pinning hooks into our own transport; a caller-supplied client (tests)
 	// brings its own trust and is left alone.
 	if c.pinPath != "" && c.http == own {
+		// VerifyPeerCertificate is skipped on a resumed TLS session, which
+		// would let a resumed connection dodge the pin check. It cannot happen
+		// here: Go only resumes when tls.Config.ClientSessionCache is set,
+		// net/http never sets one, and neither do we — so every connection is
+		// a full handshake and the pin is always checked. Keep it that way; if
+		// a session cache is ever added, move this to VerifyConnection, which
+		// runs on resumed handshakes too.
+		//nolint:gosec // G123: resumption is off, see above.
 		tr.TLSClientConfig.VerifyPeerCertificate = c.verifyPin
 	}
 	return c

internal/driver/wax/pin_test.go

@@ -77,9 +77,15 @@ func TestCachePathsAreSafeAndStable(t *testing.T) {
 			seen[p] = host
 		}
 	}
-	// Stable, so a device keeps its own cache across restarts.
-	if PinPathIn(base, "ap1") != PinPathIn(base, "ap1") {
-		t.Error("a host's cache path must not change between calls")
+	// Stable, so a device keeps its own cache across restarts. Held in
+	// variables rather than compared inline: two identical calls side by side
+	// read as a tautology (and staticcheck flags them as one), but the point
+	// is that the path is derived from the host alone, with nothing random or
+	// time-based mixed in.
+	first := PinPathIn(base, "ap1")
+	second := PinPathIn(base, "ap1")
+	if first != second {
+		t.Errorf("a host's cache path must not change between calls: %s then %s", first, second)
 	}
 	// A pin and a session are separate files.
 	if PinPathIn(base, "ap1") == SessionPathIn(base, "ap1") {