-
Notifications
You must be signed in to change notification settings - Fork 0
fix(security): reject malformed dependency-review identity before compare #1045
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
Changes from all commits
285bb39
241ef9d
887bf99
cef601e
5979c13
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,10 +16,12 @@ | |
| # 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. Malformed revisions or repository names fail closed before the | ||
| # network call. Every other probe outcome fails closed without printing the | ||
| # response body. Diagnostics include allowlisted repository visibility. 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 +259,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 | ||
|
|
@@ -268,36 +272,51 @@ jobs: | |
| BASE_SHA: ${{ github.event.pull_request.base.sha }} | ||
| HEAD_SHA: ${{ github.event.pull_request.head.sha }} | ||
| REPOSITORY: ${{ github.repository }} | ||
| REPOSITORY_VISIBILITY: ${{ github.event.repository.visibility }} | ||
| run: | | ||
| set -euo pipefail | ||
|
|
||
| api_url="${GITHUB_API_URL:-https://api.github.com}" | ||
| response_file="$(mktemp)" | ||
| case "${REPOSITORY_VISIBILITY}" in | ||
| public|private|internal) visibility="${REPOSITORY_VISIBILITY}" ;; | ||
| *) visibility="unknown" ;; | ||
| esac | ||
| revision_pattern='^[0-9a-fA-F]{40}$|^[0-9a-fA-F]{64}$' | ||
| repository_pattern='^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$' | ||
| if ! [[ "${BASE_SHA}" =~ $revision_pattern ]] || ! [[ "${HEAD_SHA}" =~ $revision_pattern ]]; then | ||
| echo "::error::Dependency review evidence unavailable for ${REPOSITORY} (visibility ${visibility}) at exact base ${BASE_SHA} and head ${HEAD_SHA}: HTTP unavailable; curl exit uncalled. Malformed revision. Supply the pull request's exact 40- or 64-character hex base and head SHAs, then rerun. Failing closed." | ||
|
Contributor
Author
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. This error interpolates the untrusted revision and repository. Name the failure class and the operator next step only: resupply the pull-request event's exact 40- or 64-character hex SHAs. Do not echo the raw invalid values. After identity is validated, echoing those now-safe SHAs on a later transport failure remains useful. |
||
| exit 1 | ||
| fi | ||
| if ! [[ "${REPOSITORY}" =~ $repository_pattern ]]; then | ||
| echo "::error::Dependency review evidence unavailable for ${REPOSITORY} (visibility ${visibility}) at exact base ${BASE_SHA} and head ${HEAD_SHA}: HTTP unavailable; curl exit uncalled. Malformed repository. Use the canonical owner/name and rerun. Failing closed." | ||
| exit 1 | ||
| fi | ||
| 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" ;; | ||
|
Contributor
Author
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.
Classify |
||
| "") 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} (visibility ${visibility}) 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' | ||
| uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0 | ||
| with: | ||
| fail-on-severity: moderate | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| # 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 rejects a base or head revision that is not exactly 40 or 64 hexadecimal characters, and a repository name that is not `owner/name`, before any network call (curl exit `uncalled`). It discards the response body and logs only repository identity, allowlisted visibility (`public`, `private`, `internal`, or `unknown`), exact base/head revisions, the normalized HTTP status, and the numeric transport exit. Credentials, response bodies, and raw untrusted visibility strings are never diagnostic output. After a successful probe the pinned action is not independently skippable. Executable regressions record the exact compare argv so a hardcoded `supported=true` path cannot satisfy the success contract. | ||
|
|
||
| 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). RFC 3986 forbids unvalidated path segments in a request-target (Berners-Lee et al., 2005). CWE-20 requires rejecting malformed identity before it is interpolated into that target (MITRE, n.d.). 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. | ||
| - Malformed revision or repository: fail closed with HTTP `unavailable` and curl exit `uncalled` before opening a socket. Re-supply the pull request's exact hex SHAs and canonical `owner/name`, then rerun. | ||
| - Any other result: fail the job and retain exact repository, allowlisted visibility, base/head, status, and transport-exit evidence. An HTTP `200` emitted by a failed or partial transfer is unavailable evidence. Do not infer a root cause from HTTP `403` or `404`. | ||
| - 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. | ||
|
|
||
| ## Known canary | ||
|
|
||
| ContextualWisdomLab/EgressWeave#66, Security Scan run `31108241013`, job `92638903658`, compared `10d0c51daf2ad278d66f43be479df8cf6b08ba6d...c038a9509d1a8eae8561cc9081e67e12bd373d42` and received HTTP `403`. The required workflow printed the skip warning, omitted `actions/dependency-review-action`, and still concluded success. Downstream tracking: ContextualWisdomLab/EgressWeave#76. Keep ContextualWisdomLab/.github#810 open until a protected-main public consumer run proves a non-200 or failed-transfer comparison cannot green this job. | ||
|
|
||
| ## 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, reject malformed revisions and repository names before the network call, prove the success path invoked the exact compare URL, 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 | ||
|
|
||
| Berners-Lee, T., Fielding, R., & Masinter, L. (2005). *Uniform Resource | ||
| Identifier (URI): Generic syntax* (RFC 3986). Internet Engineering Task | ||
| Force. https://doi.org/10.17487/RFC3986 | ||
|
|
||
| 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 | ||
|
|
||
| GitHub. (n.d.). *Webhook events and payloads*. GitHub Docs. Retrieved August 16, 2026, from https://docs.github.com/en/webhooks/webhook-events-and-payloads#repository | ||
|
|
||
| MITRE. (n.d.). *CWE-20: Improper input validation*. Retrieved August 16, | ||
| 2026, from https://cwe.mitre.org/data/definitions/20.html | ||
|
|
||
| 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/ |
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.
^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$matchesContextualWisdomLab/..and../.github. After interpolation the compare URL becomes/repos/ContextualWisdomLab/../dependency-graph/compare/....Reject a complete owner or name segment that is
.or..before curl. KeepContextualWisdomLab/.githublegal. Prove this with an executable case that would otherwise print HTTP 200 and writesupported=true.