This repository was archived by the owner on Apr 16, 2026. It is now read-only.
Merged
Conversation
PR SummaryMedium Risk Overview Adds repo-local Reviewed by Cursor Bugbot for commit a563956. Bugbot is set up for automated code reviews on this repo. Configure here. |
7 tasks
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Dead
Valid()method never called by jwt/v5- Renamed the dead
Valid()hook toValidate()so jwt/v5 enforces the missingexpcheck and added a regression test for exp-less tokens.
- Renamed the dead
Preview (01b4092161)
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -7,7 +7,7 @@
pull_request:
jobs:
- test:
+ proto:
runs-on: ubuntu-latest
steps:
- name: Checkout
@@ -16,9 +16,9 @@
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 @@
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 @@
- 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: Test
- run: make test
+ - 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: Verify module checksums
+ run: go mod verify
diff --git a/.golangci.yml b/.golangci.yml
new file mode 100644
--- /dev/null
+++ b/.golangci.yml
@@ -1,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
--- 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,11 +9,23 @@
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
--- a/cmd/asb-worker/main.go
+++ b/cmd/asb-worker/main.go
@@ -62,8 +62,9 @@
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
--- a/internal/api/connectapi/server_test.go
+++ b/internal/api/connectapi/server_test.go
@@ -161,7 +161,8 @@
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/api/httpapi/server.go b/internal/api/httpapi/server.go
--- a/internal/api/httpapi/server.go
+++ b/internal/api/httpapi/server.go
@@ -414,7 +414,7 @@
}
body := http.MaxBytesReader(w, r.Body, s.maxBody)
- defer body.Close()
+ defer func() { _ = body.Close() }()
decoder := json.NewDecoder(body)
decoder.DisallowUnknownFields()
diff --git a/internal/app/cleanup.go b/internal/app/cleanup.go
--- a/internal/app/cleanup.go
+++ b/internal/app/cleanup.go
@@ -242,10 +242,3 @@
return nil
}
-
-func (s *Service) cleanupSummary(stats *CleanupStats) string {
- if stats == nil {
- return ""
- }
- return fmt.Sprintf("approvals=%d sessions=%d grants=%d artifacts=%d", stats.ApprovalsExpired, stats.SessionsExpired, stats.GrantsExpired, stats.ArtifactsExpired)
-}
diff --git a/internal/bootstrap/service.go b/internal/bootstrap/service.go
--- a/internal/bootstrap/service.go
+++ b/internal/bootstrap/service.go
@@ -576,6 +576,7 @@
}
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
@@ -608,6 +609,7 @@
}
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
@@ -628,6 +630,7 @@
}
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
--- a/internal/connectors/github/app_token_source_test.go
+++ b/internal/connectors/github/app_token_source_test.go
@@ -30,7 +30,11 @@
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)
}
@@ -98,8 +102,10 @@
_, _ = 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/client.go b/internal/connectors/vaultdb/client.go
--- a/internal/connectors/vaultdb/client.go
+++ b/internal/connectors/vaultdb/client.go
@@ -65,7 +65,7 @@
if err != nil {
return nil, err
}
- defer response.Body.Close()
+ defer func() { _ = response.Body.Close() }()
body, err := io.ReadAll(response.Body)
if err != nil {
return nil, err
@@ -115,7 +115,7 @@
if err != nil {
return err
}
- defer response.Body.Close()
+ defer func() { _ = response.Body.Close() }()
payload, err := io.ReadAll(response.Body)
if err != nil {
return err
diff --git a/internal/connectors/vaultdb/connector_test.go b/internal/connectors/vaultdb/connector_test.go
--- a/internal/connectors/vaultdb/connector_test.go
+++ b/internal/connectors/vaultdb/connector_test.go
@@ -16,7 +16,8 @@
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
--- a/internal/crypto/sessionjwt/manager.go
+++ b/internal/crypto/sessionjwt/manager.go
@@ -5,8 +5,8 @@
"fmt"
"time"
+ "github.com/evalops/asb/internal/core"
"github.com/golang-jwt/jwt/v5"
- "github.com/evalops/asb/internal/core"
)
type Manager struct {
@@ -28,7 +28,10 @@
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,
@@ -79,8 +82,8 @@
}, nil
}
-func (c *claims) Valid() error {
- if c.RegisteredClaims.ExpiresAt == nil {
+func (c *claims) Validate() error {
+ if c.ExpiresAt == nil {
return fmt.Errorf("%w: missing exp", core.ErrUnauthorized)
}
return jwt.NewValidator(jwt.WithTimeFunc(time.Now)).Validate(c.RegisteredClaims)
diff --git a/internal/crypto/sessionjwt/manager_test.go b/internal/crypto/sessionjwt/manager_test.go
new file mode 100644
--- /dev/null
+++ b/internal/crypto/sessionjwt/manager_test.go
@@ -1,0 +1,48 @@
+package sessionjwt
+
+import (
+ "crypto/ed25519"
+ "errors"
+ "strings"
+ "testing"
+
+ "github.com/evalops/asb/internal/core"
+ "github.com/golang-jwt/jwt/v5"
+)
+
+func TestManager_VerifyRejectsTokenWithoutExpiration(t *testing.T) {
+ t.Parallel()
+
+ _, privateKey, err := ed25519.GenerateKey(nil)
+ if err != nil {
+ t.Fatalf("GenerateKey() error = %v", err)
+ }
+
+ manager, err := NewManager(privateKey)
+ if err != nil {
+ t.Fatalf("NewManager() error = %v", err)
+ }
+
+ token := jwt.NewWithClaims(jwt.SigningMethodEdDSA, jwt.MapClaims{
+ "sid": "sess_123",
+ "tenant_id": "t_acme",
+ "agent_id": "agent_123",
+ "run_id": "run_123",
+ "tool_context": []string{
+ "github",
+ },
+ "workload_hash": "sha256:test",
+ })
+ raw, err := token.SignedString(privateKey)
+ if err != nil {
+ t.Fatalf("SignedString() error = %v", err)
+ }
+
+ if _, err := manager.Verify(raw); err == nil {
+ t.Fatal("Verify() error = nil, want non-nil")
+ } else if !errors.Is(err, core.ErrUnauthorized) {
+ t.Fatalf("Verify() error = %v, want unauthorized", err)
+ } else if !strings.Contains(err.Error(), "missing exp") {
+ t.Fatalf("Verify() error = %v, want missing exp", err)
+ }
+}
diff --git a/internal/migrate/runner.go b/internal/migrate/runner.go
--- a/internal/migrate/runner.go
+++ b/internal/migrate/runner.go
@@ -54,6 +54,7 @@
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
--- a/internal/migrate/runner_test.go
+++ b/internal/migrate/runner_test.go
@@ -62,7 +62,7 @@
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)
}
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 0159b8c. Configure here.
haasonsaas
force-pushed
the
codex/asb-ci-hardening
branch
from
01b4092 to
b1814a5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
go run github.com/rhysd/actionlint/cmd/actionlint@latest .github/workflows/ci.ymlbuf lintmake proto-checkgo test ./... -count=1go test ./... -race -count=1GOTOOLCHAIN=go1.26.0 go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.11.3 run --timeout=5m ./...make lint security-scan coverageCloses #16