From b4489dcabb51127549fb7935625858fe405a51a2 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 03:43:05 +0900 Subject: [PATCH] fix(codeql): release runners with exact job wake-up Signed-off-by: Seongho Bae --- .github/workflows/codeql-pr.yml | 170 ++++++--------- .github/workflows/codeql-scan-dispatch.yml | 93 ++++++++- ...required-workflow-dispatch-architecture.md | 48 ++--- tests/test_codeql_pr_workflow_contract.py | 116 +++++++---- ..._codeql_scan_dispatch_workflow_contract.py | 196 +++++++++++++++++- 5 files changed, 450 insertions(+), 173 deletions(-) diff --git a/.github/workflows/codeql-pr.yml b/.github/workflows/codeql-pr.yml index b540c49069..f529641ca6 100644 --- a/.github/workflows/codeql-pr.yml +++ b/.github/workflows/codeql-pr.yml @@ -4,8 +4,10 @@ # stays required-workflow-safe by never calling codeql-action itself: it # detects languages, dispatches the actual scan via repository_dispatch to # codeql-scan-dispatch.yml (which runs natively, unrestricted, in -# ContextualWisdomLab/.github), and polls for a codeql-dispatch/ -# commit status that handler publishes back onto this PR's head. Design: +# ContextualWisdomLab/.github). The shard then fails intentionally to release +# its runner; the handler publishes codeql-dispatch/ and reruns only +# that exact failed job. On rerun the shard reads the terminal status once. +# Design: # docs/adr/0025-codeql-required-workflow-dispatch-architecture.md. The # merge-preview scan (analyze-merge) is required nowhere (PR #1766) and was # dropped, not migrated. @@ -38,7 +40,7 @@ concurrency: # superseded head survive a close event indefinitely (it and the closing # run would land in different groups and never cancel each other). A # narrower risk remains -- a delayed dispatch for an older head could still - # transiently evict a newer head's in-flight poll before that older run's + # transiently evict a newer head's in-flight dispatch before that older run's # own live-head recheck self-aborts -- tracked as a follow-up requiring a # dedicated cleanup job, not a one-line group change. group: >- @@ -155,20 +157,10 @@ jobs: matrix: ${{ fromJSON(needs.detect-languages.outputs.matrix) }} steps: - name: Request current-head CodeQL scan dispatch - # Dispatch+poll live as sequential steps of ONE job (mirroring - # opencode-review.yml's opencode-review-target job) specifically so a - # dispatch failure fails this job directly -- no needs-based skip to - # worry about, and (below) the poll step can read this step's own - # `outcome` within the same shard. Each shard dispatches only ITS OWN - # language (not the full matrix): dispatching the full matrix from a - # single shard would leave every OTHER shard blind to that one - # shard's dispatch failure, each polling the full 3-hour deadline - # before self-timing-out for a scan that was never actually - # requested. One dispatch per language costs the same total .github-side - # work as one dispatch carrying every language (N single-language - # scans either way) while letting every shard fail closed immediately - # on its own dispatch failure instead of only detecting it 3 hours - # later. + # Each shard dispatches only its own language and passes its exact + # run/job identity. The shard intentionally fails after dispatch so + # its runner is released; the trusted handler later reruns that one + # failed job after publishing a terminal current-head verdict. id: dispatch if: needs.detect-languages.outputs.code == 'true' env: @@ -183,6 +175,9 @@ jobs: PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} LANGUAGE: ${{ matrix.language }} BUILD_MODE: ${{ matrix.build-mode }} + RUN_ATTEMPT: ${{ github.run_attempt }} + REQUIRED_RUN_ID: ${{ github.run_id }} + REQUIRED_JOB_ID: ${{ job.check_run_id }} run: | set -euo pipefail live_pr="$(gh api "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}")" @@ -201,6 +196,35 @@ jobs: exit 0 fi + statuses="$(gh api "repos/${TARGET_REPOSITORY}/commits/${PR_HEAD_SHA}/statuses")" + verdict_state="$(printf '%s' "$statuses" | jq -r --arg ctx "codeql-dispatch/${LANGUAGE}" ' + [ + .[] + | select(.context == $ctx) + | select( + (.creator.login // "" | ascii_downcase) as $creator + | $creator == "opencode-agent" or $creator == "opencode-agent[bot]" + ) + ] + | first // {} | .state // empty + ')" + case "$verdict_state" in + success|failure|error) + echo "verdict=${verdict_state}" >>"$GITHUB_OUTPUT" + echo "Found authenticated current-head CodeQL verdict for ${LANGUAGE}: ${verdict_state}." + exit 0 + ;; + esac + if [ "$RUN_ATTEMPT" != "1" ]; then + echo "::error::Exact CodeQL job was rerun without an authenticated terminal verdict." + exit 1 + fi + if ! [[ "$REQUIRED_RUN_ID" =~ ^[1-9][0-9]*$ ]] || + ! [[ "$REQUIRED_JOB_ID" =~ ^[1-9][0-9]*$ ]]; then + echo "::error::CodeQL dispatch requires canonical current run and job ids." + exit 1 + fi + if [ -z "${ACTIONS_ID_TOKEN_REQUEST_TOKEN:-}" ] || [ -z "${ACTIONS_ID_TOKEN_REQUEST_URL:-}" ]; then echo "::error::CodeQL scan dispatch requires GitHub OIDC." exit 1 @@ -227,103 +251,39 @@ jobs: --arg pr_head_sha "$PR_HEAD_SHA" \ --arg language "$LANGUAGE" \ --arg build_mode "$BUILD_MODE" \ - '{event_type:"codeql-scan",client_payload:{target_repository:$target_repository,pr_number:$pr_number,pr_base_ref:$pr_base_ref,pr_base_sha:$pr_base_sha,pr_head_ref:$pr_head_ref,pr_head_sha:$pr_head_sha,matrix:[{language:$language,"build-mode":$build_mode}]}}' | + --arg required_run_id "$REQUIRED_RUN_ID" \ + --arg required_job_id "$REQUIRED_JOB_ID" \ + --arg required_language "$LANGUAGE" \ + '{event_type:"codeql-scan",client_payload:{target_repository:$target_repository,pr_number:$pr_number,pr_base_ref:$pr_base_ref,pr_base_sha:$pr_base_sha,pr_head_ref:$pr_head_ref,pr_head_sha:$pr_head_sha,matrix:[{language:$language,"build-mode":$build_mode}],required_run_id:$required_run_id,required_job_id:$required_job_id,required_language:$required_language}}' | GH_TOKEN="$app_token" gh api -X POST repos/ContextualWisdomLab/.github/dispatches --input - + echo "verdict=pending" >>"$GITHUB_OUTPUT" - - name: Fail closed without a current-head CodeQL dispatch verdict - if: needs.detect-languages.outputs.code == 'true' + - name: Release runner or enforce current-head CodeQL verdict + if: always() && needs.detect-languages.outputs.code == 'true' env: - GH_TOKEN: ${{ github.token }} - TARGET_REPOSITORY: ${{ github.event.pull_request.base.repo.full_name || github.repository }} - PR_NUMBER: ${{ github.event.pull_request.number }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} LANGUAGE: ${{ matrix.language }} DISPATCH_OUTCOME: ${{ steps.dispatch.outcome }} + VERDICT_STATE: ${{ steps.dispatch.outputs.verdict }} run: | set -euo pipefail if [ "$DISPATCH_OUTCOME" != "success" ]; then - echo "::error::CodeQL scan dispatch did not succeed (outcome=${DISPATCH_OUTCOME}); failing closed without polling." + echo "::error::CodeQL scan dispatch or exact-head verdict read did not succeed (outcome=${DISPATCH_OUTCOME})." exit 1 fi - - poll_interval_seconds=30 - max_poll_transport_failures=3 - poll_failures=0 - # Wall-clock backstop distinct from max_poll_transport_failures: - # that counter only bounds *consecutive transport failures*, so a - # dispatched scan that never posts a status -- while every - # individual `gh api` call keeps succeeding -- would otherwise poll - # forever. Mirrors opencode-review.yml's identical 3-hour bound. - poll_deadline_epoch=$(( $(date -u +%s) + 10800 )) - while :; do - if [ "$(date -u +%s)" -ge "$poll_deadline_epoch" ]; then - echo "::error::No current-head CodeQL dispatch verdict after 180 minutes of polling; failing closed and releasing the runner." + case "$VERDICT_STATE" in + success) + echo "Current-head CodeQL dispatch verdict for ${LANGUAGE}: success." + ;; + failure|error) + echo "::error::CodeQL dispatch scan for ${LANGUAGE} did not pass (state=${VERDICT_STATE}). See the linked dispatch run for SARIF evidence." exit 1 - fi - if ! live_pr="$(timeout 30s gh api "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}")"; then - poll_failures=$((poll_failures + 1)) - if [ "$poll_failures" -ge "$max_poll_transport_failures" ]; then - echo "::error::Live pull request read failed ${poll_failures} consecutive times while polling; failing closed and releasing the runner." - exit 1 - fi - echo "::warning::Live pull request read failed while polling (${poll_failures}/${max_poll_transport_failures}); retrying after revalidation delay." - sleep "$poll_interval_seconds" - continue - fi - poll_failures=0 - live_head="$(printf '%s' "$live_pr" | jq -r '.head.sha // empty')" - live_state="$(printf '%s' "$live_pr" | jq -r 'if (.state | type) == "string" then .state else empty end')" - if [ -z "$live_head" ] || [ -z "$live_state" ]; then - echo "::error::Could not validate live pull request state while polling for a current-head CodeQL verdict." + ;; + pending) + echo "::error::CodeQL scan dispatched. The dispatch workflow will rerun this exact failed CodeQL job after publishing its terminal verdict." exit 1 - fi - if [ "${live_head,,}" != "${HEAD_SHA,,}" ]; then - echo "::notice::Pull request head moved while waiting for a current-head CodeQL verdict; retiring superseded poll." - exit 0 - fi - if [ "$live_state" = "closed" ]; then - echo "PR closed while waiting for the current-head CodeQL verdict; the poll is no longer required." - exit 0 - fi - if ! statuses="$(timeout 30s gh api "repos/${TARGET_REPOSITORY}/commits/${HEAD_SHA}/statuses")"; then - poll_failures=$((poll_failures + 1)) - if [ "$poll_failures" -ge "$max_poll_transport_failures" ]; then - echo "::error::Commit statuses read failed ${poll_failures} consecutive times while polling; failing closed and releasing the runner." - exit 1 - fi - echo "::warning::Commit statuses read failed while polling (${poll_failures}/${max_poll_transport_failures}); revalidating live PR state before retry." - sleep "$poll_interval_seconds" - continue - fi - poll_failures=0 - # A commit status is writable by anyone with statuses:write on - # this repository, so matching on .context alone would let a - # malicious PR forge its own passing "codeql-dispatch/" - # status and skip being scanned (ADR 0025, "Poll target cannot be - # spoofed by the PR author"). codeql-scan-dispatch.yml mints its - # publishing token via the same OIDC audience - # (opencode-github-action) opencode-review-dispatch.yml uses, so - # the legitimate status always carries that app's bot identity -- - # mirror opencode-review.yml's opencode-agent/opencode-agent[bot] - # creator check rather than trusting the context name alone. - verdict_state="$(printf '%s' "$statuses" | jq -r --arg ctx "codeql-dispatch/${LANGUAGE}" ' - [ - .[] - | select(.context == $ctx) - | select( - (.creator.login // "" | ascii_downcase) as $creator - | $creator == "opencode-agent" or $creator == "opencode-agent[bot]" - ) - ] - | first // {} | .state // empty - ')" - if [ "$verdict_state" = "success" ] || [ "$verdict_state" = "failure" ] || [ "$verdict_state" = "error" ]; then - break - fi - sleep "$poll_interval_seconds" - done - if [ "$verdict_state" != "success" ]; then - echo "::error::CodeQL dispatch scan for ${LANGUAGE} did not pass (state=${verdict_state}). See the linked dispatch run (codeql-scan-dispatch.yml in ContextualWisdomLab/.github) for SARIF evidence." - exit 1 - fi - echo "Current-head CodeQL dispatch verdict for ${LANGUAGE}: success." + ;; + *) + echo "::error::CodeQL shard has no authenticated current-head verdict or dispatch receipt." + exit 1 + ;; + esac diff --git a/.github/workflows/codeql-scan-dispatch.yml b/.github/workflows/codeql-scan-dispatch.yml index 2934731071..b3bcd2be33 100644 --- a/.github/workflows/codeql-scan-dispatch.yml +++ b/.github/workflows/codeql-scan-dispatch.yml @@ -1,7 +1,7 @@ # Runs github/codeql-action outside any required-workflow context. GitHub # categorically refuses to admit init/analyze inside a required workflow # (docs/doctoring/codeql-pr-required-workflow-always-fails.md); this file is -# the native execution half of the dispatch+poll design implemented by +# the native execution half of the dispatch+exact-job-wake design implemented by # ContextualWisdomLab/.github#1778. Do not add workflow_dispatch here to allow # manual testing: # test_no_central_workflow_exposes_branch_selected_manual_dispatch (in @@ -48,6 +48,9 @@ jobs: head_ref: ${{ steps.validate.outputs.head_ref }} head_sha: ${{ steps.validate.outputs.head_sha }} matrix: ${{ steps.validate.outputs.matrix }} + required_run_id: ${{ steps.validate.outputs.required_run_id }} + required_job_id: ${{ steps.validate.outputs.required_job_id }} + required_language: ${{ steps.validate.outputs.required_language }} steps: - name: Exchange OpenCode app token for target repository metadata reads id: metadata_read_app_token @@ -143,6 +146,9 @@ jobs: SUPPLIED_HEAD_REF: ${{ github.event.client_payload.pr_head_ref || '' }} SUPPLIED_HEAD_SHA: ${{ github.event.client_payload.pr_head_sha || '' }} SUPPLIED_MATRIX: ${{ github.event.client_payload.matrix || '' }} + SUPPLIED_REQUIRED_RUN_ID: ${{ github.event.client_payload.required_run_id || '' }} + SUPPLIED_REQUIRED_JOB_ID: ${{ github.event.client_payload.required_job_id || '' }} + SUPPLIED_REQUIRED_LANGUAGE: ${{ github.event.client_payload.required_language || '' }} run: | set -euo pipefail if [ -z "$ALLOWED_DISPATCH_ACTOR" ] || @@ -161,9 +167,16 @@ jobs: matrix_json="$(printf '%s' "$SUPPLIED_MATRIX" | jq -c '.' 2>/dev/null || true)" if [ -z "$matrix_json" ] || - [ "$(printf '%s' "$matrix_json" | jq 'type == "array" and length > 0')" != "true" ] || + [ "$(printf '%s' "$matrix_json" | jq 'type == "array" and length == 1')" != "true" ] || [ "$(printf '%s' "$matrix_json" | jq '[.[] | select((.language | type == "string") and (.language | test("^[a-z0-9-]+$")) and (."build-mode" | type == "string"))] | length == ($ARGS.positional[0] | tonumber)' --args "$(printf '%s' "$matrix_json" | jq 'length')")" != "true" ]; then - printf '::error::CodeQL scan dispatch matrix was missing, empty, or contained an entry without a valid language/build-mode. matrix=%s\n' "${SUPPLIED_MATRIX:-}" + printf '::error::CodeQL scan dispatch matrix must contain exactly one valid language/build-mode shard. matrix=%s\n' "${SUPPLIED_MATRIX:-}" + exit 1 + fi + matrix_language="$(printf '%s' "$matrix_json" | jq -r '.[0].language // empty')" + if ! [[ "$SUPPLIED_REQUIRED_RUN_ID" =~ ^[1-9][0-9]*$ ]] || + ! [[ "$SUPPLIED_REQUIRED_JOB_ID" =~ ^[1-9][0-9]*$ ]] || + [ "$SUPPLIED_REQUIRED_LANGUAGE" != "$matrix_language" ]; then + printf '::error::CodeQL wake identity is missing, non-canonical, or does not match the dispatched language.\n' exit 1 fi @@ -207,6 +220,9 @@ jobs: echo "matrix<>"$GITHUB_OUTPUT" printf 'Validated current live metadata for %s#%s: base=%s/%s head=%s/%s.\n' "$TARGET_REPOSITORY" "$PR_NUMBER" "$live_base_ref" "$live_base_sha" "$live_head_ref" "$live_head_sha" @@ -216,7 +232,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 30 permissions: - actions: read + actions: write contents: read security-events: read id-token: write @@ -375,6 +391,7 @@ jobs: retention-days: 7 - name: Publish CodeQL dispatch status + id: publish_status if: always() env: TARGET_APP_STATUS_TOKEN: ${{ steps.target_app_token.outputs.token || '' }} @@ -443,5 +460,71 @@ jobs: exit 0 fi - echo "::error::Could not publish the CodeQL dispatch status after all configured credentials failed; the poller in codeql-pr.yml will time out and fail closed instead of reading a stale or missing verdict." + echo "::error::Could not publish the CodeQL dispatch status after all configured credentials failed; the exact required job will remain failed and will not be woken with stale or missing evidence." exit 1 + + - name: Wake exact CodeQL required job + if: >- + always() + && steps.publish_status.outcome == 'success' + && needs.validate-dispatch.outputs.target_repository != '' + && needs.validate-dispatch.outputs.pr_number != '' + && needs.validate-dispatch.outputs.head_sha != '' + && github.event.client_payload.required_run_id != '' + && github.event.client_payload.required_job_id != '' + env: + GH_TOKEN: ${{ needs.validate-dispatch.outputs.target_repository == github.repository && github.token || secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN }} + TARGET_REPOSITORY: ${{ needs.validate-dispatch.outputs.target_repository }} + PR_NUMBER: ${{ needs.validate-dispatch.outputs.pr_number }} + HEAD_SHA: ${{ needs.validate-dispatch.outputs.head_sha }} + REQUIRED_RUN_ID: ${{ needs.validate-dispatch.outputs.required_run_id }} + REQUIRED_JOB_ID: ${{ needs.validate-dispatch.outputs.required_job_id }} + REQUIRED_LANGUAGE: ${{ needs.validate-dispatch.outputs.required_language }} + WAKE_TOKEN_SOURCE: ${{ needs.validate-dispatch.outputs.target_repository == github.repository && 'github-token' || secrets.PR_REVIEW_MERGE_TOKEN != '' && 'PR_REVIEW_MERGE_TOKEN' || secrets.OPENCODE_APPROVE_TOKEN != '' && 'OPENCODE_APPROVE_TOKEN' || 'unavailable' }} + run: | + set -euo pipefail + if [ -z "${GH_TOKEN:-}" ] || [ "$WAKE_TOKEN_SOURCE" = "unavailable" ]; then + echo "::error::Actions-capable CodeQL wake credential is unavailable." + exit 1 + fi + if ! [[ "$REQUIRED_RUN_ID" =~ ^[1-9][0-9]*$ ]] || + ! [[ "$REQUIRED_JOB_ID" =~ ^[1-9][0-9]*$ ]] || + ! [[ "$REQUIRED_LANGUAGE" =~ ^[a-z0-9-]+$ ]]; then + echo "::error::CodeQL wake identity is non-canonical." + exit 1 + fi + + pull="$(gh api "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}")" + live_state="$(printf '%s' "$pull" | jq -r '.state // empty')" + live_head="$(printf '%s' "$pull" | jq -r '.head.sha // empty')" + if [ "$live_state" != "open" ] || [ "$live_head" != "$HEAD_SHA" ]; then + echo "::error::CodeQL wake rejected a closed PR or stale head." + exit 1 + fi + + run="$(gh api "repos/${TARGET_REPOSITORY}/actions/runs/${REQUIRED_RUN_ID}")" + run_identity="$(printf '%s' "$run" | jq -r --arg head "$HEAD_SHA" --argjson run_id "$REQUIRED_RUN_ID" ' + select(.id == $run_id) + | select(.event == "pull_request") + | select(.path == ".github/workflows/codeql-pr.yml") + | select(.head_sha == $head) + | .id // empty + ')" + expected_name="CodeQL compatibility analysis (${REQUIRED_LANGUAGE})" + job="$(gh api "repos/${TARGET_REPOSITORY}/actions/jobs/${REQUIRED_JOB_ID}")" + job_identity="$(printf '%s' "$job" | jq -r --arg head "$HEAD_SHA" --arg name "$expected_name" --argjson run_id "$REQUIRED_RUN_ID" --argjson job_id "$REQUIRED_JOB_ID" ' + select(.id == $job_id) + | select(.run_id == $run_id) + | select(.head_sha == $head) + | select(.name == $name) + | select(.status == "completed" and .conclusion == "failure") + | .id // empty + ')" + if [ "$run_identity" != "$REQUIRED_RUN_ID" ] || + [ "$job_identity" != "$REQUIRED_JOB_ID" ]; then + echo "::error::CodeQL wake rejected missing or ambiguous exact run/job identity." + exit 1 + fi + + gh api -X POST "repos/${TARGET_REPOSITORY}/actions/jobs/${REQUIRED_JOB_ID}/rerun" >/dev/null + echo "Re-ran exact failed CodeQL job ${REQUIRED_JOB_ID} for ${REQUIRED_LANGUAGE} on ${HEAD_SHA}." diff --git a/docs/adr/0025-codeql-required-workflow-dispatch-architecture.md b/docs/adr/0025-codeql-required-workflow-dispatch-architecture.md index 8c1cffb8fd..d9d820f014 100644 --- a/docs/adr/0025-codeql-required-workflow-dispatch-architecture.md +++ b/docs/adr/0025-codeql-required-workflow-dispatch-architecture.md @@ -75,9 +75,8 @@ check. Both should coexist. Follow the same required-workflow-entrypoint-dispatches-to-native-execution pattern already proven by `strix.yml` (`repository_dispatch` + `Fetch pull request head for trusted scan` + `Publish same-head manual Strix -status`) and `opencode-review.yml` (`Request current-head OpenCode review -execution` dispatch + `Fail closed without a current-head OpenCode verdict` -bounded poll). Concretely: +status`) and OpenCode's runner-release plus exact run/job wake-up contract. +Concretely: ``` codeql-pr.yml (required workflow, runs in target repo context) @@ -95,21 +94,16 @@ codeql-pr.yml (required workflow, runs in target repo context) state first (open, not draft-exempt in the same way OpenCode's dispatch step already does) before dispatching. - analyze-head (matrix) -- RENAMED INTERNALLY, SAME REQUIRED-CHECK NAME: + analyze-head (matrix) -- SAME REQUIRED-CHECK NAME: "CodeQL compatibility analysis (${{ matrix.language }})". - needs: [detect-languages, dispatch-analysis]. - No codeql-action reference. Polls (bounded - wall-clock deadline + transport-failure - tolerance, identical shape to opencode-review.yml's - poll loop) for a commit status posted by the - dispatch handler at context - "codeql-dispatch/${{ matrix.language }}" on - the live PR head SHA, re-validating live PR - head/state each iteration exactly like - opencode-review.yml's poll does (a superseded - head must retire this poll, not report a - stale result). Reflects the polled - conclusion as this job's own exit code. + No codeql-action reference. On attempt one it + dispatches its exact run id, job id, language, + and head, then fails intentionally to release + the runner. The trusted handler publishes the + terminal status and reruns only that failed + job. On attempt two the shard reads the + authenticated current-head status once and + reflects it as this job's own exit code. .github/workflows/codeql-scan-dispatch.yml (NEW, runs natively in .github, NOT admitted through the ruleset, so codeql-action is unrestricted here) @@ -149,6 +143,12 @@ NOT admitted through the ruleset, so codeql-action is unrestricted here) .github-side run for audit trail (mirrors strix.yml's "Preserve CodeQL SARIF evidence" / artifact retention today). + -- Re-fetch the open PR, exact required workflow + run, and exact failed language job; + require matching path/head/run/job/name before + calling the single-job rerun endpoint. Missing, + stale, closed, or mismatched identity fails + closed and leaves the required job failed. ``` ## Scope decision: `analyze-merge` is dropped, not migrated @@ -168,7 +168,7 @@ blocker for this one. PR from the API and refuse to scan or publish anything if the dispatched `pr_head_sha` no longer matches the live head, exactly like `strix.yml`'s existing `Validate repository dispatch against live pull request metadata` - step and `opencode-review.yml`'s poll-time revalidation. A forged or stale + step and the exact-job wake-time revalidation. A forged or stale dispatch must never be able to make an unrelated head appear scanned. - **Cross-repository checkout trust boundary:** the scan step checks out arbitrary target-repository PR-head content into `.github`'s own runner. @@ -183,11 +183,11 @@ blocker for this one. on the *target* repository only, following the same per-repository app-token minting `strix.yml` already performs — never a token with broader org access. -- **Poll target cannot be spoofed by the PR author:** a commit status is +- **Verdict target cannot be spoofed by the PR author:** a commit status is writable by anyone with `statuses:write` on the repository (including, depending on token scoping, a workflow running with the default `GITHUB_TOKEN` in some configurations) — confirm during implementation - that the polling job in `codeql-pr.yml` verifies the status update's + that the rerun job in `codeql-pr.yml` verifies the status update's `creator`/`avatar_url`/app identity matches the expected dispatch-handler app, not merely the context name, so a malicious PR cannot forge its own passing status. `strix.yml`'s manual-status-publish step already documents @@ -219,10 +219,8 @@ blocker for this one. requirement on `scripts/ci/`) to the org's central CI surface — more surface area to maintain, offset by removing ~70 lines of duplicated inline Python between `analyze-head`/`analyze-merge` today. - the `pr_review_merge_scheduler.py`-scale poll/dispatch pattern is already - proven at scale (Strix, OpenCode, Noema all use it today) and this is the - fourth application of the same design, not a new pattern to validate from - scratch. + exact run/job wake-up follows the OpenCode runner-release pattern while + avoiding one occupied runner per language for the scan's full duration. - Re-admitting `codeql-pr.yml` to ruleset `18156473` must happen only after this design is implemented, tested, and its `detect-languages`/ `dispatch-analysis`/`analyze-head` jobs are confirmed free of any @@ -236,7 +234,7 @@ blocker for this one. 1. Implement `scripts/ci/codeql_sarif_gate.py` + its test, extracted from the current inline gate in `codeql-pr.yml`. 2. Implement `codeql-scan-dispatch.yml` per the design above. -3. Rewrite `codeql-pr.yml`'s `analyze-head` job into the dispatch+poll shape; +3. Rewrite `codeql-pr.yml`'s `analyze-head` job into the dispatch+exact-job-wake shape; delete `analyze-merge` (tracked as future work, not silently lost — this ADR is the record). 4. Add a permanent contract test asserting no `codeql-action` reference diff --git a/tests/test_codeql_pr_workflow_contract.py b/tests/test_codeql_pr_workflow_contract.py index a314770217..90612e9bc8 100644 --- a/tests/test_codeql_pr_workflow_contract.py +++ b/tests/test_codeql_pr_workflow_contract.py @@ -14,7 +14,7 @@ def test_codeql_pr_workflow_structure() -> None: - """codeql-pr.yml stays required-workflow-safe: no codeql-action, dispatch+poll instead. + """codeql-pr.yml stays required-workflow-safe: dispatch, release, then exact wake-up. See docs/adr/0025-codeql-required-workflow-dispatch-architecture.md. codeql-action/init and codeql-action/analyze are categorically disallowed @@ -53,26 +53,17 @@ def test_codeql_pr_workflow_structure() -> None: assert "refs/pull/{0}/merge" not in workflow assert "event_type:\"codeql-scan\"" in workflow assert "repos/ContextualWisdomLab/.github/dispatches" in workflow - # Polls for the context codeql-scan-dispatch.yml publishes; doesn't - # publish it itself (that happens on the .github side only). + # Reads the authenticated context codeql-scan-dispatch.yml publishes; it + # never publishes that status from the required workflow. assert '--arg ctx "codeql-dispatch/${LANGUAGE}"' in workflow - assert "commits/${HEAD_SHA}/statuses" in workflow + assert "commits/${PR_HEAD_SHA}/statuses" in workflow def test_codeql_pr_dispatches_one_language_per_shard_not_the_full_matrix() -> None: """Every shard dispatches, but only its own language, not the full matrix. - Two designs were tried and rejected before this one (see - docs/adr/0025-codeql-required-workflow-dispatch-architecture.md history - and .github#1778's review thread): (a) only the first shard dispatches - with the full matrix, which leaves every OTHER shard blind to that one - shard's dispatch failure -- each polls the full 3-hour deadline before - self-timing-out for a scan that was never requested; (b) every shard - dispatches the full matrix, which triggers N redundant full-matrix scans - on the .github side. Dispatching one shard's own single language avoids - both: N dispatches total (same real work as one N-language dispatch), - and each shard can read its own steps.dispatch.outcome for the poll step - below to fail closed immediately, not after 3 hours. + Each shard carries its own run, job, language, and head identity so the + trusted dispatcher can wake only that intentionally failed job. """ workflow = WORKFLOW_PATH.read_text(encoding="utf-8") @@ -81,16 +72,16 @@ def test_codeql_pr_dispatches_one_language_per_shard_not_the_full_matrix() -> No assert "needs.detect-languages.outputs.matrix).include[0]" not in workflow assert "DISPATCH_OUTCOME: ${{ steps.dispatch.outcome }}" in workflow assert workflow.count("- name: Request current-head CodeQL scan dispatch") == 1 - assert workflow.count("- name: Fail closed without a current-head CodeQL dispatch verdict") == 1 + assert workflow.count("- name: Release runner or enforce current-head CodeQL verdict") == 1 RUN_BLOCK_STEP_NAMES = ( "Request current-head CodeQL scan dispatch", - "Fail closed without a current-head CodeQL dispatch verdict", + "Release runner or enforce current-head CodeQL verdict", ) -def test_codeql_pr_dispatch_and_poll_run_blocks_are_valid_bash() -> None: +def test_codeql_pr_dispatch_and_release_run_blocks_are_valid_bash() -> None: """Both run: blocks in analyze-head must be syntactically valid Bash.""" workflow_text = WORKFLOW_PATH.read_text(encoding="utf-8") @@ -112,17 +103,21 @@ def test_codeql_pr_dispatch_and_poll_run_blocks_are_valid_bash() -> None: assert result.returncode == 0, f"{step_name}: {result.stderr}" -POLL_STEP_NAME = "Fail closed without a current-head CodeQL dispatch verdict" +DISPATCH_STEP_NAME = "Request current-head CodeQL scan dispatch" +VERDICT_STEP_NAME = "Release runner or enforce current-head CodeQL verdict" -def _run_poll_step(tmp_path: Path, statuses: list[dict]) -> subprocess.CompletedProcess[str]: - """Execute the real poll shell block against a fake `gh api` returning a fixed live PR and status list.""" +def _run_verdict_read( + tmp_path: Path, statuses: list[dict] +) -> tuple[subprocess.CompletedProcess[str], subprocess.CompletedProcess[str]]: + """Execute the real one-shot status read and verdict enforcement blocks.""" bash = shutil.which("bash") jq = shutil.which("jq") assert bash is not None and jq is not None, "bash and jq are required to run this test" workflow_text = WORKFLOW_PATH.read_text(encoding="utf-8") - script = _extract_run_block(workflow_text, POLL_STEP_NAME) + dispatch_script = _extract_run_block(workflow_text, DISPATCH_STEP_NAME) + verdict_script = _extract_run_block(workflow_text, VERDICT_STEP_NAME) head_sha = "b" * 40 live_pr = {"head": {"sha": head_sha}, "state": "open"} @@ -143,7 +138,8 @@ def _run_poll_step(tmp_path: Path, statuses: list[dict]) -> subprocess.Completed ) fake_gh.chmod(0o755) - env = { + output = tmp_path / "github-output" + dispatch_env = { **os.environ, "PATH": f"{fake_bin}:{os.environ['PATH']}", "FAKE_PULL_JSON": json.dumps(live_pr), @@ -151,28 +147,50 @@ def _run_poll_step(tmp_path: Path, statuses: list[dict]) -> subprocess.Completed "GH_TOKEN": "fake-token", "TARGET_REPOSITORY": "ContextualWisdomLab/naruon", "PR_NUMBER": "42", - "HEAD_SHA": head_sha, + "PR_HEAD_SHA": head_sha, + "LANGUAGE": "python", + "BUILD_MODE": "none", + "BASE_REF": "main", + "BASE_SHA": "a" * 40, + "HEAD_REF": "feature", + "RUN_ATTEMPT": "2", + "REQUIRED_RUN_ID": "42", + "REQUIRED_JOB_ID": "43", + "GITHUB_OUTPUT": str(output), + } + dispatch_result = subprocess.run( + [bash], input=dispatch_script, text=True, capture_output=True, check=False, + env=dispatch_env, timeout=60, + ) + output_values = dict( + line.split("=", 1) for line in output.read_text(encoding="utf-8").splitlines() + ) + verdict_env = { + **os.environ, "LANGUAGE": "python", "DISPATCH_OUTCOME": "success", + "VERDICT_STATE": output_values["verdict"], } - return subprocess.run( - [bash], input=script, text=True, capture_output=True, check=False, env=env, timeout=60 + verdict_result = subprocess.run( + [bash], input=verdict_script, text=True, capture_output=True, check=False, + env=verdict_env, timeout=60, ) + return dispatch_result, verdict_result -def test_codeql_pr_poll_step_ignores_a_status_forged_by_a_non_opencode_creator(tmp_path: Path) -> None: +def test_codeql_pr_one_shot_read_ignores_status_forged_by_non_opencode_creator(tmp_path: Path) -> None: """A PR-forged 'codeql-dispatch/: success' status must not stand in for the real verdict. Only a status published by codeql-scan-dispatch.yml's own app identity (opencode-agent[bot], minted via the same OIDC exchange - opencode-review-dispatch.yml uses) may satisfy the poll -- matching the + opencode-review-dispatch.yml uses) may satisfy the verdict read -- matching the context string alone is not enough, since anyone with statuses:write on the repository can publish an arbitrary context (ADR 0025, "Poll target cannot be spoofed by the PR author"). This proves the forged success is skipped in favor of the legitimate (here, failing) verdict rather than accepted. """ - result = _run_poll_step( + dispatch_result, verdict_result = _run_verdict_read( tmp_path, statuses=[ {"context": "codeql-dispatch/python", "state": "success", "creator": {"login": "attacker"}}, @@ -183,13 +201,14 @@ def test_codeql_pr_poll_step_ignores_a_status_forged_by_a_non_opencode_creator(t }, ], ) - assert result.returncode == 1, result.stderr - assert "did not pass (state=failure)" in result.stdout + assert dispatch_result.returncode == 0, dispatch_result.stderr + assert verdict_result.returncode == 1, verdict_result.stderr + assert "did not pass (state=failure)" in verdict_result.stdout -def test_codeql_pr_poll_step_accepts_the_opencode_agent_creator(tmp_path: Path) -> None: +def test_codeql_pr_one_shot_read_accepts_the_opencode_agent_creator(tmp_path: Path) -> None: """The legitimate handler's own success status is accepted once creator identity matches.""" - result = _run_poll_step( + dispatch_result, verdict_result = _run_verdict_read( tmp_path, statuses=[ { @@ -199,8 +218,9 @@ def test_codeql_pr_poll_step_accepts_the_opencode_agent_creator(tmp_path: Path) } ], ) - assert result.returncode == 0, result.stderr - assert "Current-head CodeQL dispatch verdict for python: success." in result.stdout + assert dispatch_result.returncode == 0, dispatch_result.stderr + assert verdict_result.returncode == 0, verdict_result.stderr + assert "Current-head CodeQL dispatch verdict for python: success." in verdict_result.stdout def test_codeql_action_steps_use_one_version_per_workflow() -> None: @@ -216,3 +236,29 @@ def test_codeql_action_steps_use_one_version_per_workflow() -> None: ) assert len(refs) == 1, f"scheduled-security-scan.yml mixes CodeQL action refs: {sorted(refs)}" + + +def test_codeql_shard_releases_runner_and_dispatches_exact_wake_identity() -> None: + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + shard = workflow.split(" analyze-head:\n", 1)[1] + + assert "while :; do" not in shard + assert "poll_interval_seconds" not in shard + assert "sleep " not in shard + assert "job.check_run_id" in shard + assert "required_run_id:$required_run_id" in shard + assert "required_job_id:$required_job_id" in shard + assert "required_language:$required_language" in shard + assert "The dispatch workflow will rerun this exact failed CodeQL job" in shard + assert "commits/${PR_HEAD_SHA}/statuses" in shard + + +def test_codeql_required_workflow_does_not_gain_actions_write() -> None: + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + permissions = workflow.split("permissions:\n", 1)[1].split("\njobs:\n", 1)[0] + shard_permissions = workflow.split(" analyze-head:\n", 1)[1].split( + " strategy:\n", 1 + )[0] + + assert "actions: write" not in permissions + assert "actions: write" not in shard_permissions diff --git a/tests/test_codeql_scan_dispatch_workflow_contract.py b/tests/test_codeql_scan_dispatch_workflow_contract.py index 1b2c2ed662..a3b9de22c0 100644 --- a/tests/test_codeql_scan_dispatch_workflow_contract.py +++ b/tests/test_codeql_scan_dispatch_workflow_contract.py @@ -1,7 +1,7 @@ """Structure and shell-syntax contract for the new codeql-scan-dispatch.yml handler. ContextualWisdomLab/.github#1772 designs this file as the native -(non-required-workflow) half of the CodeQL dispatch+poll rewrite, and +(non-required-workflow) half of the CodeQL dispatch architecture, and ContextualWisdomLab/.github#1778 wires the required entrypoint to it. This guards the handler's structure and shell syntax, mirroring the established pattern in tests/test_opencode_workflow_shell_syntax.py and @@ -32,6 +32,7 @@ "Fetch the pinned CodeQL SARIF gate script", "Materialize pull request head for CodeQL scan", "Publish CodeQL dispatch status", + "Wake exact CodeQL required job", ) @@ -99,7 +100,7 @@ def _run_validate_step(tmp_path: Path, env_overrides: dict[str, str], pull_reque script = _extract_run_block(workflow_text, VALIDATE_STEP_NAME) fake_bin = tmp_path / "bin" - fake_bin.mkdir() + fake_bin.mkdir(parents=True) fake_gh = fake_bin / "gh" fake_gh.write_text( "#!/usr/bin/env bash\n" @@ -126,6 +127,9 @@ def _run_validate_step(tmp_path: Path, env_overrides: dict[str, str], pull_reque "SUPPLIED_HEAD_REF": "feature", "SUPPLIED_HEAD_SHA": "b" * 40, "SUPPLIED_MATRIX": json.dumps([{"language": "python", "build-mode": "none"}]), + "SUPPLIED_REQUIRED_RUN_ID": "42", + "SUPPLIED_REQUIRED_JOB_ID": "43", + "SUPPLIED_REQUIRED_LANGUAGE": "python", **env_overrides, } result = subprocess.run([bash], input=script, text=True, capture_output=True, check=False, env=env) @@ -152,6 +156,9 @@ def test_codeql_scan_dispatch_validate_step_accepts_matching_live_metadata(tmp_p assert "pr_number=42" in output_text assert "head_sha=" + "b" * 40 in output_text assert '[{"language":"python","build-mode":"none"}]' in output_text + assert "required_run_id=42" in output_text + assert "required_job_id=43" in output_text + assert "required_language=python" in output_text def test_codeql_scan_dispatch_validate_step_rejects_actor_mismatch(tmp_path): @@ -205,7 +212,7 @@ def test_codeql_scan_dispatch_validate_step_rejects_malformed_matrix(tmp_path): ) assert result.returncode == 1 - assert "matrix was missing, empty, or contained an entry without a valid language/build-mode" in result.stdout + assert "matrix must contain exactly one valid language/build-mode shard" in result.stdout def test_codeql_scan_dispatch_validate_step_rejects_stale_head_sha(tmp_path): @@ -242,3 +249,186 @@ def test_codeql_scan_dispatch_is_not_in_the_required_workflow_ruleset_scope(): assert ".github/workflows/codeql-pr.yml" in required_paths assert ".github/workflows/codeql-scan-dispatch.yml" not in required_paths + + +def test_dispatch_wakes_only_the_exact_failed_codeql_job() -> None: + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + wake = workflow.split(" - name: Wake exact CodeQL required job\n", 1)[1].split( + "\n\n - name:", 1 + )[0] + + assert "steps.publish_status.outcome == 'success'" in wake + assert 'gh api "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}"' in wake + assert 'gh api "repos/${TARGET_REPOSITORY}/actions/runs/${REQUIRED_RUN_ID}"' in wake + assert 'gh api "repos/${TARGET_REPOSITORY}/actions/jobs/${REQUIRED_JOB_ID}"' in wake + assert 'select(.event == "pull_request")' in wake + assert 'select(.path == ".github/workflows/codeql-pr.yml")' in wake + assert "select(.head_sha == $head)" in wake + assert "select(.run_id == $run_id)" in wake + assert "select(.name == $name)" in wake + assert 'select(.status == "completed" and .conclusion == "failure")' in wake + assert 'actions/jobs/${REQUIRED_JOB_ID}/rerun' in wake + assert "rerun-failed-jobs" not in wake + assert "while " not in wake + assert "sleep " not in wake + + +def test_dispatch_wake_has_only_trusted_actions_write_boundary() -> None: + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + scan = workflow.split(" scan:\n", 1)[1] + scan_permissions = scan.split(" strategy:\n", 1)[0] + + assert "actions: write" in scan_permissions + assert "pull_request:" not in workflow + assert "pull_request_target:" not in workflow + assert "github.event.client_payload.required_run_id != ''" in scan + assert "github.event.client_payload.required_job_id != ''" in scan + + +def _run_wake_step( + tmp_path: Path, + *, + pull: dict | None = None, + run: dict | None = None, + job: dict | None = None, +) -> tuple[subprocess.CompletedProcess[str], Path]: + """Execute the exact wake block against fixture-backed GitHub API responses.""" + bash = shutil.which("bash") + jq = shutil.which("jq") + assert bash is not None and jq is not None, "bash and jq are required to run this test" + + head_sha = "b" * 40 + pull = pull or {"state": "open", "head": {"sha": head_sha}} + run = run or { + "id": 42, + "event": "pull_request", + "path": ".github/workflows/codeql-pr.yml", + "head_sha": head_sha, + "status": "completed", + "conclusion": "failure", + } + job = job or { + "id": 43, + "run_id": 42, + "head_sha": head_sha, + "name": "CodeQL compatibility analysis (python)", + "status": "completed", + "conclusion": "failure", + } + script = _extract_run_block( + WORKFLOW_PATH.read_text(encoding="utf-8"), "Wake exact CodeQL required job" + ) + fake_bin = tmp_path / "bin" + fake_bin.mkdir(parents=True) + post_log = tmp_path / "posts" + fake_gh = fake_bin / "gh" + fake_gh.write_text( + "#!/usr/bin/env bash\n" + "set -euo pipefail\n" + 'test "$1" = api\n' + 'if [ "${2:-}" = "-X" ]; then\n' + ' test "$3" = POST\n' + ' printf \'%s\\n\' "$4" >>"$FAKE_POST_LOG"\n' + " exit 0\n" + "fi\n" + 'case "$2" in\n' + ' */pulls/*) printf \'%s\\n\' "$FAKE_PULL_JSON" ;;\n' + ' */actions/runs/*) printf \'%s\\n\' "$FAKE_RUN_JSON" ;;\n' + ' */actions/jobs/*) printf \'%s\\n\' "$FAKE_JOB_JSON" ;;\n' + " *) exit 1 ;;\n" + "esac\n", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + env = { + **os.environ, + "PATH": f"{fake_bin}:{os.environ['PATH']}", + "FAKE_PULL_JSON": json.dumps(pull), + "FAKE_RUN_JSON": json.dumps(run), + "FAKE_JOB_JSON": json.dumps(job), + "FAKE_POST_LOG": str(post_log), + "GH_TOKEN": "fake-token", + "WAKE_TOKEN_SOURCE": "PR_REVIEW_MERGE_TOKEN", + "TARGET_REPOSITORY": "ContextualWisdomLab/naruon", + "PR_NUMBER": "42", + "HEAD_SHA": head_sha, + "REQUIRED_RUN_ID": "42", + "REQUIRED_JOB_ID": "43", + "REQUIRED_LANGUAGE": "python", + } + result = subprocess.run( + [bash], input=script, text=True, capture_output=True, check=False, env=env + ) + return result, post_log + + +def test_dispatch_wake_reruns_only_fixture_bound_exact_job(tmp_path: Path) -> None: + result, post_log = _run_wake_step(tmp_path) + + assert result.returncode == 0, result.stderr + assert post_log.read_text(encoding="utf-8").splitlines() == [ + "repos/ContextualWisdomLab/naruon/actions/jobs/43/rerun" + ] + + +def test_dispatch_wake_rejects_stale_head_and_closed_pr(tmp_path: Path) -> None: + stale_result, stale_log = _run_wake_step( + tmp_path / "stale", pull={"state": "open", "head": {"sha": "c" * 40}} + ) + closed_result, closed_log = _run_wake_step( + tmp_path / "closed", pull={"state": "closed", "head": {"sha": "b" * 40}} + ) + + assert stale_result.returncode == 1 + assert closed_result.returncode == 1 + assert not stale_log.exists() + assert not closed_log.exists() + + +def test_dispatch_wake_rejects_ambiguous_or_nonfailed_job_identity(tmp_path: Path) -> None: + wrong_job_result, wrong_job_log = _run_wake_step( + tmp_path / "wrong-job", + job={ + "id": 43, + "run_id": 999, + "head_sha": "b" * 40, + "name": "CodeQL compatibility analysis (python)", + "status": "completed", + "conclusion": "failure", + }, + ) + successful_job_result, successful_job_log = _run_wake_step( + tmp_path / "successful-job", + job={ + "id": 43, + "run_id": 42, + "head_sha": "b" * 40, + "name": "CodeQL compatibility analysis (python)", + "status": "completed", + "conclusion": "success", + }, + ) + + assert wrong_job_result.returncode == 1 + assert successful_job_result.returncode == 1 + assert "missing or ambiguous exact run/job identity" in wrong_job_result.stdout + assert not wrong_job_log.exists() + assert not successful_job_log.exists() + + +def test_dispatch_wake_allows_parallel_language_rerun_on_same_exact_run(tmp_path: Path) -> None: + """Another language may already have moved the shared run back to in_progress.""" + result, post_log = _run_wake_step( + tmp_path, + run={ + "id": 42, + "event": "pull_request", + "path": ".github/workflows/codeql-pr.yml", + "head_sha": "b" * 40, + "status": "in_progress", + "conclusion": None, + }, + ) + + assert result.returncode == 0, result.stderr + assert post_log.exists()