scorecard: add SARIF evidence upload to Code Scanning (forked version)#796
Closed
justaugustus wants to merge 12 commits intoossf:mainfrom
Closed
scorecard: add SARIF evidence upload to Code Scanning (forked version)#796justaugustus wants to merge 12 commits intoossf:mainfrom
justaugustus wants to merge 12 commits intoossf:mainfrom
Conversation
Bootstrap the openspec/ directory in Allstar and add a proposal for evidence upload capability in the Scorecard policy. This is the first use of OpenSpec in the Allstar repo, establishing the spec-driven development workflow for future changes. The proposal covers: - SARIF upload to GitHub Code Scanning API (Phase 1) - Alignment with Scorecard v6 evidence engine direction (ossf/scorecard#4952) - Config model: upload: {sarif: true} on scorecard.yaml - Dual execution model (per-check loop + full Run() for SARIF) - Change detection to avoid redundant uploads - Non-blocking error handling Design decisions were made through structured interview covering scope, execution model, config, governance, rate limiting, testing, and v6 alignment. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Stephen Augustus <foo@auggie.dev>
Add UploadConfig struct with a SARIF boolean field to the Scorecard
policy configuration. This enables org admins to opt in to SARIF
upload via:
upload:
sarif: true
The config follows Allstar's existing merge hierarchy:
- OrgConfig sets the default (concrete value)
- RepoConfig overrides via pointer (nil = not set)
- DisableRepoOverride prevents repo-level overrides
Upload is disabled by default (zero value = false).
Includes 5 new table-driven tests covering:
- Default disabled behavior
- Org-level enable
- OrgRepo override of org setting
- Repo override of all settings
- Repo override blocked by DisableRepoOverride
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Stephen Augustus <foo@auggie.dev>
Add generateSARIF() which runs all configured Scorecard checks at once via sc.Run() and converts the result to SARIF 2.1.0 using the existing Result.AsSARIF() method from scorecard v5.4.0. This is the "dual execution" path described in the evidence-upload proposal: the main Check() method continues to run checks individually for issue text generation, while generateSARIF() runs them together to produce a complete SARIF document. Key implementation details: - Constructs ScorecardPolicy from Allstar's checks + threshold config (each check marked as ENFORCED with the threshold score) - Uses scorecard's docs.Read() for remediation guidance in SARIF output - Follows scorecard-action's format.go pattern for AsSARIF() parameters Includes tests for: - Basic single-check SARIF generation - Multiple check SARIF generation - Error propagation from failed scorecard runs Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Stephen Augustus <foo@auggie.dev>
Add uploadToCodeScanning() which uploads SARIF content to GitHub's Code Scanning API using go-github/v74's CodeScanning.UploadSarif(). Implementation: - Gets default branch and HEAD commit SHA from the GitHub API - Compresses SARIF with gzip and encodes as base64 (required by API) - Constructs github.SarifAnalysis with commit SHA, ref, and tool name - Calls CodeScanning.UploadSarif() for the actual upload Uses mockable function variables (codeScanningUploadFunc, getDefaultBranchRefFunc) following the existing pattern in scorecard.go for testability. Includes tests for: - gzip + base64 compression round-trip - Upload with correct SarifAnalysis fields (SHA, ref, tool name) - Error propagation from failed API calls Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Stephen Augustus <foo@auggie.dev>
Add uploadSARIF() which orchestrates SARIF generation, change detection, and upload. It skips the upload when the SARIF content has not changed since the last upload for the same repository. Change detection uses SHA-256 hashing of SARIF content stored in an in-memory map, following the same caching pattern used by pkg/scorecard/scorecard.go (scClientsLocal/scClientsRemote with mutex). This mirrors how pkg/issue/issue.go uses SHA-256 to detect issue text changes. Includes tests for: - First call uploads, second call with same result skips - Changed results trigger a new upload Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Stephen Augustus <foo@auggie.dev>
Wire uploadSARIF() into the Scorecard policy's Check() method. When upload.sarif is true in the merged config, Check() calls uploadSARIF() after the per-check loop completes. Upload errors are non-blocking: logged as warnings but do not affect the policy check result (Result.Pass). This ensures transient GitHub API failures don't disrupt the enforcement loop. Includes integration tests for: - Check() with upload enabled triggers SARIF upload - Check() with upload disabled does not trigger upload - SARIF upload failure does not affect Result.Pass Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Stephen Augustus <foo@auggie.dev>
Add documentation for the new SARIF upload feature: - README.md: Add "SARIF Upload" section under "Generic Scorecard Check" with config example, requirements, and behavior notes - operator.md: Add note about security_events:write permission needed for GitHub App when using SARIF upload Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Stephen Augustus <foo@auggie.dev>
dosubot
bot
added
the
size:XXL
justaugustus
added a commit
to uwu-tools/.github
that referenced
this pull request
Mar 22, 2026
Add upload.sarif: true to the Scorecard policy config. This enables uploading Scorecard SARIF results to each repo's Security > Code Scanning tab. Requires ossf/allstar#796 (evidence-upload branch). Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Stephen Augustus <foo@auggie.dev>
3 tasks
justaugustus
added a commit
to uwu-tools/.github
that referenced
this pull request
Mar 22, 2026
Add upload.sarif: true to the Scorecard policy config. This enables uploading Scorecard SARIF results to each repo's Security > Code Scanning tab. Requires ossf/allstar#796 (evidence-upload branch). Signed-off-by: Stephen Augustus <foo@auggie.dev> Co-authored-by: Claude <noreply@anthropic.com>
The GitHub App UI labels the permission as "Code scanning alerts" (under Repository permissions), not "Security events". The API scope remains `security_events`. Updated all documentation to reference both the UI label and API scope, discovered during manual testing of the self-hosted operator path. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Stephen Augustus <foo@auggie.dev>
Change detection was comparing hashes of SARIF content, which includes timestamps and other run-specific metadata that differ between runs even when findings are identical. This caused every cycle to upload, defeating the purpose of change detection. Fix: compare the repository HEAD commit SHA instead. If the repo hasn't been pushed to since the last upload, skip the scan and upload entirely. This is simpler, more reliable, and also avoids the cost of a redundant scorecard run. Verified with a continuous mode test: - First cycle: "SARIF uploaded to Code Scanning." - Second cycle: "SARIF unchanged, skipping upload." Also refactors uploadToCodeScanning into uploadToCodeScanningWithRef to avoid fetching branch info twice (once for change detection, once for upload). Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Stephen Augustus <foo@auggie.dev>
Update all documentation to describe the commit SHA comparison approach for change detection, replacing references to SHA-256 hash of SARIF content. The SARIF content hash approach was found to be unreliable during testing because SARIF includes timestamps that differ between runs. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Stephen Augustus <foo@auggie.dev>
All implementation phases, documentation, and verification steps have been completed and tested, including: - Manual self-hosted operator SARIF upload to Code Scanning - Change detection verified in continuous mode (skip on second cycle) - Updated verification to reflect go vet (golangci-lint has a pre-existing version mismatch in this environment) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Stephen Augustus <foo@auggie.dev>
Remove extra blank line between functions caught by the gofumpt formatter in CI. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Stephen Augustus <foo@auggie.dev>
justaugustus
changed the title
Member
Author
|
Closing this in favor of a feature branch PR —> #797 |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closing this in favor of a feature branch PR —> #797