diff --git a/.github/workflows/validate_georelays.yml b/.github/workflows/validate_georelays.yml new file mode 100644 index 00000000..09276d10 --- /dev/null +++ b/.github/workflows/validate_georelays.yml @@ -0,0 +1,114 @@ +name: Validate GeoRelay Data + +# The weekly automation validates a relay snapshot before pushing its branch, +# but that check lives inside the scheduled job: a relay-directory change that +# arrives as a pull request is otherwise reviewed on faith. This runs the same +# validator against the reviewed baseline and publishes the resulting hash, so +# a proposed snapshot can be matched against the one the automation published. +on: + pull_request: + paths: + - "relays/online_relays_gps.csv" + - "scripts/validate_georelays.py" + - "scripts/tests/**" + - ".github/workflows/validate_georelays.yml" + +# Both jobs run against pull-request content. Read-only, no secrets, and never +# pull_request_target. +permissions: + contents: read + +# A superseded run has nothing left to report; the sibling PR workflows do not +# set this, so drop it if the repo prefers them uniform. +concurrency: + group: validate-georelays-${{ github.ref }} + cancel-in-progress: true + +jobs: + # The trust boundary. Nothing authored by the pull request executes in this + # job, and the bytes it validates come from a git object rather than a + # worktree that PR-authored code could have rewritten. + gate: + name: Validate relay directory + runs-on: ubuntu-latest + timeout-minutes: 10 + + steps: + - name: Check out the reviewed base + # base.sha, not the merge ref: the pull request's validator and its + # tests are never present in this workspace, so the gate cannot be + # influenced by code under review. + uses: actions/checkout@v5 + with: + ref: ${{ github.event.pull_request.base.sha }} + persist-credentials: false + + - name: Validate the proposed relay directory + env: + PR_NUMBER: ${{ github.event.pull_request.number }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + + # Read the candidate as a git object pinned to the event's head commit. + # Reading it from a checked-out worktree would let PR-authored code + # swap the file first, so the hash below would describe a snapshot + # that never merges. + git fetch --no-tags --depth=1 origin "refs/pull/${PR_NUMBER}/head" + fetched_sha="$(git rev-parse FETCH_HEAD)" + if [[ "$fetched_sha" != "$HEAD_SHA" ]]; then + echo "::error::pull request head moved to $fetched_sha since this event ($HEAD_SHA); a newer run validates it" + exit 1 + fi + git show "${HEAD_SHA}:relays/online_relays_gps.csv" > "$RUNNER_TEMP/candidate.csv" + + # Validator and baseline are the checked-out base commit's, so a change + # cannot relax the rule that judges it. --output is required and gets a + # verbatim copy of --input; it stays in RUNNER_TEMP. + python3 scripts/validate_georelays.py \ + --input "$RUNNER_TEMP/candidate.csv" \ + --baseline relays/online_relays_gps.csv \ + --output "$RUNNER_TEMP/validated-copy.csv" \ + --github-output "$RUNNER_TEMP/summary.txt" + + # Publish the hash so a reviewer can match a proposed snapshot against + # the one the automation published, instead of taking it on faith. + { + echo "### GeoRelay validation" + echo + sed 's/^/- /' "$RUNNER_TEMP/summary.txt" + } >> "$GITHUB_STEP_SUMMARY" + + # Runs code authored by the pull request, so it is an honest-mistake check and + # not a trust boundary: it catches a validator change that would invalidate the + # committed directory and break the weekly automation after merge. + proposed-validator: + name: Test the proposed validator + runs-on: ubuntu-latest + timeout-minutes: 10 + + steps: + - name: Check out the pull request + uses: actions/checkout@v5 + with: + persist-credentials: false + + - name: Test GeoRelay validator + run: python3 -m unittest discover -s scripts/tests -p "test_*.py" -v + + - name: Run the proposed validator against the real directory + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: | + set -euo pipefail + + # Without this the proposed validator only ever sees unit-test + # fixtures, and a new constraint can make the committed directory + # invalid while this workflow stays green. + git fetch --no-tags --depth=1 origin "$BASE_SHA" + git show "${BASE_SHA}:relays/online_relays_gps.csv" > "$RUNNER_TEMP/baseline.csv" + + python3 scripts/validate_georelays.py \ + --input relays/online_relays_gps.csv \ + --baseline "$RUNNER_TEMP/baseline.csv" \ + --output "$RUNNER_TEMP/proposed-copy.csv" diff --git a/scripts/tests/test_georelay_pr_workflow.py b/scripts/tests/test_georelay_pr_workflow.py new file mode 100644 index 00000000..5e49e8e0 --- /dev/null +++ b/scripts/tests/test_georelay_pr_workflow.py @@ -0,0 +1,89 @@ +import re +from pathlib import Path +import unittest + + +REPOSITORY_ROOT = Path(__file__).resolve().parents[2] +WORKFLOW_PATH = REPOSITORY_ROOT / ".github/workflows/validate_georelays.yml" +RELAY_DATA_PATH = "relays/online_relays_gps.csv" +VALIDATOR_PATH = "scripts/validate_georelays.py" + + +class ValidateGeoRelaysWorkflowTests(unittest.TestCase): + """The relay directory is reviewed data. A pull request that changes it must + be judged by the base branch's validator, on the bytes that would actually + merge, and the job that does so must not run anything the pull request + wrote -- the `gate` job is the trust boundary, `proposed-validator` is not. + """ + + @classmethod + def setUpClass(cls) -> None: + cls.workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + # Comments explain why a construct is absent and would otherwise trip + # the assertions looking for it, so directives are checked without them. + cls.directives = "\n".join( + line for line in cls.workflow.splitlines() if not line.lstrip().startswith("#") + ) + + def job(self, name: str) -> str: + block = re.search(rf"^ {name}:\n(.*?)(?=^ \S|\Z)", self.directives, re.MULTILINE | re.DOTALL) + self.assertIsNotNone(block, f"no {name} job in the workflow") + return block.group(1) + + def test_runs_on_untrusted_pull_request_trigger(self) -> None: + self.assertIn("pull_request:", self.directives) + # pull_request_target would run PR-authored code with repository secrets. + self.assertNotIn("pull_request_target", self.directives) + + def test_no_job_holds_a_write_scope(self) -> None: + self.assertIn("permissions:\n contents: read", self.directives) + write_scope = re.search(r"^\s*[\w-]+:\s*write\s*$", self.directives, re.MULTILINE) + self.assertIsNone(write_scope, "a job running PR-authored code must not hold a write scope") + + def test_checkout_does_not_persist_credentials(self) -> None: + self.assertEqual(2, self.directives.count("persist-credentials: false")) + + def test_relay_data_changes_trigger_the_workflow(self) -> None: + paths_block = re.search(r"paths:\n((?:\s+- .+\n)+)", self.directives) + self.assertIsNotNone(paths_block) + self.assertIn(RELAY_DATA_PATH, paths_block.group(1)) + self.assertIn(VALIDATOR_PATH, paths_block.group(1)) + + def test_gate_checks_out_the_reviewed_base(self) -> None: + # Checking out the merge ref would put the pull request's validator and + # tests in the gate's workspace. + self.assertIn("ref: ${{ github.event.pull_request.base.sha }}", self.job("gate")) + + def test_gate_runs_nothing_the_pull_request_wrote(self) -> None: + gate = self.job("gate") + self.assertNotIn("unittest discover", gate) + # The candidate is read as a git object, never from the worktree, so + # code running earlier in the job cannot substitute the bytes validated. + self.assertIn('git show "${HEAD_SHA}:' + RELAY_DATA_PATH + '"', gate) + self.assertIn('--input "$RUNNER_TEMP/candidate.csv"', gate) + self.assertNotIn(f"--input {RELAY_DATA_PATH}", gate) + + def test_gate_pins_both_inputs_to_the_event(self) -> None: + gate = self.job("gate") + self.assertIn("HEAD_SHA: ${{ github.event.pull_request.head.sha }}", gate) + # A head that moved after the event means these bytes are not the ones + # this run was asked about. + self.assertIn('if [[ "$fetched_sha" != "$HEAD_SHA" ]]; then', gate) + self.assertIn("exit 1", gate) + + def test_gate_validation_cannot_write_to_the_reviewed_file(self) -> None: + gate = self.job("gate") + self.assertIn('--output "$RUNNER_TEMP/', gate) + self.assertNotIn(f"--output {RELAY_DATA_PATH}", gate) + + def test_proposed_validator_is_exercised_against_the_real_directory(self) -> None: + # Unit-test fixtures are small and synthetic; a new constraint can pass + # them while invalidating the committed directory. + job = self.job("proposed-validator") + self.assertIn("unittest discover", job) + self.assertIn(f"--input {RELAY_DATA_PATH}", job) + self.assertIn('git show "${BASE_SHA}:' + RELAY_DATA_PATH + '"', job) + + +if __name__ == "__main__": + unittest.main()