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/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/.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_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: 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"],