From ea28a1453c2136a1f381cb2c5f045b0142371bfc Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 22:12:06 +0900 Subject: [PATCH 1/2] fix(dispatch): accept a list of trusted dispatcher identities Two trusted workflows send the opencode-review repository_dispatch: opencode-review.yml through the OpenCode GitHub App (sender opencode-agent[bot], introduced by #1497) and pr-review-merge-scheduler.yml through its own token chain (sender github-actions[bot]). The authorization gate in opencode-review-dispatch.yml compared both actor and sender against a single-valued variable that still names only github-actions[bot], so every app-token dispatch has failed at the first job -- 9611 failures to 466 successes over the workflow's lifetime, and no open PR holds a successful review on its current head (#1929). Parse ALLOWED_DISPATCH_ACTOR as a comma-separated list, exactly as the adjacent ALLOWED_DISPATCH_TARGETS block already does. Semantics are preserved otherwise: actor and sender must both equal the SAME listed identity (a dispatch whose actor and sender are two different listed identities is still rejected), and an empty allowlist admits nothing. This change does not alter the variable. Which identities belong on the list is an authorization decision for the repository owner; this only makes the gate able to express more than one. A single-valued variable keeps working unchanged. Contract test extended in tests/test_opencode_agent_contract.py: both identities pass with a listed allowlist (whitespace around commas tolerated), an unlisted identity is rejected, and mismatched actor/sender is rejected. Negative control: the extended test fails against the unmodified gate on origin/main. REVIEW_DISPATCH_BLOB_SHA recomputed via git hash-object. Verified: 2890 passed, coverage 100%, interrogate 100%. Co-Authored-By: Claude Fable 5.1 --- .../workflows/opencode-review-dispatch.yml | 24 +++++++++-- tests/test_opencode_agent_contract.py | 41 +++++++++++++++++++ ...t_pr_review_autofix_nvidia_nim_contract.py | 2 +- 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index ade10b37c4..26e8555967 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -124,10 +124,26 @@ jobs: run: | set -euo pipefail if [ "$EVENT_NAME" = "repository_dispatch" ]; then - if [ -z "$ALLOWED_DISPATCH_ACTOR" ] || - [ "$DISPATCH_ACTOR" != "$ALLOWED_DISPATCH_ACTOR" ] || - [ "$DISPATCH_SENDER" != "$ALLOWED_DISPATCH_ACTOR" ]; then - printf '::error::repository_dispatch authorization rejected actor=%s sender=%s because both must match the configured scheduler identity.\n' "${DISPATCH_ACTOR:-}" "${DISPATCH_SENDER:-}" + # More than one trusted identity dispatches this workflow: + # opencode-review.yml sends through the OpenCode GitHub App + # (opencode-agent[bot]) while pr-review-merge-scheduler.yml sends + # with its own token chain. Accept a comma-separated allowlist, + # parsed exactly like ALLOWED_DISPATCH_TARGETS below. The actor + # AND the sender must both equal the SAME allowlisted identity; + # an empty allowlist admits nothing. + actor_allowed=0 + IFS=',' read -r -a allowed_dispatch_actors <<<"$ALLOWED_DISPATCH_ACTOR" + for allowed_actor in "${allowed_dispatch_actors[@]}"; do + allowed_actor="${allowed_actor//[[:space:]]/}" + if [ -n "$allowed_actor" ] && + [ "$DISPATCH_ACTOR" = "$allowed_actor" ] && + [ "$DISPATCH_SENDER" = "$allowed_actor" ]; then + actor_allowed=1 + break + fi + done + if [ "$actor_allowed" -ne 1 ]; then + printf '::error::repository_dispatch authorization rejected actor=%s sender=%s because both must match one configured scheduler identity.\n' "${DISPATCH_ACTOR:-}" "${DISPATCH_SENDER:-}" exit 1 fi diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 72a8b44e56..37ec068db9 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -1121,9 +1121,50 @@ def test_opencode_repository_dispatch_authorization_is_fail_closed(): assert authorized.returncode == 0, authorized.stderr assert "Authorized repository_dispatch actor=" in authorized.stdout + # Two trusted identities dispatch this workflow: opencode-review.yml through + # the OpenCode GitHub App and pr-review-merge-scheduler.yml through its own + # token chain. The allowlist is a comma-separated list parsed like + # ALLOWED_DISPATCH_TARGETS, whitespace tolerated, and each identity must + # match on BOTH actor and sender. + multi_allowlist = "github-actions[bot], opencode-agent[bot]" + for identity in ("github-actions[bot]", "opencode-agent[bot]"): + listed = subprocess.run( + ["bash", "-c", shell], + env={ + **base_env, + "ALLOWED_DISPATCH_ACTOR": multi_allowlist, + "DISPATCH_ACTOR": identity, + "DISPATCH_SENDER": identity, + }, + text=True, + capture_output=True, + check=False, + ) + assert listed.returncode == 0, listed.stderr + assert f"Authorized repository_dispatch actor={identity}" in listed.stdout + for overrides, expected_reason in ( ({"ALLOWED_DISPATCH_ACTOR": ""}, "rejected actor="), ({"DISPATCH_SENDER": "seonghobae"}, "rejected actor="), + # A listed allowlist still rejects an identity that is not on it. + ( + { + "ALLOWED_DISPATCH_ACTOR": multi_allowlist, + "DISPATCH_ACTOR": "seonghobae", + "DISPATCH_SENDER": "seonghobae", + }, + "rejected actor=seonghobae", + ), + # Actor and sender must be the SAME listed identity, not each some + # listed identity -- a dispatch where they differ is still rejected. + ( + { + "ALLOWED_DISPATCH_ACTOR": multi_allowlist, + "DISPATCH_ACTOR": "opencode-agent[bot]", + "DISPATCH_SENDER": "github-actions[bot]", + }, + "rejected actor=opencode-agent[bot]", + ), ( {"ALLOWED_DISPATCH_TARGETS": "ContextualWisdomLab/.github"}, "rejected target=ContextualWisdomLab/naruon", diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 8d4397c42d..2d2304aaf1 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -17,7 +17,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "ade10b37c43d0f2b46490b2196c893244afc3d49" +REVIEW_DISPATCH_BLOB_SHA = "26e8555967171a5f3974602ac05700c27bddebf1" def _workflow_text(path: Path) -> str: From d44aa07d58e657c841172560af5b9d080e2f988a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 22:25:36 +0900 Subject: [PATCH 2/2] fix(dispatch): parse the actor allowlist identically in all three consumers vars.OPENCODE_REPOSITORY_DISPATCH_ACTOR is read by three workflows, and the first commit widened only one of them: opencode-review-dispatch.yml:127 covered by the previous commit codeql-scan-dispatch.yml:155 byte-identical gate, was still exact-match pr-review-fix-scheduler.yml:156 same three conditions, different error line Left as-is, codeql-scan-dispatch would keep rejecting the App identity once #1925's toJSON fix lets it reach line 155, and the scheduler would too. Three consumers of one variable with two parsers is the next drift, so all three now run the same comma-separated parse with the same semantics: actor and sender must both equal the SAME listed identity, empty list admits nothing, single value unchanged. The scheduler keeps its own error line. Tests extended in place for both: a listed identity passes (whitespace around commas tolerated), an unlisted one is rejected, and actor/sender that are two different listed identities are rejected. Negative control: both extended tests fail against the unmodified gates on origin/main. The codeql helper creates tmp_path/bin, so each invocation gets its own subdirectory. No blob-SHA pin references either newly edited workflow. No open PR touches either gate: #1926 covers codeql-scan-dispatch 146-152 and #1741 covers pr-review-fix-scheduler 210-217. Verified: 2891 passed, coverage 100%, interrogate 100%. Co-Authored-By: Claude Fable 5.1 --- .github/workflows/codeql-scan-dispatch.yml | 21 +++++++-- .github/workflows/pr-review-fix-scheduler.yml | 19 ++++++-- ..._codeql_scan_dispatch_workflow_contract.py | 46 +++++++++++++++++++ tests/test_pr_review_fix_hourly_contract.py | 31 +++++++++++++ 4 files changed, 110 insertions(+), 7 deletions(-) diff --git a/.github/workflows/codeql-scan-dispatch.yml b/.github/workflows/codeql-scan-dispatch.yml index 1ad28f3086..343e7af6ae 100644 --- a/.github/workflows/codeql-scan-dispatch.yml +++ b/.github/workflows/codeql-scan-dispatch.yml @@ -152,10 +152,23 @@ jobs: SUPPLIED_REQUIRED_LANGUAGE: ${{ github.event.client_payload.required_language || '' }} run: | set -euo pipefail - if [ -z "$ALLOWED_DISPATCH_ACTOR" ] || - [ "$DISPATCH_ACTOR" != "$ALLOWED_DISPATCH_ACTOR" ] || - [ "$DISPATCH_SENDER" != "$ALLOWED_DISPATCH_ACTOR" ]; then - printf '::error::repository_dispatch authorization rejected actor=%s sender=%s because both must match the configured scheduler identity.\n' "${DISPATCH_ACTOR:-}" "${DISPATCH_SENDER:-}" + # ALLOWED_DISPATCH_ACTOR is a comma-separated allowlist shared with + # opencode-review-dispatch.yml and pr-review-fix-scheduler.yml; all + # three parse it the same way. Actor AND sender must both equal the + # SAME listed identity, and an empty allowlist admits nothing. + actor_allowed=0 + IFS=',' read -r -a allowed_dispatch_actors <<<"$ALLOWED_DISPATCH_ACTOR" + for allowed_actor in "${allowed_dispatch_actors[@]}"; do + allowed_actor="${allowed_actor//[[:space:]]/}" + if [ -n "$allowed_actor" ] && + [ "$DISPATCH_ACTOR" = "$allowed_actor" ] && + [ "$DISPATCH_SENDER" = "$allowed_actor" ]; then + actor_allowed=1 + break + fi + done + if [ "$actor_allowed" -ne 1 ]; then + printf '::error::repository_dispatch authorization rejected actor=%s sender=%s because both must match one configured scheduler identity.\n' "${DISPATCH_ACTOR:-}" "${DISPATCH_SENDER:-}" exit 1 fi printf 'Authorized repository_dispatch actor=%s sender=%s target=%s.\n' "$DISPATCH_ACTOR" "$DISPATCH_SENDER" "$TARGET_REPOSITORY" diff --git a/.github/workflows/pr-review-fix-scheduler.yml b/.github/workflows/pr-review-fix-scheduler.yml index dc9c7415ca..0c0c05c151 100644 --- a/.github/workflows/pr-review-fix-scheduler.yml +++ b/.github/workflows/pr-review-fix-scheduler.yml @@ -153,9 +153,22 @@ jobs: # Only the direct repository_dispatch surface needs sender binding; # cross-repository invocations still pass the configured allowlist. if [ "$EVENT_NAME" = "repository_dispatch" ]; then - if [ -z "$ALLOWED_DISPATCH_ACTOR" ] || - [ "$DISPATCH_ACTOR" != "$ALLOWED_DISPATCH_ACTOR" ] || - [ "$DISPATCH_SENDER" != "$ALLOWED_DISPATCH_ACTOR" ]; then + # ALLOWED_DISPATCH_ACTOR is a comma-separated allowlist shared with + # opencode-review-dispatch.yml and codeql-scan-dispatch.yml; all + # three parse it the same way. Actor AND sender must both equal the + # SAME listed identity, and an empty allowlist admits nothing. + actor_allowed=0 + IFS=',' read -r -a allowed_dispatch_actors <<<"$ALLOWED_DISPATCH_ACTOR" + for allowed_actor in "${allowed_dispatch_actors[@]}"; do + allowed_actor="${allowed_actor//[[:space:]]/}" + if [ -n "$allowed_actor" ] && + [ "$DISPATCH_ACTOR" = "$allowed_actor" ] && + [ "$DISPATCH_SENDER" = "$allowed_actor" ]; then + actor_allowed=1 + break + fi + done + if [ "$actor_allowed" -ne 1 ]; then echo "::error::Scheduler repository dispatch actor or sender is unauthorized." exit 1 fi diff --git a/tests/test_codeql_scan_dispatch_workflow_contract.py b/tests/test_codeql_scan_dispatch_workflow_contract.py index dbc0e4bb73..dba6cbfacd 100644 --- a/tests/test_codeql_scan_dispatch_workflow_contract.py +++ b/tests/test_codeql_scan_dispatch_workflow_contract.py @@ -180,6 +180,52 @@ def test_codeql_scan_dispatch_validate_step_rejects_actor_mismatch(tmp_path): assert "authorization rejected actor=" in result.stdout +def test_codeql_scan_dispatch_validate_step_accepts_any_listed_dispatcher(tmp_path): + """ALLOWED_DISPATCH_ACTOR is a comma-separated allowlist shared by all three + dispatch consumers; each listed identity passes when actor and sender both + equal it, an unlisted one is rejected, and actor/sender that are two + *different* listed identities are still rejected.""" + # _run_validate_step creates tmp_path/bin, so each invocation needs its + # own directory. + allowlist = "github-actions[bot], opencode-agent[bot]" + for identity in ("github-actions[bot]", "opencode-agent[bot]"): + result = _run_validate_step( + tmp_path / identity.replace("[", "").replace("]", ""), + { + "ALLOWED_DISPATCH_ACTOR": allowlist, + "DISPATCH_ACTOR": identity, + "DISPATCH_SENDER": identity, + }, + _matching_pull_request(), + ) + assert result.returncode == 0, result.stderr + assert f"Authorized repository_dispatch actor={identity}" in result.stdout + + unlisted = _run_validate_step( + tmp_path / "unlisted", + { + "ALLOWED_DISPATCH_ACTOR": allowlist, + "DISPATCH_ACTOR": "seonghobae", + "DISPATCH_SENDER": "seonghobae", + }, + _matching_pull_request(), + ) + assert unlisted.returncode == 1 + assert "authorization rejected actor=seonghobae" in unlisted.stdout + + mismatched = _run_validate_step( + tmp_path / "mismatched", + { + "ALLOWED_DISPATCH_ACTOR": allowlist, + "DISPATCH_ACTOR": "opencode-agent[bot]", + "DISPATCH_SENDER": "github-actions[bot]", + }, + _matching_pull_request(), + ) + assert mismatched.returncode == 1 + assert "authorization rejected actor=opencode-agent[bot]" in mismatched.stdout + + def test_codeql_scan_dispatch_validate_step_accepts_any_org_repository(tmp_path): """Unlike opencode-review-dispatch.yml, any ContextualWisdomLab repo is accepted. diff --git a/tests/test_pr_review_fix_hourly_contract.py b/tests/test_pr_review_fix_hourly_contract.py index 4157aaf521..994145b469 100644 --- a/tests/test_pr_review_fix_hourly_contract.py +++ b/tests/test_pr_review_fix_hourly_contract.py @@ -195,12 +195,43 @@ def test_scheduler_validates_dispatch_authority_before_credentials() -> None: check=False, ).returncode == 0 + # ALLOWED_DISPATCH_ACTOR is a comma-separated allowlist shared with the two + # dispatch workflows; every listed identity passes when actor and sender + # both equal it, whitespace around commas tolerated. + allowlist = "github-actions[bot], opencode-agent[bot]" + for identity in ("github-actions[bot]", "opencode-agent[bot]"): + assert subprocess.run( + ["bash"], + input=shell, + text=True, + env={ + **base_env, + "ALLOWED_DISPATCH_ACTOR": allowlist, + "DISPATCH_ACTOR": identity, + "DISPATCH_SENDER": identity, + }, + check=False, + ).returncode == 0 + for override in ( {"DISPATCH_SENDER": "untrusted"}, {"DISPATCH_ACTOR": "untrusted"}, {"TARGET_REPOSITORY": "ContextualWisdomLab/unapproved"}, {"ALLOWED_DISPATCH_ACTOR": ""}, {"ALLOWED_TARGET_REPOSITORIES": ""}, + # A listed allowlist still rejects an unlisted identity. + { + "ALLOWED_DISPATCH_ACTOR": allowlist, + "DISPATCH_ACTOR": "untrusted", + "DISPATCH_SENDER": "untrusted", + }, + # Actor and sender must be the SAME listed identity, not each some + # listed identity. + { + "ALLOWED_DISPATCH_ACTOR": allowlist, + "DISPATCH_ACTOR": "opencode-agent[bot]", + "DISPATCH_SENDER": "github-actions[bot]", + }, ): assert subprocess.run( ["bash"],