[codex] Harden ASB CI quality gates (#63)

haasonsaas · web-flow · commit 73f03d20a360 · 2026-04-15T14:43:09.000-07:00
* Harden ASB CI quality gates

* fmt: normalize authn import ordering

* Fix ASB buf breaking CI fetch
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -7,7 +7,7 @@ on:
   pull_request:
 
 jobs:
-  test:
+  proto:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout
@@ -16,9 +16,9 @@ jobs:
           fetch-depth: 0
 
       - name: Setup Go
-        uses: evalops/service-runtime/.github/actions/setup-go-service@ca09da8302203b96582332dbcababe9f2d906d10
+        uses: evalops/service-runtime/.github/actions/setup-go-service@cac5638c2935870453cf669e803936a80e17ac7b
         with:
-          go-version-file: go.mod
+          go-version: "1.26.2"
           cache: true
 
       - name: Setup Buf
@@ -32,7 +32,9 @@ jobs:
           go install connectrpc.com/connect/cmd/protoc-gen-connect-go@v1.19.1
 
       - name: Format
-        run: make fmt
+        run: |
+          make fmt
+          git diff --exit-code
 
       - name: Buf lint
         run: buf lint
@@ -41,7 +43,6 @@ jobs:
         if: github.event_name == 'pull_request'
         run: |
           if git show origin/main:buf.yaml >/dev/null 2>&1; then
-            git fetch origin main:refs/remotes/origin/main
             buf breaking --against '.git#ref=refs/remotes/origin/main'
           else
             echo "Skipping: no buf.yaml on main yet"
@@ -50,8 +51,75 @@ jobs:
       - name: Verify Proto Sync
         run: make proto-check
 
-      - name: Vet
-        run: go vet ./...
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+        with:
+          fetch-depth: 0
+
+      - name: Setup Go
+        uses: evalops/service-runtime/.github/actions/setup-go-service@cac5638c2935870453cf669e803936a80e17ac7b
+        with:
+          go-version: "1.26.2"
+          cache: true
+          run-go-test: "true"
+          go-test-race: "true"
+          go-test-args: "./... -count=1"
+
+      - name: Build
+        run: go build ./...
+
+      - name: Collect coverage
+        id: coverage
+        run: |
+          go test ./... -coverprofile=coverage.out -covermode=atomic -count=1
+          coverage="$(go tool cover -func=coverage.out | awk '/^total:/ {sub(/%/, "", $3); print $3}')"
+          echo "value=${coverage}" >> "$GITHUB_OUTPUT"
+          awk -v got="${coverage}" -v floor="40" 'BEGIN { if ((got + 0) < (floor + 0)) exit 1 }'
+
+      - name: Publish coverage summary
+        run: |
+          echo "Total coverage: ${{ steps.coverage.outputs.value }}%" >> "$GITHUB_STEP_SUMMARY"
+
+      - name: Upload coverage artifact
+        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
+        with:
+          name: coverage.out
+          path: coverage.out
+
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+        with:
+          fetch-depth: 0
+
+      - name: Setup Go
+        uses: evalops/service-runtime/.github/actions/setup-go-service@cac5638c2935870453cf669e803936a80e17ac7b
+        with:
+          go-version: "1.26.2"
+          cache: true
+          run-golangci-lint: "true"
+          golangci-lint-args: "--timeout=5m ./..."
+
+  security:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+        with:
+          fetch-depth: 0
+
+      - name: Setup Go
+        uses: evalops/service-runtime/.github/actions/setup-go-service@cac5638c2935870453cf669e803936a80e17ac7b
+        with:
+          go-version: "1.26.2"
+          cache: true
+          run-gosec: "true"
+          run-govulncheck: "true"
 
-      - name: Test
-        run: make test
+      - name: Verify module checksums
+        run: go mod verify
diff --git a/.golangci.yml b/.golangci.yml
@@ -0,0 +1,17 @@
+version: "2"
+
+run:
+  timeout: 5m
+
+linters:
+  enable:
+    - errcheck
+    - govet
+    - ineffassign
+    - staticcheck
+    - unused
+    - gosec
+
+  settings:
+    errcheck:
+      check-type-assertions: true
diff --git a/Makefile b/Makefile
@@ -1,17 +1,30 @@
-GO ?= go
+TOOLCHAIN ?= go1.26.2
+GO ?= env GOTOOLCHAIN=$(TOOLCHAIN) go
 
-.PHONY: fmt test vet lint install-hooks proto proto-check migrate run-api run-worker
+.PHONY: fmt test test-race vet lint security-scan coverage install-hooks proto proto-check migrate run-api run-worker
 
 fmt:
 	$(GO) fmt ./...
 
 test:
 	$(GO) test ./...
 
+test-race:
+	$(GO) test -race ./...
+
 vet:
 	$(GO) vet ./...
 
-lint: vet
+lint:
+	golangci-lint run ./...
+
+security-scan:
+	$(GO) mod verify
+	gosec ./cmd/... ./internal/...
+	env GOTOOLCHAIN=$(TOOLCHAIN) govulncheck ./...
+
+coverage:
+	$(GO) test ./... -coverprofile=coverage.out -covermode=atomic
 
 proto:
 	bash scripts/sync-proto.sh
diff --git a/cmd/asb-worker/main.go b/cmd/asb-worker/main.go
@@ -62,8 +62,9 @@ func main() {
 		mux := http.NewServeMux()
 		mux.Handle("/metrics", promhttp.Handler())
 		metricsServer = &http.Server{
-			Addr:    metricsAddr,
-			Handler: mux,
+			Addr:              metricsAddr,
+			Handler:           mux,
+			ReadHeaderTimeout: 5 * time.Second,
 		}
 		go func() {
 			if err := metricsServer.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
diff --git a/internal/api/connectapi/server_test.go b/internal/api/connectapi/server_test.go
@@ -161,7 +161,8 @@ func TestServer_CreateSessionAcceptsOIDCAttestationKind(t *testing.T) {
 				t.Fatalf("unexpected attestation = %#v", req.Attestation)
 			}
 			return &core.CreateSessionResponse{
-				SessionID:    "sess_oidc",
+				SessionID: "sess_oidc",
+				// #nosec G101 -- Synthetic session token fixture for transport tests.
 				SessionToken: "eyJ.oidc",
 				ExpiresAt:    time.Date(2026, 4, 15, 6, 0, 0, 0, time.UTC),
 			}, nil
diff --git a/internal/authn/delegationjwt/validator.go b/internal/authn/delegationjwt/validator.go
@@ -6,8 +6,8 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/golang-jwt/jwt/v5"
 	"github.com/evalops/asb/internal/core"
+	"github.com/golang-jwt/jwt/v5"
 )
 
 type Config struct {
diff --git a/internal/authn/delegationjwt/validator_test.go b/internal/authn/delegationjwt/validator_test.go
@@ -6,9 +6,9 @@ import (
 	"testing"
 	"time"
 
-	"github.com/golang-jwt/jwt/v5"
 	"github.com/evalops/asb/internal/authn/delegationjwt"
 	"github.com/evalops/asb/internal/core"
+	"github.com/golang-jwt/jwt/v5"
 )
 
 func TestValidator_ValidateSignedDelegation(t *testing.T) {
diff --git a/internal/authn/k8s/verifier.go b/internal/authn/k8s/verifier.go
@@ -5,8 +5,8 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/golang-jwt/jwt/v5"
 	"github.com/evalops/asb/internal/core"
+	"github.com/golang-jwt/jwt/v5"
 )
 
 type Keyfunc func(ctx context.Context, token *jwt.Token) (any, error)
diff --git a/internal/authn/k8s/verifier_test.go b/internal/authn/k8s/verifier_test.go
@@ -6,9 +6,9 @@ import (
 	"testing"
 	"time"
 
-	"github.com/golang-jwt/jwt/v5"
 	"github.com/evalops/asb/internal/authn/k8s"
 	"github.com/evalops/asb/internal/core"
+	"github.com/golang-jwt/jwt/v5"
 )
 
 func TestVerifier_VerifyProjectedServiceAccountToken(t *testing.T) {
diff --git a/internal/bootstrap/service.go b/internal/bootstrap/service.go
@@ -615,6 +615,7 @@ func redisPoolStats(client *goredis.Client) func() *goredis.PoolStats {
 }
 
 func loadPublicKey(path string) (any, error) {
+	// #nosec G304,G703 -- Public-key paths come from explicit operator configuration.
 	contents, err := os.ReadFile(path)
 	if err != nil {
 		return nil, err
@@ -647,6 +648,7 @@ func loadEd25519PublicKey(path string) (ed25519.PublicKey, error) {
 }
 
 func loadEd25519PrivateKey(path string) (ed25519.PrivateKey, error) {
+	// #nosec G304,G703 -- Private-key paths come from explicit operator configuration.
 	contents, err := os.ReadFile(path)
 	if err != nil {
 		return nil, err
@@ -667,6 +669,7 @@ func loadEd25519PrivateKey(path string) (ed25519.PrivateKey, error) {
 }
 
 func loadRSAPrivateKey(path string) (*rsa.PrivateKey, error) {
+	// #nosec G304,G703 -- Private-key paths come from explicit operator configuration.
 	contents, err := os.ReadFile(path)
 	if err != nil {
 		return nil, err
diff --git a/internal/connectors/github/app_token_source_test.go b/internal/connectors/github/app_token_source_test.go
@@ -31,7 +31,11 @@ func TestAppTokenSource_TokenForRepoUsesInstallationTokenAndCaches(t *testing.T)
 	tokenRequests := 0
 
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		claims := parseAppJWT(t, privateKey.Public().(*rsa.PublicKey), r.Header.Get("Authorization"))
+		publicKey, ok := privateKey.Public().(*rsa.PublicKey)
+		if !ok {
+			t.Fatalf("public key = %T, want *rsa.PublicKey", privateKey.Public())
+		}
+		claims := parseAppJWT(t, publicKey, r.Header.Get("Authorization"))
 		if claims.Issuer != "123" {
 			t.Fatalf("issuer = %q, want 123", claims.Issuer)
 		}
@@ -170,8 +174,10 @@ func TestAppTokenSource_TokenForRepoRefreshesWhenTokenNearExpiry(t *testing.T) {
 			_, _ = w.Write([]byte(`{"id":987}`))
 		case "/app/installations/987/access_tokens":
 			tokenRequests++
+			// #nosec G101 -- Synthetic installation tokens for cache refresh tests.
 			tokenValue := "inst-token-1"
 			if tokenRequests > 1 {
+				// #nosec G101 -- Synthetic installation tokens for cache refresh tests.
 				tokenValue = "inst-token-2"
 			}
 			_, _ = w.Write([]byte(`{"token":"` + tokenValue + `","expires_at":"` + now.Add(6*time.Minute).Format(time.RFC3339) + `"}`))
diff --git a/internal/connectors/vaultdb/connector_test.go b/internal/connectors/vaultdb/connector_test.go
@@ -16,7 +16,8 @@ func TestConnector_IssueAndRevokeDynamicCredentials(t *testing.T) {
 
 	client := &fakeVaultClient{
 		lease: &vaultdb.LeaseCredentials{
-			Username:      "v-token-user",
+			Username: "v-token-user",
+			// #nosec G101 -- Synthetic password fixture exercises DSN escaping.
 			Password:      "secret:/?#[]@",
 			LeaseID:       "database/creds/analytics_ro/123",
 			LeaseDuration: 10 * time.Minute,
diff --git a/internal/crypto/sessionjwt/manager.go b/internal/crypto/sessionjwt/manager.go
@@ -28,7 +28,10 @@ func NewManager(privateKey ed25519.PrivateKey) (*Manager, error) {
 	if len(privateKey) == 0 {
 		return nil, fmt.Errorf("%w: private key is required", core.ErrInvalidRequest)
 	}
-	publicKey := privateKey.Public().(ed25519.PublicKey)
+	publicKey, ok := privateKey.Public().(ed25519.PublicKey)
+	if !ok {
+		return nil, fmt.Errorf("%w: private key public component is %T, want ed25519.PublicKey", core.ErrInvalidRequest, privateKey.Public())
+	}
 	return &Manager{
 		privateKey: privateKey,
 		publicKey:  publicKey,
diff --git a/internal/migrate/runner.go b/internal/migrate/runner.go
@@ -54,6 +54,7 @@ func Discover(dir string) ([]Migration, error) {
 			return nil, fmt.Errorf("invalid migration filename %q", entry.Name())
 		}
 		path := filepath.Join(dir, entry.Name())
+		// #nosec G304 -- Paths come from the filtered migration directory listing.
 		contents, err := os.ReadFile(path)
 		if err != nil {
 			return nil, err
diff --git a/internal/migrate/runner_test.go b/internal/migrate/runner_test.go
@@ -62,7 +62,7 @@ func TestRunner_UpAppliesPendingMigrations(t *testing.T) {
 
 func writeMigration(t *testing.T, dir string, name string, contents string) {
 	t.Helper()
-	if err := os.WriteFile(filepath.Join(dir, name), []byte(contents), 0o644); err != nil {
+	if err := os.WriteFile(filepath.Join(dir, name), []byte(contents), 0o600); err != nil {
 		t.Fatalf("WriteFile() error = %v", err)
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -6,8 +6,8 @@ import (`
`6`	`6`	`"fmt"`
`7`	`7`	`"time"`
`8`	`8`
`9`		`- "github.com/golang-jwt/jwt/v5"`
`10`	`9`	`"github.com/evalops/asb/internal/core"`
	`10`	`+ "github.com/golang-jwt/jwt/v5"`
`11`	`11`	`)`
`12`	`12`
`13`	`13`	`type Config struct {`
Original file line number	Diff line number	Diff line change
`@@ -6,9 +6,9 @@ import (`
`6`	`6`	`"testing"`
`7`	`7`	`"time"`
`8`	`8`
`9`		`- "github.com/golang-jwt/jwt/v5"`
`10`	`9`	`"github.com/evalops/asb/internal/authn/delegationjwt"`
`11`	`10`	`"github.com/evalops/asb/internal/core"`
	`11`	`+ "github.com/golang-jwt/jwt/v5"`
`12`	`12`	`)`
`13`	`13`
`14`	`14`	`func TestValidator_ValidateSignedDelegation(t *testing.T) {`
Original file line number	Diff line number	Diff line change
`@@ -5,8 +5,8 @@ import (`
`5`	`5`	`"fmt"`
`6`	`6`	`"strings"`
`7`	`7`
`8`		`- "github.com/golang-jwt/jwt/v5"`
`9`	`8`	`"github.com/evalops/asb/internal/core"`
	`9`	`+ "github.com/golang-jwt/jwt/v5"`
`10`	`10`	`)`
`11`	`11`
`12`	`12`	`type Keyfunc func(ctx context.Context, token *jwt.Token) (any, error)`
Original file line number	Diff line number	Diff line change
`@@ -615,6 +615,7 @@ func redisPoolStats(client goredis.Client) func() goredis.PoolStats {`
`615`	`615`	`}`
`616`	`616`
`617`	`617`	`func loadPublicKey(path string) (any, error) {`
	`618`	`+ // #nosec G304,G703 -- Public-key paths come from explicit operator configuration.`
`618`	`619`	`contents, err := os.ReadFile(path)`
`619`	`620`	`if err != nil {`
`620`	`621`	`return nil, err`
`@@ -647,6 +648,7 @@ func loadEd25519PublicKey(path string) (ed25519.PublicKey, error) {`
`647`	`648`	`}`
`648`	`649`
`649`	`650`	`func loadEd25519PrivateKey(path string) (ed25519.PrivateKey, error) {`
	`651`	`+ // #nosec G304,G703 -- Private-key paths come from explicit operator configuration.`
`650`	`652`	`contents, err := os.ReadFile(path)`
`651`	`653`	`if err != nil {`
`652`	`654`	`return nil, err`
`@@ -667,6 +669,7 @@ func loadEd25519PrivateKey(path string) (ed25519.PrivateKey, error) {`
`667`	`669`	`}`
`668`	`670`
`669`	`671`	`func loadRSAPrivateKey(path string) (*rsa.PrivateKey, error) {`
	`672`	`+ // #nosec G304,G703 -- Private-key paths come from explicit operator configuration.`
`670`	`673`	`contents, err := os.ReadFile(path)`
`671`	`674`	`if err != nil {`
`672`	`675`	`return nil, err`