ci: enable strict golangci-lint (#782)

Author: cssbruno

.github/workflows/golangci-lint.yml

@@ -0,0 +1,28 @@
+name: GolangCI-Lint
+
+on:
+  workflow_dispatch:
+
+permissions:
+  contents: read
+
+jobs:
+  golangci-lint:
+    name: Strict lint
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Check out repository
+        uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
+
+      - name: Set up Go
+        uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6
+        with:
+          go-version: '1.26.4'
+          cache: true
+
+      - name: Install golangci-lint
+        run: go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2
+
+      - name: Run strict GolangCI-Lint
+        run: scripts/check-golangci-lint.sh

.golangci.yml

@@ -0,0 +1,54 @@
+version: "2"
+
+run:
+  timeout: 10m
+  tests: true
+  modules-download-mode: readonly
+
+linters:
+  default: none
+  enable:
+    - asasalint
+    - asciicheck
+    - bidichk
+    - bodyclose
+    - copyloopvar
+    - durationcheck
+    - errcheck
+    - errchkjson
+    - errorlint
+    - fatcontext
+    - gocheckcompilerdirectives
+    - gocritic
+    - gomoddirectives
+    - govet
+    - ineffassign
+    - makezero
+    - misspell
+    - nilerr
+    - nilnesserr
+    - nilnil
+    - nolintlint
+    - nosprintfhostport
+    - predeclared
+    - rowserrcheck
+    - sqlclosecheck
+    - staticcheck
+    - testableexamples
+    - thelper
+    - unconvert
+    - unparam
+    - unused
+    - wastedassign
+
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
+  uniq-by-line: false
+
+formatters:
+  enable:
+    - gci
+    - gofmt
+    - gofumpt
+    - goimports

addons/auth/auth_test.go

@@ -691,7 +691,7 @@ func TestSessionRejectsTamperedCookie(t *testing.T) {
 	}
 	cookie := recorder.Result().Cookies()[0]
 	// Flip the role inside the signed payload without re-signing.
-	cookie.Value = cookie.Value + "x"
+	cookie.Value += "x"
 
 	request := httptest.NewRequest(http.MethodGet, "/", nil)
 	request.AddCookie(cookie)
@@ -825,9 +825,9 @@ func TestSessionCookieDefaultsToSecureAttributes(t *testing.T) {
 	if !cookie.Expires.Equal(now.Add(DefaultSessionTTL)) {
 		t.Fatalf("cookie expires at %v, want %v", cookie.Expires, now.Add(DefaultSessionTTL))
 	}
-	clear := sessions.ClearCookie()
-	if clear.Name != DefaultSessionCookie || clear.Path != "/" || !clear.HttpOnly || !clear.Secure || clear.SameSite != http.SameSiteLaxMode {
-		t.Fatalf("unexpected default clear-cookie attributes: %#v", clear)
+	clearCookie := sessions.ClearCookie()
+	if clearCookie.Name != DefaultSessionCookie || clearCookie.Path != "/" || !clearCookie.HttpOnly || !clearCookie.Secure || clearCookie.SameSite != http.SameSiteLaxMode {
+		t.Fatalf("unexpected default clear-cookie attributes: %#v", clearCookie)
 	}
 }
 
@@ -851,9 +851,9 @@ func TestSessionCookieHelpers(t *testing.T) {
 	if cookie.Name != DefaultSessionCookie || cookie.Value == "" || !cookie.HttpOnly {
 		t.Fatalf("unexpected issued cookie: %#v", cookie)
 	}
-	clear := sessions.ClearCookie()
-	if clear.Name != DefaultSessionCookie || clear.MaxAge >= 0 {
-		t.Fatalf("unexpected clear cookie: %#v", clear)
+	clearCookie := sessions.ClearCookie()
+	if clearCookie.Name != DefaultSessionCookie || clearCookie.MaxAge >= 0 {
+		t.Fatalf("unexpected clear cookie: %#v", clearCookie)
 	}
 }
 
@@ -1085,5 +1085,7 @@ func TestNewReportsEnvNameWithoutLeakingSecret(t *testing.T) {
 
 // Sessions must satisfy the Provider interface so it can be registered with the
 // generated RegisterAuthProvider hook.
-var _ Provider = (*Sessions)(nil)
-var _ PasswordHasher = PBKDF2Hasher{}
+var (
+	_ Provider       = (*Sessions)(nil)
+	_ PasswordHasher = PBKDF2Hasher{}
+)

addons/auth/password.go

@@ -8,7 +8,6 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
-	"hash"
 	"strconv"
 	"strings"
 )
@@ -144,5 +143,5 @@ func decodeHash(encoded string) (iterations int, salt, key []byte, err error) {
 }
 
 func pbkdf2SHA256(password string, salt []byte, iterations, keyLength int) ([]byte, error) {
-	return pbkdf2.Key(func() hash.Hash { return sha256.New() }, password, salt, iterations, keyLength)
+	return pbkdf2.Key(sha256.New, password, salt, iterations, keyLength)
 }

addons/auth/session.go

@@ -263,7 +263,7 @@ func sessionSigningKeys(options Options) (SigningKey, map[string]SigningKey, err
 
 func validateSigningKeyID(label string, id string) error {
 	if strings.Contains(id, ".") {
-		return fmt.Errorf("gowdk auth: %s id %q must not contain .", label, id)
+		return fmt.Errorf("gowdk auth: %s id %q must not contain dot", label, id)
 	}
 	return nil
 }
@@ -406,12 +406,12 @@ func (sessions *Sessions) Revoke(ctx context.Context, request *http.Request) err
 	if sessions.mode != SessionModeRevocable || request == nil {
 		return nil
 	}
-	cookie, err := request.Cookie(sessions.cookie)
-	if err != nil {
+	cookie, ok := sessions.requestCookie(request)
+	if !ok {
 		return nil
 	}
-	payload, err := sessions.verify(cookie.Value)
-	if err != nil {
+	payload, ok := sessions.verifiedPayload(cookie.Value)
+	if !ok {
 		return nil
 	}
 	if strings.TrimSpace(payload.SessionID) == "" {
@@ -456,24 +456,24 @@ func (sessions *Sessions) ClearCookie() http.Cookie {
 // principal and no error, meaning unauthenticated.
 func (sessions *Sessions) Principal(request *http.Request) (*Principal, error) {
 	if request == nil {
-		return nil, nil
+		return unauthenticatedPrincipal()
 	}
-	cookie, err := request.Cookie(sessions.cookie)
-	if err != nil {
-		return nil, nil
+	cookie, ok := sessions.requestCookie(request)
+	if !ok {
+		return unauthenticatedPrincipal()
 	}
-	payload, err := sessions.verify(cookie.Value)
-	if err != nil {
-		return nil, nil
+	payload, ok := sessions.verifiedPayload(cookie.Value)
+	if !ok {
+		return unauthenticatedPrincipal()
 	}
 	if sessions.now().Unix() >= payload.Expires {
-		return nil, nil
+		return unauthenticatedPrincipal()
 	}
 	if sessions.mode == SessionModeRevocable {
 		return sessions.revocablePrincipal(request.Context(), payload)
 	}
 	if strings.TrimSpace(payload.ID) == "" {
-		return nil, nil
+		return unauthenticatedPrincipal()
 	}
 	return &Principal{
 		ID:                   payload.ID,
@@ -485,21 +485,21 @@ func (sessions *Sessions) Principal(request *http.Request) (*Principal, error) {
 
 func (sessions *Sessions) revocablePrincipal(ctx context.Context, payload sessionPayload) (*Principal, error) {
 	if strings.TrimSpace(payload.SessionID) == "" {
-		return nil, nil
+		return unauthenticatedPrincipal()
 	}
 	record, err := sessions.store.LookupSession(ctx, payload.SessionID)
 	if err != nil {
 		if errors.Is(err, ErrSessionNotFound) {
-			return nil, nil
+			return unauthenticatedPrincipal()
 		}
 		return nil, err
 	}
 	now := sessions.now()
 	if record.Revoked || record.expired(now) || strings.TrimSpace(record.Principal.ID) == "" {
-		return nil, nil
+		return unauthenticatedPrincipal()
 	}
 	if sessionRecordAuthorizationVersion(record) != payload.AuthorizationVersion {
-		return nil, nil
+		return unauthenticatedPrincipal()
 	}
 	if sessions.idleTTL > 0 {
 		toucher, ok := sessions.store.(SessionToucher)
@@ -516,6 +516,27 @@ func (sessions *Sessions) revocablePrincipal(ctx context.Context, payload sessio
 	return &principal, nil
 }
 
+func unauthenticatedPrincipal() (*Principal, error) {
+	var principal *Principal
+	return principal, nil
+}
+
+func (sessions *Sessions) requestCookie(request *http.Request) (*http.Cookie, bool) {
+	cookie, err := request.Cookie(sessions.cookie)
+	if err != nil {
+		return nil, false
+	}
+	return cookie, true
+}
+
+func (sessions *Sessions) verifiedPayload(token string) (sessionPayload, bool) {
+	payload, err := sessions.verify(token)
+	if err != nil {
+		return sessionPayload{}, false
+	}
+	return payload, true
+}
+
 func sessionRecordAuthorizationVersion(record SessionRecord) string {
 	if record.Principal.AuthorizationVersion != "" {
 		return record.Principal.AuthorizationVersion

addons/db/db.go

@@ -81,7 +81,9 @@ func OpenWithOptions(driver, dsn string, options Options) (*sql.DB, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), pingTimeout)
 	defer cancel()
 	if err := database.PingContext(ctx); err != nil {
-		database.Close()
+		if closeErr := database.Close(); closeErr != nil {
+			return nil, fmt.Errorf("gowdk db: ping %q: %w; close: %w", driver, err, closeErr)
+		}
 		return nil, fmt.Errorf("gowdk db: ping %q: %w", driver, err)
 	}
 	return database, nil

addons/db/db_test.go

@@ -186,7 +186,7 @@ func TestApplyMigrationsReservesBeforeRunningMigrationSQL(t *testing.T) {
 	if reserve < 0 || migration < 0 || finalize < 0 {
 		t.Fatalf("did not find reservation, migration, and finalize statements in %#v", state.executed)
 	}
-	if !(reserve < migration && migration < finalize) {
+	if reserve >= migration || migration >= finalize {
 		t.Fatalf("migration was not reserved before user SQL and finalized after it: %#v", state.executed)
 	}
 }

addons/realtime/realtime_test.go

@@ -22,7 +22,5 @@ func TestNewSSEReturnsPresentationFanout(t *testing.T) {
 		WithSSEReplayLimit(4),
 		WithSSEAudienceFromRequest(func(*http.Request) []string { return []string{"tenant:test"} }),
 	)
-	if fanout == nil {
-		t.Fatal("expected SSE fanout")
-	}
+	_ = fanout
 }

addons/tailwind/tailwind.go

@@ -72,7 +72,9 @@ func (p processor) ProcessCSS(context gowdk.CSSContext) (gowdk.CSSResult, error)
 	if err != nil {
 		return gowdk.CSSResult{}, err
 	}
-	defer os.RemoveAll(tempDir)
+	defer func() {
+		_ = os.RemoveAll(tempDir)
+	}()
 
 	tempOutput := filepath.Join(tempDir, "app.css")
 	workingDir := cssWorkingDir(context)

docs/engineering/ci.md

@@ -37,6 +37,10 @@ Required pull-request lanes:
 
 The release lanes live outside the pull-request CI workflow:
 
+- `.github/workflows/golangci-lint.yml`: manual strict lint workflow. It runs
+  `scripts/check-golangci-lint.sh` with the pinned GolangCI-Lint version. Do
+  not make this a required pull-request lane until the repository is clean under
+  the configured lint set.
 - `.github/workflows/release-dry-run.yml`: scheduled weekly and manual; packages
   CLI/VS Code artifacts, writes checksums, and uploads workflow artifacts. This
   is GitHub-only because it uses Actions artifact upload.
@@ -58,6 +62,7 @@ Run the same local checks before handoff when relevant:
   scripts/vulncheck-go-modules.sh
   go build ./cmd/gowdk
   scripts/check-dead-code.sh
+  scripts/check-golangci-lint.sh
   ```
 
 - VS Code extension checks:
@@ -177,6 +182,27 @@ after confirming a clean baseline without broad suppressions; if an intentional
 entry point needs a suppression, keep it local to the declaration and document
 why external reachability is expected.
 
+## GolangCI-Lint
+
+`.golangci.yml` is intentionally strict:
+
+```sh
+scripts/check-golangci-lint.sh
+```
+
+The gate verifies the config and runs `golangci-lint run` with correctness,
+resource-handling, formatting, maintainability, and dead-code linters enabled.
+Test files are included, module downloads are readonly, issue counts are
+uncapped, and existing findings are not hidden through exclusions.
+The wrapper uses `golangci-lint` when `v2.12.2` is on `PATH`; otherwise it falls back to
+`go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2`. CI
+also installs that pinned version before running the wrapper.
+
+Do not add broad `linters.exclusions` or `issues.exclude` entries to make a
+dirty baseline pass. Fix the finding, add a narrow `//nolint:<linter>` with a
+reason where the code is intentionally exceptional, or lower strictness only
+with an owning engineering note that explains the tradeoff.
+
 ## Release Smoke
 
 After publishing a tag, verify the current machine's release artifact locally: