Skip to content

feat: adds custom scorecard certifier#2815

Open
shreyasHpandya wants to merge 1 commit intoguacsec:mainfrom
guidewire-oss:feat/add-custom-scorecard-certifer-new
Open

feat: adds custom scorecard certifier#2815
shreyasHpandya wants to merge 1 commit intoguacsec:mainfrom
guidewire-oss:feat/add-custom-scorecard-certifer-new

Conversation

@shreyasHpandya
Copy link
Contributor

@shreyasHpandya shreyasHpandya commented Nov 7, 2025

Fixes #2783

Description of the PR

See comment #2815 (comment)

PR Checklist

  • All commits have a Developer Certificate of Origin (DCO) -- they are generated using -s flag to git commit.
  • All new changes are covered by tests
  • If GraphQL schema is changed, make generate has been run
  • If GraphQL schema is changed, GraphQL client updates/additions have been made
  • If OpenAPI spec is changed, make generate has been run
  • If ent schema is changed, make generate has been run
  • If collectsub protobuf has been changed, make proto has been run
  • All CI checks are passing (tests and formatting)
  • All dependent PRs have already been merged

@kusari-inspector
Copy link

kusari-inspector bot commented Nov 7, 2025

Kusari Inspector

Kusari Analysis Results:

Proceed with these changes

✅ No Flagged Issues Detected
All values appear to be within acceptable risk parameters.

No pinned version dependency changes, code issues or exposed secrets detected!

Note

View full detailed analysis result for more information on the output and the checks that were run.


@kusari-inspector rerun - Trigger a re-analysis of this PR
@kusari-inspector feedback [your message] - Send feedback to our AI and team
See Kusari's documentation for setup and configuration.
Commit: 81bb024, performed at: 2025-11-24T03:05:07Z

Found this helpful? Give it a 👍 or 👎 reaction!

req.Header.Set("Accept", "application/json")

resp, err := httpClient.Do(req)
defer func() {

Choose a reason for hiding this comment

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

Issue: Fix nil pointer dereference in defer statement

Recommended Code Changes:

resp, err := httpClient.Do(req)
if err != nil {
    return nil, fmt.Errorf("scorecard request failed: %w", err)
}
defer func() {
    _ = resp.Body.Close()
}()

@pull-request-size pull-request-size bot added size/L and removed size/M labels Nov 24, 2025
@kusari-inspector
Copy link

Kusari PR Analysis rerun based on - 81bb024 performed at: 2025-11-24T03:05:26Z - link to updated analysis

@ANIRUDH-333 ANIRUDH-333 force-pushed the feat/add-custom-scorecard-certifer-new branch from 0b3c224 to e0151f0 Compare November 26, 2025 09:09
@gaganhr94 gaganhr94 force-pushed the feat/add-custom-scorecard-certifer-new branch 3 times, most recently from 845216e to 3f18d64 Compare December 2, 2025 05:36
@gaganhr94 gaganhr94 force-pushed the feat/add-custom-scorecard-certifer-new branch 2 times, most recently from 7becd0d to 15c5e84 Compare January 15, 2026 07:55
@gaganhr94
Copy link
Contributor

Scorecard improvements description for better understanding

When querying the Scorecard API with a specific commitSHA that returns 404,
retry the request without the commit parameter to get the latest scorecard
for the default branch.

Design Decisions:

  • API-First with Retry: If a commit-specific query fails, retry without
    commitSHA rather than immediately falling back to local computation.
    This maximizes API usage (faster, no auth required).

  • Silent Fallback for Missing Commits: When the requested commit isn't in
    the API, we return the latest scorecard instead. This is a conscious
    decision because:

    • Scorecard's purpose is to assess current repository health and
      maintenance practices, not point-in-time snapshots
    • A recent scorecard provides more value than no scorecard for trust
      decisions
    • Many checks (branch protection, security policy) are repository-wide
      and don't vary between commits
  • Tag Without CommitSHA Skips API: When a tag is provided without a
    commitSHA, skip the API entirely and use local computation, which can
    resolve tags to commits via ListReleases().

  • HEAD Treated as Empty: Upstream GUAC code sets commitSHA to "HEAD" when
    no specific commit is known, so we treat "HEAD" the same as empty.

@gaganhr94
Copy link
Contributor

Hi @mihaimaruseac @pxp928, can you please review this PR when you get a chance. Thanks !

ghToken: s,
}, nil
}
} No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

The file should end with a newline

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

Comment on lines +114 to +115
// Log warning but allow initialization without token
// The API path will still work, only local computation will fail
Copy link
Member

Choose a reason for hiding this comment

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

This comment doesn't seem useful.

It looks like you used AI to generate the PR, but did not clean the output.

Copy link
Contributor

Choose a reason for hiding this comment

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

Previously, a GitHub token was required for Scorecard to start. After this change, startup no longer depends on the token, so just logs a warning. Added these comments intentionally to make that behaviour change explicit and improve transparency for future readers. If there’s a clearer way to convey this, I’m happy to adjust the wording.


// CertifyComponent is a certifier that generates scorecard attestations
func (s scorecard) CertifyComponent(_ context.Context, rootComponent interface{}, docChannel chan<- *processor.Document) error {
func (s scorecard) CertifyComponent(ctx context.Context, rootComponent interface{}, docChannel chan<- *processor.Document) error {
Copy link
Member

Choose a reason for hiding this comment

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

ctx is still unused... Did you try to compile the code? The compiler would have given you an error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Did compile the code and test out the scorecard certifier. Since ctx was a function parameter here, it did not give any issues during compile-time.

Will revert back to _ since we are not using ctx in the function

@gaganhr94 gaganhr94 force-pushed the feat/add-custom-scorecard-certifer-new branch 2 times, most recently from 9fc1125 to 188409c Compare February 6, 2026 15:43
Co-authored-by: Shreyas Pandya <pandyashreyas1@gmail.com>
Co-authored-by: Abhisek Agrawal <abhishek.yours4@gmail.com>
Co-authored-by: Anirudh Edpuganti <aniedpuganti@gmail.com>
Co-authored-by: Jayashree O <jaishu138@gmail.com>
Co-authored-by: Paul Joseph <k.paul.joseph@gmail.com>
Co-authored-by: Gagan H R <hrgagan4@gmail.com>
Signed-off-by: Gagan H R <hrgagan4@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feature] Add API-based scorecard fetcher as an alternative to existing scorecard certifier

3 participants