-
Notifications
You must be signed in to change notification settings - Fork 0
fix(security): bind repository scanners to exact PR head #799
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.
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
Draft
seonghobae
wants to merge
26
commits into
main
Choose a base branch
from
fix/security-scan-exact-head
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
1a25b12
test(security): require literal PR-head scanner checkout
seonghobae 7d20fb7
ci(security): execute exact-head scanner contract
seonghobae e75172d
test(security): require literal-head SARIF attribution
seonghobae a7028f2
fix(security): scan and publish literal PR-head evidence
seonghobae 115b2fc
ci(security): verify exact-head checkout and SARIF contracts
seonghobae fc37844
docs(security): record literal-head scanner evidence
seonghobae 323c07b
docs: record exact-head security scanner repair
seonghobae 682088b
test(security): prove unavailable dependency review fails closed
seonghobae 2d1603f
test(security): execute dependency-review fail-closed contract
seonghobae 7c0b6f9
fix(security): fail closed on unavailable dependency review
seonghobae 3c1653a
docs(security): document fail-closed dependency review evidence
seonghobae 5996f89
docs(security): record dependency-review fail-closed gate
seonghobae dfa2878
test(security): accept explicit unavailable-status diagnostic
seonghobae a5f39a4
docs(security): surface exact-head scanner operator contract
seonghobae acc0da8
test: require bounded body-discarding dependency probe
seonghobae c395c0f
chore(security): stage bounded dependency-review probe
seonghobae 3ccb616
fix(security): bound dependency-review support probe
seonghobae e7690e3
chore(security): remove staged timeout patch artifact
seonghobae 4dfde68
chore(ci): stage bounded PR 799 repair
seonghobae 3a382eb
ci: bootstrap verified PR 799 repair
seonghobae 3ff61fd
fix(ci): reap fatal review process groups
github-actions[bot] e4b7f0c
chore(stack): isolate exact-head scanner repair on current main
seonghobae 667e9dd
chore(ci): restage bounded PR 799 repair
seonghobae 6732b24
ci: rerun verified PR 799 repair
seonghobae 9ad6b70
chore(ci): remove temporary PR 799 repair workflow
seonghobae a6cd746
chore(ci): remove encoded PR 799 repair payload
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| name: Security Scan Exact-Head Quality CI | ||
|
|
||
| on: | ||
| pull_request: | ||
| paths: | ||
| - ".github/workflows/security-scan.yml" | ||
| - ".github/workflows/security-scan-exact-head-quality-ci.yml" | ||
| - "tests/test_security_scan_exact_head.py" | ||
| - "tests/test_security_scan_sarif_exact_head.py" | ||
|
|
||
| concurrency: | ||
| group: security-scan-exact-head-${{ github.event.pull_request.number }} | ||
| cancel-in-progress: true | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| exact-head-contract: | ||
| runs-on: ubuntu-24.04 | ||
| steps: | ||
| - name: Checkout literal pull request head | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| repository: ${{ github.event.pull_request.head.repo.full_name }} | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
| persist-credentials: false | ||
| - name: Compile exact-head contracts | ||
| run: >- | ||
| python3 -m py_compile | ||
| tests/test_security_scan_exact_head.py | ||
| tests/test_security_scan_sarif_exact_head.py | ||
| - name: Execute dependency-free exact-head contracts | ||
| run: | | ||
| python3 - <<'PY' | ||
| from tests import test_security_scan_exact_head as checkout_contract | ||
| from tests import test_security_scan_sarif_exact_head as sarif_contract | ||
|
|
||
| checkout_contract.test_repository_scanners_checkout_the_literal_pull_request_head() | ||
| checkout_contract.test_dependency_review_checkout_is_bound_to_the_same_exact_head() | ||
| checkout_contract.test_dependency_review_support_probe_fails_closed_unless_api_returns_200() | ||
| sarif_contract.test_repository_scanner_sarif_is_attributed_to_the_literal_head() | ||
| print("security scan exact-head contracts passed") | ||
| PY |
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| # Security scan exact-head evidence | ||
|
|
||
| ## Decision | ||
|
|
||
| The central `Security Scan` workflow treats the literal pull-request head as the only valid repository-scanner input. GitHub's `pull_request` event normally exposes a generated merge revision through `GITHUB_SHA`; that revision is useful for integration testing but cannot prove that Trivy or Scorecard scanned the exact current contributor head required by CWL authorization policy. | ||
|
|
||
| The dependency-review support checkout, Trivy filesystem scan, and Scorecard posture scan therefore set both: | ||
|
|
||
| ```yaml | ||
| repository: ${{ github.event.pull_request.head.repo.full_name }} | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
| ``` | ||
|
|
||
| Persisted checkout credentials remain disabled. Fork pull requests are read through their explicit head repository and immutable commit SHA; no write credential is added. | ||
|
|
||
| ## Dependency-review availability is evidence, not an optimization | ||
|
|
||
| Dependency review is a hard supply-chain gate. The support probe compares the exact pull-request base SHA with the exact pull-request head SHA through GitHub's dependency-review API. Only HTTP `200` is accepted as evidence that the pinned `actions/dependency-review-action` may execute. HTTP `403`, `404`, `000`, an empty or malformed status, a transport failure, timeout, or any other unexpected probe result is **not** a clean dependency review and fails the job closed. | ||
|
|
||
| The failure diagnostic records only the repository identifier, exact base SHA, exact head SHA, and HTTP status. The API response body is discarded rather than printed because it is unnecessary for the authorization decision and can contain operational details that do not belong in a public workflow log. Authentication material is never included in the diagnostic. | ||
|
|
||
| A dependency-neutral path classifier is not a substitute for dependency-review evidence. In particular, the workflow must not translate an unavailable API into `not-applicable` merely because another mechanism believes the current diff contains no dependency change. OSV, Trivy, CodeQL, Semgrep, Secret Scan, Scorecard, and Dependabot remain independent controls; none semantically replaces the dependency-diff gate. | ||
|
|
||
| ## Operator remediation for an unavailable gate | ||
|
|
||
| For a public GitHub.com repository, a `403` or `404` from the dependency-review comparison endpoint is treated as a repository or organization configuration problem until evidence proves otherwise. An operator should verify that the dependency graph and the GitHub security features required for dependency review are enabled for the repository and organization, that organization policy permits the endpoint, and that the workflow's read-only token receives the documented access needed by the dependency-review API and action. Rerun only after the capability or policy path is corrected; do not weaken the workflow to manufacture a green check. | ||
|
|
||
| Private or internal repositories can have different product-entitlement and policy requirements. Any exception for those repository classes must be designed as an explicit organization policy with independently reviewable entitlement evidence. It must not be inferred from a failed probe and must not weaken the public-repository canary semantics. | ||
|
|
||
| ## Durable SARIF identity | ||
|
|
||
| Scanning the head is insufficient when durable code-scanning evidence is attributed to a different revision. Trivy and Scorecard uploads explicitly bind: | ||
|
|
||
| ```yaml | ||
| ref: refs/pull/${{ github.event.pull_request.number }}/head | ||
| sha: ${{ github.event.pull_request.head.sha }} | ||
| ``` | ||
|
|
||
| GitHub's code-scanning API requires both a full Git reference and the commit SHA to which an uploaded analysis relates. The pair above states that the SARIF describes the pull-request head, not the generated merge commit. | ||
|
|
||
| ## Preserved security behavior | ||
|
|
||
| This change does not alter scanner versions, vulnerability severities, Trivy's fixable Medium-or-higher hard gate, dependency-review thresholds, Scorecard's soft posture role, SARIF sanitation, permissions, or the existing OSV base-versus-head comparison. It makes scanner input and result identity consistent and makes unavailable dependency-review evidence an explicit hard failure instead of a green skip. | ||
|
|
||
| The workflow remains fail closed for absent scanner output and actionable findings. SARIF upload failures remain separately visible without suppressing the repository-local Trivy finding gate. A queued, cancelled, skipped, failed, missing, or predecessor-head run is not current-head evidence. | ||
|
|
||
| ## Verification | ||
|
|
||
| `tests/test_security_scan_exact_head.py` verifies literal-head checkout and the rule that only an HTTP `200` support probe may reach dependency review. It also rejects the former `supported=false` / skip path and response-body logging. `tests/test_security_scan_sarif_exact_head.py` verifies durable Trivy and Scorecard SARIF attribution. The dedicated read-only quality workflow checks out the literal PR head, compiles both contracts, and executes them without package installation. | ||
|
|
||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| The initiating DiskSage evidence was Security Scan run `31070907732`, whose Trivy job log checked out `refs/remotes/pull/137/merge` rather than DiskSage PR #137 head `87ac0e08cceed3d1a766da13a8f8123912178192`. That result remains historical merge-tree evidence and is not reclassified as exact-head proof. | ||
|
|
||
| The dependency-review availability regression was reproduced on the public EgressWeave canary: a support probe returned HTTP `403`, the former workflow marked the hard action skipped, and the aggregate Security Scan still concluded success. That historical result is unavailable dependency-review evidence, not proof of a clean dependency diff. | ||
|
|
||
| ## Rollback | ||
|
|
||
| Rollback requires an independently reviewed revert and fresh exact-head security evidence. Do not restore implicit checkout, automatic SARIF revision detection, or a fail-open dependency-review support path unless an equally strict mechanism proves the same authorization properties. In particular, never convert `403`, `404`, transport failure, or another unavailable probe outcome into a successful hard gate. | ||
|
|
||
| ## APA 7th references | ||
|
|
||
| GitHub. (n.d.). *Dependency review*. GitHub Docs. Retrieved August 7, 2026, from https://docs.github.com/en/code-security/concepts/supply-chain-security/dependency-review | ||
|
|
||
| GitHub. (n.d.). *REST API endpoints for dependency review*. GitHub Docs. Retrieved August 7, 2026, from https://docs.github.com/en/enterprise-cloud@latest/rest/dependency-graph/dependency-review | ||
|
|
||
| GitHub. (n.d.). *Customizing your dependency review action configuration*. GitHub Docs. Retrieved August 7, 2026, from https://docs.github.com/en/code-security/tutorials/secure-your-dependencies/customize-dependency-review-action | ||
|
|
||
| GitHub. (n.d.). *Events that trigger workflows*. GitHub Docs. Retrieved August 6, 2026, from https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows | ||
|
|
||
| GitHub. (n.d.). *REST API endpoints for code scanning*. GitHub Docs. Retrieved August 6, 2026, from https://docs.github.com/en/rest/code-scanning/code-scanning | ||
|
|
||
| GitHub. (n.d.). *Uploading CodeQL analysis results to GitHub*. GitHub Docs. Retrieved August 6, 2026, from https://docs.github.com/en/enterprise-cloud@latest/code-security/tutorials/customize-code-scanning/upload-results | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.