Skip to content

✨ Add GitHub artifact attestation for Signed-Releases #5001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
martincostello wants to merge 1 commit into
base: main
Choose a base branch
from
Open

✨ Add GitHub artifact attestation for Signed-Releases #5001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions checks/evaluation/signed_releases.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
sce "github.com/ossf/scorecard/v5/errors"
"github.com/ossf/scorecard/v5/finding"
"github.com/ossf/scorecard/v5/probes/releasesAreSigned"
"github.com/ossf/scorecard/v5/probes/releasesHaveAttestation"
"github.com/ossf/scorecard/v5/probes/releasesHaveProvenance"
"github.com/ossf/scorecard/v5/probes/releasesHaveVerifiedProvenance"
)
Expand All @@ -37,6 +38,7 @@ func SignedReleases(name string,
) checker.CheckResult {
expectedProbes := []string{
releasesAreSigned.Probe,
releasesHaveAttestation.Probe,
releasesHaveProvenance.Probe,
}

Expand All @@ -45,9 +47,9 @@ func SignedReleases(name string,
return checker.CreateRuntimeErrorResult(name, e)
}

// keep track of releases which have provenance so we don't log about signatures
// keep track of releases which have provenance or attestation so we don't log about signatures
// on our second pass through below
hasProvenance := make(map[string]bool)
hasProvenanceOrAttestation := make(map[string]bool)

// Debug all releases and check for OutcomeNotApplicable
// All probes have OutcomeNotApplicable in case the project has no
Expand Down Expand Up @@ -78,8 +80,11 @@ func SignedReleases(name string,
loggedReleases = append(loggedReleases, releaseName)
}

if f.Probe == releasesHaveProvenance.Probe && f.Outcome == finding.OutcomeTrue {
hasProvenance[releaseName] = true
if f.Outcome == finding.OutcomeTrue {
switch f.Probe {
case releasesHaveProvenance.Probe, releasesHaveAttestation.Probe:
hasProvenanceOrAttestation[releaseName] = true
}
}
}

Expand Down Expand Up @@ -112,12 +117,18 @@ func SignedReleases(name string,
if _, ok := releaseMap[releaseName]; !ok {
releaseMap[releaseName] = 8
}
case releasesHaveProvenance.Probe:
case releasesHaveProvenance.Probe, releasesHaveAttestation.Probe:
releaseMap[releaseName] = 10
}
case finding.OutcomeFalse:
logLevel = checker.DetailWarn
if f.Probe == releasesAreSigned.Probe && hasProvenance[releaseName] {
if f.Probe == releasesAreSigned.Probe && hasProvenanceOrAttestation[releaseName] {
continue
}
// Attestation is an optional alternative to signing or provenance.
// Suppress warnings when attestation is absent to avoid noise for projects
// that use other verification methods.
if f.Probe == releasesHaveAttestation.Probe {
continue
}
Comment on lines 124 to 133
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code treats attestation as an alternative for scoring (sets 10) and suppresses warnings when the attestation probe is false, but it still logs a warning when provenance is false even if attestation is true for that release. If attestation is intended as an alternative to provenance, consider also suppressing the provenance warning for releases that already have an attestation OutcomeTrue to avoid confusing/noisy output when the check is already at max score.

Copilot uses AI. Check for mistakes.
default:
Expand Down Expand Up @@ -163,6 +174,8 @@ func getReleaseName(f *finding.Finding) string {
key = releasesAreSigned.ReleaseNameKey
case releasesHaveProvenance.Probe:
key = releasesHaveProvenance.ReleaseNameKey
case releasesHaveAttestation.Probe:
key = releasesHaveAttestation.ReleaseNameKey
}
return f.Values[key]
}
Expand Down
93 changes: 93 additions & 0 deletions checks/evaluation/signed_releases_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
sce "github.com/ossf/scorecard/v5/errors"
"github.com/ossf/scorecard/v5/finding"
"github.com/ossf/scorecard/v5/probes/releasesAreSigned"
"github.com/ossf/scorecard/v5/probes/releasesHaveAttestation"
"github.com/ossf/scorecard/v5/probes/releasesHaveProvenance"
scut "github.com/ossf/scorecard/v5/utests"
)
Expand Down Expand Up @@ -64,6 +65,17 @@ func provenanceProbe(release, asset int, outcome finding.Outcome) finding.Findin
}
}

func attestationProbe(release, asset int, outcome finding.Outcome) finding.Finding {
return finding.Finding{
Probe: releasesHaveAttestation.Probe,
Outcome: outcome,
Values: map[string]string{
releasesHaveAttestation.ReleaseNameKey: fmt.Sprintf("v%d", release),
releasesHaveAttestation.AssetNameKey: fmt.Sprintf("artifact-%d", asset),
},
}
}

