Refuse to resolve a ref under a trigger that carries secrets

docs/DEVELOPMENT.md

@@ -531,6 +531,42 @@ most of what it claimed. It is not enough to settle a difference of five, so the
 release match is a check on the batch rather than a measurement of any change in
 it.
 
+### The triggers a match can run under
+
+`scripts/resolve_ref.sh` turns a workflow input into a commit. A number is a
+pull request, so it fetches `refs/pull/<n>/head`, and the job then builds that
+commit and plays it. That is code somebody without write access wrote, running
+on a runner.
+
+It is safe here. Both match workflows run from the actions tab, which needs
+write access to the repository, they declare `contents: read` at the top, and
+neither is given a secret. It is not safe in general. Wired to
+`pull_request_target`, to `workflow_run` or to a comment event, the same script
+would run a fork's branch in a job holding this repository's secrets and a
+token that can write to it.
+
+So the script refuses. It runs under `workflow_dispatch`, `push`, `schedule`
+and `release`, which are the triggers where the ref was chosen by somebody who
+can already push here, and exits non-zero under anything else. With no
+`GITHUB_EVENT_NAME` at all it runs, since that is a machine and not a runner. A
+workflow called with `workflow_call` sees the event of the workflow that called
+it, so the release workflow's call is checked too.
+
+**What that does not cover.** The trigger is the only part of this a script can
+read. A job's `permissions:` block is exposed to nothing running inside the
+job, and the repository's secrets are not enumerable either: a secret reaches a
+step only when a step above passes it, and its absence from the environment
+says nothing about what else the job could be handed. So the check says the ref
+came from somebody with write access. It does not say the job is unprivileged,
+and a `workflow_dispatch` workflow that granted `contents: write` and passed a
+secret would pass it.
+
+The rest is the workflow's to declare and not the script's to detect. The
+`permissions: contents: read` at the top of `strength.yml` and `calibrate.yml`,
+and the fact that neither names a secret, are what make the pull request number
+input safe to offer. Anyone reusing this has to write those two lines
+themselves. A script cannot check them and does not pretend to.
+
 ### Which openings are played
 
 Strength and Calibrate both take a `book` input, and the books it can name are

scripts/resolve_ref.sh

@@ -13,6 +13,32 @@
 # each having their own idea of what a ref is.
 set -euo pipefail
 
+# Refuse outside the triggers where the ref was chosen by somebody who can
+# already push here. A number is a pull request, whose head anybody can write,
+# and the caller builds and runs whatever this prints. Under a trigger such as
+# pull_request_target or workflow_run the job also holds this repository's
+# secrets and a token that can write to it, and the two together hand a fork
+# the repository.
+#
+# GITHUB_EVENT_NAME is what this checks, because it is the only part of that
+# worth checking from here. The runner sets it on every job, an env block
+# cannot override a GITHUB_ name, and a workflow called with workflow_call sees
+# the event of the workflow that called it, so a call through a reusable
+# workflow is checked as well. It is not a check on permissions or on secrets:
+# a step cannot read either, and docs/DEVELOPMENT.md says what that leaves
+# open. Unset means this is not a runner at all, where there is nothing to
+# take.
+case "${GITHUB_EVENT_NAME-}" in
+    "" | workflow_dispatch | push | schedule | release) ;;
+    *)
+        echo "resolve_ref.sh: refusing to resolve a ref under" \
+            "${GITHUB_EVENT_NAME}. It builds a ref that somebody without" \
+            "write access can choose, and that trigger runs with this" \
+            "repository's secrets." >&2
+        exit 1
+        ;;
+esac
+
 ref=${1:?usage: resolve_ref.sh <branch|tag|commit|pull request number>}
 
 if [ "$ref" -eq "$ref" ] 2>/dev/null; then

scripts/tests/test_resolve_ref.py

@@ -7,8 +7,13 @@
 workflow input can take is resolved offline: a commit already present, a
 branch or tag that has to be fetched, and a bare number meaning a pull
 request's head.
+
+The trigger the script is run under is an argument here rather than whatever
+this process inherited, since these tests themselves run inside a workflow and
+would otherwise be checking the trigger that happened to start them.
 """
 
+import os
 import subprocess
 import sys
 from pathlib import Path
@@ -58,13 +63,20 @@ def clone(tmp_path):
     return clone
 
 
-def resolve(clone, ref):
+def resolve(clone, ref, event=None):
+    """Run the script with the named trigger, or with none, which is what a
+    machine that is not a runner looks like."""
+    environment = dict(os.environ)
+    environment.pop("GITHUB_EVENT_NAME", None)
+    if event is not None:
+        environment["GITHUB_EVENT_NAME"] = event
     return subprocess.run(
         [str(SCRIPT), ref],
         cwd=clone,
         check=False,
         capture_output=True,
         text=True,
+        env=environment,
     )
 
 
@@ -105,3 +117,45 @@ def test_a_ref_that_exists_nowhere_fails(clone):
 def test_a_number_with_no_pull_request_fails(clone):
     result = resolve(clone, "999")
     assert result.returncode != 0
+
+
+# What the script refuses, and what that refusal is and is not. It builds a ref
+# somebody without write access can choose, so it runs only under the triggers
+# where the ref was chosen by somebody who can already push. The trigger is the
+# one part of that a step can read: the permissions a job was given and the
+# secrets it holds are not visible from a script at all.
+
+
+def test_a_dispatch_is_how_the_match_workflows_run(clone):
+    assert resolve(clone, "v1.0.0", "workflow_dispatch").returncode == 0
+
+
+def test_a_push_is_how_the_release_workflow_runs(clone):
+    assert resolve(clone, "v1.0.0", "push").returncode == 0
+
+
+def test_no_trigger_at_all_is_a_machine_that_is_not_a_runner(clone):
+    assert resolve(clone, "v1.0.0").returncode == 0
+
+
+def test_pull_request_target_is_refused(clone):
+    result = resolve(clone, "7", "pull_request_target")
+    assert result.returncode != 0
+    assert "pull_request_target" in result.stderr
+    assert result.stdout == ""
+
+
+def test_workflow_run_is_refused(clone):
+    result = resolve(clone, "7", "workflow_run")
+    assert result.returncode != 0
+    assert "workflow_run" in result.stderr
+
+
+def test_a_comment_event_is_refused(clone):
+    result = resolve(clone, "7", "issue_comment")
+    assert result.returncode != 0
+    assert "issue_comment" in result.stderr
+
+
+def test_the_refusal_covers_a_named_ref_and_not_only_a_number(clone):
+    assert resolve(clone, "v1.0.0", "pull_request_target").returncode != 0