From 4c62031fb253cec6abb39ac14c0dc124db6a61e6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 16:34:08 +0900 Subject: [PATCH 01/16] test(codeql): reproduce rerun dead-end after pre-runner cancellation --- .../test_codeql_pr_rerun_recovery_contract.py | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 tests/test_codeql_pr_rerun_recovery_contract.py diff --git a/tests/test_codeql_pr_rerun_recovery_contract.py b/tests/test_codeql_pr_rerun_recovery_contract.py new file mode 100644 index 0000000000..2551e57e68 --- /dev/null +++ b/tests/test_codeql_pr_rerun_recovery_contract.py @@ -0,0 +1,113 @@ +"""Regression for CodeQL reruns whose earlier attempts never reached dispatch.""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +from pathlib import Path + +from tests.test_opencode_workflow_shell_syntax import _extract_run_block + + +REPO_ROOT = Path(__file__).resolve().parents[1] +WORKFLOW_PATH = REPO_ROOT / ".github/workflows/codeql-pr.yml" +DISPATCH_STEP_NAME = "Request current-head CodeQL scan dispatch" + + +def test_rerun_without_authenticated_verdict_can_redispatch(tmp_path: Path) -> None: + """A later attempt may dispatch when earlier attempts never produced a verdict.""" + bash = shutil.which("bash") + jq = shutil.which("jq") + assert bash is not None and jq is not None + + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + script = _extract_run_block(workflow, DISPATCH_STEP_NAME) + head_sha = "b" * 40 + base_sha = "a" * 40 + + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + dispatch_body = tmp_path / "dispatch.json" + + fake_gh = fake_bin / "gh" + fake_gh.write_text( + "#!/usr/bin/env bash\n" + "set -euo pipefail\n" + 'test "$1" = api\n' + "shift\n" + 'if [ "${1:-}" = "-X" ]; then\n' + ' test "$2" = POST\n' + ' test "$3" = "repos/ContextualWisdomLab/.github/dispatches"\n' + ' cat >"$FAKE_DISPATCH_BODY"\n' + " exit 0\n" + "fi\n" + 'case "$1" in\n' + " */pulls/*) printf '%s\\n' \"$FAKE_PULL_JSON\" ;;\n" + " */statuses) printf '%s\\n' '[]' ;;\n" + " *) exit 1 ;;\n" + "esac\n", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + + fake_curl = fake_bin / "curl" + fake_curl.write_text( + "#!/usr/bin/env bash\n" + "set -euo pipefail\n" + 'last="${@: -1}"\n' + 'case "$last" in\n' + " *audience=opencode-github-action) printf '%s\\n' '{\"value\":\"oidc-token\"}' ;;\n" + " */exchange_github_app_token) printf '%s\\n' '{\"token\":\"app-token\"}' ;;\n" + " *) exit 1 ;;\n" + "esac\n", + encoding="utf-8", + ) + fake_curl.chmod(0o755) + + output = tmp_path / "github-output" + env = { + **os.environ, + "PATH": f"{fake_bin}:{os.environ['PATH']}", + "FAKE_PULL_JSON": json.dumps({"head": {"sha": head_sha}, "state": "open"}), + "FAKE_DISPATCH_BODY": str(dispatch_body), + "GH_TOKEN": "leaf-token", + "OIDC_AUDIENCE": "opencode-github-action", + "OPENCODE_API_BASE_URL": "https://api.opencode.ai", + "TARGET_REPOSITORY": "ContextualWisdomLab/accounting-information-platform", + "PR_NUMBER": "49", + "PR_BASE_REF": "develop", + "PR_BASE_SHA": base_sha, + "PR_HEAD_REF": "fix/restore-accounting-doc-ci-evidence", + "PR_HEAD_SHA": head_sha, + "LANGUAGE": "python", + "BUILD_MODE": "none", + "RUN_ATTEMPT": "3", + "REQUIRED_RUN_ID": "33890965185", + "REQUIRED_JOB_ID": "101220582747", + "ACTIONS_ID_TOKEN_REQUEST_TOKEN": "request-token", + "ACTIONS_ID_TOKEN_REQUEST_URL": "https://oidc.example/token", + "GITHUB_OUTPUT": str(output), + } + + result = subprocess.run( + [bash], + input=script, + text=True, + capture_output=True, + check=False, + env=env, + timeout=60, + ) + + assert result.returncode == 0, result.stderr + assert "verdict=pending" in output.read_text(encoding="utf-8") + payload = json.loads(dispatch_body.read_text(encoding="utf-8")) + assert payload["event_type"] == "codeql-scan" + client_payload = payload["client_payload"] + assert client_payload["target_repository"] == "ContextualWisdomLab/accounting-information-platform" + assert client_payload["pr_head_sha"] == head_sha + assert client_payload["required_run_id"] == "33890965185" + assert client_payload["required_job_id"] == "101220582747" + assert client_payload["required_language"] == "python" From fabc998d3efffee61e288e14624d5a933026b2ff Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 16:35:16 +0900 Subject: [PATCH 02/16] fix(codeql): recover reruns with no dispatch verdict --- .github/workflows/codeql-pr.yml | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/.github/workflows/codeql-pr.yml b/.github/workflows/codeql-pr.yml index cb07ad2fab..b4ff2d4bb9 100644 --- a/.github/workflows/codeql-pr.yml +++ b/.github/workflows/codeql-pr.yml @@ -6,7 +6,10 @@ # codeql-scan-dispatch.yml (which runs natively, unrestricted, in # 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. +# that exact failed job. On rerun the shard consumes an authenticated terminal +# status when one exists. If earlier attempts never reached dispatch and no +# authenticated verdict exists, the rerun may dispatch the same exact shard; +# the central target/PR/language concurrency lane bounds duplicate recovery. # Design: # docs/adr/0025-codeql-required-workflow-dispatch-architecture.md. The # merge-preview scan (analyze-merge) is required nowhere (PR #1766) and was @@ -158,9 +161,11 @@ jobs: steps: - name: Request current-head CodeQL scan dispatch # 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. + # run, job, language, and head identity. The shard intentionally fails + # after dispatch so its runner is released; the trusted handler later + # reruns that exact failed job after publishing a terminal verdict. + # Rerun attempt count is not a dispatch receipt: earlier attempts can + # be cancelled before runner assignment and execute zero steps. id: dispatch if: needs.detect-languages.outputs.code == 'true' env: @@ -215,10 +220,6 @@ jobs: 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." From e47de34288fba93b990a39031503c3e59caeb680 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 16:36:24 +0900 Subject: [PATCH 03/16] docs(codeql): trace pre-runner rerun recovery evidence --- ...-rerun-pre-runner-cancellation-recovery.md | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 docs/doctoring/codeql-rerun-pre-runner-cancellation-recovery.md diff --git a/docs/doctoring/codeql-rerun-pre-runner-cancellation-recovery.md b/docs/doctoring/codeql-rerun-pre-runner-cancellation-recovery.md new file mode 100644 index 0000000000..3a5952616b --- /dev/null +++ b/docs/doctoring/codeql-rerun-pre-runner-cancellation-recovery.md @@ -0,0 +1,60 @@ +# CodeQL rerun recovery after pre-runner cancellation + +## Problem and exact evidence + +The required `CodeQL PR` workflow used `github.run_attempt != 1` as if it proved that an earlier attempt had successfully dispatched the native CodeQL scan. That inference is false when an earlier attempt is cancelled before runner assignment. + +`ContextualWisdomLab/accounting-information-platform` PR #49 provides the concrete reproduction on exact head `065f9ab7038bf35db4ef129827de6ab8ee6a1038`, workflow run `33890965185`. + +- Attempt 1 `Detect CodeQL languages` job `101082241642` ended `cancelled` with `runner_id=0` and `steps=[]`; its downstream compatibility job was also cancelled without execution. +- Attempt 2 `Detect CodeQL languages` job `101128192785` ended the same way: `cancelled`, `runner_id=0`, `steps=[]`; the downstream compatibility job again never executed. +- Attempt 3 finally obtained runners. The `actions` shard job `101220582725` and `python` shard job `101220582747` reached `Request current-head CodeQL scan dispatch`, found no authenticated `codeql-dispatch/` terminal status, then failed solely because `RUN_ATTEMPT=3`. +- The target exact head had no `codeql-dispatch/actions` or `codeql-dispatch/python` commit status. Thus the attempt number did not identify a prior dispatch receipt or a terminal scan verdict. + +This leaves an unchanged PR head permanently unable to obtain the required CodeQL result even after runner capacity recovers. + +## Chosen repair + +Keep the existing trust sequence: + +1. re-read the live pull request and reject closed or moved heads; +2. read only `codeql-dispatch/` statuses created by the expected `opencode-agent` identity; +3. if an authenticated terminal status exists, reflect it without dispatching; +4. otherwise validate the exact required run/job identity, obtain the OIDC-bound app token, and dispatch the exact repository/PR/head/language shard. + +Remove the `RUN_ATTEMPT != 1` veto. A rerun attempt number is execution metadata, not evidence that the dispatch step ever ran. The native handler already serializes the same target-repository / pull-request / language tuple and re-validates live PR and wake identity before publishing a verdict or rerunning the exact required job. + +This does not convert a missing CodeQL verdict to success. The required shard still fails with `verdict=pending` after dispatch and becomes successful only when the trusted handler publishes an authenticated terminal `success` status and reruns the exact job. A forged status, stale head, failed/error verdict, unavailable OIDC/app token, malformed run/job identity, or absent dispatch receipt remains fail closed. + +## Executable regression + +`tests/test_codeql_pr_rerun_recovery_contract.py` executes the production `Request current-head CodeQL scan dispatch` Bash block with: + +- `RUN_ATTEMPT=3`; +- the same live target head; +- no authenticated CodeQL status; +- mocked OIDC and app-token exchange boundaries; and +- an exact run/job/language wake identity matching the accounting-platform reproduction. + +The test requires the step to publish `verdict=pending` and to emit a `codeql-scan` repository-dispatch payload bound to `ContextualWisdomLab/accounting-information-platform`, PR #49, the exact head, run `33890965185`, job `101220582747`, and `python`. + +Before the production change, the real shell block exits at the attempt-number guard before OIDC or dispatch, so this regression is RED for the observed reason. After the guard is removed, the same shell block reaches the bounded dispatch path. + +## Risks, rollback, and acceptance + +A manually requested rerun while a prior native dispatch is still queued but has not yet published a terminal status may replace work in the existing central target/PR/language concurrency lane. This is bounded to the same exact logical shard and does not broaden repository, head, language, credential, or merge authority. If live evidence shows harmful restart churn, the successor design should add an authenticated dispatch-receipt/pending state rather than restoring attempt-number inference. + +Rollback is not `RUN_ATTEMPT != 1`; that recreates the proven dead end. A valid replacement must distinguish “prior dispatch accepted” from “prior attempt never executed” using authenticated evidence and retain exact-head fail-closed semantics. + +GREEN requires all of the following on one unchanged successor head: + +- the focused rerun-recovery regression passes; +- the existing `test_codeql_pr_workflow_contract.py` suite remains green; +- the complete central test, 100% coverage, docstring, workflow syntax, security and review gates pass; +- after protected integration, the unchanged accounting-platform PR #49 head is rerun and obtains a real authenticated terminal CodeQL verdict without provider/model or leaf-repository workaround. + +## References + +GitHub. (2026). *Re-running workflows and jobs*. GitHub Docs. https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs + +GitHub. (2026). *REST API endpoints for workflow runs*. GitHub Docs. https://docs.github.com/en/rest/actions/workflow-runs From 67a6bc95ac9a85cdf59c0f5ce08ae0a09c5a3980 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 17:54:43 +0900 Subject: [PATCH 04/16] docs(codeql): align ADR with evidence-driven rerun recovery --- ...required-workflow-dispatch-architecture.md | 67 ++++++++++++++++--- 1 file changed, 58 insertions(+), 9 deletions(-) diff --git a/docs/adr/0025-codeql-required-workflow-dispatch-architecture.md b/docs/adr/0025-codeql-required-workflow-dispatch-architecture.md index 065a9d4d0f..495f473d4d 100644 --- a/docs/adr/0025-codeql-required-workflow-dispatch-architecture.md +++ b/docs/adr/0025-codeql-required-workflow-dispatch-architecture.md @@ -96,14 +96,18 @@ codeql-pr.yml (required workflow, runs in target repo context) does) before dispatching. analyze-head (matrix) -- SAME REQUIRED-CHECK NAME: "CodeQL compatibility analysis (${{ matrix.language }})". - 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. + No codeql-action reference. Each invocation + first consumes a trusted terminal + codeql-dispatch/ status for the exact + current head when one exists. Otherwise it + validates the exact required run/job/language + identity, dispatches that one shard, records + verdict=pending, and fails intentionally to + release the runner. The trusted handler later + publishes the terminal status and reruns only + that failed job. A later run_attempt is not + treated as proof that any earlier attempt + reached the dispatch step. .github/workflows/codeql-scan-dispatch.yml (NEW, runs natively in .github, NOT admitted through the ruleset, so codeql-action is unrestricted here) @@ -170,6 +174,36 @@ dispatch was rejected because the handler validates one shard and wakes one exact required job per run; changing that contract would enlarge the security and recovery surface without solving another observed need. +### Rerun recovery is evidence-driven, not attempt-driven + +`github.run_attempt` is execution metadata. It is not an authenticated receipt +that a previous attempt reached `Request current-head CodeQL scan dispatch`. +The concrete counterexample is +`ContextualWisdomLab/accounting-information-platform#49@065f9ab7038bf35db4ef129827de6ab8ee6a1038`, +required CodeQL run `33890965185`: attempts 1 and 2 were cancelled before +runner assignment (`runner_id=0`, `steps=[]`). Attempt 3 finally ran, found no +trusted terminal `codeql-dispatch/actions` or `codeql-dispatch/python` status, +and the former `RUN_ATTEMPT != 1` guard rejected both shards before dispatch. +The unchanged consumer head was therefore unable to recover after capacity +returned. + +The required workflow must instead use authenticated evidence. For the exact +live PR head and language shard, a terminal status created by the expected +central identity is consumed. If no such terminal verdict exists, the shard +re-validates its run/job/head identity and may dispatch again regardless of the +numeric attempt. The central target/repository/PR/language concurrency key +bounds duplicate recovery; the handler independently re-validates live PR and +wake identity before it publishes a verdict or reruns the exact job. Missing +evidence remains fail closed: redispatch produces `verdict=pending`, never a +synthetic success. + +A manually requested rerun can arrive while an earlier native dispatch is still +queued but has not published a terminal status. In that case the existing +concurrency lane may replace work for the same exact logical shard. This is a +bounded restart risk, not a reason to restore attempt-number inference. If +observed churn becomes material, the successor design must add an authenticated +pending/dispatch-receipt state keyed to the same exact identity. + ## Scope decision: `analyze-merge` is dropped, not migrated `analyze-merge` ("CodeQL merge preview") is confirmed, per PR #1766's own @@ -230,6 +264,10 @@ blocker for this one. documented, evidently deliberate platform limitation ("CodeQL requires configuration at the repository level"), not a bug report candidate. +- **Use run-attempt number as a dispatch receipt:** rejected after the AIP #49 + reproduction. Earlier attempts can be cancelled before any step executes, + so an attempt number cannot prove that a native scan was requested. Only + authenticated exact-head status/receipt evidence may suppress redispatch. ## Risks and effects @@ -250,6 +288,10 @@ blocker for this one. assert zero matches, as a permanent contract test) — re-adding it with the bug still present would recreate the exact org-wide 100%-startup_failure incident this ADR exists to prevent. +- Rerun recovery can replace an already queued same-shard native dispatch when + no terminal status exists yet. The concurrency key keeps that restart within + the exact repository/PR/language identity. If this causes material churn, + add an authenticated pending receipt rather than trusting run-attempt order. ## Follow-up @@ -261,7 +303,14 @@ blocker for this one. ADR is the record). 4. Add a permanent contract test asserting no `codeql-action` reference exists anywhere in `codeql-pr.yml`. -5. Only then, re-add `.github/workflows/codeql-pr.yml` to ruleset `18156473`'s +5. Keep the rerun-recovery regression that executes the production dispatch + shell with a later `run_attempt`, no trusted terminal verdict, and exact + run/job/head identity; it must reach bounded dispatch with `verdict=pending`. +6. After protected integration, rerun the unchanged AIP #49 head and require + real authenticated terminal `codeql-dispatch/actions` and + `codeql-dispatch/python` verdicts before treating the owner repair as + effective for that consumer. +7. Only then, re-add `.github/workflows/codeql-pr.yml` to ruleset `18156473`'s required `workflows` list (admin:org PUT, same mechanism used to remove it) and verify a real PR observes a successful, correctly-named required check before declaring this ADR's status Accepted. From acfa17e84f1ef6a0da5b93c642fcdf0d67d1d814 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 12:19:36 +0900 Subject: [PATCH 05/16] test(codeql): require complete status pagination before redispatch --- tests/test_codeql_pr_rerun_recovery_contract.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_codeql_pr_rerun_recovery_contract.py b/tests/test_codeql_pr_rerun_recovery_contract.py index 2551e57e68..cac1605e7b 100644 --- a/tests/test_codeql_pr_rerun_recovery_contract.py +++ b/tests/test_codeql_pr_rerun_recovery_contract.py @@ -111,3 +111,16 @@ def test_rerun_without_authenticated_verdict_can_redispatch(tmp_path: Path) -> N assert client_payload["required_run_id"] == "33890965185" assert client_payload["required_job_id"] == "101220582747" assert client_payload["required_language"] == "python" + + +def test_status_lookup_paginates_complete_history_before_redispatch() -> None: + """Recovery must inspect every commit-status page before treating verdict as absent.""" + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + script = _extract_run_block(workflow, DISPATCH_STEP_NAME) + + assert ( + 'gh api --paginate --slurp ' + '"repos/${TARGET_REPOSITORY}/commits/${PR_HEAD_SHA}/statuses?per_page=100"' + in script + ) + assert ".[][]" in script From 7628274f3e146e32fba124fe3e21e1fef8b107b3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 12:20:35 +0900 Subject: [PATCH 06/16] fix(codeql): inspect complete status history before redispatch --- .github/workflows/codeql-pr.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/codeql-pr.yml b/.github/workflows/codeql-pr.yml index b4ff2d4bb9..3c737aea90 100644 --- a/.github/workflows/codeql-pr.yml +++ b/.github/workflows/codeql-pr.yml @@ -201,10 +201,10 @@ jobs: exit 0 fi - statuses="$(gh api "repos/${TARGET_REPOSITORY}/commits/${PR_HEAD_SHA}/statuses")" + statuses="$(gh api --paginate --slurp "repos/${TARGET_REPOSITORY}/commits/${PR_HEAD_SHA}/statuses?per_page=100")" verdict_state="$(printf '%s' "$statuses" | jq -r --arg ctx "codeql-dispatch/${LANGUAGE}" ' [ - .[] + .[][] | select(.context == $ctx) | select( (.creator.login // "" | ascii_downcase) as $creator From 622ea74b8bd125a6383c2a1c89868844163ac568 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 12:21:14 +0900 Subject: [PATCH 07/16] docs(codeql): record complete status-history recovery guard --- ...-rerun-pre-runner-cancellation-recovery.md | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/docs/doctoring/codeql-rerun-pre-runner-cancellation-recovery.md b/docs/doctoring/codeql-rerun-pre-runner-cancellation-recovery.md index 3a5952616b..f257883d46 100644 --- a/docs/doctoring/codeql-rerun-pre-runner-cancellation-recovery.md +++ b/docs/doctoring/codeql-rerun-pre-runner-cancellation-recovery.md @@ -26,6 +26,16 @@ Remove the `RUN_ATTEMPT != 1` veto. A rerun attempt number is execution metadata This does not convert a missing CodeQL verdict to success. The required shard still fails with `verdict=pending` after dispatch and becomes successful only when the trusted handler publishes an authenticated terminal `success` status and reruns the exact job. A forged status, stale head, failed/error verdict, unavailable OIDC/app token, malformed run/job identity, or absent dispatch receipt remains fail closed. +## Follow-up review: complete status-history authority + +Current-head review on `e72ae30e3e989396b8cfdd1d850f7db1f45c6a7e` found a second defect in the same evidence boundary. `GET /commits/{sha}/statuses` was read without pagination. Treating an empty default response page as proof that no authenticated terminal `codeql-dispatch/` verdict exists is unsafe on a commit with enough status history to push an older trusted verdict to a later page. The recovery path could then redispatch even though terminal authority already existed. + +The rejected alternatives are increasing an assumed first-page size without pagination, trusting the combined commit-status summary, or restoring `RUN_ATTEMPT` inference. None proves absence of the exact creator-bound language status across the complete history. + +RED `acfa17e84f1ef6a0da5b93c642fcdf0d67d1d814` extends the focused contract to require a paginated, slurped status lookup and page-flattening before absence can authorize redispatch. Minimal repair `7628274f3e146e32fba124fe3e21e1fef8b107b3` changes only that read boundary: `gh api --paginate --slurp .../statuses?per_page=100` collects every page, and the existing trusted-context/creator filter runs across `.[][]`. Live PR/head validation, OIDC/app-token exchange, exact run/job/language binding, pending fail-closed behavior, handler validation and concurrency are unchanged. + +The security effect is narrower than “more reliable pagination”: **absence is now established over the complete status population before dispatch authority is exercised**. An authenticated terminal status on any page therefore prevents a redundant redispatch. If GitHub changes the status API representation, the focused regression must fail rather than silently fall back to first-page semantics. + ## Executable regression `tests/test_codeql_pr_rerun_recovery_contract.py` executes the production `Request current-head CodeQL scan dispatch` Bash block with: @@ -36,19 +46,21 @@ This does not convert a missing CodeQL verdict to success. The required shard st - mocked OIDC and app-token exchange boundaries; and - an exact run/job/language wake identity matching the accounting-platform reproduction. -The test requires the step to publish `verdict=pending` and to emit a `codeql-scan` repository-dispatch payload bound to `ContextualWisdomLab/accounting-information-platform`, PR #49, the exact head, run `33890965185`, job `101220582747`, and `python`. +The test requires the step to publish `verdict=pending` and to emit a `codeql-scan` repository-dispatch payload bound to `ContextualWisdomLab/accounting-information-platform`, PR #49, the exact head, run `33890965185`, job `101220582747`, and `python`. The companion status-history contract requires `--paginate --slurp`, an explicit `per_page=100`, and page flattening before the trusted verdict filter. -Before the production change, the real shell block exits at the attempt-number guard before OIDC or dispatch, so this regression is RED for the observed reason. After the guard is removed, the same shell block reaches the bounded dispatch path. +Before the production change, the original regression exits at the attempt-number guard before OIDC or dispatch. Before the pagination repair, the status-history contract fails because the production read asks only for the default first page. After both repairs, the same shell block reaches the bounded dispatch path only when the complete authenticated status history contains no terminal verdict. ## Risks, rollback, and acceptance A manually requested rerun while a prior native dispatch is still queued but has not yet published a terminal status may replace work in the existing central target/PR/language concurrency lane. This is bounded to the same exact logical shard and does not broaden repository, head, language, credential, or merge authority. If live evidence shows harmful restart churn, the successor design should add an authenticated dispatch-receipt/pending state rather than restoring attempt-number inference. -Rollback is not `RUN_ATTEMPT != 1`; that recreates the proven dead end. A valid replacement must distinguish “prior dispatch accepted” from “prior attempt never executed” using authenticated evidence and retain exact-head fail-closed semantics. +Pagination adds API reads proportional to commit-status history, bounded at 100 statuses per page. That cost is accepted because a false “verdict absent” decision authorizes external dispatch; status absence therefore requires complete evidence rather than a first-page heuristic. + +Rollback is not `RUN_ATTEMPT != 1` and not a non-paginated status read; either recreates a proven dead end or an incomplete-authority check. A valid replacement must distinguish “prior dispatch accepted” from “prior attempt never executed” using authenticated complete-history evidence and retain exact-head fail-closed semantics. GREEN requires all of the following on one unchanged successor head: -- the focused rerun-recovery regression passes; +- the focused rerun-recovery and complete-status-history regressions pass; - the existing `test_codeql_pr_workflow_contract.py` suite remains green; - the complete central test, 100% coverage, docstring, workflow syntax, security and review gates pass; - after protected integration, the unchanged accounting-platform PR #49 head is rerun and obtains a real authenticated terminal CodeQL verdict without provider/model or leaf-repository workaround. @@ -58,3 +70,5 @@ GREEN requires all of the following on one unchanged successor head: GitHub. (2026). *Re-running workflows and jobs*. GitHub Docs. https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs GitHub. (2026). *REST API endpoints for workflow runs*. GitHub Docs. https://docs.github.com/en/rest/actions/workflow-runs + +GitHub. (2026). *REST API endpoints for commit statuses*. GitHub Docs. https://docs.github.com/en/rest/commits/statuses From bf732f923a411d00251086d6e42e87f7e9988d7c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 12:22:49 +0900 Subject: [PATCH 08/16] test(codeql): exercise paginated empty-verdict recovery --- tests/test_codeql_pr_rerun_recovery_contract.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/test_codeql_pr_rerun_recovery_contract.py b/tests/test_codeql_pr_rerun_recovery_contract.py index cac1605e7b..50fff499af 100644 --- a/tests/test_codeql_pr_rerun_recovery_contract.py +++ b/tests/test_codeql_pr_rerun_recovery_contract.py @@ -43,9 +43,16 @@ def test_rerun_without_authenticated_verdict_can_redispatch(tmp_path: Path) -> N ' cat >"$FAKE_DISPATCH_BODY"\n' " exit 0\n" "fi\n" + 'if [ "${1:-}" = "--paginate" ]; then\n' + ' test "${2:-}" = "--slurp"\n' + ' case "${3:-}" in\n' + " */statuses?per_page=100) printf '%s\\n' '[[]]' ;;\n" + " *) exit 1 ;;\n" + " esac\n" + " exit 0\n" + "fi\n" 'case "$1" in\n' " */pulls/*) printf '%s\\n' \"$FAKE_PULL_JSON\" ;;\n" - " */statuses) printf '%s\\n' '[]' ;;\n" " *) exit 1 ;;\n" "esac\n", encoding="utf-8", From 4bf80b99b6908c0323ac406d7e30e8346e09a50d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 23:36:06 +0900 Subject: [PATCH 09/16] test(codeql): align verdict fixtures with paginated status reads Match exact gh arguments and page-shaped responses. Preserve trusted-publisher assertions and exercise second-page success and failure after a full page of forged statuses. Signed-off-by: Seongho Bae --- tests/test_codeql_pr_workflow_contract.py | 53 ++++++++++++++++++++--- 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/tests/test_codeql_pr_workflow_contract.py b/tests/test_codeql_pr_workflow_contract.py index 90612e9bc8..d9e6503e63 100644 --- a/tests/test_codeql_pr_workflow_contract.py +++ b/tests/test_codeql_pr_workflow_contract.py @@ -6,6 +6,8 @@ import sys from pathlib import Path +import pytest + from tests.test_opencode_workflow_shell_syntax import _extract_run_block @@ -108,7 +110,7 @@ def test_codeql_pr_dispatch_and_release_run_blocks_are_valid_bash() -> None: def _run_verdict_read( - tmp_path: Path, statuses: list[dict] + tmp_path: Path, statuses: list[dict], *, second_page: list[dict] | None = None ) -> tuple[subprocess.CompletedProcess[str], subprocess.CompletedProcess[str]]: """Execute the real one-shot status read and verdict enforcement blocks.""" bash = shutil.which("bash") @@ -129,11 +131,14 @@ def _run_verdict_read( "#!/usr/bin/env bash\n" "set -euo pipefail\n" 'test "$1" = api\n' - 'case "$2" in\n' - " */pulls/*) printf '%s\\n' \"$FAKE_PULL_JSON\" ;;\n" - " */statuses) printf '%s\\n' \"$FAKE_STATUSES_JSON\" ;;\n" - " *) exit 1 ;;\n" - "esac\n", + 'if [ "$#" = 2 ] && [ "$2" = "repos/ContextualWisdomLab/naruon/pulls/42" ]; then\n' + " printf '%s\\n' \"$FAKE_PULL_JSON\"\n" + 'elif [ "$#" = 4 ] && [ "$2" = --paginate ] && [ "$3" = --slurp ] &&\n' + ' [ "$4" = "repos/ContextualWisdomLab/naruon/commits/${PR_HEAD_SHA}/statuses?per_page=100" ]; then\n' + " printf '%s\\n' \"$FAKE_STATUSES_JSON\"\n" + "else\n" + " exit 1\n" + "fi\n", encoding="utf-8", ) fake_gh.chmod(0o755) @@ -143,7 +148,9 @@ def _run_verdict_read( **os.environ, "PATH": f"{fake_bin}:{os.environ['PATH']}", "FAKE_PULL_JSON": json.dumps(live_pr), - "FAKE_STATUSES_JSON": json.dumps(statuses), + "FAKE_STATUSES_JSON": json.dumps( + [statuses] if second_page is None else [statuses, second_page] + ), "GH_TOKEN": "fake-token", "TARGET_REPOSITORY": "ContextualWisdomLab/naruon", "PR_NUMBER": "42", @@ -162,6 +169,7 @@ def _run_verdict_read( [bash], input=dispatch_script, text=True, capture_output=True, check=False, env=dispatch_env, timeout=60, ) + assert dispatch_result.returncode == 0, dispatch_result.stderr output_values = dict( line.split("=", 1) for line in output.read_text(encoding="utf-8").splitlines() ) @@ -223,6 +231,37 @@ def test_codeql_pr_one_shot_read_accepts_the_opencode_agent_creator(tmp_path: Pa assert "Current-head CodeQL dispatch verdict for python: success." in verdict_result.stdout +@pytest.mark.parametrize("state,exit_code", [("success", 0), ("failure", 1)]) +def test_codeql_pr_reads_trusted_verdict_on_second_page( + tmp_path: Path, state: str, exit_code: int +) -> None: + """A full first page of forged successes cannot hide a later trusted verdict.""" + dispatch_result, verdict_result = _run_verdict_read( + tmp_path, + statuses=[ + { + "context": "codeql-dispatch/python", + "state": "success", + "creator": {"login": "attacker"}, + } + for _ in range(100) + ], + second_page=[ + { + "context": "codeql-dispatch/python", + "state": state, + "creator": {"login": "opencode-agent[bot]"}, + } + ], + ) + assert dispatch_result.returncode == 0, dispatch_result.stderr + assert verdict_result.returncode == exit_code, verdict_result.stderr + if state == "success": + assert "Current-head CodeQL dispatch verdict for python: success." in verdict_result.stdout + else: + assert "did not pass (state=failure)" in verdict_result.stdout + + def test_codeql_action_steps_use_one_version_per_workflow() -> None: """Prevent CodeQL init/analyze version splits from failing the scheduled scan.""" workflow = (REPO_ROOT / ".github/workflows/scheduled-security-scan.yml").read_text( From 951d0ecd1b5398a9eac293a13bba220a6528df24 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 23:44:17 +0900 Subject: [PATCH 10/16] fix(scheduler): require explicit open live PR identity Request PR state in GraphQL and preserve it in REST normalization. Reject missing state and empty or malformed heads before OpenCode dispatch, Strix dispatch, or Strix job rerun. Preserve explicit positive fixtures and add fail-closed regressions. Focused RED: 17 failed, 19 passed; final scheduler regressions: 380 passed under both normal and GITHUB_ACTIONS=true environments with warnings treated as errors. No dispatch, permission, queue, or cancellation policy changes. Signed-off-by: Seongho Bae --- ...duler-explicit-open-live-dispatch-guard.md | 48 ++++++++ scripts/ci/pr_review_merge_scheduler_core.py | 13 ++- tests/test_pr_review_merge_scheduler.py | 1 + tests/test_scheduler_live_dispatch_guard.py | 107 ++++++++++++++++++ tests/test_strix_rerun_job_selection.py | 2 + 5 files changed, 168 insertions(+), 3 deletions(-) create mode 100644 docs/doctoring/scheduler-explicit-open-live-dispatch-guard.md create mode 100644 tests/test_scheduler_live_dispatch_guard.py diff --git a/docs/doctoring/scheduler-explicit-open-live-dispatch-guard.md b/docs/doctoring/scheduler-explicit-open-live-dispatch-guard.md new file mode 100644 index 0000000000..333adfccea --- /dev/null +++ b/docs/doctoring/scheduler-explicit-open-live-dispatch-guard.md @@ -0,0 +1,48 @@ +# Scheduler의 명시적 OPEN·현재 head 확인 + +## 원인과 범위 + +#1902의 후속 조사에서 CodeQL 복구 primitive보다 먼저 고칠 공통 결함을 확인했다. +`4bf80b99b6908c0323ac406d7e30e8346e09a50d`의 +`scripts/ci/pr_review_merge_scheduler_core.py`는 GraphQL 공통 PR fragment에서 +`state`를 요청하지 않았고, REST PR 정규화에서도 그 필드를 보존하지 않았다. +그런데 `live_dispatch_head_matches`는 누락되거나 빈 state를 OPEN으로 취급했다. +단일 PR 조회는 닫힌 PR도 반환하므로 head가 그대로면 닫힌 PR을 허용할 수 있었다. +또한 양쪽 head를 빈 문자열로 대체해 비교했으므로 빈 값끼리도 일치했다. + +영향 범위는 OpenCode repository dispatch, Strix의 기존 job rerun, +Strix repository dispatch 직전의 공통 guard다. 이번 수정은 이 세 경로의 +새 실행 요청을 막는 조건만 다룬다. 앞서 수행되는 stale-run cleanup의 순서나 +cancellation 정책은 바꾸지 않는다. + +## 수정 + +- GraphQL 공통 fragment가 PR state를 실제로 요청한다. +- REST fallback은 원본 state를 대문자로 보존하고, 누락은 빈 값으로 남긴다. +- guard는 정확히 한 PR, 명시적 `OPEN`, 양쪽의 문자열 타입 40자리 hex SHA, + 대소문자를 제외한 동일 head를 모두 요구한다. +- 기존 정상 fixture는 `OPEN`을 명시한다. 누락 사례를 정상 fixture로 대체하지 않는다. + +토큰, 권한, trigger, queue, concurrency, dispatch payload는 변경하지 않았다. +CodeQL primitive도 추가하지 않았다. 조회 직후 PR 상태가 바뀔 수 있는 경쟁 조건과 +중복 전송의 원자성은 여전히 미해결이며, 이 guard는 exact-once 보장이 아니다. +Cross-repo target callback의 Actions-write 권한도 별도 미해결 조건이다. + +## 회귀 검증 + +`tests/test_scheduler_live_dispatch_guard.py`는 실제 guard와 세 caller를 실행하고 +외부 API 및 실행 요청만 대체한다. 누락·빈 값·CLOSED·MERGED·UNKNOWN은 dispatch와 +rerun에 도달하지 않아야 하며, OPEN의 정상 경로는 계속 도달해야 한다. +별도 사례가 빈 값, 잘못된 길이, 비-hex, 비문자열, 서로 다른 SHA를 거부하고 +GraphQL 실제 query와 REST fallback의 state 전달을 확인한다. + +Production 수정 전 새 회귀는 17 failed / 19 passed였다. 이후 실제 live head만 +잘못된 사례 두 건도 추가했다. 최종 관련 5파일은 `-W error`를 적용해 정상 환경에서 +380 passed, `GITHUB_ACTIONS=true` 환경에서도 380 passed를 확인했다. +검증 명령은 다음과 같다. + +```sh +python -m pytest -q -W error tests/test_scheduler_live_dispatch_guard.py tests/test_pr_review_merge_scheduler.py tests/test_strix_rerun_job_selection.py tests/test_repository_branch_coverage_review_schedulers.py tests/test_pr_review_fix_scheduler_rest_workflow_identity.py +``` + +로컬 회귀 통과는 실제 GitHub dispatch, protected merge, 대상 job 복구의 증거가 아니다. diff --git a/scripts/ci/pr_review_merge_scheduler_core.py b/scripts/ci/pr_review_merge_scheduler_core.py index c4e9d28ebd..839b40812b 100644 --- a/scripts/ci/pr_review_merge_scheduler_core.py +++ b/scripts/ci/pr_review_merge_scheduler_core.py @@ -167,17 +167,23 @@ def review_dispatch_admitted(component: str, repo: str, pr: dict[str, Any]) -> b def live_dispatch_head_matches(repo: str, pr: dict[str, Any]) -> bool: """Re-read the authoritative PR immediately before an Actions side effect.""" live = fetch_pr(validate_github_repository(repo), int(pr["number"])) + expected_head = pr.get("headRefOid") + live_head = live[0].get("headRefOid") if len(live) == 1 else None return ( len(live) == 1 - and str(live[0].get("state") or "OPEN").upper() == "OPEN" - and str(live[0].get("headRefOid") or "").lower() - == str(pr.get("headRefOid") or "").lower() + and live[0].get("state") == "OPEN" + and isinstance(expected_head, str) + and isinstance(live_head, str) + and GIT_SHA_RE.fullmatch(expected_head) is not None + and GIT_SHA_RE.fullmatch(live_head) is not None + and live_head.lower() == expected_head.lower() ) PULL_REQUEST_FIELDS_FRAGMENT = """\ fragment SchedulerPullRequestFields on PullRequest { number + state title author { login } isDraft @@ -1341,6 +1347,7 @@ def rest_pr_node(repo: str, pr: dict[str, Any]) -> dict[str, Any]: ) return { "number": number, + "state": str(pr.get("state") or "").upper(), "title": pr.get("title"), "author": {"login": ((pr.get("user") or {}).get("login"))}, "isDraft": bool(pr.get("draft")), diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 2cbda7f85b..4d979086c6 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -60,6 +60,7 @@ def fake_fine_grained_github_token(body): def make_pr(**overrides): value = { "number": 1, + "state": "OPEN", "title": "Central review", "author": {"login": "pull-request-author"}, "isDraft": False, diff --git a/tests/test_scheduler_live_dispatch_guard.py b/tests/test_scheduler_live_dispatch_guard.py new file mode 100644 index 0000000000..35e6345a2d --- /dev/null +++ b/tests/test_scheduler_live_dispatch_guard.py @@ -0,0 +1,107 @@ +"""Fail-closed live PR evidence before review dispatch or exact Strix rerun.""" + +import pytest + +from scripts.ci import pr_review_merge_scheduler as sched + + +HEAD = "a" * 40 + + +def candidate(): + """Return explicit open PR metadata with canonical refs and SHAs.""" + return { + "number": 7, "state": "OPEN", "headRefOid": HEAD, + "baseRefOid": "b" * 40, "baseRefName": "main", "headRefName": "feature", + } + + +@pytest.mark.parametrize("caller", ["opencode", "strix-dispatch", "strix-rerun"]) +@pytest.mark.parametrize("state", [None, "", "CLOSED", "MERGED", "UNKNOWN", "OPEN"]) +def test_live_state_gates_all_three_side_effects(monkeypatch, caller, state): + """Only an explicitly open live PR may reach any guarded side effect.""" + pr = candidate() + live = candidate() + if state is None: + live.pop("state") + else: + live["state"] = state + effects = [] + monkeypatch.setattr(sched, "fetch_pr", lambda *_: [live]) + monkeypatch.setattr(sched, "require_github_actions_control_actor", lambda *_: None) + monkeypatch.setattr(sched, "review_dispatch_admitted", lambda *_: True) + monkeypatch.setattr(sched, "active_opencode_run_refs", lambda *_: ([], [])) + monkeypatch.setattr(sched, "active_review_run_refs", lambda *_, **__: ([], [])) + monkeypatch.setattr(sched, "_cancel_revalidated_review_run_refs", lambda *_: ([], [])) + monkeypatch.setattr(sched, "active_workflow_runs", lambda *_: []) + monkeypatch.setattr(sched, "complete_paginated_pr_contexts", lambda *_: None) + monkeypatch.setattr(sched, "matching_actions_run_id", lambda *_: None) + monkeypatch.setattr(sched, "discover_opencode_required_run_id", lambda *_: None) + monkeypatch.setattr(sched, "matching_actions_job_id", lambda *_: "202" if caller == "strix-rerun" else None) + monkeypatch.setattr(sched, "repository_dispatch_target", lambda _: "ContextualWisdomLab/.github") + monkeypatch.setattr(sched, "run_github_dispatch", lambda *_, **__: effects.append("dispatch")) + monkeypatch.setattr(sched, "rerun_actions_job", lambda *_, **__: effects.append("rerun")) + dispatch = sched.dispatch_opencode_review if caller == "opencode" else sched.dispatch_strix_evidence + result = dispatch("owner/repo", "review", pr, dry_run=False) + if state == "OPEN": + assert result == ("rerun" if caller == "strix-rerun" else "dispatched") + assert effects == ["rerun" if caller == "strix-rerun" else "dispatch"] + else: + assert result == "stale_head" + assert effects == [] + + +@pytest.mark.parametrize("expected,observed,accepted", [ + (HEAD, HEAD, True), (HEAD.upper(), HEAD, True), + (HEAD, "b" * 40, False), ("", "", False), (None, None, False), + ("bad", "bad", False), ("g" * 40, "g" * 40, False), + ("a" * 39, "a" * 39, False), ("a" * 41, "a" * 41, False), + (123, 123, False), (HEAD, None, False), (None, HEAD, False), + (HEAD, "g" * 40, False), (HEAD, "", False), +]) +def test_live_guard_requires_two_canonical_matching_heads(monkeypatch, expected, observed, accepted): + """Equal empty, malformed, or non-string heads are not identity evidence.""" + pr = candidate() + pr["headRefOid"] = expected + monkeypatch.setattr(sched, "fetch_pr", lambda *_: [{"state": "OPEN", "headRefOid": observed}]) + assert sched.live_dispatch_head_matches("owner/repo", pr) is accepted + + +@pytest.mark.parametrize("rows", [[], [candidate(), candidate()]]) +def test_live_guard_rejects_missing_or_ambiguous_pr(monkeypatch, rows): + """The live lookup must identify exactly one PR.""" + monkeypatch.setattr(sched, "fetch_pr", lambda *_: rows) + assert not sched.live_dispatch_head_matches("owner/repo", candidate()) + + +def test_exact_graphql_fetch_requests_pr_state(monkeypatch): + """The actual single-PR query must request state in its shared fragment.""" + def graphql(query, **fields): + fragment = query.split("fragment SchedulerPullRequestFields on PullRequest {", 1)[1] + assert " state" in fragment.split(" author", 1)[0].splitlines() + assert fields == {"owner": "owner", "name": "repo", "number": 7} + return {"data": {"repository": {"pullRequest": {**candidate(), "state": "CLOSED"}}}} + + monkeypatch.setattr(sched, "gh_graphql", graphql) + monkeypatch.setattr(sched, "complete_all_pr_reviews", lambda *_: None) + monkeypatch.setattr(sched, "enrich_rest_mergeable_states", lambda *_: None) + assert sched.fetch_pr("owner/repo", 7)[0]["state"] == "CLOSED" + assert not sched.live_dispatch_head_matches("owner/repo", candidate()) + + +@pytest.mark.parametrize("state,normalized", [("open", "OPEN"), ("closed", "CLOSED"), (None, "")]) +def test_rest_normalization_preserves_state_without_open_default(monkeypatch, state, normalized): + """REST fallback must retain closure and must not manufacture open state.""" + pr = {"number": 7, "head": {"sha": HEAD}} + if state is not None: + pr["state"] = state + responses = { + "repos/owner/repo/pulls/7": pr, + f"repos/owner/repo/commits/{HEAD}/check-runs?per_page=100": {}, + f"repos/owner/repo/commits/{HEAD}/check-suites?per_page=100": {}, + f"repos/owner/repo/commits/{HEAD}/status": {}, + "repos/owner/repo/pulls/7/files?per_page=20": [], + } + monkeypatch.setattr(sched, "gh_api_json", lambda endpoint: responses[endpoint]) + monkeypatch.setattr(sched, "fetch_all_pr_reviews_rest", lambda *_: []) + assert sched.fetch_pr_rest("owner/repo", 7)[0]["state"] == normalized diff --git a/tests/test_strix_rerun_job_selection.py b/tests/test_strix_rerun_job_selection.py index c1926b2ce3..23a09891dd 100644 --- a/tests/test_strix_rerun_job_selection.py +++ b/tests/test_strix_rerun_job_selection.py @@ -23,6 +23,8 @@ def test_dispatch_strix_reruns_scan_job_not_sibling_publisher(monkeypatch) -> No """A skipped status-publisher sibling must never be selected as the Strix rerun target.""" pr = { "number": 1055, + "state": "OPEN", + "headRefOid": "a" * 40, "statusCheckRollup": { "contexts": { "nodes": [ From 9bfe575aebe39c756f772c1fbdbba3f36e471d6d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 00:19:33 +0900 Subject: [PATCH 11/16] fix(scheduler): bind Strix reruns to verified job identity Validate selected check, job, run, workflow and publisher before rerunning Strix. Preserve PR-target base-SHA executions through association and target-title checks; defer dispatch runs without authenticated target provenance. Local mock-only regressions: 402 passed in normal and CI environments with warnings treated as errors. No token, permission, queue or cancellation changes. Signed-off-by: Seongho Bae --- .../strix-rerun-job-identity-binding.md | 52 +++++++ scripts/ci/pr_review_merge_scheduler_core.py | 92 ++++++++++- tests/test_pr_review_merge_scheduler.py | 17 +++ tests/test_scheduler_live_dispatch_guard.py | 2 + tests/test_strix_job_binding.py | 144 ++++++++++++++++++ tests/test_strix_rerun_job_selection.py | 2 + 6 files changed, 308 insertions(+), 1 deletion(-) create mode 100644 docs/doctoring/strix-rerun-job-identity-binding.md create mode 100644 tests/test_strix_job_binding.py diff --git a/docs/doctoring/strix-rerun-job-identity-binding.md b/docs/doctoring/strix-rerun-job-identity-binding.md new file mode 100644 index 0000000000..039c22d919 --- /dev/null +++ b/docs/doctoring/strix-rerun-job-identity-binding.md @@ -0,0 +1,52 @@ +# Strix 재실행 대상 job의 신원 결합 + +## 확인한 결함 + +현재 main `ee5567f7b15f0441a61ec2435415603b9518f1c6`과 #1902의 +`951d0ecd1b5398a9eac293a13bba220a6528df24`에서 Strix 재실행 선택 경로를 비교했다. +관련 core 차이는 이전 OPEN-state 수리뿐이었다. 이번 작업은 951 위에서 진행하며 +main을 merge하거나 다른 세션의 workflow 변경을 덮어쓰지 않았다. + +기존 선택기는 check의 details URL에서 job ID만 추출했다. 직전 live guard는 +PR snapshot이 최신인지 확인했지만, 선택한 job이 그 PR head를 스캔했는지는 +확인하지 않았다. 로컬 mock-only 회귀에서 실제 caller와 rerun wrapper를 실행한 +결과 8 failed / 2 passed였다. 실패 사례는 job/run 조회 없이 mock POST에 도달했다. +실제 GitHub 위조 요청이나 job 재실행을 실행한 결과가 아니다. + +## 최소 수리와 보류 조건 + +기존 selector와 API 조회 helper를 유지하고 Strix rerun 분기에 검증 하나를 추가했다. +GraphQL과 REST 정규화는 selected check의 database ID를 보존한다. + +- selected check URL은 같은 repo의 정확한 run/job을 지정해야 한다. +- 실제 job의 ID, run ID, 이름, 완료 상태와 재실행 가능한 실패 결론을 확인한다. + 현재 허용 결론은 failure, cancelled, timed_out이다. 다른 결론은 자동 재실행을 보류한다. +- job이 가리키는 실제 check ID가 selected check와 같아야 한다. Check publisher는 + github-actions여야 하며 check suite와 run의 연결도 일치해야 한다. +- 실제 run과 workflow 조회는 같은 repo의 `.github/workflows/strix.yml`, + `Strix Security Scan` 이름을 확인한다. +- pull_request_target은 정확히 하나의 PR association, base/head repository, + association의 PR head, event에서 생성한 정확한 run-name이 모두 일치해야 한다. + job/run의 top-level head_sha가 base SHA인 정상 사례를 허용한다. 이 필드를 + PR head로 간주하지 않는다. 누락되거나 상충하는 repository 식별자는 거부한다. +- repository_dispatch는 제어 코드의 실행 SHA만으로 target head를 증명할 수 없다. + 이 경로에는 인증된 target receipt를 소비하는 계약이 없으므로, 제목이 맞더라도 + 자동 재실행을 보류한다. push 등 다른 event도 새로 허용하지 않는다. +- 검증이 끝난 뒤 live PR을 다시 확인한다. API 실패나 불완전한 metadata는 + `identity_unverified`로 보류하고 새 dispatch로 우회하지 않는다. 세 상위 caller도 + 이를 실행 완료가 아닌 wait로 보고한다. + +## 검증과 한계 + +`tests/test_strix_job_binding.py`는 실제 REST 정규화, selector, live guard, +dispatch caller, actor 검사, rerun wrapper를 실행한다. 외부 명령은 모두 mock 경계에서 +차단한다. 정상 대조군은 top-level base SHA와 PR head SHA, REST repository URL 형식을 +포함한다. 음성 사례는 stale·상충·누락·다른 repo/workflow/publisher/event·API 실패 및 +검증 중 head 이동을 포함한다. 정상 사례는 네 metadata GET과 단일 mock POST를 요구한다. + +기존 state-only, 명령형식, sibling 선택 테스트 세 곳은 각자의 검증 대상을 유지하도록 +새 guard만 국소적으로 대체했다. 신원 결합 자체는 별도 회귀에서 실제 구현을 사용한다. + +권한, 토큰 선택, actor allowlist, queue, concurrency, CodeQL primitive는 변경하지 않았다. +조회와 POST 사이의 원자성, cross-repo callback 권한, hosted 복구는 해결했다고 주장하지 +않는다. 신뢰할 provenance가 없는 역사적 run은 자동 복구가 보류될 수 있다. diff --git a/scripts/ci/pr_review_merge_scheduler_core.py b/scripts/ci/pr_review_merge_scheduler_core.py index 839b40812b..95ebb5dc14 100644 --- a/scripts/ci/pr_review_merge_scheduler_core.py +++ b/scripts/ci/pr_review_merge_scheduler_core.py @@ -232,6 +232,7 @@ def live_dispatch_head_matches(repo: str, pr: dict[str, Any]) -> bool: nodes { __typename ... on CheckRun { + databaseId name status conclusion @@ -312,7 +313,7 @@ def live_dispatch_head_matches(repo: str, pr: dict[str, Any]) -> bool: nodes { __typename ... on CheckRun { - name status conclusion startedAt detailsUrl + databaseId name status conclusion startedAt detailsUrl checkSuite { createdAt workflowRun { workflow { name } } } } ... on StatusContext { context state } @@ -1282,6 +1283,7 @@ def rest_check_node( workflow = {"name": workflow_name} if workflow_name else {} return { "__typename": "CheckRun", + "databaseId": check.get("id"), "name": check.get("name"), "status": (check.get("status") or "").upper(), "conclusion": (check.get("conclusion") or "").upper() if check.get("conclusion") else None, @@ -2913,6 +2915,8 @@ def post_update_branch_followup( if wait_reason: return f"{head_note}; {wait_reason}" dispatch_result = dispatch_strix_evidence(repo, security_workflow, updated_pr, dry_run=dry_run) + if dispatch_result in {"identity_unverified", "stale_head"}: + return f"{head_note}; Strix rerun waits for verified current-target job identity" if dispatch_result == "admission_deferred": return f"{head_note}; bounded admission budget is exhausted" if dispatch_result == "already_running": @@ -3747,6 +3751,84 @@ def is_strix_scan_check_run(node: dict[str, Any]) -> bool: ) +def strix_rerun_identity_verified(repo: str, pr: dict[str, Any], job_id: str) -> bool: + """Bind a selected Strix job to authenticated native PR-target run evidence. + + Dispatch runs require target provenance beyond their control-plane SHA; + without an authenticated target receipt this path deliberately defers them. + A PR-target execution SHA may be the base SHA, so it is never used as the + target PR head. Association and trusted workflow run-name must agree instead. + """ + try: + repo = validate_github_repository(repo) + head = validate_git_sha(pr["headRefOid"]).lower() + head_repo = validate_github_repository(pr["headRepository"]["nameWithOwner"]) + if not re.fullmatch(r"[1-9][0-9]*", job_id): + return False + candidates = [node for node in context_nodes(pr) + if is_strix_scan_check_run(node) + and actions_job_id_from_details_url(node.get("detailsUrl")) == job_id] + if len(candidates) != 1: + return False + selected = candidates[0] + url_match = re.fullmatch( + rf"https://github\.com/{re.escape(repo)}/actions/runs/([1-9][0-9]*)/job/{job_id}", + selected.get("detailsUrl") or "", + ) + if not url_match: + return False + run_id = url_match.group(1) + job = gh_api_json(f"repos/{repo}/actions/jobs/{job_id}") + if (job.get("id") != int(job_id) or job.get("run_id") != int(run_id) + or job.get("name") != "strix" or job.get("status") != "completed" + or job.get("conclusion") not in {"failure", "cancelled", "timed_out"}): + return False + check_match = re.fullmatch( + rf"https://api\.github\.com/repos/{re.escape(repo)}/check-runs/([1-9][0-9]*)", + job.get("check_run_url") or "", + ) + if not check_match or selected.get("databaseId") != int(check_match.group(1)): + return False + check = gh_api_json(f"repos/{repo}/check-runs/{check_match.group(1)}") + run_data = gh_api_json(f"repos/{repo}/actions/runs/{run_id}") + if (check.get("id") != selected["databaseId"] or check.get("name") != "strix" + or check.get("app", {}).get("slug") != "github-actions" + or run_data.get("id") != int(run_id) + or run_data.get("repository", {}).get("full_name") != repo + or run_data.get("event") != "pull_request_target" + or run_data.get("status") != "completed" + or run_data.get("name") != "Strix Security Scan" + or run_data.get("path") != ".github/workflows/strix.yml" + or not check.get("check_suite", {}).get("id") + or check["check_suite"]["id"] != run_data.get("check_suite_id")): + return False + workflow_id = run_data.get("workflow_id") + if type(workflow_id) is not int or workflow_id <= 0: + return False + workflow = gh_api_json(f"repos/{repo}/actions/workflows/{workflow_id}") + if (workflow.get("id") != workflow_id + or workflow.get("name") != "Strix Security Scan" + or workflow.get("path") != ".github/workflows/strix.yml"): + return False + associations = run_data.get("pull_requests") or [] + if len(associations) != 1 or associations[0].get("number") != int(pr["number"]): + return False + association = associations[0] + for side, expected_repo in (("base", repo), ("head", head_repo)): + repository = association[side]["repo"] + if (not (repository.get("full_name") or repository.get("url")) + or (repository.get("full_name") is not None and repository["full_name"] != expected_repo) + or (repository.get("url") is not None + and repository["url"] != f"https://api.github.com/repos/{expected_repo}")): + return False + return ( + validate_git_sha(association["head"]["sha"]).lower() == head + and run_data.get("display_title") == f"Strix Security Scan {repo}#{pr['number']}@{head}" + ) + except (RuntimeError, ValueError, TypeError, KeyError, AttributeError): + return False + + def dispatch_strix_evidence(repo: str, workflow: str, pr: dict[str, Any], *, dry_run: bool) -> str: """Dispatch same-head Strix workflow evidence before OpenCode reviews.""" job_id = matching_actions_job_id(pr, is_strix_scan_check_run) @@ -3755,6 +3837,10 @@ def dispatch_strix_evidence(repo: str, workflow: str, pr: dict[str, Any], *, dry return "admission_deferred" if not dry_run and not live_dispatch_head_matches(repo, pr): return "stale_head" + if not dry_run and not strix_rerun_identity_verified(repo, pr, job_id): + return "identity_unverified" + if not dry_run and not live_dispatch_head_matches(repo, pr): + return "stale_head" rerun_actions_job(repo, job_id, dry_run=dry_run, action="rerun-strix-evidence") return "rerun" if not dry_run else "dry_run" if dry_run: @@ -4106,6 +4192,8 @@ def dispatch_draft_review_only( f"draft PR review-only dispatch; current head has no completed Strix evidence; {wait_reason}", ) dispatch_result = dispatch_strix_evidence(repo, security_workflow, pr, dry_run=dry_run) + if dispatch_result in {"identity_unverified", "stale_head"}: + return Decision(number, "wait", "Strix rerun waits for verified current-target job identity") if dispatch_result == "admission_deferred": return Decision(number, "wait", "draft PR review-only dispatch; bounded admission budget is exhausted") if dispatch_result == "already_running": @@ -4910,6 +4998,8 @@ def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decisio if wait_reason: return decide("wait", f"current head has no completed Strix evidence; {wait_reason}") dispatch_result = dispatch_strix_evidence(repo, security_workflow, pr, dry_run=dry_run) + if dispatch_result in {"identity_unverified", "stale_head"}: + return decide("wait", "Strix rerun waits for verified current-target job identity") if dispatch_result == "admission_deferred": return decide("wait", "bounded admission budget is exhausted") if dispatch_result == "already_running": diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 4d979086c6..60dd4249ea 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -4612,6 +4612,8 @@ def fake_run(args, stdin=None): } ) sched.dispatch_opencode_review("owner/repo", "OpenCode Review", required_workflow_pr, dry_run=False) + # Command-shape contract; selected-job binding is exercised separately. + monkeypatch.setattr(sched, "strix_rerun_identity_verified", lambda *_: True) sched.dispatch_strix_evidence("owner/repo", "Strix Security Scan", required_workflow_pr, dry_run=False) assert calls[:2] == [ [ @@ -7790,6 +7792,16 @@ def test_draft_pr_review_only_dispatch_waits_when_strix_already_running(monkeypa assert decision.reason == "draft PR review-only dispatch; same-head Strix evidence is still running" +@pytest.mark.parametrize("result", ["identity_unverified", "stale_head"]) +@pytest.mark.parametrize("draft", [False, True]) +def test_unverified_strix_rerun_is_reported_as_wait(monkeypatch, result, draft): + """A withheld rerun must never be reported as a successful security dispatch.""" + monkeypatch.setattr(sched, "dispatch_strix_evidence", lambda *_, **__: result) + decision = inspect(make_pr(isDraft=draft), allow_draft_review_dispatch=draft) + assert decision.action == "wait" + assert "verified current-target job identity" in decision.reason + + def test_draft_pr_review_only_dispatch_waits_when_repository_is_busy(monkeypatch): monkeypatch.setattr( sched, @@ -8199,6 +8211,11 @@ def followup(updated_pr, **overrides): statusCheckRollup={"contexts": {"nodes": [strix_check(status="IN_PROGRESS", conclusion="")]}}, ) ) + for withheld in ("identity_unverified", "stale_head"): + monkeypatch.setattr(sched, "dispatch_strix_evidence", lambda *_, **__: withheld) + assert "waits for verified current-target job identity" in followup( + make_pr(headRefOid="new-head") + ) assert "same-head OpenCode review is already running" in followup( make_pr( headRefOid="new-head", diff --git a/tests/test_scheduler_live_dispatch_guard.py b/tests/test_scheduler_live_dispatch_guard.py index 35e6345a2d..23b80aa3cb 100644 --- a/tests/test_scheduler_live_dispatch_guard.py +++ b/tests/test_scheduler_live_dispatch_guard.py @@ -30,6 +30,8 @@ def test_live_state_gates_all_three_side_effects(monkeypatch, caller, state): monkeypatch.setattr(sched, "fetch_pr", lambda *_: [live]) monkeypatch.setattr(sched, "require_github_actions_control_actor", lambda *_: None) monkeypatch.setattr(sched, "review_dispatch_admitted", lambda *_: True) + # This suite isolates live PR state; job provenance has its own real-caller suite. + monkeypatch.setattr(sched, "strix_rerun_identity_verified", lambda *_: True) monkeypatch.setattr(sched, "active_opencode_run_refs", lambda *_: ([], [])) monkeypatch.setattr(sched, "active_review_run_refs", lambda *_, **__: ([], [])) monkeypatch.setattr(sched, "_cancel_revalidated_review_run_refs", lambda *_: ([], [])) diff --git a/tests/test_strix_job_binding.py b/tests/test_strix_job_binding.py new file mode 100644 index 0000000000..09c880e4f1 --- /dev/null +++ b/tests/test_strix_job_binding.py @@ -0,0 +1,144 @@ +"""Mock-only Strix rerun identity contracts through the real scheduler caller.""" + +import json + +import pytest + +from scripts.ci import pr_review_merge_scheduler as sched + + +@pytest.mark.parametrize("case,allowed", [ + ("current-associated-head-top-level-base", True), + ("current-associated-head-top-level-head", True), + ("stale-associated-head", False), + ("stale-association-top-level-current", False), + ("contradictory-title", False), + ("missing-association", False), + ("foreign-details-repository", False), + ("different-workflow-path", False), + ("untrusted-check-publisher", False), + ("unrelated-push-event", False), + ("dispatch-without-target-receipt", False), + ("api-unavailable", False), + ("wrong-job-run", False), + ("wrong-check-suite", False), + ("contradictory-repository", False), + ("missing-check-id", False), + ("current-head-moves-during-binding", False), + ("repository-api-url-only", True), +]) +def test_actual_strix_rerun_caller_binds_selected_job(monkeypatch, case, allowed): + """Selected job/run provenance, not a current PR snapshot alone, authorizes rerun.""" + repo = "owner/repo" + current_head, stale_head, base_sha = "b" * 40, "a" * 40, "c" * 40 + job_id, run_id, suite_id, workflow_id = 202, 101, 303, 404 + associated_head = stale_head if case.startswith("stale-") else current_head + title_head = stale_head if case in {"stale-associated-head", "contradictory-title"} else current_head + execution_sha = current_head if case in { + "current-associated-head-top-level-head", "stale-association-top-level-current" + } else base_sha + details_repo = "other/repo" if case == "foreign-details-repository" else repo + publisher = "third-party-app" if case == "untrusted-check-publisher" else "github-actions" + actual_check_id = 606 if publisher != "github-actions" else 505 + check = { + "id": 505, "name": "strix", "status": "completed", "conclusion": "failure", + "head_sha": current_head, + "details_url": f"https://github.com/{details_repo}/actions/runs/{run_id}/job/{job_id}", + "app": {"slug": publisher}, "check_suite": {"id": suite_id}, + } + node = sched.rest_check_node( + check, {}, {} if publisher != "github-actions" else {suite_id: "Strix Security Scan"} + ) + if case == "missing-check-id": + node.pop("databaseId") + pr = { + "number": 7, "state": "OPEN", "headRefOid": current_head, + "baseRefOid": base_sha, "headRefName": "feature", "baseRefName": "main", + "headRepository": {"nameWithOwner": repo}, + "statusCheckRollup": {"contexts": {"nodes": [node]}}, + } + job = { + "id": job_id, "run_id": run_id, "head_sha": execution_sha, + "name": "strix", "status": "completed", "conclusion": "failure", + "check_run_url": f"https://api.github.com/repos/{repo}/check-runs/{actual_check_id}", + "html_url": f"https://github.com/{repo}/actions/runs/{run_id}/job/{job_id}", + } + run = { + "id": run_id, "head_sha": execution_sha, "workflow_id": workflow_id, + "check_suite_id": suite_id, "repository": {"full_name": repo}, + "event": "push" if case == "unrelated-push-event" else "pull_request_target", + "path": ".github/workflows/other.yml" if case == "different-workflow-path" else ".github/workflows/strix.yml", + "name": "Strix Security Scan", "status": "completed", "conclusion": "failure", + "display_title": f"Strix Security Scan {repo}#7@{title_head}", + "pull_requests": [] if case == "missing-association" else [{ + "number": 7, "base": {"sha": base_sha, "repo": {"name": "repo", "full_name": repo}}, + "head": {"sha": associated_head, "repo": {"name": "repo", "full_name": repo}}, + }], + } + if case == "dispatch-without-target-receipt": + run["event"] = "repository_dispatch" + run["pull_requests"] = [] + if case == "wrong-job-run": + job["run_id"] = 999 + if case == "wrong-check-suite": + run["check_suite_id"] = 999 + if case == "contradictory-repository": + run["pull_requests"][0]["head"]["repo"]["url"] = "https://api.github.com/repos/other/repo" + if case == "repository-api-url-only": + for side in ("base", "head"): + run["pull_requests"][0][side]["repo"] = {"name": "repo", "url": f"https://api.github.com/repos/{repo}"} + reads, posts = [], [] + responses = { + f"repos/{repo}/actions/jobs/{job_id}": job, + f"repos/{repo}/actions/runs/{run_id}": run, + f"repos/{repo}/check-runs/505": check, + f"repos/{repo}/actions/workflows/{workflow_id}": { + "id": workflow_id, "path": run["path"], "name": "Strix Security Scan", + }, + } + if actual_check_id != 505: + responses[f"repos/{repo}/check-runs/{actual_check_id}"] = { + **check, "id": actual_check_id, "app": {"slug": "github-actions"}, + } + + def read(endpoint): + reads.append(endpoint) + if case == "api-unavailable": + raise RuntimeError("metadata unavailable") + assert endpoint in responses, f"Unexpected metadata lookup: {endpoint}" + return responses[endpoint] + + def actions(args, *, stdin=None): + assert stdin is None + if args == ["gh", "api", "-X", "POST", f"repos/{repo}/actions/jobs/{job_id}/rerun"]: + posts.append(args) + return "" + assert args[:2] == ["gh", "api"] and len(args) == 3 + return json.dumps(read(args[2])) + + def no_external_call(*args, **kwargs): + pytest.fail("Unexpected unmocked command boundary") + + monkeypatch.setenv("GITHUB_ACTIONS", "true") + monkeypatch.setenv("SCHEDULER_ACTIONS_TOKEN", "fixture-token") + monkeypatch.setattr(sched, "run", no_external_call) + monkeypatch.setattr(sched, "run_with_env", no_external_call) + monkeypatch.setattr(sched, "gh_api_json", read) + monkeypatch.setattr(sched, "run_github_read", actions) + monkeypatch.setattr(sched, "run_github_actions", actions) + monkeypatch.setattr(sched, "review_dispatch_admitted", lambda *_: True) + def fetch_pr(*_): + if case == "current-head-moves-during-binding" and reads: + return [{**pr, "headRefOid": "d" * 40}] + return [pr] + + monkeypatch.setattr(sched, "fetch_pr", fetch_pr) + # Keep real selection, live guard, caller, control-actor check and rerun wrapper. + result = sched.dispatch_strix_evidence(repo, "Strix Security Scan", pr, dry_run=False) + if allowed: + assert result == "rerun" + assert len(posts) == 1 + assert len(reads) == 4 + else: + assert posts == [], f"Unsafe rerun reached POST without binding metadata; reads={reads}; run={run}" + assert result in {"identity_unverified", "stale_head"} diff --git a/tests/test_strix_rerun_job_selection.py b/tests/test_strix_rerun_job_selection.py index 23a09891dd..39b557daf7 100644 --- a/tests/test_strix_rerun_job_selection.py +++ b/tests/test_strix_rerun_job_selection.py @@ -41,6 +41,8 @@ def record_rerun(repo: str, job_id: str, *, dry_run: bool, action: str) -> None: monkeypatch.setattr(sched, "rerun_actions_job", record_rerun) monkeypatch.setattr(sched, "fetch_pr", lambda *_args: [pr]) + # Keep this test focused on sibling selection, not the independent API binding. + monkeypatch.setattr(sched, "strix_rerun_identity_verified", lambda *_args: True) assert ( sched.dispatch_strix_evidence( From fe64f24931ec91b8578edb5b5eadf219074a52a7 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 00:41:44 +0900 Subject: [PATCH 12/16] test(ci): cover remaining Strix rerun identity rejections Signed-off-by: Seongho Bae --- tests/test_strix_job_binding.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_strix_job_binding.py b/tests/test_strix_job_binding.py index 09c880e4f1..46319f720d 100644 --- a/tests/test_strix_job_binding.py +++ b/tests/test_strix_job_binding.py @@ -26,12 +26,18 @@ ("missing-check-id", False), ("current-head-moves-during-binding", False), ("repository-api-url-only", True), + ("zero-job-id", False), + ("ambiguous-job-candidates", False), + ("invalid-workflow-id", False), + ("mismatched-workflow-metadata", False), ]) def test_actual_strix_rerun_caller_binds_selected_job(monkeypatch, case, allowed): """Selected job/run provenance, not a current PR snapshot alone, authorizes rerun.""" repo = "owner/repo" current_head, stale_head, base_sha = "b" * 40, "a" * 40, "c" * 40 job_id, run_id, suite_id, workflow_id = 202, 101, 303, 404 + if case == "zero-job-id": + job_id = 0 associated_head = stale_head if case.startswith("stale-") else current_head title_head = stale_head if case in {"stale-associated-head", "contradictory-title"} else current_head execution_sha = current_head if case in { @@ -57,6 +63,8 @@ def test_actual_strix_rerun_caller_binds_selected_job(monkeypatch, case, allowed "headRepository": {"nameWithOwner": repo}, "statusCheckRollup": {"contexts": {"nodes": [node]}}, } + if case == "ambiguous-job-candidates": + pr["statusCheckRollup"]["contexts"]["nodes"].append({**node, "databaseId": 506}) job = { "id": job_id, "run_id": run_id, "head_sha": execution_sha, "name": "strix", "status": "completed", "conclusion": "failure", @@ -82,6 +90,8 @@ def test_actual_strix_rerun_caller_binds_selected_job(monkeypatch, case, allowed job["run_id"] = 999 if case == "wrong-check-suite": run["check_suite_id"] = 999 + if case == "invalid-workflow-id": + run["workflow_id"] = 0 if case == "contradictory-repository": run["pull_requests"][0]["head"]["repo"]["url"] = "https://api.github.com/repos/other/repo" if case == "repository-api-url-only": @@ -96,6 +106,8 @@ def test_actual_strix_rerun_caller_binds_selected_job(monkeypatch, case, allowed "id": workflow_id, "path": run["path"], "name": "Strix Security Scan", }, } + if case == "mismatched-workflow-metadata": + responses[f"repos/{repo}/actions/workflows/{workflow_id}"]["path"] = ".github/workflows/other.yml" if actual_check_id != 505: responses[f"repos/{repo}/check-runs/{actual_check_id}"] = { **check, "id": actual_check_id, "app": {"slug": "github-actions"}, @@ -142,3 +154,12 @@ def fetch_pr(*_): else: assert posts == [], f"Unsafe rerun reached POST without binding metadata; reads={reads}; run={run}" assert result in {"identity_unverified", "stale_head"} + expected_reads = { + "zero-job-id": 0, + "ambiguous-job-candidates": 0, + "invalid-workflow-id": 3, + "mismatched-workflow-metadata": 4, + } + if case in expected_reads: + assert result == "identity_unverified" + assert len(reads) == expected_reads[case] From b966f826085f8beabf4884e56ebca1d19b6c74e2 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 09:00:58 +0900 Subject: [PATCH 13/16] fix(codeql): require preserved SARIF before terminal publication Block terminal status publication and exact-job wake when SARIF upload does not succeed. Preserve existing finding verdicts and document the unresolved receipt boundary. Co-authored-by: Codex Signed-off-by: Seongho Bae --- .github/workflows/codeql-scan-dispatch.yml | 7 ++ ...odeql-sarif-upload-publication-boundary.md | 21 ++++ ..._codeql_scan_dispatch_workflow_contract.py | 96 +++++++++++++++++++ 3 files changed, 124 insertions(+) create mode 100644 docs/doctoring/codeql-sarif-upload-publication-boundary.md diff --git a/.github/workflows/codeql-scan-dispatch.yml b/.github/workflows/codeql-scan-dispatch.yml index 1c9dda3e45..a35a2f16dd 100644 --- a/.github/workflows/codeql-scan-dispatch.yml +++ b/.github/workflows/codeql-scan-dispatch.yml @@ -397,11 +397,13 @@ jobs: run: python3 "$RUNNER_TEMP/codeql_sarif_gate.py" codeql-results-dispatch - name: Preserve CodeQL SARIF evidence + id: sarif_upload if: always() && hashFiles('codeql-results-dispatch/**/*.sarif') != '' uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: codeql-dispatch-${{ matrix.language }}-${{ github.run_id }}-${{ github.run_attempt }} path: codeql-results-dispatch + if-no-files-found: error retention-days: 7 - name: Publish CodeQL dispatch status @@ -416,8 +418,13 @@ jobs: HEAD_SHA: ${{ needs.validate-dispatch.outputs.head_sha }} LANGUAGE: ${{ matrix.language }} GATE_OUTCOME: ${{ steps.gate.outcome }} + SARIF_UPLOAD_OUTCOME: ${{ steps.sarif_upload.outcome }} run: | set -euo pipefail + if [ "${SARIF_UPLOAD_OUTCOME:-}" != "success" ]; then + echo "::error::CodeQL SARIF evidence was not preserved; terminal status publication and exact-job wake are blocked." + exit 1 + fi case "$GATE_OUTCOME" in success) state="success" diff --git a/docs/doctoring/codeql-sarif-upload-publication-boundary.md b/docs/doctoring/codeql-sarif-upload-publication-boundary.md new file mode 100644 index 0000000000..133c0fc015 --- /dev/null +++ b/docs/doctoring/codeql-sarif-upload-publication-boundary.md @@ -0,0 +1,21 @@ +# CodeQL SARIF 보존과 terminal 게시 경계 + +기준 `fe64f24931ec91b8578edb5b5eadf219074a52a7`의 handler는 gate가 +성공하면 SARIF artifact upload 실패와 무관하게 success status를 게시했다. +게시 성공 조건만 보는 callback도 원래 required job을 깨울 수 있었다. + +기존 upload action에 ID를 붙이고 실제 outcome이 `success`일 때만 +terminal 게시를 허용한다. 실패·skip·누락·취소는 게시 전에 실패하며, +기존 callback의 publication-success 조건 때문에 wake도 실행하지 않는다. +정상 upload 뒤 gate의 success/failure/error 판정은 그대로 유지한다. +파일이 없는 upload도 성공으로 처리하지 않는다. + +기존 `test_codeql_scan_dispatch_workflow_contract.py`의 실제 shell 추출과 +fake-gh를 사용한다. upload failure/skipped/빈 값/cancelled 대조군은 수정 전 +success POST와 mock wake가 발생해 RED였다. 외부 API나 실제 scan은 실행하지 않는다. + +이는 전체 receipt 또는 dedupe 수리가 아니다. 기대 trusted workflow SHA의 +독립적인 출처와 cross-repository artifact 읽기 권한은 여전히 후속 gate다. +기존 terminal status를 publisher·head·language만으로 재사용하여 다른 +base/workflow의 성공을 승계할 수 있는 소비자 취약점도 이번 변경으로 해결되지 않는다. +동일 입력 증명, 자동 wake 재조정, admission 원자성도 이번 변경이 보장하지 않는다. diff --git a/tests/test_codeql_scan_dispatch_workflow_contract.py b/tests/test_codeql_scan_dispatch_workflow_contract.py index 71fa43541f..9890c0bde8 100644 --- a/tests/test_codeql_scan_dispatch_workflow_contract.py +++ b/tests/test_codeql_scan_dispatch_workflow_contract.py @@ -17,13 +17,109 @@ import sys from pathlib import Path +import pytest + from scripts.ci import audit_central_required_workflows as ruleset_audit from tests.test_opencode_workflow_shell_syntax import _extract_run_block from tests.test_required_workflow_queue_contract import ( workflow_level_cancels_in_progress, workflow_level_concurrency_group, + workflow_step, ) + +@pytest.mark.parametrize( + ("gate", "upload", "expected_state"), + [ + ("success", "failure", None), + ("success", "skipped", None), + ("success", "", None), + ("success", "cancelled", None), + ("success", "success", "success"), + ("failure", "success", "failure"), + ("skipped", "success", "error"), + ], +) +def test_terminal_publication_requires_preserved_sarif( + tmp_path: Path, gate: str, upload: str, expected_state: str | None +) -> None: + """Execute production publication shell; missing artifacts cannot wake jobs.""" + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + script = _extract_run_block(workflow, "Publish CodeQL dispatch status") + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + post_log = tmp_path / "status-posts" + fake_gh = fake_bin / "gh" + fake_gh.write_text( + "#!/usr/bin/env bash\nset -euo pipefail\n" + 'test "$1" = api && test "$2" = -X && test "$3" = POST\n' + 'test "$4" = "repos/ContextualWisdomLab/naruon/statuses/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"\n' + 'test "$5" = -f\n' + 'printf "%s\\n" "$6" >>"$FAKE_POST_LOG"\n', + encoding="utf-8", + ) + fake_gh.chmod(0o755) + result = subprocess.run( + [shutil.which("bash") or "bash"], input=script, text=True, + capture_output=True, check=False, timeout=30, + env={ + **os.environ, + "PATH": f"{fake_bin}:{os.environ['PATH']}", + "FAKE_POST_LOG": str(post_log), + "GATE_OUTCOME": gate, "SARIF_UPLOAD_OUTCOME": upload, + "TARGET_APP_STATUS_TOKEN": "fixture-token", + "PR_REVIEW_MERGE_STATUS_TOKEN": "", + "OPENCODE_APPROVE_STATUS_TOKEN": "", "GITHUB_STATUS_READ_TOKEN": "", + "TARGET_REPOSITORY": "ContextualWisdomLab/naruon", + "HEAD_SHA": "b" * 40, "LANGUAGE": "python", + "GITHUB_SERVER_URL": "https://github.com", + "GITHUB_REPOSITORY": "ContextualWisdomLab/.github", "GITHUB_RUN_ID": "99", + }, + ) + # The actual workflow only admits wake when publication succeeded. + wake = workflow_step(workflow, "Wake exact CodeQL required job") + assert wake.split(" env:", 1)[0] == ( + " - name: Wake exact CodeQL required job\n" + " if: >-\n" + " always()\n" + " && steps.publish_status.outcome == 'success'\n" + " && needs.validate-dispatch.outputs.target_repository != ''\n" + " && needs.validate-dispatch.outputs.pr_number != ''\n" + " && needs.validate-dispatch.outputs.head_sha != ''\n" + " && github.event.client_payload.required_run_id != ''\n" + " && github.event.client_payload.required_job_id != ''\n" + ) + wake_posts = [] + if result.returncode == 0: + wake_result, wake_log = _run_wake_step(tmp_path / "wake") + assert wake_result.returncode == 0, wake_result.stderr + wake_posts = wake_log.read_text(encoding="utf-8").splitlines() + if expected_state is None: + assert not post_log.exists(), result.stdout + assert result.returncode != 0 + assert wake_posts == [] + else: + assert result.returncode == 0, result.stderr + assert post_log.read_text(encoding="utf-8").splitlines() == [f"state={expected_state}"] + assert wake_posts == ["repos/ContextualWisdomLab/naruon/actions/jobs/43/rerun"] + + +def test_terminal_publication_binds_actual_upload_step_outcome() -> None: + """The tested shell input must come from the existing artifact action.""" + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + upload = workflow_step(workflow, "Preserve CodeQL SARIF evidence") + assert upload.split(" uses:", 1)[0] == ( + " - name: Preserve CodeQL SARIF evidence\n" + " id: sarif_upload\n" + " if: always() && hashFiles('codeql-results-dispatch/**/*.sarif') != ''\n" + ) + assert " uses: actions/upload-artifact@" in upload + assert " if-no-files-found: error" in upload.splitlines() + publish = workflow_step(workflow, "Publish CodeQL dispatch status") + env = publish.split(" env:\n", 1)[1].split(" run:", 1)[0] + binding = [line for line in env.splitlines() if "SARIF_UPLOAD_OUTCOME" in line] + assert binding == [" SARIF_UPLOAD_OUTCOME: ${{ steps.sarif_upload.outcome }}"] + REPO_ROOT = Path(__file__).resolve().parents[1] WORKFLOW_PATH = REPO_ROOT / ".github/workflows/codeql-scan-dispatch.yml" VALIDATE_STEP_NAME = "Bind workflow inputs to live organization pull request metadata" From 8c11d860f6562fb726086576780dfe7e361badd1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 09:20:28 +0900 Subject: [PATCH 14/16] fix(codeql): verify live base before consuming terminal verdicts Compare already-fetched live base identity with event inputs before status consumption. Keep historical verdict provenance and artifact authority as unresolved follow-ups. Co-authored-by: Codex Signed-off-by: Seongho Bae --- .github/workflows/codeql-pr.yml | 13 ++++ .../codeql-live-base-terminal-boundary.md | 20 ++++++ tests/test_codeql_pr_workflow_contract.py | 71 ++++++++++++++++--- 3 files changed, 95 insertions(+), 9 deletions(-) create mode 100644 docs/doctoring/codeql-live-base-terminal-boundary.md diff --git a/.github/workflows/codeql-pr.yml b/.github/workflows/codeql-pr.yml index 3c737aea90..8f179ffa2b 100644 --- a/.github/workflows/codeql-pr.yml +++ b/.github/workflows/codeql-pr.yml @@ -201,6 +201,19 @@ jobs: exit 0 fi + live_base_repository="$(printf '%s' "$live_pr" | jq -r '.base.repo.full_name | select(type == "string")')" + live_base_ref="$(printf '%s' "$live_pr" | jq -r '.base.ref | select(type == "string")')" + live_base_sha="$(printf '%s' "$live_pr" | jq -r '.base.sha | select(type == "string")')" + if [ "$live_base_repository" != "$TARGET_REPOSITORY" ] || + [ -z "$live_base_ref" ] || [ -z "${PR_BASE_REF:-}" ] || + ! [[ "$live_base_sha" =~ ^[0-9a-fA-F]{40}$ ]] || + ! [[ "${PR_BASE_SHA:-}" =~ ^[0-9a-fA-F]{40}$ ]] || + [ "$live_base_ref" != "$PR_BASE_REF" ] || + [ "$live_base_sha" != "$PR_BASE_SHA" ]; then + echo "::error::CodeQL live base metadata is missing, malformed, or differs from the event base; terminal verdict reuse is blocked." + exit 1 + fi + statuses="$(gh api --paginate --slurp "repos/${TARGET_REPOSITORY}/commits/${PR_HEAD_SHA}/statuses?per_page=100")" verdict_state="$(printf '%s' "$statuses" | jq -r --arg ctx "codeql-dispatch/${LANGUAGE}" ' [ diff --git a/docs/doctoring/codeql-live-base-terminal-boundary.md b/docs/doctoring/codeql-live-base-terminal-boundary.md new file mode 100644 index 0000000000..b42b31b2e2 --- /dev/null +++ b/docs/doctoring/codeql-live-base-terminal-boundary.md @@ -0,0 +1,20 @@ +# CodeQL terminal 소비 전 live base 검증 + +기준 `b966f826085f8beabf4884e56ebca1d19b6c74e2`에서는 이미 조회한 PR의 +state/head만 확인하고 terminal status를 소비했다. 이벤트 이후 base가 +바뀌거나 base 정보가 없어도 trusted publisher의 같은-head 성공을 받아들였다. + +기존 handler와 같은 base repository/ref/SHA 일치 계약을 소비 직전에 적용한다. +이미 받은 PR 응답을 사용하며 추가 API·권한·대기·자동 재dispatch는 없다. +누락·잘못된 자료형/SHA·불일치에서는 status 조회 전에 실패한다. + +기존 실제 shell/fake-gh 테스트의 fixture를 production `PR_BASE_REF`, +`PR_BASE_SHA`, `PR_HEAD_REF` 이름으로 교정했다. live base 음성 8개와 +event base 음성 3개가 RED였으며, 거부 경로는 PR GET 한 번만 허용해 +status 조회 및 모든 POST가 없음을 확인한다. 정상 publisher·실패 verdict· +두 번째 페이지 status 회귀는 유지한다. + +이 검사는 이벤트와 현재 live PR의 base 일치만 보장한다. 이전 verdict 자체가 +어느 base/trusted workflow에서 생성됐는지는 증명하지 않는다. run-linked +receipt의 독립적인 기대 workflow SHA와 중앙 artifact 읽기 권한은 미결이며, +dedupe·자동 wake·admission 직렬화도 이번 범위가 아니다. diff --git a/tests/test_codeql_pr_workflow_contract.py b/tests/test_codeql_pr_workflow_contract.py index d9e6503e63..e0876f066f 100644 --- a/tests/test_codeql_pr_workflow_contract.py +++ b/tests/test_codeql_pr_workflow_contract.py @@ -110,7 +110,9 @@ def test_codeql_pr_dispatch_and_release_run_blocks_are_valid_bash() -> None: def _run_verdict_read( - tmp_path: Path, statuses: list[dict], *, second_page: list[dict] | None = None + tmp_path: Path, statuses: list[dict], *, second_page: list[dict] | None = None, + base: dict | None = None, env_overrides: dict[str, str] | None = None, + expect_dispatch_failure: bool = False, ) -> tuple[subprocess.CompletedProcess[str], subprocess.CompletedProcess[str]]: """Execute the real one-shot status read and verdict enforcement blocks.""" bash = shutil.which("bash") @@ -122,7 +124,13 @@ def _run_verdict_read( verdict_script = _extract_run_block(workflow_text, VERDICT_STEP_NAME) head_sha = "b" * 40 - live_pr = {"head": {"sha": head_sha}, "state": "open"} + live_pr = { + "head": {"sha": head_sha}, "state": "open", + "base": base if base is not None else { + "repo": {"full_name": "ContextualWisdomLab/naruon"}, + "ref": "main", "sha": "a" * 40, + }, + } fake_bin = tmp_path / "bin" fake_bin.mkdir() @@ -130,6 +138,7 @@ def _run_verdict_read( fake_gh.write_text( "#!/usr/bin/env bash\n" "set -euo pipefail\n" + 'printf "%s\\n" "$*" >>"$FAKE_CALL_LOG"\n' 'test "$1" = api\n' 'if [ "$#" = 2 ] && [ "$2" = "repos/ContextualWisdomLab/naruon/pulls/42" ]; then\n' " printf '%s\\n' \"$FAKE_PULL_JSON\"\n" @@ -152,32 +161,39 @@ def _run_verdict_read( [statuses] if second_page is None else [statuses, second_page] ), "GH_TOKEN": "fake-token", + "FAKE_CALL_LOG": str(tmp_path / "gh-calls"), "TARGET_REPOSITORY": "ContextualWisdomLab/naruon", "PR_NUMBER": "42", "PR_HEAD_SHA": head_sha, "LANGUAGE": "python", "BUILD_MODE": "none", - "BASE_REF": "main", - "BASE_SHA": "a" * 40, - "HEAD_REF": "feature", + "PR_BASE_REF": "main", + "PR_BASE_SHA": "a" * 40, + "PR_HEAD_REF": "feature", "RUN_ATTEMPT": "2", "REQUIRED_RUN_ID": "42", "REQUIRED_JOB_ID": "43", "GITHUB_OUTPUT": str(output), + **(env_overrides or {}), } dispatch_result = subprocess.run( [bash], input=dispatch_script, text=True, capture_output=True, check=False, env=dispatch_env, timeout=60, ) - assert dispatch_result.returncode == 0, dispatch_result.stderr + if expect_dispatch_failure: + assert dispatch_result.returncode != 0, dispatch_result.stdout + else: + assert dispatch_result.returncode == 0, dispatch_result.stderr output_values = dict( - line.split("=", 1) for line in output.read_text(encoding="utf-8").splitlines() + line.split("=", 1) for line in ( + output.read_text(encoding="utf-8").splitlines() if output.exists() else [] + ) ) verdict_env = { **os.environ, "LANGUAGE": "python", - "DISPATCH_OUTCOME": "success", - "VERDICT_STATE": output_values["verdict"], + "DISPATCH_OUTCOME": "success" if dispatch_result.returncode == 0 else "failure", + "VERDICT_STATE": output_values.get("verdict", ""), } verdict_result = subprocess.run( [bash], input=verdict_script, text=True, capture_output=True, check=False, @@ -186,6 +202,43 @@ def _run_verdict_read( return dispatch_result, verdict_result +@pytest.mark.parametrize("field,value", [ + ("repo", {"full_name": "ContextualWisdomLab/other"}), + ("repo", {}), ("ref", "other"), ("ref", ""), ("ref", 42), + ("sha", "c" * 40), ("sha", ""), ("sha", "not-a-sha"), +]) +def test_codeql_terminal_rejects_invalid_live_base_before_status_read( + tmp_path: Path, field: str, value: object, +) -> None: + """A genuine old success cannot excuse missing or changed event base inputs.""" + base = {"repo": {"full_name": "ContextualWisdomLab/naruon"}, + "ref": "main", "sha": "a" * 40} + base[field] = value + dispatch, verdict = _run_verdict_read(tmp_path, [ + {"context": "codeql-dispatch/python", "state": "success", + "creator": {"login": "opencode-agent[bot]"}}, + ], base=base, expect_dispatch_failure=True) + assert "base" in dispatch.stdout.lower() + assert verdict.returncode == 1 + assert (tmp_path / "gh-calls").read_text().splitlines() == [ + "api repos/ContextualWisdomLab/naruon/pulls/42" + ] + + +@pytest.mark.parametrize("field,value", [ + ("PR_BASE_SHA", ""), ("PR_BASE_SHA", "invalid"), ("PR_BASE_REF", ""), +]) +def test_codeql_terminal_rejects_missing_or_malformed_event_base( + tmp_path: Path, field: str, value: str, +) -> None: + _dispatch, verdict = _run_verdict_read(tmp_path, [], + env_overrides={field: value}, expect_dispatch_failure=True) + assert verdict.returncode == 1 + assert (tmp_path / "gh-calls").read_text().splitlines() == [ + "api repos/ContextualWisdomLab/naruon/pulls/42" + ] + + 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. From 3d8a8b6dc2c1494d24cfb9c4ffb7fd8f8edb6465 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 09:29:22 +0900 Subject: [PATCH 15/16] test(codeql): bind rerun recovery fixture to live base Supply the real base repository, ref and SHA required by the production dispatch shell. Preserve later-attempt redispatch coverage without weakening the live-base guard. Co-authored-by: Codex Signed-off-by: Seongho Bae --- tests/test_codeql_pr_rerun_recovery_contract.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/test_codeql_pr_rerun_recovery_contract.py b/tests/test_codeql_pr_rerun_recovery_contract.py index 50fff499af..0df30ddbab 100644 --- a/tests/test_codeql_pr_rerun_recovery_contract.py +++ b/tests/test_codeql_pr_rerun_recovery_contract.py @@ -77,7 +77,13 @@ def test_rerun_without_authenticated_verdict_can_redispatch(tmp_path: Path) -> N env = { **os.environ, "PATH": f"{fake_bin}:{os.environ['PATH']}", - "FAKE_PULL_JSON": json.dumps({"head": {"sha": head_sha}, "state": "open"}), + "FAKE_PULL_JSON": json.dumps({ + "head": {"sha": head_sha}, "state": "open", + "base": { + "repo": {"full_name": "ContextualWisdomLab/accounting-information-platform"}, + "ref": "develop", "sha": base_sha, + }, + }), "FAKE_DISPATCH_BODY": str(dispatch_body), "GH_TOKEN": "leaf-token", "OIDC_AUDIENCE": "opencode-github-action", From 82ca0b8fe67177a98ca01f8dc12441c782f1760f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 09:37:10 +0900 Subject: [PATCH 16/16] fix(codeql): require preserved SARIF before status --- .github/workflows/codeql-scan-dispatch.yml | 7 ++ .../codeql-sarif-publication-boundary.md | 9 +++ ..._codeql_scan_dispatch_workflow_contract.py | 75 +++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 docs/doctoring/codeql-sarif-publication-boundary.md diff --git a/.github/workflows/codeql-scan-dispatch.yml b/.github/workflows/codeql-scan-dispatch.yml index 1c9dda3e45..a35a2f16dd 100644 --- a/.github/workflows/codeql-scan-dispatch.yml +++ b/.github/workflows/codeql-scan-dispatch.yml @@ -397,11 +397,13 @@ jobs: run: python3 "$RUNNER_TEMP/codeql_sarif_gate.py" codeql-results-dispatch - name: Preserve CodeQL SARIF evidence + id: sarif_upload if: always() && hashFiles('codeql-results-dispatch/**/*.sarif') != '' uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: codeql-dispatch-${{ matrix.language }}-${{ github.run_id }}-${{ github.run_attempt }} path: codeql-results-dispatch + if-no-files-found: error retention-days: 7 - name: Publish CodeQL dispatch status @@ -416,8 +418,13 @@ jobs: HEAD_SHA: ${{ needs.validate-dispatch.outputs.head_sha }} LANGUAGE: ${{ matrix.language }} GATE_OUTCOME: ${{ steps.gate.outcome }} + SARIF_UPLOAD_OUTCOME: ${{ steps.sarif_upload.outcome }} run: | set -euo pipefail + if [ "${SARIF_UPLOAD_OUTCOME:-}" != "success" ]; then + echo "::error::CodeQL SARIF evidence was not preserved; terminal status publication and exact-job wake are blocked." + exit 1 + fi case "$GATE_OUTCOME" in success) state="success" diff --git a/docs/doctoring/codeql-sarif-publication-boundary.md b/docs/doctoring/codeql-sarif-publication-boundary.md new file mode 100644 index 0000000000..640e40aa0e --- /dev/null +++ b/docs/doctoring/codeql-sarif-publication-boundary.md @@ -0,0 +1,9 @@ +# CodeQL SARIF publication boundary + +The central CodeQL dispatch handler publishes a terminal commit status only after the same matrix shard has successfully preserved its SARIF artifact. A successful finding gate without durable evidence is not a successful scan contract: upload failure, a skipped upload, cancellation, or a missing outcome fails closed before any status credential is used and therefore before the exact required job can be woken. + +`actions/upload-artifact` owns the evidence boundary. The upload step has a stable step identifier and rejects an empty artifact input. The status-publication step consumes that step's outcome and accepts only `success`; it does not infer preservation from a generated local file or from the SARIF gate result. The gate result continues to determine whether preserved evidence represents a passing or failing security verdict. + +Executable regression coverage runs the real publication shell against a fixture-backed GitHub API. The success control permits one exact-head status post. Upload outcomes `failure`, `skipped`, `cancelled`, and empty each exit before a post, preventing a false terminal success and the downstream exact-job rerun. + +This source repair does not change repository-dispatch actor authorization or cross-repository credential authority. Those remain separate configuration and GitHub App permission boundaries tracked in ContextualWisdomLab/.github issue #1929. diff --git a/tests/test_codeql_scan_dispatch_workflow_contract.py b/tests/test_codeql_scan_dispatch_workflow_contract.py index 71fa43541f..f0308aca9a 100644 --- a/tests/test_codeql_scan_dispatch_workflow_contract.py +++ b/tests/test_codeql_scan_dispatch_workflow_contract.py @@ -17,6 +17,8 @@ import sys from pathlib import Path +import pytest + from scripts.ci import audit_central_required_workflows as ruleset_audit from tests.test_opencode_workflow_shell_syntax import _extract_run_block from tests.test_required_workflow_queue_contract import ( @@ -27,6 +29,7 @@ REPO_ROOT = Path(__file__).resolve().parents[1] WORKFLOW_PATH = REPO_ROOT / ".github/workflows/codeql-scan-dispatch.yml" VALIDATE_STEP_NAME = "Bind workflow inputs to live organization pull request metadata" +PUBLISH_STATUS_STEP_NAME = "Publish CodeQL dispatch status" RUN_BLOCK_STEP_NAMES = ( "Exchange OpenCode app token for target repository metadata reads", @@ -80,6 +83,9 @@ def test_codeql_scan_dispatch_workflow_structure(): assert "scripts/ci/codeql_sarif_gate.py" in workflow assert 'context="codeql-dispatch/${LANGUAGE}"' in workflow assert "OPENCODE_REPOSITORY_DISPATCH_ACTOR" in workflow + assert "id: sarif_upload" in workflow + assert "if-no-files-found: error" in workflow + assert "SARIF_UPLOAD_OUTCOME: ${{ steps.sarif_upload.outcome }}" in workflow # Deliberately NOT vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS: that allowlist # scopes a gradual ~12-repo OpenCode review rollout, while ruleset # 18156473 covers ~ALL org repos except noema/.github/IRT-bibliography-set @@ -281,6 +287,75 @@ def test_codeql_scan_dispatch_validate_step_rejects_malformed_matrix(tmp_path): assert "matrix must contain exactly one valid language/build-mode shard" in result.stdout +def _run_publish_status_step( + tmp_path: Path, sarif_upload_outcome: str +) -> tuple[subprocess.CompletedProcess[str], Path]: + """Execute the real status-publication shell with a fixture-backed GitHub API.""" + bash = shutil.which("bash") + assert bash is not None, "bash is required to run this test" + + script = _extract_run_block( + WORKFLOW_PATH.read_text(encoding="utf-8"), PUBLISH_STATUS_STEP_NAME + ) + 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' + 'test "$2" = -X\n' + 'test "$3" = POST\n' + 'printf \'%s\\n\' "$4" >>"$FAKE_POST_LOG"\n', + encoding="utf-8", + ) + fake_gh.chmod(0o755) + env = { + **os.environ, + "PATH": f"{fake_bin}:{os.environ['PATH']}", + "FAKE_POST_LOG": str(post_log), + "TARGET_APP_STATUS_TOKEN": "target-app-token", + "GITHUB_STATUS_READ_TOKEN": "github-token", + "PR_REVIEW_MERGE_STATUS_TOKEN": "", + "OPENCODE_APPROVE_STATUS_TOKEN": "", + "TARGET_REPOSITORY": "ContextualWisdomLab/naruon", + "HEAD_SHA": "b" * 40, + "LANGUAGE": "python", + "GATE_OUTCOME": "success", + "SARIF_UPLOAD_OUTCOME": sarif_upload_outcome, + "GITHUB_SERVER_URL": "https://github.com", + "GITHUB_REPOSITORY": "ContextualWisdomLab/.github", + "GITHUB_RUN_ID": "42", + } + result = subprocess.run( + [bash], input=script, text=True, capture_output=True, check=False, env=env + ) + return result, post_log + + +@pytest.mark.parametrize("outcome", ["failure", "skipped", "cancelled", ""]) +def test_dispatch_status_rejects_unpreserved_sarif_evidence( + tmp_path: Path, outcome: str +) -> None: + """A missing SARIF artifact blocks terminal success and exact-job wake-up.""" + result, post_log = _run_publish_status_step(tmp_path, outcome) + + assert result.returncode == 1 + assert "SARIF evidence was not preserved" in result.stdout + assert not post_log.exists() + + +def test_dispatch_status_accepts_preserved_sarif_evidence(tmp_path: Path) -> None: + """A successful SARIF upload permits the exact-head terminal status boundary.""" + result, post_log = _run_publish_status_step(tmp_path, "success") + + assert result.returncode == 0, result.stderr + assert post_log.read_text(encoding="utf-8").splitlines() == [ + "repos/ContextualWisdomLab/naruon/statuses/" + "b" * 40 + ] + + def test_codeql_scan_dispatch_validate_step_rejects_stale_head_sha(tmp_path): """A dispatch whose supplied head SHA no longer matches the live PR head is rejected.""" stale_pull_request = _matching_pull_request()