func TestSignedReleases(t *testing.T) {
t.Parallel()
tests := []struct {
Expand All @@ -76,6 +88,7 @@ func TestSignedReleases(t *testing.T) {
findings: []finding.Finding{
signedProbe(0, 0, finding.OutcomeTrue),
provenanceProbe(0, 0, finding.OutcomeFalse),
attestationProbe(0, 0, finding.OutcomeFalse),
},
result: scut.TestReturn{
Score: 8,
Expand All @@ -89,6 +102,7 @@ func TestSignedReleases(t *testing.T) {
findings: []finding.Finding{
signedProbe(0, 0, finding.OutcomeTrue),
provenanceProbe(0, 0, finding.OutcomeTrue),
attestationProbe(0, 0, finding.OutcomeFalse),
},
result: scut.TestReturn{
Score: 10,
Expand All @@ -101,6 +115,7 @@ func TestSignedReleases(t *testing.T) {
findings: []finding.Finding{
signedProbe(0, 0, finding.OutcomeFalse),
provenanceProbe(0, 0, finding.OutcomeTrue),
attestationProbe(0, 0, finding.OutcomeFalse),
},
result: scut.TestReturn{
Score: checker.MaxResultScore,
Expand All @@ -109,19 +124,50 @@ func TestSignedReleases(t *testing.T) {
NumberOfDebug: 1,
},
},
{
name: "Has one release with attestation but no signature or provenance",
findings: []finding.Finding{
signedProbe(0, 0, finding.OutcomeFalse),
provenanceProbe(0, 0, finding.OutcomeFalse),
attestationProbe(0, 0, finding.OutcomeTrue),
},
result: scut.TestReturn{
Score: checker.MaxResultScore,
NumberOfInfo: 1,
NumberOfWarn: 1, // provenance warning still fires
NumberOfDebug: 1,
},
},
{
name: "Has one release with attestation and signature",
findings: []finding.Finding{
signedProbe(0, 0, finding.OutcomeTrue),
provenanceProbe(0, 0, finding.OutcomeFalse),
attestationProbe(0, 0, finding.OutcomeTrue),
},
result: scut.TestReturn{
Score: checker.MaxResultScore,
NumberOfInfo: 2, // signed + attestation
NumberOfWarn: 1, // provenance warning still fires
NumberOfDebug: 1,
},
},

{
name: "3 releases. One release has one signed, and one release has provenance.",
findings: []finding.Finding{
// Release 1:
signedProbe(release0, asset1, finding.OutcomeTrue),
provenanceProbe(release0, asset0, finding.OutcomeFalse),
attestationProbe(release0, asset0, finding.OutcomeFalse),
// Release 2
signedProbe(release1, asset0, finding.OutcomeFalse),
provenanceProbe(release1, asset0, finding.OutcomeFalse),
attestationProbe(release1, asset0, finding.OutcomeFalse),
// Release 3
signedProbe(release2, asset0, finding.OutcomeFalse),
provenanceProbe(release2, asset1, finding.OutcomeTrue),
attestationProbe(release2, asset0, finding.OutcomeFalse),
},
result: scut.TestReturn{
Score: 6,
Expand All @@ -136,18 +182,23 @@ func TestSignedReleases(t *testing.T) {
// Release 1:
signedProbe(release0, asset1, finding.OutcomeTrue),
provenanceProbe(release0, asset1, finding.OutcomeFalse),
attestationProbe(release0, asset1, finding.OutcomeFalse),
// Release 2:
signedProbe(release1, asset0, finding.OutcomeTrue),
provenanceProbe(release1, asset0, finding.OutcomeFalse),
attestationProbe(release1, asset0, finding.OutcomeFalse),
// Release 3:
signedProbe(release2, asset0, finding.OutcomeFalse),
provenanceProbe(release2, asset0, finding.OutcomeTrue),
attestationProbe(release2, asset0, finding.OutcomeFalse),
// Release 4, Asset 1:
signedProbe(release3, asset0, finding.OutcomeFalse),
provenanceProbe(release3, asset0, finding.OutcomeTrue),
attestationProbe(release3, asset0, finding.OutcomeFalse),
// Release 5, Asset 1:
signedProbe(release4, asset0, finding.OutcomeFalse),
provenanceProbe(release4, asset0, finding.OutcomeFalse),
attestationProbe(release4, asset0, finding.OutcomeFalse),
},
result: scut.TestReturn{
Score: 7,
Expand All @@ -162,18 +213,23 @@ func TestSignedReleases(t *testing.T) {
// Release 1:
signedProbe(release0, asset1, finding.OutcomeTrue),
provenanceProbe(release0, asset1, finding.OutcomeFalse),
attestationProbe(release0, asset1, finding.OutcomeFalse),
// Release 2:
signedProbe(release1, asset0, finding.OutcomeTrue),
provenanceProbe(release1, asset0, finding.OutcomeFalse),
attestationProbe(release1, asset0, finding.OutcomeFalse),
// Release 3:
signedProbe(release2, asset0, finding.OutcomeTrue),
provenanceProbe(release2, asset0, finding.OutcomeFalse),
attestationProbe(release2, asset0, finding.OutcomeFalse),
// Release 4:
signedProbe(release3, asset0, finding.OutcomeTrue),
provenanceProbe(release3, asset0, finding.OutcomeFalse),
attestationProbe(release3, asset0, finding.OutcomeFalse),
// Release 5:
signedProbe(release4, asset0, finding.OutcomeTrue),
provenanceProbe(release4, asset0, finding.OutcomeFalse),
attestationProbe(release4, asset0, finding.OutcomeFalse),
},
result: scut.TestReturn{
Score: 8,
Expand All @@ -189,22 +245,28 @@ func TestSignedReleases(t *testing.T) {
// Release 1, Asset 1:
signedProbe(release0, asset0, finding.OutcomeTrue),
provenanceProbe(release0, asset0, finding.OutcomeTrue),
attestationProbe(release0, asset0, finding.OutcomeFalse),
// Release 2:
// Release 2, Asset 1:
signedProbe(release1, asset0, finding.OutcomeTrue),
provenanceProbe(release1, asset0, finding.OutcomeTrue),
attestationProbe(release1, asset0, finding.OutcomeFalse),
// Release 3, Asset 1:
signedProbe(release2, asset0, finding.OutcomeTrue),
provenanceProbe(release2, asset0, finding.OutcomeTrue),
attestationProbe(release2, asset0, finding.OutcomeFalse),
// Release 4, Asset 1:
signedProbe(release3, asset0, finding.OutcomeTrue),
provenanceProbe(release3, asset0, finding.OutcomeTrue),
attestationProbe(release3, asset0, finding.OutcomeFalse),
// Release 5, Asset 1:
signedProbe(release4, asset0, finding.OutcomeTrue),
provenanceProbe(release4, asset0, finding.OutcomeTrue),
attestationProbe(release4, asset0, finding.OutcomeFalse),
// Release 6, Asset 1:
signedProbe(release5, asset0, finding.OutcomeTrue),
provenanceProbe(release5, asset0, finding.OutcomeTrue),
attestationProbe(release5, asset0, finding.OutcomeFalse),
},
result: scut.TestReturn{
Score: checker.InconclusiveResultScore,
Expand All @@ -213,6 +275,37 @@ func TestSignedReleases(t *testing.T) {
NumberOfDebug: 6, // 1 for each release
},
},
{
name: "5 releases. All have attestation.",
findings: []finding.Finding{
// Release 1:
signedProbe(release0, asset0, finding.OutcomeFalse),
provenanceProbe(release0, asset0, finding.OutcomeFalse),
attestationProbe(release0, asset0, finding.OutcomeTrue),
// Release 2:
signedProbe(release1, asset0, finding.OutcomeFalse),
provenanceProbe(release1, asset0, finding.OutcomeFalse),
attestationProbe(release1, asset0, finding.OutcomeTrue),
// Release 3:
signedProbe(release2, asset0, finding.OutcomeFalse),
provenanceProbe(release2, asset0, finding.OutcomeFalse),
attestationProbe(release2, asset0, finding.OutcomeTrue),
// Release 4:
signedProbe(release3, asset0, finding.OutcomeFalse),
provenanceProbe(release3, asset0, finding.OutcomeFalse),
attestationProbe(release3, asset0, finding.OutcomeTrue),
// Release 5:
signedProbe(release4, asset0, finding.OutcomeFalse),
provenanceProbe(release4, asset0, finding.OutcomeFalse),
attestationProbe(release4, asset0, finding.OutcomeTrue),
},
result: scut.TestReturn{
Score: 10,
NumberOfInfo: 5, // 1 attestation per release
NumberOfWarn: 5, // provenance warnings still fire
NumberOfDebug: 5,
},
},
}

for _, tt := range tests {
Expand Down
71 changes: 63 additions & 8 deletions clients/githubrepo/releases.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,19 @@
sce "github.com/ossf/scorecard/v5/errors"
)

const (
ownerEndpointUser = "users"
ownerEndpointOrg = "orgs"
)

type releasesHandler struct {
client *github.Client
once *sync.Once
ctx context.Context
errSetup error
repourl *Repo
releases []clients.Release
client *github.Client
once *sync.Once
ctx context.Context
errSetup error
repourl *Repo
releases []clients.Release
ownerEndpointPrefix string
}

func (handler *releasesHandler) init(ctx context.Context, repourl *Repo) {
Expand All @@ -55,10 +61,58 @@
handler.errSetup = sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("githubv4.Query: %v", err))
}
handler.releases = releasesFrom(releases)
handler.ownerEndpointPrefix = handler.resolveOwnerEndpointPrefix()
handler.checkAttestations()
})
return handler.errSetup
}

Comment on lines +64 to 69
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkAttestations() is invoked unconditionally during ListReleases() setup and iterates over every release/asset returned by Repositories.ListReleases (default 30 releases). This adds a potentially large number of extra REST calls (and likely 401/403s without an auth token) even for checks that don't need attestations, which can noticeably increase runtime and risk API rate limiting. Consider deferring attestation lookups to the attestation probe (or gating/limiting them to the same lookback window the probe evaluates).

Suggested change
handler.ownerEndpointPrefix = handler.resolveOwnerEndpointPrefix()
handler.checkAttestations()
})
return handler.errSetup
}
})
return handler.errSetup
}
// setupAttestations lazily enriches the already-loaded releases with
// attestation metadata. This is intentionally separate from setup so that
// generic release listing does not incur the additional per-asset API calls.
func (handler *releasesHandler) setupAttestations() error {
if err := handler.setup(); err != nil {
return err
}
handler.ownerEndpointPrefix = handler.resolveOwnerEndpointPrefix()
handler.checkAttestations()
return nil
}

