refactor: modernize codebase with idiomatic Go patterns and linter updates

.golangci.yml

@@ -1,49 +1,118 @@
 version: "2"
+output:
+  sort-order:
+    - file
 linters:
   default: none
   enable:
+    - bidichk
     - bodyclose
-    - dogsled
-    - dupl
+    - depguard
     - errcheck
-    - exhaustive
-    - goconst
+    - forbidigo
+    - gocheckcompilerdirectives
     - gocritic
-    - gocyclo
-    - goprintffuncname
-    - gosec
     - govet
     - ineffassign
-    - misspell
+    - mirror
+    - modernize
     - nakedret
-    - noctx
+    - nilnil
     - nolintlint
-    - rowserrcheck
+    - perfsprint
+    - revive
     - staticcheck
+    - testifylint
     - unconvert
     - unparam
     - unused
-    - whitespace
+    - usestdlibvars
+    - usetesting
+    - wastedassign
+  settings:
+    depguard:
+      rules:
+        main:
+          deny:
+            - pkg: io/ioutil
+              desc: use os or io instead
+            - pkg: golang.org/x/exp
+              desc: it's experimental and unreliable
+            - pkg: github.com/pkg/errors
+              desc: use builtin errors package instead
+    nolintlint:
+      allow-unused: false
+      require-explanation: true
+      require-specific: true
+    gocritic:
+      enabled-checks:
+        - equalFold
+      disabled-checks: []
+    revive:
+      severity: error
+      rules:
+        - name: blank-imports
+        - name: constant-logical-expr
+        - name: context-as-argument
+        - name: context-keys-type
+        - name: dot-imports
+        - name: empty-lines
+        - name: error-return
+        - name: error-strings
+        - name: exported
+        - name: identical-branches
+        - name: if-return
+        - name: increment-decrement
+        - name: modifies-value-receiver
+        - name: package-comments
+        - name: redefines-builtin-id
+        - name: superfluous-else
+        - name: time-naming
+        - name: unexported-return
+        - name: var-declaration
+        - name: var-naming
+          disabled: true
+    staticcheck:
+      checks:
+        - all
+    testifylint: {}
+    usetesting:
+      os-temp-dir: true
+    forbidigo:
+      forbid:
+        - pattern: "^(print|println)$"
+          msg: "use fmt.Print* instead of built-in print/println"
+    perfsprint:
+      concat-loop: false
+    govet:
+      enable:
+        - nilness
+        - unusedwrite
   exclusions:
     generated: lax
     presets:
       - comments
       - common-false-positives
       - legacy
       - std-error-handling
-    paths:
-      - third_party$
-      - builtin$
-      - examples$
+    rules:
+      - linters:
+          - errcheck
+          - staticcheck
+          - unparam
+        path: _test\.go
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
 formatters:
   enable:
     - gofmt
     - gofumpt
-    - goimports
     - golines
+  settings:
+    gofumpt:
+      extra-rules: true
   exclusions:
     generated: lax
-    paths:
-      - third_party$
-      - builtin$
-      - examples$
+run:
+  timeout: 10m

Makefile

@@ -56,7 +56,7 @@ fmt: install-golangci-lint
 
 ## lint: run golangci-lint to check for issues
 lint: install-golangci-lint
-	golangci-lint run
+	unset GOROOT && golangci-lint run
 
 ## build_linux_amd64: build the authgate binary for linux amd64
 build_linux_amd64: generate

browser_flow.go

