-
Notifications
You must be signed in to change notification settings - Fork 0
fix(security): fail closed on unavailable dependency review #897
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
Open
seonghobae
wants to merge
1
commit into
main
Choose a base branch
from
fix/dependency-review-fail-closed-current-main-20260809
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.
+136
−26
Open
Changes from all commits
Commits
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 |
|---|---|---|
|
|
@@ -16,10 +16,10 @@ | |
| # pull_request workflows upload to refs/pull/N/merge, so no single ref ever holds | ||
| # all tools. Bundling at the workflow/check level is ref-independent. | ||
| # | ||
| # NOTE on dependency-review: dependency graph can be unavailable on some repos. | ||
| # Treat that as "not enforceable here" instead of making the required workflow | ||
| # unsatisfiable; keep medium-or-higher dependency findings hard-failing where the | ||
| # API is supported. | ||
| # NOTE on dependency-review: unavailable evidence is not a clean result. Only | ||
| # an exact base/head comparison returning HTTP 200 may reach the pinned hard | ||
| # gate. Every other probe outcome fails closed without printing the response | ||
| # body. See docs/doctoring/dependency-review-fail-closed.md. | ||
| # | ||
| # NOTE on trivy-fs: it scans the whole repo, so a pre-existing FIXABLE | ||
| # MEDIUM/HIGH/CRITICAL finding blocks every PR in that repo until it is fixed. | ||
|
|
@@ -257,9 +257,11 @@ jobs: | |
| contents: read | ||
| pull-requests: read | ||
| steps: | ||
| - name: Checkout | ||
| - name: Checkout exact head | ||
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| with: | ||
| repository: ${{ github.event.pull_request.head.repo.full_name }} | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
| persist-credentials: false | ||
| - name: Check dependency review support | ||
| id: dependency_review_support | ||
|
|
@@ -272,30 +274,31 @@ jobs: | |
| set -euo pipefail | ||
|
|
||
| api_url="${GITHUB_API_URL:-https://api.github.com}" | ||
| response_file="$(mktemp)" | ||
| set +e | ||
| status="$( | ||
| curl -fsS -o "$response_file" -w '%{http_code}' \ | ||
| curl -sS --connect-timeout 10 --max-time 30 \ | ||
| -o /dev/null \ | ||
| -w '%{http_code}' \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
| -H "Authorization: Bearer ${GH_TOKEN}" \ | ||
| -H "X-GitHub-Api-Version: 2022-11-28" \ | ||
| "${api_url}/repos/${REPOSITORY}/dependency-graph/compare/${BASE_SHA}...${HEAD_SHA}" \ | ||
| || true | ||
| "${api_url}/repos/${REPOSITORY}/dependency-graph/compare/${BASE_SHA}...${HEAD_SHA}" | ||
| )" | ||
| curl_status=$? | ||
| set -e | ||
|
|
||
| if [ "$status" = "200" ]; then | ||
| echo "supported=true" >>"$GITHUB_OUTPUT" | ||
| exit 0 | ||
| fi | ||
| case "$status" in | ||
| [0-9][0-9][0-9]) http_status="$status" ;; | ||
| "") http_status="unavailable" ;; | ||
| *) http_status="malformed" ;; | ||
| esac | ||
|
|
||
| if [ "$status" = "403" ] || [ "$status" = "404" ]; then | ||
| echo "::warning::Dependency review is unavailable for ${REPOSITORY}; skipping dependency-review hard gate." | ||
| echo "supported=false" >>"$GITHUB_OUTPUT" | ||
| exit 0 | ||
| if [ "$curl_status" -ne 0 ] || [ "$http_status" != "200" ]; then | ||
| echo "::error::Dependency review evidence unavailable for ${REPOSITORY} at exact base ${BASE_SHA} and head ${HEAD_SHA}: HTTP ${http_status}; curl exit ${curl_status}. Verify dependency-graph/security configuration and GitHub service behavior, then rerun. Failing closed." | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "::error::Dependency review support check failed with HTTP ${status}." | ||
| cat "$response_file" | ||
| exit 1 | ||
| echo "supported=true" >>"$GITHUB_OUTPUT" | ||
| - name: Dependency review | ||
| if: steps.dependency_review_support.outputs.supported == 'true' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this support step is later skipped, |
||
| uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0 | ||
|
|
||
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,50 @@ | ||
| # Dependency review fail-closed operations | ||
|
|
||
| Status: `active_pr` until the matching workflow and regression contract are present on protected `main`; thereafter `implemented_on_protected_main`. | ||
|
|
||
| ## Decision | ||
|
|
||
| Dependency review is a hard supply-chain gate. The central workflow accepts only HTTP `200` from GitHub's exact `BASE_SHA...HEAD_SHA` comparison before invoking the immutably pinned dependency-review action. A `403`, `404`, empty or malformed status, timeout, transport failure, truncated exchange, or other unexpected outcome is unavailable evidence and fails closed. | ||
|
|
||
| The support probe has a 10-second connection limit and 30-second total limit. It preserves curl's transport exit code separately from the bounded HTTP status and requires transport exit `0` plus exact HTTP `200`. It discards the response body and logs only repository identity, exact base/head revisions, the normalized HTTP status, and the numeric transport exit. Credentials and response bodies are never diagnostic output. | ||
|
|
||
| RFC 9110 §15.3.1 defines `200` as a completed successful representation, not as a status that can be inferred after a truncated transfer (Fielding et al., 2022). NIST SP 800-53 Rev. 5 RA-5 and SA-12 require that vulnerability and supply-chain evidence be obtained, not assumed absent (National Institute of Standards and Technology, 2020). SLSA v1.0 likewise treats missing provenance as unverified rather than passing (SLSA, 2023). An HTTP `403` or `404` is therefore unavailable evidence, not a clean skip. | ||
|
|
||
| ## Identity and authority | ||
|
|
||
| The dependency-review job checks out the pull request's explicit head repository and immutable head SHA with persisted credentials disabled. The API comparison independently binds the event's exact base and head revisions. The job retains `contents: read` and `pull-requests: read`; it receives no write, OIDC, model, release, package, or deployment authority. | ||
|
|
||
| Checks, status contexts, review submissions, and merge authorization remain separate evidence classes. OSV, Trivy, CodeQL, Semgrep, Secret Scan, Scorecard, and Dependabot are complementary controls and are not semantic substitutes for dependency review. | ||
|
|
||
| ## Failure classification and remediation | ||
|
|
||
| - Transport exit `0` plus HTTP `200`: proceed to the pinned dependency-review action. | ||
| - Any other result: fail the job and retain exact repository/base/head/status and transport-exit evidence. An HTTP `200` emitted by a failed or partial transfer is unavailable evidence. | ||
| - Public repository failure: verify dependency graph and security configuration, organization policy, token read access, and GitHub service health. | ||
| - Private or internal exception: require a separately reviewed organization policy with explicit entitlement evidence and compensating controls. Never infer `not-applicable` from an unavailable response. | ||
|
|
||
| Retries are operator-initiated only after the capability or service condition changes. Do not rerun unchanged evidence repeatedly and do not convert an unavailable endpoint into a green skip. | ||
|
|
||
| ## Acceptance and rollback | ||
|
|
||
| Acceptance requires the permanent queue contract to reject the former `supported=false` path, require bounded probing and discarded bodies, require exact-head checkout, and prove that only `200` reaches the action. Exact-head CI/security evidence, current review, protected integration, and a real protected-main consumer run remain required. | ||
|
|
||
| Rollback requires an independently reviewed revert and fresh exact-head evidence. A rollback must not restore the `403`/`404` success path or print an API response body. | ||
|
|
||
| ## References | ||
|
|
||
| Fielding, R., Nottingham, M., & Reschke, J. (Eds.). (2022). *HTTP semantics* | ||
| (RFC 9110). Internet Engineering Task Force. https://doi.org/10.17487/RFC9110 | ||
|
|
||
| GitHub. (n.d.). *Dependency review*. GitHub Docs. Retrieved August 9, 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 9, 2026, from https://docs.github.com/en/rest/dependency-graph/dependency-review | ||
|
|
||
| GitHub. (n.d.). *Dependency graph*. GitHub Docs. Retrieved August 9, 2026, from https://docs.github.com/en/code-security/concepts/supply-chain-security/dependency-graph | ||
|
|
||
| National Institute of Standards and Technology. (2020). *Security and | ||
| privacy controls for information systems and organizations* (NIST SP | ||
| 800-53 Rev. 5). https://doi.org/10.6028/NIST.SP.800-53r5 | ||
|
|
||
| SLSA. (2023). *SLSA v1.0: Supply-chain Levels for Software Artifacts*. | ||
| Open Source Security Foundation. https://slsa.dev/spec/v1.0/ |
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#810 requires the probe to record repository visibility with the exact base/head and status, without guessing a 403 root cause. This line logs repo and SHAs only. Allowlist
public|private|internalfromgithub.event.repository.visibilityand emitunknownfor any other value in this diagnostic.