Copilot uses AI. Check for mistakes.
// resolveOwnerEndpointPrefix determines whether the repo owner is a GitHub
// user or organization and returns the corresponding API path prefix.
// It calls the GitHub Users API once so that hasAttestation can use a single
// endpoint per asset instead of trying both.
func (handler *releasesHandler) resolveOwnerEndpointPrefix() string {
user, _, err := handler.client.Users.Get(handler.ctx, handler.repourl.owner)
if err != nil {
// Fall back to users; hasAttestation will skip on 404.
return ownerEndpointUser
}
if strings.EqualFold(user.GetType(), "Organization") {
return ownerEndpointOrg
}
return ownerEndpointUser
Comment on lines +74 to +83
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says "hasAttestation will skip on 404", but hasAttestation currently returns false for all errors and does not special-case 404s. This also means if Users.Get fails (rate limit/network) for an org-owned repo, the fallback to users/... can cause all attestation checks to incorrectly return false due to hitting the wrong endpoint. Consider either (a) updating the comment, or (b) handling 404 by retrying the alternate users/orgs prefix once before returning false.

Copilot uses AI. Check for mistakes.
}

func (handler *releasesHandler) checkAttestations() {
for i := range handler.releases {
for j := range handler.releases[i].Assets {
asset := &handler.releases[i].Assets[j]
if asset.Digest == "" {
continue
}
asset.HasAttestation = handler.hasAttestation(asset.Digest)
}
}
}

