ci: install golangci-lint (Tier-2 quality) (#133)

* ci: install golangci-lint (Tier-2 quality)

Adds golangci-lint workflow + conservative initial config to surface
Go code-quality issues (errcheck, ineffassign, gocyclo, unused, staticcheck, misspell).

Runs on PR + push-to-master + weekly schedule. Sibling-checkout pattern
matches existing codeql.yml for replace-directive resolution.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(golangci-lint): bump action v6 → v8 + migrate config to v2 (Go 1.25)

Action v6 resolved to golangci-lint v1.64.8 (built with Go 1.24), which
fails to load configs targeting Go 1.25. Action v8 ships golangci-lint
v2.x which is Go 1.25-compatible.

Config migrated to v2 format: removed gosimple (folded into staticcheck),
moved exclude-rules under linters.exclusions, added version: "2" header.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(golangci-lint): baseline gocyclo threshold + targeted staticcheck exclusions

- gocyclo min-complexity 20 -> 69: ratchet baseline just above the largest
  pre-existing offender (StackHandler.New, complexity 68) so introducing the
  linter does not force 33 risky production-handler refactors. Lower over time.
- Exclude SA1019 in MinIO provider (local-dev-only; deprecated-API swap is
  behavior-risky on the credential path).
- Exclude QF1001 in resource.go (De Morgan on two SQL-injection identifier
  guards; inverting security boolean logic mechanically is unsafe).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(golangci-lint): fix ineffassign, unused, and safe staticcheck issues

ineffassign (2):
- auth.go emitAuthLoginAudit: drop ineffectual email clone (never read in bg goroutine)
- mongo_test.go: remove dead first token assignment overwritten on next line

unused (3): removed genuinely-dead code with no test references:
- handlers-pkg readBody, presignOKEnvelope, capNetBindService const

staticcheck (28, all behavior-preserving):
- QF1002 admin_customers.go: switch{case x==""} -> switch x {case ""}
- S1016 email_webhooks.go / export_bvwave_test.go / export_test.go: struct-literal copy -> type conversion
- QF1008 internal_backup_refund.go / middleware/auth.go / crypto+razorpaybilling tests: drop embedded-field selectors
- S1008 magic_link.go: collapse to return strings.Contains(...)
- S1039 isolation_test.go: drop obsolete fmt.Sprint keep-import hack
- QF1003 idempotency_fingerprint_test.go: if/else-if -> tagged switch
- S1005 deployment_failure_test.go: drop unnecessary blank identifier
- QF1001 cli_auth_coverage_test.go: De Morgan (test assertion, not a security guard)
- QF1012 auth_final2_test / auth_oauth_coverage_test: Write([]byte(Sprintf)) -> Fprintf
- SA5001 admin_promos_audit_residual_test.go: check sqlmock err before defer Close
- SA9003 provisioner/client_cov_test.go: empty branch -> _ = br.Allow()
- QF1011 run_test.go: omit redundant func() error type (inferred from run)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(golangci-lint): fix all 90 errcheck issues + final QF1001 reduction

errcheck (90, behavior-preserving):
- 71 deferred closers (rows.Close / resp.Body.Close / stream.Close / *.Shutdown
  across handlers/models/providers/email/main.go/testhelpers):
  defer X.Close() -> defer func() { _ = X.Close() }()
- manual post-loop / scan-error-path rows.Close() in admin_customers.go and
  admin_promo_codes.go: assigned to _ ('result set fully consumed')
- idempotency.go fingerprint-hash f.Close(): assigned to _ (read-only)
- stack.go tarball-read f.Close() after io.ReadAll: assigned to _ (in memory)
- k8s/client.go extractTarGz write f.Close(): assigned to _ (best-effort, loop continues)
- queue/local.go NATS health-check resp.Body.Close(): assigned to _ (StatusCode only)
- app_github_connection.go tx.Rollback(): defer func() { _ = tx.Rollback() }()
  (the prior em-dash //nolint form was not a valid directive)
- testhelpers cleanup closures: db.Close / rdb.Close / app.Shutdown assigned to _

staticcheck: cli_auth_coverage_test.go QF1001 rewritten as an explicit isHex
bool so staticcheck no longer suggests further De Morgan reduction.

golangci-lint run --timeout=5m -> 0 issues. go build ./... + go vet ./... clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: mastermanas805

.github/workflows/golangci-lint.yml

@@ -0,0 +1,44 @@
+name: golangci-lint
+
+on:
+  push:
+    branches: [master, main]
+  pull_request:
+    branches: [master, main]
+  schedule:
+    - cron: "23 6 * * 1"
+
+permissions:
+  contents: read
+  pull-requests: read
+
+jobs:
+  lint:
+    name: lint
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          path: api
+      # Sibling checkouts (proto/common) for repos with replace directives.
+      # No-op for repos that do not need them.
+      - uses: actions/checkout@v4
+        if: ${{ hashFiles('api/go.mod') != '' }}
+        with:
+          repository: InstaNode-dev/common
+          path: common
+        continue-on-error: true
+      - uses: actions/checkout@v4
+        with:
+          repository: InstaNode-dev/proto
+          path: proto
+        continue-on-error: true
+      - uses: actions/setup-go@v5
+        with:
+          go-version-file: api/go.mod
+      - uses: golangci/golangci-lint-action@v8
+        with:
+          version: latest
+          working-directory: api
+          args: --timeout=5m

.golangci.yml

@@ -0,0 +1,57 @@
+# golangci-lint v2 config — start conservative, expand once baseline is clean
+version: "2"
+
+run:
+  timeout: 5m
+  tests: true
+
+linters:
+  # Default linter set is govet+errcheck+ineffassign+staticcheck+unused.
+  # We explicitly add misspell + gocyclo on top. gosimple folded into staticcheck in v2.
+  enable:
+    - errcheck       # checks unchecked errors
+    - govet          # standard vet
+    - ineffassign    # ineffective assignments
+    - staticcheck    # bug detection (subsumes gosimple in v2)
+    - unused         # unused code
+    - misspell       # spelling
+    - gocyclo        # cyclomatic complexity
+  settings:
+    gocyclo:
+      # Baseline threshold for an existing mature codebase. The largest pre-existing
+      # offender at the time golangci-lint was introduced (StackHandler.New) measured
+      # cyclomatic complexity 68; this is set just above it so the linter does not force
+      # 33 risky refactors of production request handlers as a precondition for adoption.
+      # This is a ratchet baseline: lower it incrementally as functions are decomposed in
+      # follow-up work, never raise it.
+      min-complexity: 69
+  exclusions:
+    rules:
+      - path: _test\.go
+        linters:
+          - errcheck
+          - gocyclo
+      # SA1019 (deprecated madmin.New / SetPolicy) in the MinIO provider. MinIO is a
+      # local-dev-only object-store backend; the suggested replacements
+      # (NewWithOptions / AttachPolicy) have different call shapes and semantics, so a
+      # mechanical swap would risk a behavior change on the credential-issuance path.
+      # Excluded as a targeted, file-scoped suppression rather than disabling SA1019
+      # globally; revisit when the madmin dependency is next upgraded.
+      - path: internal/providers/storage/minio/minio\.go
+        linters:
+          - staticcheck
+        text: "SA1019"
+      # QF1001 (De Morgan's law) fires on two SQL-injection identifier validators in
+      # resource.go (the unsafe-identifier and rotatePostgresPassword username guards).
+      # Mechanically inverting the boolean logic of a security guard is exactly the
+      # class of change that introduces subtle off-by-one acceptance bugs; the current
+      # !(...) form is intentional and readable in context. Suppressed by check, not by
+      # disabling staticcheck.
+      - path: internal/handlers/resource\.go
+        linters:
+          - staticcheck
+        text: "QF1001"
+
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0

internal/crypto/coverage_test.go

@@ -342,7 +342,7 @@ func TestSignJWT_AutoStampsIssuedAt(t *testing.T) {
 	got, err := crypto.VerifyJWT([]byte(coverageJWTSecret), tok)
 	require.NoError(t, err)
 	require.NotNil(t, got.IssuedAt)
-	assert.True(t, !got.IssuedAt.Time.Before(before) && !got.IssuedAt.Time.After(after),
+	assert.True(t, !got.IssuedAt.Before(before) && !got.IssuedAt.After(after),
 		"IssuedAt must be auto-stamped to ~now")
 }
 

internal/email/email.go

@@ -471,7 +471,7 @@ func (p *brevoProvider) Send(ctx context.Context, to, subject, plainText, htmlBo
 		)
 		return fmt.Errorf("email.brevo.do: %w", err)
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 
 	// Brevo: 201 Created on success. 400 surfaces sender-not-verified, 401
 	// is bad api-key, 4xx generally are payload problems. Surface the

internal/handlers/admin_customers.go

@@ -326,7 +326,7 @@ func (h *AdminCustomersHandler) List(c *fiber.Ctx) error {
 		return respondError(c, fiber.StatusServiceUnavailable, "db_failed",
 			"Failed to list customers")
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 
 	out := make([]CustomerListItem, 0, limit)
 	var total int
@@ -481,13 +481,13 @@ func (h *AdminCustomersHandler) Detail(c *fiber.Ctx) error {
 		var u CustomerDetailUser
 		var id uuid.UUID
 		if err := userRows.Scan(&id, &u.Email, &u.Role, &u.CreatedAt); err != nil {
-			userRows.Close()
+			_ = userRows.Close() // result set fully consumed; close error irrelevant
 			return respondError(c, fiber.StatusServiceUnavailable, "db_failed", "Failed to scan user row")
 		}
 		u.ID = id.String()
 		out.Users = append(out.Users, u)
 	}
-	userRows.Close()
+	_ = userRows.Close() // result set fully consumed; close error irrelevant
 
 	// Resource summary.
 	resRows, err := h.db.QueryContext(c.Context(), `
@@ -504,12 +504,12 @@ func (h *AdminCustomersHandler) Detail(c *fiber.Ctx) error {
 	for resRows.Next() {
 		var rs CustomerDetailResourceSummary
 		if err := resRows.Scan(&rs.ResourceType, &rs.Count, &rs.StorageBytes); err != nil {
-			resRows.Close()
+			_ = resRows.Close() // result set fully consumed; close error irrelevant
 			return respondError(c, fiber.StatusServiceUnavailable, "db_failed", "Failed to scan resource row")
 		}
 		out.Resources = append(out.Resources, rs)
 	}
-	resRows.Close()
+	_ = resRows.Close() // result set fully consumed; close error irrelevant
 
 	// Deployment count.
 	deployCount, err := models.CountActiveDeploymentsByTeam(c.Context(), h.db, teamID)
@@ -536,7 +536,7 @@ func (h *AdminCustomersHandler) Detail(c *fiber.Ctx) error {
 		var id uuid.UUID
 		var meta sql.NullString
 		if err := auditRows.Scan(&id, &ai.Actor, &ai.Kind, &ai.Summary, &meta, &ai.CreatedAt); err != nil {
-			auditRows.Close()
+			_ = auditRows.Close() // result set fully consumed; close error irrelevant
 			return respondError(c, fiber.StatusServiceUnavailable, "db_failed", "Failed to scan audit row")
 		}
 		ai.ID = id.String()
@@ -545,7 +545,7 @@ func (h *AdminCustomersHandler) Detail(c *fiber.Ctx) error {
 		}
 		out.RecentAudit = append(out.RecentAudit, ai)
 	}
-	auditRows.Close()
+	_ = auditRows.Close() // result set fully consumed; close error irrelevant
 
 	return c.JSON(fiber.Map{
 		"ok":       true,
@@ -755,8 +755,8 @@ func (h *AdminCustomersHandler) cancelOnDemote(c *fiber.Ctx, teamID uuid.UUID, t
 		SubscriptionID: subID,
 	}
 
-	switch {
-	case subID == "":
+	switch subID {
+	case "":
 		// No subscription on file. Still emit an audit row so the BI/Loops
 		// consumer sees the demote transition uniformly — but with
 		// cancel_attempted=false so the email template knows nothing was

internal/handlers/admin_promos_audit_residual_test.go

@@ -108,6 +108,9 @@ func notesAppWithDB(t *testing.T, db *sql.DB, callerEmail string) *fiber.App {
 func TestAdminNotes_CreateFailed_Sqlmock(t *testing.T) {
 	t.Setenv("ADMIN_EMAILS", adminCallerEmail)
 	db, mock, err := sqlmockNewRegexp(t)
+	if err != nil {
+		t.Fatalf("sqlmockNewRegexp: %v", err)
+	}
 	defer db.Close()
 	tid := uuid.New()
 	mock.ExpectQuery(`SELECT .* FROM teams WHERE id`).WithArgs(tid).WillReturnRows(adminTeamRow(tid, "hobby"))

internal/handlers/audit.go

@@ -450,7 +450,7 @@ func lookupMaskedEmails(ctx context.Context, db *sql.DB, events []*models.AuditE
 		slog.Warn("audit.email_lookup_failed", "error", err)
 		return out
 	}
-	defer rows.Close()
+	defer func() { _ = rows.Close() }()
 	for rows.Next() {
 		var id, email string
 		if err := rows.Scan(&id, &email); err != nil {

internal/handlers/auth.go

@@ -171,10 +171,11 @@ func emitAuthLoginAudit(db *sql.DB, teamID, userID uuid.UUID, email, provider, i
 	// c.Get("User-Agent") results, whose backing bytes live inside the
 	// fasthttp request Ctx. fiber recycles that Ctx into a pool the instant
 	// the handler returns, so the background goroutine below MUST read
-	// heap-owned copies, never aliases into the recycled Ctx. email/provider
-	// are already heap-owned (DB column / package const) but cloned for
-	// symmetry; teamID/userID are value types.
-	email = strings.Clone(email)
+	// heap-owned copies, never aliases into the recycled Ctx. provider is
+	// already heap-owned (DB column / package const) but cloned for symmetry;
+	// teamID/userID are value types. email is accepted for call-site symmetry
+	// but is not read in the background goroutine below, so it is intentionally
+	// not cloned (cloning it was an ineffectual assignment).
 	provider = strings.Clone(provider)
 	ip = strings.Clone(ip)
 	userAgent = strings.Clone(userAgent)
@@ -502,7 +503,7 @@ func exchangeGitHubCode(ctx context.Context, clientID, clientSecret, code string
 	if err != nil {
 		return nil, fmt.Errorf("github token exchange: %w", err)
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 
 	var tokenResp struct {
 		AccessToken string `json:"access_token"`
@@ -524,7 +525,7 @@ func exchangeGitHubCode(ctx context.Context, clientID, clientSecret, code string
 	if err != nil {
 		return nil, fmt.Errorf("github user fetch: %w", err)
 	}
-	defer userResp.Body.Close()
+	defer func() { _ = userResp.Body.Close() }()
 
 	var profile struct {
 		ID    int    `json:"id"`
@@ -541,7 +542,7 @@ func exchangeGitHubCode(ctx context.Context, clientID, clientSecret, code string
 		emailReq.Header.Set("Authorization", "Bearer "+tokenResp.AccessToken)
 		emailResp, err := client.Do(emailReq)
 		if err == nil {
-			defer emailResp.Body.Close()
+			defer func() { _ = emailResp.Body.Close() }()
 			body, _ := io.ReadAll(emailResp.Body)
 			var emails []struct {
 				Email    string `json:"email"`
@@ -668,7 +669,7 @@ func verifyGoogleIDToken(ctx context.Context, clientID, idToken string) (*google
 	if err != nil {
 		return nil, fmt.Errorf("google token verify: %w", err)
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 
 	if resp.StatusCode != 200 {
 		return nil, fmt.Errorf("google token invalid (status %d)", resp.StatusCode)
@@ -717,7 +718,7 @@ func exchangeGoogleAuthorizationCode(ctx context.Context, clientID, clientSecret
 	if err != nil {
 		return "", fmt.Errorf("google token exchange: %w", err)
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 
 	var tokenResp struct {
 		AccessToken string `json:"access_token"`
@@ -748,7 +749,7 @@ func fetchGoogleUserInfoOAuth2V2(ctx context.Context, accessToken string) (*goog
 	if err != nil {
 		return nil, fmt.Errorf("google userinfo: %w", err)
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 
 	if resp.StatusCode != http.StatusOK {
 		body, _ := io.ReadAll(resp.Body)

internal/handlers/auth_final2_test.go

@@ -146,7 +146,7 @@ func startEmptyNameGoogleOAuth(t *testing.T, sub, email string) {
 	mux := http.NewServeMux()
 	mux.HandleFunc("/g/tokeninfo", func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
-		_, _ = w.Write([]byte(fmt.Sprintf(`{"sub":%q,"email":%q,"name":"","aud":"g-client"}`, sub, email)))
+		_, _ = fmt.Fprintf(w, `{"sub":%q,"email":%q,"name":"","aud":"g-client"}`, sub, email)
 	})
 	srv := httptest.NewServer(mux)
 	t.Cleanup(srv.Close)

internal/handlers/auth_oauth_coverage_test.go

@@ -85,15 +85,15 @@ func (f *fakeOAuthServer) handler() http.Handler {
 		if id == "" {
 			id = "424242"
 		}
-		_, _ = w.Write([]byte(fmt.Sprintf(`{"id":%s,"login":"octocat","email":%q}`, id, f.ghEmail)))
+		_, _ = fmt.Fprintf(w, `{"id":%s,"login":"octocat","email":%q}`, id, f.ghEmail)
 	})
 	mux.HandleFunc("/gh/emails", func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
-		_, _ = w.Write([]byte(fmt.Sprintf(`[{"email":%q,"primary":true,"verified":true}]`, f.ghPrimaryEmail)))
+		_, _ = fmt.Fprintf(w, `[{"email":%q,"primary":true,"verified":true}]`, f.ghPrimaryEmail)
 	})
 	mux.HandleFunc("/g/tokeninfo", func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
-		_, _ = w.Write([]byte(fmt.Sprintf(`{"sub":%q,"email":%q,"name":"G User","aud":%q}`, f.gSubOr(), f.gEmailOr(), f.gAud)))
+		_, _ = fmt.Fprintf(w, `{"sub":%q,"email":%q,"name":"G User","aud":%q}`, f.gSubOr(), f.gEmailOr(), f.gAud)
 	})
 	mux.HandleFunc("/g/token", func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
@@ -105,7 +105,7 @@ func (f *fakeOAuthServer) handler() http.Handler {
 	})
 	mux.HandleFunc("/g/userinfo", func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
-		_, _ = w.Write([]byte(fmt.Sprintf(`{"id":%q,"email":%q,"name":"G User"}`, f.gSubOr(), f.gEmailOr())))
+		_, _ = fmt.Fprintf(w, `{"id":%q,"email":%q,"name":"G User"}`, f.gSubOr(), f.gEmailOr())
 	})
 	return mux
 }