diff --git a/.github/workflows/ci-guards.yml b/.github/workflows/ci-guards.yml index 284b3dc1e421..375ae2e713ae 100644 --- a/.github/workflows/ci-guards.yml +++ b/.github/workflows/ci-guards.yml @@ -974,6 +974,10 @@ jobs: if: ${{ matrix.group == 'release-notary' }} run: python3 tests/test_release_homebrew_gate.py + - name: Validate the Homebrew cask update keeps run metadata out of scripts + if: ${{ matrix.group == 'release-notary' }} + run: python3 tests/test_ci_homebrew_untrusted_input.py + - name: Validate universal GhosttyKit and Release build settings if: ${{ matrix.group == 'release-notary' }} run: ./tests/test_ci_universal_release_settings.sh diff --git a/.github/workflows/update-homebrew.yml b/.github/workflows/update-homebrew.yml index 0fb1d9adc91c..45844c9ac16f 100644 --- a/.github/workflows/update-homebrew.yml +++ b/.github/workflows/update-homebrew.yml @@ -41,7 +41,17 @@ jobs: # The cask downloads that DMG from the published release and verifies its # SHA256 below, so the only thing it needs from the run is that the signed # build job succeeded. - if: ${{ github.event_name == 'workflow_dispatch' || github.event.workflow_run.conclusion != 'cancelled' }} + # `workflow_run` matches the source by display name, so a run of any + # workflow with that name (a fork pull request's copy, for one) triggers + # this. Accept only the real release workflow run from a tag push in this + # repository; everything else about the run is attacker-controlled. + if: >- + ${{ github.event_name == 'workflow_dispatch' || ( + github.event.workflow_run.conclusion != 'cancelled' && + github.event.workflow_run.path == '.github/workflows/release.yml' && + github.event.workflow_run.head_repository.full_name == github.repository && + github.event.workflow_run.event == 'push' + ) }} runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || vars.LINUX_RUNNER || 'blacksmith-4vcpu-ubuntu-2404' }} timeout-minutes: 5 outputs: @@ -79,12 +89,19 @@ jobs: steps: - name: Get version id: version + # Run metadata and the dispatch input reach the script only through + # env: substituted into the script text, a branch name like + # `$(cmd)` would run with this job's tap token. + env: + INPUT_VERSION: ${{ github.event.inputs.version }} + HEAD_BRANCH: ${{ github.event.workflow_run.head_branch }} + EVENT_NAME: ${{ github.event_name }} run: | - if [ -n "${{ github.event.inputs.version }}" ]; then - VERSION="${{ github.event.inputs.version }}" + if [ -n "$INPUT_VERSION" ]; then + VERSION="$INPUT_VERSION" else # workflow_run: extract tag from the triggering workflow's head branch - VERSION="${{ github.event.workflow_run.head_branch }}" + VERSION="$HEAD_BRANCH" fi VERSION="${VERSION#v}" if [ -z "$VERSION" ]; then @@ -92,7 +109,7 @@ jobs: exit 1 fi if ! [[ "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then - if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then + if [ "$EVENT_NAME" = "workflow_dispatch" ]; then echo "Invalid version: ${VERSION}" >&2 exit 1 fi diff --git a/tests/test-execution.toml b/tests/test-execution.toml index fde54a719f1a..e70b0fd3cb8f 100644 --- a/tests/test-execution.toml +++ b/tests/test-execution.toml @@ -1513,3 +1513,7 @@ lane = "linux-guard" [[test]] path = "tests/test_ci_fast_guard_status.py" lane = "linux-guard" + +[[test]] +path = "tests/test_ci_homebrew_untrusted_input.py" +lane = "linux-guard" diff --git a/tests/test_ci_homebrew_untrusted_input.py b/tests/test_ci_homebrew_untrusted_input.py new file mode 100755 index 000000000000..e9a0bf763b89 --- /dev/null +++ b/tests/test_ci_homebrew_untrusted_input.py @@ -0,0 +1,90 @@ +#!/usr/bin/env python3 +"""update-homebrew.yml must not let a triggering run's metadata run code. + +The job holds the Homebrew tap token. `workflow_run` matches the source +workflow by display name, and `github.event.workflow_run.head_branch` comes +from whoever pushed the run's branch, so its value is attacker data. GitHub +substitutes `${{ ... }}` into a `run:` script as text before the shell parses +it: a branch named `$(touch${IFS}pwned)` used to run as a command. Values must +reach the script through `env:`, and the gate must accept only the real +release workflow run from a tag push in this repository. + +This test renders each step the way the runner does (expressions substituted +into `run:` and `env:`), runs the version step with a hostile branch name, and +checks that nothing executed. +""" + +import os +import re +import subprocess +import sys +import tempfile + +import yaml + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +HOMEBREW = os.path.join(ROOT, ".github", "workflows", "update-homebrew.yml") +FAILURES = [] + + +def _check(cond, msg): + if not cond: + FAILURES.append(msg) + print(f"FAIL: {msg}") + else: + print(f"ok: {msg}") + + +def render(text, context): + """Substitute `${{ expr }}` like the runner: known paths get their value, + anything else becomes empty.""" + return re.sub(r"\$\{\{\s*([^}]+?)\s*\}\}", lambda m: context.get(m.group(1).strip(), ""), str(text)) + + +def main(): + workflow = yaml.safe_load(open(HOMEBREW, encoding="utf-8")) + job = workflow["jobs"]["update-cask"] + step = next(s for s in job["steps"] if s.get("id") == "version") + + with tempfile.TemporaryDirectory() as tmp: + marker = os.path.join(tmp, "pwned") + hostile = f"$(touch${{IFS}}{marker})" + context = { + "github.event.workflow_run.head_branch": hostile, + "github.event.inputs.version": "", + "github.event_name": "workflow_run", + } + output = os.path.join(tmp, "output") + open(output, "w").close() + env = {"PATH": os.environ.get("PATH", "/usr/bin:/bin"), "GITHUB_OUTPUT": output} + for key, value in (step.get("env") or {}).items(): + env[key] = render(value, context) + script = render(step["run"], context) + subprocess.run(["bash", "-e", "-c", script], env=env, cwd=tmp, capture_output=True, text=True) + _check(not os.path.exists(marker), "a hostile head_branch never runs as a command in the version step") + _check("skip=true" in open(output).read(), "a non-release branch name is skipped, not used as a version") + + for name, job_def in workflow["jobs"].items(): + for s in job_def.get("steps", []): + run = str(s.get("run", "")) + for expr in re.findall(r"\$\{\{\s*([^}]+?)\s*\}\}", run): + _check( + not expr.startswith(("github.event.workflow_run", "github.event.inputs")), + f"{name}: `{s.get('name')}` does not substitute {expr} into its script", + ) + + gate_if = str(workflow["jobs"]["gate"].get("if", "")) + for condition in ( + "github.event.workflow_run.path == '.github/workflows/release.yml'", + "github.event.workflow_run.head_repository.full_name == github.repository", + "github.event.workflow_run.event == 'push'", + ): + _check(condition in gate_if, f"the gate requires {condition}") + + if FAILURES: + print(f"\n{len(FAILURES)} failure(s)") + sys.exit(1) + + +if __name__ == "__main__": + main()