@@ -132,7 +132,7 @@ func performBrowserFlowWithUpdates(
 		Step:       1,
 		TotalSteps: 3,
 		Message:    "Opening browser",
-		Data: map[string]interface{}{
+		Data: map[string]any{
 			"url": authURL,
 		},
 	}
@@ -152,7 +152,7 @@ func performBrowserFlowWithUpdates(
 		Step:       2,
 		TotalSteps: 3,
 		Message:    "Waiting for callback",
-		Data: map[string]interface{}{
+		Data: map[string]any{
 			"port": callbackPort,
 		},
 	}
@@ -179,7 +179,7 @@ func performBrowserFlowWithUpdates(
 				updates <- tui.FlowUpdate{
 					Type:     tui.TimerTick,
 					Progress: progress,
-					Data: map[string]interface{}{
+					Data: map[string]any{
 						"elapsed": elapsed,
 						"timeout": callbackTimeout,
 					},

callback.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"html"
 	"net"
@@ -18,7 +19,7 @@ const (
 // ErrCallbackTimeout is returned when no browser callback is received within callbackTimeout.
 // Callers can use errors.Is to distinguish a timeout from other authorization errors
 // and decide whether to fall back to Device Code Flow.
-var ErrCallbackTimeout = fmt.Errorf("browser authorization timed out")
+var ErrCallbackTimeout = errors.New("browser authorization timed out")
 
 // callbackResult holds the outcome of the local callback round-trip.
 type callbackResult struct {

callback_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -18,7 +19,8 @@ type callbackServerResult struct {
 // startCallbackServerAsync starts the callback server in a goroutine and
 // returns a channel that will receive the result (storage or error).
 func startCallbackServerAsync(
-	t *testing.T, ctx context.Context, port int, state string,
+	t *testing.T, ctx context.Context, //nolint:revive // t before ctx in test helpers
+	port int, state string,
 	exchangeFn func(context.Context, string) (*TokenStorage, error),
 ) chan callbackServerResult {
 	t.Helper()
@@ -37,7 +39,7 @@ func noExchangeFn(t *testing.T) func(context.Context, string) (*TokenStorage, er
 	t.Helper()
 	return func(_ context.Context, _ string) (*TokenStorage, error) {
 		t.Error("exchangeFn should not be called")
-		return nil, fmt.Errorf("should not be called")
+		return nil, errors.New("should not be called")
 	}
 }
 
@@ -68,7 +70,7 @@ func TestCallbackServer_Success(t *testing.T) {
 		"http://127.0.0.1:%d/callback?code=mycode123&state=%s",
 		port, state,
 	)
-	resp, err := http.Get(callbackURL) //nolint:noctx,gosec
+	resp, err := http.Get(callbackURL) //nolint:noctx,gosec // test-only HTTP call to local server
 	if err != nil {
 		t.Fatalf("GET callback failed: %v", err)
 	}
@@ -105,7 +107,7 @@ func TestCallbackServer_StateMismatch(t *testing.T) {
 		"http://127.0.0.1:%d/callback?code=mycode&state=wrong-state",
 		port,
 	)
-	resp, err := http.Get(callbackURL) //nolint:noctx,gosec
+	resp, err := http.Get(callbackURL) //nolint:noctx,gosec // test-only HTTP call to local server
 	if err != nil {
 		t.Fatalf("GET callback failed: %v", err)
 	}
@@ -136,7 +138,7 @@ func TestCallbackServer_OAuthError(t *testing.T) {
 		"http://127.0.0.1:%d/callback?error=access_denied&error_description=User+denied&state=%s",
 		port, state,
 	)
-	resp, err := http.Get(callbackURL) //nolint:noctx,gosec
+	resp, err := http.Get(callbackURL) //nolint:noctx,gosec // test-only HTTP call to local server
 	if err != nil {
 		t.Fatalf("GET callback failed: %v", err)
 	}
@@ -166,14 +168,14 @@ func TestCallbackServer_ExchangeFailure(t *testing.T) {
 
 	ch := startCallbackServerAsync(t, context.Background(), port, state,
 		func(_ context.Context, _ string) (*TokenStorage, error) {
-			return nil, fmt.Errorf("unauthorized_client: unauthorized_client")
+			return nil, errors.New("unauthorized_client: unauthorized_client")
 		})
 
 	callbackURL := fmt.Sprintf(
 		"http://127.0.0.1:%d/callback?code=mycode&state=%s",
 		port, state,
 	)
-	resp, err := http.Get(callbackURL) //nolint:noctx,gosec
+	resp, err := http.Get(callbackURL) //nolint:noctx,gosec // test-only HTTP call to local server
 	if err != nil {
 		t.Fatalf("GET callback failed: %v", err)
 	}
@@ -208,7 +210,7 @@ func TestCallbackServer_DoubleCallback(t *testing.T) {
 	done := make(chan error, 2)
 	for range 2 {
 		go func() {
-			resp, err := http.Get(url) //nolint:noctx,gosec
+			resp, err := http.Get(url) //nolint:noctx,gosec // test-only HTTP call to local server
 			if err == nil {
 				resp.Body.Close()
 			}
@@ -247,7 +249,7 @@ func TestCallbackServer_MissingCode(t *testing.T) {
 		"http://127.0.0.1:%d/callback?state=%s",
 		port, state,
 	)
-	resp, err := http.Get(callbackURL) //nolint:noctx,gosec
+	resp, err := http.Get(callbackURL) //nolint:noctx,gosec // test-only HTTP call to local server
 	if err != nil {
 		t.Fatalf("GET callback failed: %v", err)
 	}

config.go

@@ -2,11 +2,13 @@ package main
 
 import (
 	"crypto/tls"
+	"errors"
 	"flag"
 	"fmt"
 	"net/http"
 	"net/url"
 	"os"
+	"strconv"
 	"strings"
 	"time"
 
@@ -107,7 +109,7 @@ func initConfig() {
 	// Resolve callback port (int flag needs special handling).
 	portStr := ""
 	if *flagCallbackPort != 0 {
-		portStr = fmt.Sprintf("%d", *flagCallbackPort)
+		portStr = strconv.Itoa(*flagCallbackPort)
 	}
 	portStr = getConfig(portStr, "CALLBACK_PORT", "8888")
 	if _, err := fmt.Sscanf(portStr, "%d", &callbackPort); err != nil || callbackPort <= 0 {
@@ -185,7 +187,7 @@ func getEnv(key, defaultValue string) string {
 
 func validateServerURL(rawURL string) error {
 	if rawURL == "" {
-		return fmt.Errorf("server URL cannot be empty")
+		return errors.New("server URL cannot be empty")
 	}
 	u, err := url.Parse(rawURL)
 	if err != nil {
@@ -195,7 +197,7 @@ func validateServerURL(rawURL string) error {
 		return fmt.Errorf("URL scheme must be http or https, got: %s", u.Scheme)
 	}
 	if u.Host == "" {
-		return fmt.Errorf("URL must include a host")
+		return errors.New("URL must include a host")
 	}
 	return nil
 }