fix(cli): verify SLSA provenance via GitHub attestation API#548
fix(cli): verify SLSA provenance via GitHub attestation API#548
Conversation
The previous OCI-referrer-based provenance lookup never worked because actions/attest-build-provenance stores attestations in GitHub's attestation store, not as OCI referrers in the registry. - Rewrite provenance.go to fetch from GitHub attestation API (GET /repos/OWNER/REPO/attestations/DIGEST) instead of OCI referrers - Parse Sigstore v0.3 bundles from the API response - Cap response size to 5MB to prevent memory exhaustion - Make provenance failure a hard error (was warn-only) - Remove unused OCI-referrer-based code, types, and imports - Rewrite tests for the GitHub API approach with mock HTTP server - Remove --registry-referrers-mode and COSIGN_EXPERIMENTAL from CI Tested locally: all 3 images show "SLSA provenance: verified". Closes #532
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the SLSA provenance verification process within the CLI. It transitions from an unreliable OCI-referrer-based method to a more robust approach that directly leverages the GitHub Attestation API. This change enhances the security posture by making provenance verification failures hard errors and simplifies the codebase by removing deprecated logic. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaces OCI referrer-based provenance retrieval with GitHub Attestation API queries. Fetches JSON attestation bundles for a digest, iterates bundles, performs cryptographic verification per bundle, and treats the first successfully verified bundle as proof of provenance. Adds API constants, HTTP client, test hooks, and updated error mapping. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Verify
participant HTTP as Attestation HTTP Client
participant GH as GitHub Attestation API
participant Ver as Bundle Verifier
CLI->>HTTP: GET /repos/{owner}/{repo}/check-runs/attestations?digest={sha256}
HTTP->>GH: Request attestation bundles for digest
GH-->>HTTP: 200 OK with JSON array of bundles
HTTP-->>CLI: returns []json.RawMessage (bundles)
CLI->>Ver: for each bundle -> verifyProvenanceBundle(bundle, digest, sev, certID)
Ver->>Ver: cryptographic signature & certificate checks
alt bundle valid
Ver-->>CLI: success
CLI->>CLI: mark ProvenanceVerified = true
CLI-->>HTTP: stop iterating
else invalid
Ver-->>CLI: error (aggregate)
CLI->>Ver: try next bundle
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Code Review
This pull request provides a significant improvement by refactoring the SLSA provenance verification to use the GitHub Attestation API, which fixes the previously non-functional implementation based on OCI referrers. The new implementation is cleaner, simpler, and correctly verifies provenance. Making provenance verification failure a hard error is a great step towards a more secure default. The code changes are well-structured and include comprehensive tests for the new API interaction. I've added a few minor suggestions to further improve code clarity and robustness.
cli/internal/verify/provenance.go
Outdated
| defaultGitHubAPIBase = "https://api.github.com" | ||
|
|
||
| // githubAttestationOwnerRepo is the GitHub owner/repo for attestation lookups. | ||
| // Derived from the image repository prefix (aureliolo/synthorg-*). |
There was a problem hiding this comment.
The comment here states that the owner/repo is derived from the image repository prefix. However, ImageRepoPrefix is aureliolo/synthorg- (lowercase a), while this constant uses Aureliolo (uppercase A). While this is the correct path for the GitHub API, the comment is slightly misleading. To improve clarity, consider updating the comment to remove the reference to the image prefix.
| // Derived from the image repository prefix (aureliolo/synthorg-*). | |
| // This is the canonical path for the project on GitHub. |
| type githubAttestationResponse struct { | ||
| Attestations []struct { | ||
| Bundle json.RawMessage `json:"bundle"` | ||
| } `json:"attestations"` | ||
| } |
There was a problem hiding this comment.
For better readability and maintainability, consider defining a named struct for the items in the Attestations slice instead of using an anonymous struct. This makes the code easier to understand and reuse if needed.
For example:
type githubAttestation struct {
Bundle json.RawMessage `json:"bundle"`
}
type githubAttestationResponse struct {
Attestations []githubAttestation `json:"attestations"`
}
cli/internal/verify/provenance.go
Outdated
| req.Header.Set("Accept", "application/json") | ||
|
|
||
| layerReader, err := layers[0].Uncompressed() | ||
| resp, err := http.DefaultClient.Do(req) |
There was a problem hiding this comment.
For better robustness and to avoid potential side-effects from other packages modifying the global http.DefaultClient, it's recommended to instantiate a private http.Client for this package. This isolates your HTTP requests from global state and improves testability.
For example:
// At package level
var attestationClient = &http.Client{}
// In fetchGitHubAttestations
resp, err := attestationClient.Do(req)There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/internal/verify/provenance_test.go`:
- Around line 31-125: Add two tests for fetchGitHubAttestations: one that
returns a response body larger than the 5MB limit to exercise the size-check
branch (ensure the test server set via setGitHubAPIBase returns a huge payload
and asserts the error matches the size-rejection path in provenance.go), and one
mixed-bundle test where the HTTP JSON contains multiple attestations with the
first bundle invalid/corrupt and a later bundle valid (assert
fetchGitHubAttestations succeeds and returns the valid bundle), referencing
fetchGitHubAttestations, setGitHubAPIBase, githubAPIBase and
ErrNoProvenanceAttestations to locate and wire the tests into the existing
suite.
In `@cli/internal/verify/provenance.go`:
- Around line 66-74: The loop currently returns success as soon as
verifyProvenanceBundle(...) succeeds, but that function only checks
signatures/digests and not the in-toto predicate type; update the logic so you
only treat a bundle as a successful SLSA provenance verification if the verified
statement's predicate type starts with SLSAProvenancePredicatePrefix (and/or
matches DSSEPayloadType as appropriate). Concretely: inside
verifyProvenanceBundle (after calling sev.Verify() and obtaining
VerificationResult), check VerificationResult.Statement.PredicateType and return
an error if it does not start with SLSAProvenancePredicatePrefix; also change
the main loop around verifyProvenanceBundle so it does not return nil unless
verifyProvenanceBundle confirms the predicate type, and apply the same
predicate-type check in any other call sites of verifyProvenanceBundle.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ca6c163b-3d5f-4ace-997f-70c0ac76f862
📒 Files selected for processing (3)
cli/internal/verify/provenance.gocli/internal/verify/provenance_test.gocli/internal/verify/verify.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CLI Test (windows-latest)
🧰 Additional context used
📓 Path-based instructions (2)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: Go: runcd cli && golangci-lint runbefore committing CLI code
Go: runcd cli && go vet ./...to check for common Go mistakes
Files:
cli/internal/verify/provenance_test.gocli/internal/verify/verify.gocli/internal/verify/provenance.go
cli/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Go: use
testing.Ffor fuzz testing; organize fuzz targets in test files withFuzzTargetnaming
Files:
cli/internal/verify/provenance_test.go
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to .github/workflows/docker.yml : CI Docker: build → scan → push to GHCR + cosign sign + SLSA L3 provenance via attest-build-provenance (images only pushed after Trivy/Grype scans pass).
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to .github/workflows/docker.yml : CI Docker: build → scan → push to GHCR + cosign sign + SLSA L3 provenance via attest-build-provenance (images only pushed after Trivy/Grype scans pass).
Applied to files:
cli/internal/verify/provenance.go
🧬 Code graph analysis (3)
cli/internal/verify/provenance_test.go (2)
cli/internal/verify/provenance.go (2)
ErrNoProvenanceAttestations(42-42)VerifyProvenance(56-76)cli/internal/verify/verify.go (1)
ImageRef(27-32)
cli/internal/verify/verify.go (1)
cli/internal/verify/provenance.go (1)
VerifyProvenance(56-76)
cli/internal/verify/provenance.go (1)
cli/internal/verify/verify.go (1)
ImageRef(27-32)
| func TestFetchGitHubAttestationsNotFound(t *testing.T) { | ||
| srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| switch { | ||
| case r.URL.Path == "/v2/": | ||
| w.WriteHeader(http.StatusOK) | ||
| case strings.Contains(r.URL.Path, "/referrers/"): | ||
| w.Header().Set("Content-Type", "application/vnd.oci.image.index.v1+json") | ||
| _, _ = w.Write([]byte(emptyIndex)) | ||
| default: | ||
| w.WriteHeader(http.StatusNotFound) | ||
| } | ||
| w.WriteHeader(http.StatusNotFound) | ||
| })) | ||
| defer srv.Close() | ||
|
|
||
| host := strings.TrimPrefix(srv.URL, "http://") | ||
| ref := ImageRef{ | ||
| Registry: host, | ||
| Repository: repo, | ||
| Tag: "1.0.0", | ||
| Digest: testDigest, | ||
| } | ||
| // Temporarily override the API base for testing. | ||
| origBase := githubAPIBase | ||
| defer func() { setGitHubAPIBase(origBase) }() | ||
| setGitHubAPIBase(srv.URL) | ||
|
|
||
| err := VerifyProvenance(context.Background(), ref, nil, sigverify.CertificateIdentity{}) | ||
| _, err := fetchGitHubAttestations(context.Background(), testDigest) | ||
| if err == nil { | ||
| t.Fatal("expected error when no attestations found") | ||
| t.Fatal("expected error for 404 response") | ||
| } | ||
| if !strings.Contains(err.Error(), "no SLSA provenance attestations") { | ||
| t.Errorf("unexpected error: %v", err) | ||
| if !errors.Is(err, ErrNoProvenanceAttestations) { | ||
| t.Errorf("expected ErrNoProvenanceAttestations, got: %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestValidateSLSAPredicate(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| predicateType string | ||
| payloadType string | ||
| wantErr bool | ||
| errContains string | ||
| }{ | ||
| { | ||
| name: "valid SLSA v1", | ||
| predicateType: "https://slsa.dev/provenance/v1", | ||
| payloadType: DSSEPayloadType, | ||
| }, | ||
| { | ||
| name: "wrong predicate type", | ||
| predicateType: "https://example.com/wrong-type", | ||
| payloadType: DSSEPayloadType, | ||
| wantErr: true, | ||
| errContains: "unexpected predicate type", | ||
| }, | ||
| { | ||
| name: "wrong payload type", | ||
| predicateType: "https://slsa.dev/provenance/v1", | ||
| payloadType: "application/octet-stream", | ||
| wantErr: true, | ||
| errContains: "unexpected DSSE payload type", | ||
| }, | ||
| } | ||
| func TestFetchGitHubAttestationsEmptyResponse(t *testing.T) { | ||
| srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = w.Write([]byte(`{"attestations": []}`)) | ||
| })) | ||
| defer srv.Close() | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| statement := inTotoStatement{PredicateType: tt.predicateType} | ||
| statementJSON, err := json.Marshal(statement) | ||
| if err != nil { | ||
| t.Fatalf("failed to marshal statement: %v", err) | ||
| } | ||
|
|
||
| envelope := dsseEnvelope{ | ||
| PayloadType: tt.payloadType, | ||
| Payload: base64.StdEncoding.EncodeToString(statementJSON), | ||
| } | ||
|
|
||
| err = validateSLSAPredicate(envelope) | ||
| if (err != nil) != tt.wantErr { | ||
| t.Fatalf("validateSLSAPredicate() error = %v, wantErr %v", err, tt.wantErr) | ||
| } | ||
| if tt.errContains != "" && err != nil { | ||
| if !strings.Contains(err.Error(), tt.errContains) { | ||
| t.Errorf("error %q should contain %q", err.Error(), tt.errContains) | ||
| } | ||
| } | ||
| }) | ||
| origBase := githubAPIBase | ||
| defer func() { setGitHubAPIBase(origBase) }() | ||
| setGitHubAPIBase(srv.URL) | ||
|
|
||
| _, err := fetchGitHubAttestations(context.Background(), testDigest) | ||
| if err == nil { | ||
| t.Fatal("expected error for empty attestations") | ||
| } | ||
| if !errors.Is(err, ErrNoProvenanceAttestations) { | ||
| t.Errorf("expected ErrNoProvenanceAttestations, got: %v", err) | ||
| } | ||
| } | ||
|
|
||
| // TestVerifyProvenanceInvalidAttestation verifies that VerifyProvenance rejects | ||
| // an attestation with an invalid structure. The mock attestation has a non-SLSA | ||
| // predicate and no Sigstore bundle, so VerifyProvenance may fail at either | ||
| // validateSLSAPredicate or verifyAttestationBundle — the test exercises the | ||
| // general invalid-attestation error path, not a specific validation step. | ||
| func TestVerifyProvenanceInvalidAttestation(t *testing.T) { | ||
| repo := "test/image" | ||
| attestDigest := "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" | ||
|
|
||
| statement := inTotoStatement{ | ||
| PredicateType: "https://example.com/not-slsa", | ||
| func TestFetchGitHubAttestationsSuccess(t *testing.T) { | ||
| bundle := json.RawMessage(`{"mediaType": "application/vnd.dev.sigstore.bundle.v0.3+json"}`) | ||
| resp := githubAttestationResponse{ | ||
| Attestations: []struct { | ||
| Bundle json.RawMessage `json:"bundle"` | ||
| }{ | ||
| {Bundle: bundle}, | ||
| }, | ||
| } | ||
| statementJSON, err := json.Marshal(statement) | ||
| respJSON, err := json.Marshal(resp) | ||
| if err != nil { | ||
| t.Fatalf("failed to marshal statement: %v", err) | ||
| } | ||
| envelope := dsseEnvelope{ | ||
| PayloadType: DSSEPayloadType, | ||
| Payload: base64.StdEncoding.EncodeToString(statementJSON), | ||
| t.Fatalf("marshaling response: %v", err) | ||
| } | ||
| envelopeJSON, err := json.Marshal(envelope) | ||
|
|
||
| srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| // Verify the URL path contains the digest. | ||
| if !strings.Contains(r.URL.Path, testDigest) { | ||
| t.Errorf("expected digest in URL path, got: %s", r.URL.Path) | ||
| } | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = w.Write(respJSON) | ||
| })) | ||
| defer srv.Close() | ||
|
|
||
| origBase := githubAPIBase | ||
| defer func() { setGitHubAPIBase(origBase) }() | ||
| setGitHubAPIBase(srv.URL) | ||
|
|
||
| bundles, err := fetchGitHubAttestations(context.Background(), testDigest) | ||
| if err != nil { | ||
| t.Fatalf("failed to marshal envelope: %v", err) | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
| if len(bundles) != 1 { | ||
| t.Fatalf("expected 1 bundle, got %d", len(bundles)) | ||
| } | ||
| } | ||
|
|
||
| attestManifest := ociManifest{ | ||
| SchemaVersion: 2, | ||
| MediaType: "application/vnd.oci.image.manifest.v1+json", | ||
| Config: ociDescriptor{ | ||
| MediaType: "application/vnd.oci.image.config.v1+json", | ||
| Digest: "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", | ||
| Size: 2, | ||
| }, | ||
| Layers: []ociLayerDescriptor{ | ||
| { | ||
| MediaType: "application/vnd.in-toto+json", | ||
| Digest: "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc", | ||
| Size: len(envelopeJSON), | ||
| }, | ||
| }, | ||
| func TestFetchGitHubAttestationsServerError(t *testing.T) { | ||
| srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.WriteHeader(http.StatusInternalServerError) | ||
| })) | ||
| defer srv.Close() | ||
|
|
||
| origBase := githubAPIBase | ||
| defer func() { setGitHubAPIBase(origBase) }() | ||
| setGitHubAPIBase(srv.URL) | ||
|
|
||
| _, err := fetchGitHubAttestations(context.Background(), testDigest) | ||
| if err == nil { | ||
| t.Fatal("expected error for 500 response") | ||
| } | ||
| attestManifestJSON, err := json.Marshal(attestManifest) | ||
| if err != nil { | ||
| t.Fatalf("failed to marshal attestation manifest: %v", err) | ||
| if !strings.Contains(err.Error(), fmt.Sprintf("HTTP %d", http.StatusInternalServerError)) { | ||
| t.Errorf("expected HTTP status in error, got: %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add regressions for the new size-limit and bundle-selection branches.
The new suite never hits the >5 MB rejection on Lines 113-119 in cli/internal/verify/provenance.go or the "earlier bundles fail, later bundle succeeds" path on Lines 66-74 there. Please add one oversized-response case and one mixed-bundle case so those two new branches cannot regress silently.
Also applies to: 127-155
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/internal/verify/provenance_test.go` around lines 31 - 125, Add two tests
for fetchGitHubAttestations: one that returns a response body larger than the
5MB limit to exercise the size-check branch (ensure the test server set via
setGitHubAPIBase returns a huge payload and asserts the error matches the
size-rejection path in provenance.go), and one mixed-bundle test where the HTTP
JSON contains multiple attestations with the first bundle invalid/corrupt and a
later bundle valid (assert fetchGitHubAttestations succeeds and returns the
valid bundle), referencing fetchGitHubAttestations, setGitHubAPIBase,
githubAPIBase and ErrNoProvenanceAttestations to locate and wire the tests into
the existing suite.
- Add predicate type check after cryptographic verification to prevent SBOM or other attestation types from passing as SLSA provenance (CodeRabbit critical finding) - Use private http.Client instead of http.DefaultClient for isolation - Extract named githubAttestation struct from anonymous inline struct - Fix misleading comment about owner/repo derivation - Update test to use named struct type Addresses CodeRabbit and Gemini review feedback.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/internal/verify/provenance_test.go`:
- Line 146: The test passes nil for the verifier to VerifyProvenance which
relies on loadBundle failing before sev.Verify is invoked, making it fragile;
update the test to construct and pass a minimal valid verifier instance (or a
lightweight mock/stub implementing the verifier interface) to VerifyProvenance
instead of nil so sev.Verify is never nil, or alternatively add a clear comment
next to the VerifyProvenance call documenting the implicit dependence on
loadBundle failing; refer to the VerifyProvenance function and the sev.Verify
call and ensure the test initializes the same verifier type used elsewhere in
tests to avoid nil pointer dereference.
In `@cli/internal/verify/provenance.go`:
- Around line 177-187: The code currently only checks PredicateType when result
!= nil && result.Statement != nil, which lets a successful sev.Verify() that
returns a nil result/statement pass validation; update the verification logic to
treat a nil result or nil result.Statement as an error (return a descriptive
fmt.Errorf) immediately after calling sev.Verify(), then continue to validate
result.Statement.PredicateType (referencing sev.Verify, the result variable, and
result.Statement.PredicateType) so the function never treats a missing statement
as a successful SLSA provenance verification.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9bc5f7fd-58cb-438c-adee-9187de571035
📒 Files selected for processing (2)
cli/internal/verify/provenance.gocli/internal/verify/provenance_test.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (2)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: Go: runcd cli && golangci-lint runbefore committing CLI code
Go: runcd cli && go vet ./...to check for common Go mistakes
Files:
cli/internal/verify/provenance.gocli/internal/verify/provenance_test.go
cli/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Go: use
testing.Ffor fuzz testing; organize fuzz targets in test files withFuzzTargetnaming
Files:
cli/internal/verify/provenance_test.go
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to .github/workflows/docker.yml : CI Docker: build → scan → push to GHCR + cosign sign + SLSA L3 provenance via attest-build-provenance (images only pushed after Trivy/Grype scans pass).
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to .github/workflows/docker.yml : CI Docker: build → scan → push to GHCR + cosign sign + SLSA L3 provenance via attest-build-provenance (images only pushed after Trivy/Grype scans pass).
Applied to files:
cli/internal/verify/provenance.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Fix everything valid — never skip when review agents find valid issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes). No deferring, no 'out of scope' skipping.
Applied to files:
cli/internal/verify/provenance.go
🧬 Code graph analysis (2)
cli/internal/verify/provenance.go (1)
cli/internal/verify/verify.go (1)
ImageRef(27-32)
cli/internal/verify/provenance_test.go (2)
cli/internal/verify/provenance.go (2)
ErrNoProvenanceAttestations(44-44)VerifyProvenance(63-83)cli/internal/verify/verify.go (1)
ImageRef(27-32)
🔇 Additional comments (7)
cli/internal/verify/provenance_test.go (3)
31-123: Add tests for size limit and mixed-bundle scenarios.The test suite doesn't exercise:
- The
>5 MBresponse rejection (lines 127-129 in provenance.go)- The "earlier bundles fail, later bundle succeeds" path (lines 73-81 in provenance.go)
These branches could regress silently without coverage.
16-29: LGTM!Test correctly validates that an empty digest returns an appropriate error.
42-42: No action needed.testDigestis properly defined incli/internal/verify/digest_test.goas a package-level constant and is correctly accessible to other test files in the same package.cli/internal/verify/provenance.go (4)
16-39: LGTM!Constants are well-documented with appropriate values. The 30-second timeout and 5MB response cap provide reasonable DoS protection for the attestation API interactions.
50-56: Good isolation pattern.Using a dedicated
attestationHTTPClientinstead ofhttp.DefaultClientprevents side effects from other packages modifying global HTTP state.
63-83: LGTM!The first-success-wins pattern with error aggregation is appropriate for trying multiple attestation bundles.
96-150: LGTM!The size-capping logic (read
maxAttestationResponseBytes+1, then reject if exceeded) correctly prevents memory exhaustion without requiring a second read. The error handling properly distinguishes 404 (no attestations) from other HTTP errors.
| } | ||
|
|
||
| err = VerifyProvenance(context.Background(), ref, nil, sigverify.CertificateIdentity{}) | ||
| err := VerifyProvenance(context.Background(), ref, nil, sigverify.CertificateIdentity{}) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Nil verifier makes this test fragile.
The test passes nil for the verifier, relying on loadBundle to fail before sev.Verify() is called. If loadBundle becomes more permissive in the future, this test will panic with a nil pointer dereference.
Consider creating a minimal valid verifier or documenting this assumption explicitly in a comment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/internal/verify/provenance_test.go` at line 146, The test passes nil for
the verifier to VerifyProvenance which relies on loadBundle failing before
sev.Verify is invoked, making it fragile; update the test to construct and pass
a minimal valid verifier instance (or a lightweight mock/stub implementing the
verifier interface) to VerifyProvenance instead of nil so sev.Verify is never
nil, or alternatively add a clear comment next to the VerifyProvenance call
documenting the implicit dependence on loadBundle failing; refer to the
VerifyProvenance function and the sev.Verify call and ensure the test
initializes the same verifier type used elsewhere in tests to avoid nil pointer
dereference.
If sev.Verify() succeeds but returns nil result or nil statement, return an explicit error instead of silently skipping the SLSA predicate type validation.
🤖 I have created a release *beep* *boop* --- ## [0.3.5](v0.3.4...v0.3.5) (2026-03-18) ### Features * **api:** auto-wire backend services at startup ([#555](#555)) ([0e52c47](0e52c47)) ### Bug Fixes * **api:** resolve WebSocket 403 rejection ([#549](#549)) ([#556](#556)) ([60453d2](60453d2)) * **cli:** verify SLSA provenance via GitHub attestation API ([#548](#548)) ([91d4f79](91d4f79)), closes [#532](#532) ### Performance * **test:** speed up test suite -- reduce Hypothesis examples and eliminate real sleeps ([#557](#557)) ([d5f3a41](d5f3a41)) ### Refactoring * replace _ErrorResponseSpec NamedTuple with TypedDict ([#554](#554)) ([71cc6e1](71cc6e1)) ### Maintenance * **docker:** suppress pydantic v1 warning on Python 3.14 ([#552](#552)) ([cbe1f05](cbe1f05)), closes [#551](#551) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary
actions/attest-build-provenancestores attestations in GitHub's attestation store, not as OCI referrers in the registryprovenance.goto fetch from the GitHub attestation API (GET /repos/OWNER/REPO/attestations/DIGEST) which returns Sigstore v0.3 bundles--registry-referrers-mode=oci-1-1andCOSIGN_EXPERIMENTALfrom CI (cosign v3 default is correct for cosign signatures; provenance uses GitHub API now)Tested locally
Test plan
go vet ./...passesgo test ./...passes (5 new provenance tests with mock HTTP server)golangci-lint runpassessynthorg updateverifies both cosign and SLSA on fresh installCloses #532