diff --git a/scripts/ci/check_self_hosted_runner_policy.py b/scripts/ci/check_self_hosted_runner_policy.py index 1074b56448..886602263c 100644 --- a/scripts/ci/check_self_hosted_runner_policy.py +++ b/scripts/ci/check_self_hosted_runner_policy.py @@ -6,12 +6,15 @@ fails closed: every self-hosted job must belong to an explicitly allowed workflow, use the exact dedicated label set, and avoid runner groups (unavailable for this personal-account repository). Every pull_request job must also carry -the exact same-repository guard. Triggers whose payload can name a fork pull -request (pull_request_target, workflow_run, issue_comment, pull_request_review, -pull_request_review_comment) never reach a self-hosted job, whatever the guard -says; push / schedule / workflow_dispatch only run repository-owned refs and are -accepted. Dynamic ``runs-on`` expressions are rejected because their target -cannot be proved statically. +the exact same-repository guard. A self-hosted job is gated on a trigger +allowlist (pull_request, push, schedule, workflow_dispatch): these only run +repository-owned refs, and are checked for the same-repo guard when +pull_request is present. Any other trigger is rejected fail-closed -- the +known-dangerous ones carry specific codes (pull_request_target, workflow_run, +issue_comment, pull_request_review, pull_request_review_comment), and anything +unlisted (e.g. check_run / check_suite, whose payload can name a fork pull +request) is refused by default. Dynamic ``runs-on`` expressions are rejected +because their target cannot be proved statically. Exit codes: 0 policy satisfied (including the explicit baseline of zero self-hosted jobs) @@ -122,6 +125,22 @@ "pull_request_review", "pull_request_review_comment", }) +# Safe triggers for a self-hosted job (#14201, tranche 2). push, schedule and +# workflow_dispatch only run repository-owned refs, so the `== null` branch of +# the universal guard is safe there by construction (NanoClaw concern 2, +# measured on the routed workflows: banner-guard.yml / notebook-cell-source- +# parses.yml / hooks-parity.yml rely on push / schedule). pull_request is safe +# only WITH the same-repo guard (enforced above). Everything else is rejected +# fail-closed: a denylist alone (the FORK_REACHABLE_TRIGGERS form of #14148) is +# fail-OPEN on any trigger we have not enumerated -- check_run / check_suite, +# whose payloads carry pull_requests[], and any future GitHub event that does +# the same. Default-deny is the form that does not perish with each new event. +SAFE_SELF_HOSTED_TRIGGERS = frozenset({ + "pull_request", + "push", + "schedule", + "workflow_dispatch", +}) DEFAULT_WORKFLOWS_DIR = Path(__file__).resolve().parents[2] / ".github" / "workflows" @@ -448,6 +467,27 @@ def scan_workflows(workflows_dir: Path = DEFAULT_WORKFLOWS_DIR) -> ScanResult: f"{trigger} can check out a fork pull request and must never " "reach a self-hosted runner", )) + # Fail-closed default deny (#14201, tranche 2). The known-dangerous + # triggers above carry specific codes; ANY other trigger outside the + # safe set is refused by default. A denylist alone (the #14148 + # FORK_REACHABLE_TRIGGERS form) is fail-OPEN on anything unlisted. + known_unsafe = FORK_REACHABLE_TRIGGERS | { + "pull_request_target", + "workflow_run", + "workflow_call", + } + unknown_unsafe = sorted( + triggers - SAFE_SELF_HOSTED_TRIGGERS - known_unsafe + ) + if unknown_unsafe: + violations.append(Violation( + path.name, + str(job_name), + "UNSAFE_TRIGGER", + "self-hosted job is gated on a trigger outside the safe set " + "(pull_request, push, schedule, workflow_dispatch): " + + ", ".join(unknown_unsafe), + )) if path.name not in SELF_HOSTED_WORKFLOW_ALLOWLIST: violations.append(Violation( diff --git a/scripts/tests/test_check_self_hosted_runner_policy.py b/scripts/tests/test_check_self_hosted_runner_policy.py index fa358ebda8..9c35e665f0 100644 --- a/scripts/tests/test_check_self_hosted_runner_policy.py +++ b/scripts/tests/test_check_self_hosted_runner_policy.py @@ -192,6 +192,82 @@ def test_push_and_schedule_triggers_remain_accepted_with_universal_guard(tmp_pat assert result.self_hosted_jobs == 1 +def test_check_run_and_check_suite_fail_closed(tmp_path): + """#14201 (tranche 2) : le denylist de #14148 (FORK_REACHABLE_TRIGGERS) est + fail-OPEN sur tout trigger non enumere. check_run / check_suite portent + pull_requests[] et un head_sha -- un `checkout ${{ ... head_sha }}` + executerait du code de fork sur le runner, comme les triggers de + commentaire que #14148 a fermes. L'allowlist default-deny doit les rejeter.""" + write_workflow(tmp_path, "pr-gate-stale-sweep", """ + name: check-driven + on: + check_run: + types: [completed] + check_suite: + types: [completed] + jobs: + test: + runs-on: [self-hosted, coursia-ephemeral, coursia-fast-guards] + steps: + - run: echo unsafe + """) + result = policy.scan_workflows(tmp_path) + assert result.broken == [] + [violation] = result.violations + assert violation.code == "UNSAFE_TRIGGER" + assert "check_run" in violation.message + assert "check_suite" in violation.message + + +def test_unknown_trigger_is_default_denied(tmp_path): + """#14201 (tranche 2) : un trigger inconnu du checker doit etre refuse par + defaut (fail-closed), pas laisse passer parce que la liste des triggers + dangereux est une enumeration. La plupart des candidats futurs de GitHub + (release, deployment, registry_package, ...) sont ici rejetes.""" + write_workflow(tmp_path, "pr-gate-stale-sweep", """ + name: future-event + on: + release: + types: [published] + jobs: + test: + runs-on: [self-hosted, coursia-ephemeral, coursia-fast-guards] + steps: + - run: echo unsafe + """) + result = policy.scan_workflows(tmp_path) + assert result.broken == [] + assert "UNSAFE_TRIGGER" in codes(result) + + +def test_safe_allowlist_triggers_are_accepted(tmp_path): + """Contre-controle de la frontiere allowlist : les QUATRE triggers du + depot -- pull_request, push, schedule, workflow_dispatch -- sont acceptes + ensemble sur un job self-hosted garde. C'est le complement du contre- + controle de #14148 (push/schedule/workflow_dispatch seuls) : aucun des + triggers reels de la tranche 1 ne doit devenir UNSAFE_TRIGGER.""" + write_workflow(tmp_path, "pr-gate-stale-sweep", """ + name: allowlist-safe + on: + pull_request: + push: + branches: [main] + schedule: + - cron: '7 3 * * *' + workflow_dispatch: + jobs: + test: + if: ${{ github.event.pull_request.head.repo.full_name == null || github.event.pull_request.head.repo.full_name == github.repository }} + runs-on: [self-hosted, coursia-ephemeral, coursia-fast-guards] + steps: + - run: echo safe + """) + result = policy.scan_workflows(tmp_path) + assert result.broken == [] + assert result.violations == [] + assert result.self_hosted_jobs == 1 + + def test_universal_guard_weakened_by_or_is_rejected(tmp_path): """#13874 FN-safety : meme avec la forme universelle, l'ajout d'un `|| always()` ou `|| true` reintroduit le trou. La garde universelle