From b1814a58ead3f8302e8726f61bf82f0cb6ca38c3 Mon Sep 17 00:00:00 2001 From: Jonathan Haas Date: Wed, 15 Apr 2026 14:15:28 -0700 Subject: [PATCH 1/3] Harden ASB CI quality gates --- .github/workflows/ci.yml | 85 +++++++++++++++++-- .golangci.yml | 17 ++++ Makefile | 19 ++++- cmd/asb-worker/main.go | 5 +- internal/api/connectapi/server_test.go | 3 +- internal/bootstrap/service.go | 3 + .../github/app_token_source_test.go | 8 +- internal/connectors/vaultdb/connector_test.go | 3 +- internal/crypto/sessionjwt/manager.go | 5 +- internal/migrate/runner.go | 1 + internal/migrate/runner_test.go | 2 +- 11 files changed, 133 insertions(+), 18 deletions(-) create mode 100644 .golangci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d5ea7d1..abd0102 100644 --- 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 @@ -50,8 +52,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 new file mode 100644 index 0000000..8482e3f --- /dev/null +++ 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 index 0a46d91..b4e8692 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,7 @@ -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 ./... @@ -8,10 +9,22 @@ 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 index 14b8662..970f2d9 100644 --- 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 index a4d98e3..a3e9f27 100644 --- 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/bootstrap/service.go b/internal/bootstrap/service.go index b1a298b..5faa0a0 100644 --- 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 index 8dfa2ec..37c6333 100644 --- 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 index 65b3812..ddce4b0 100644 --- 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 index 9c61c9c..f0d2a75 100644 --- 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 index 7237b9e..9d5f4d1 100644 --- 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 index ae2597c..2c663c4 100644 --- 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) } } From 7d583da75c1c839a6bfcb64caa747652564b85ec Mon Sep 17 00:00:00 2001 From: Jonathan Haas Date: Wed, 15 Apr 2026 14:36:28 -0700 Subject: [PATCH 2/3] fmt: normalize authn import ordering --- internal/authn/delegationjwt/validator.go | 2 +- internal/authn/delegationjwt/validator_test.go | 2 +- internal/authn/k8s/verifier.go | 2 +- internal/authn/k8s/verifier_test.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/authn/delegationjwt/validator.go b/internal/authn/delegationjwt/validator.go index b6c9be9..f8775b0 100644 --- 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 index cd5fae1..fdb3e76 100644 --- 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 index cf1072d..fafed81 100644 --- 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 index a482c3d..7eb0889 100644 --- 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) { From a563956bbebb3dd162a0a25e4f2a0052ea1fa87f Mon Sep 17 00:00:00 2001 From: Jonathan Haas Date: Wed, 15 Apr 2026 14:39:44 -0700 Subject: [PATCH 3/3] Fix ASB buf breaking CI fetch --- .github/workflows/ci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index abd0102..7dfd7e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,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"