type attestationResponse struct {
Attestations []interface{} `json:"attestations"`
}

func (handler *releasesHandler) hasAttestation(digest string) bool {
endpoint := fmt.Sprintf("%s/%s/attestations/%s", handler.ownerEndpointPrefix, handler.repourl.owner, digest)
req, err := handler.client.NewRequest("GET", endpoint, nil)
if err != nil {
return false

Check warning on line 106 in clients/githubrepo/releases.go

View check run for this annotation

Codecov / codecov/patch

clients/githubrepo/releases.go#L106 

Added line #L106 was not covered by tests
}
var body attestationResponse
_, err = handler.client.Do(handler.ctx, req, &body)
if err != nil {
return false
}
return len(body.Attestations) > 0
}

func (handler *releasesHandler) getReleases() ([]clients.Release, error) {
if err := handler.setup(); err != nil {
return nil, fmt.Errorf("error during graphqlHandler.setup: %w", err)
Expand All @@ -76,8 +130,9 @@
}
for _, a := range r.Assets {
release.Assets = append(release.Assets, clients.ReleaseAsset{
Name: a.GetName(),
URL: r.GetHTMLURL(),
Name: a.GetName(),
URL: r.GetHTMLURL(),
Digest: a.GetDigest(),
})
}
releases = append(releases, release)
Expand Down
Loading
Loading