diff --git a/.github/workflows/maint-52-sync-dev-versions.yml b/.github/workflows/maint-52-sync-dev-versions.yml index 4c79f0e4e..ee6d04a3d 100644 --- a/.github/workflows/maint-52-sync-dev-versions.yml +++ b/.github/workflows/maint-52-sync-dev-versions.yml @@ -112,9 +112,11 @@ jobs: - name: Build repo matrix id: repos + env: + INPUT_REPOS: ${{ inputs.repos }} run: | - if [ -n "${{ inputs.repos }}" ]; then - repos="${{ inputs.repos }}" + if [ -n "$INPUT_REPOS" ]; then + repos="$INPUT_REPOS" else repos=$(python scripts/list_registered_consumer_repos.py --separator ',') fi @@ -126,6 +128,11 @@ jobs: | map(select(. != "")) | map(gsub("^\\s+|\\s+$"; "")) ') + invalid_repos=$(jq -r '.[] | select(test("^[A-Za-z0-9][A-Za-z0-9_.-]*/[A-Za-z0-9][A-Za-z0-9_.-]*$") | not)' <<<"$json_array") + if [ -n "$invalid_repos" ]; then + echo "Invalid owner/repository value(s): $invalid_repos" >&2 + exit 1 + fi echo "matrix={\"repo\":$json_array}" >> "$GITHUB_OUTPUT" echo "Repos to sync: $json_array" @@ -151,8 +158,9 @@ jobs: - name: Clone consumer repo env: GH_TOKEN: ${{ env.REPO_TOKEN }} + MATRIX_REPO: ${{ matrix.repo }} run: | - gh repo clone ${{ matrix.repo }} consumer -- --depth=1 + gh repo clone "$MATRIX_REPO" consumer -- --depth=1 - name: Download sync script uses: actions/download-artifact@v8 @@ -269,8 +277,10 @@ jobs: - name: Skip - no pyproject.toml if: steps.check.outputs.has_pyproject != 'true' + env: + MATRIX_REPO: ${{ matrix.repo }} run: | - echo "Skipping ${{ matrix.repo }}: no pyproject.toml found" + echo "Skipping $MATRIX_REPO: no pyproject.toml found" - name: Create sync PR if: >- diff --git a/.github/workflows/maint-69-sync-labels.yml b/.github/workflows/maint-69-sync-labels.yml index 183030efa..24e574c16 100644 --- a/.github/workflows/maint-69-sync-labels.yml +++ b/.github/workflows/maint-69-sync-labels.yml @@ -68,9 +68,11 @@ jobs: - name: Determine target repos id: repos + env: + INPUT_REPOS: ${{ inputs.repos }} run: | - if [ "${{ inputs.repos }}" != "all" ] && [ -n "${{ inputs.repos }}" ]; then - repos="${{ inputs.repos }}" + if [ "$INPUT_REPOS" != "all" ] && [ -n "$INPUT_REPOS" ]; then + repos="$INPUT_REPOS" else repos=$(python scripts/list_registered_consumer_repos.py --separator ',') fi diff --git a/.github/workflows/maint-70-fix-integration-formatting.yml b/.github/workflows/maint-70-fix-integration-formatting.yml index 5de77b2de..9b80de9be 100644 --- a/.github/workflows/maint-70-fix-integration-formatting.yml +++ b/.github/workflows/maint-70-fix-integration-formatting.yml @@ -113,6 +113,8 @@ jobs: - name: Commit and push changes if: steps.changes.outputs.has_changes == 'true' + env: + COMMIT_MESSAGE: ${{ inputs.commit_message }} run: | cd integration-tests git config user.name "github-actions[bot]" @@ -121,7 +123,7 @@ jobs: run_url="https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" git add -A - git commit -m "${{ inputs.commit_message }}" \ + git commit -m "$COMMIT_MESSAGE" \ -m "" \ -m "Applied by: ${run_url}" diff --git a/.github/workflows/reusable-codex-run.yml b/.github/workflows/reusable-codex-run.yml index b7c4f6e3e..8bda73385 100644 --- a/.github/workflows/reusable-codex-run.yml +++ b/.github/workflows/reusable-codex-run.yml @@ -860,6 +860,12 @@ jobs: CODEX_HOME: ${{ runner.temp }}/.codex CODEX_MODEL_CANDIDATES: ${{ steps.codex_model.outputs.candidates }} CODEX_MODEL_SELECTION_REASON: ${{ steps.codex_model.outputs.selection_reason }} + PROMPT_FILE: ${{ steps.prompt.outputs.file }} + PR_NUM: ${{ inputs.pr_number }} + SANDBOX: ${{ inputs.sandbox }} + EXTRA_ARGS_RAW: ${{ inputs.codex_args }} + MAX_RUNTIME_MIN: ${{ inputs.max_runtime_minutes }} + TARGET_BRANCH: ${{ inputs.pr_ref || github.ref_name }} run: | set -euo pipefail @@ -869,9 +875,7 @@ jobs: chmod 600 "$CODEX_HOME/auth.json" # Build codex exec command - PROMPT_FILE="${{ steps.prompt.outputs.file }}" # Use PR-specific output filename to avoid merge conflicts - PR_NUM="${{ inputs.pr_number }}" if [ -n "${PR_NUM}" ]; then OUTPUT_FILE="codex-output-${PR_NUM}.md" SESSION_JSONL="codex-session-${PR_NUM}.jsonl" @@ -879,8 +883,6 @@ jobs: OUTPUT_FILE="codex-output.md" SESSION_JSONL="codex-session.jsonl" fi - SANDBOX="${{ inputs.sandbox }}" - EXTRA_ARGS_RAW="${{ inputs.codex_args }}" # Default sandbox if not specified if [ -z "$SANDBOX" ]; then @@ -949,7 +951,6 @@ jobs: # When the job approaches the timeout limit, this background process # commits and pushes any uncommitted work so it isn't lost to the # job cancellation. It fires once, 5 minutes before max_runtime. - MAX_RUNTIME_MIN=${{ inputs.max_runtime_minutes }} GRACE_MIN=5 WATCHDOG_DELAY=$(( (MAX_RUNTIME_MIN - GRACE_MIN) * 60 )) echo "watchdog-saved=false" >> "$GITHUB_OUTPUT" @@ -961,7 +962,6 @@ jobs: echo "::warning::Pre-timeout watchdog fired "\ "(${GRACE_MIN}m before ${MAX_RUNTIME_MIN}m limit)" - TARGET_BRANCH="${{ inputs.pr_ref }}" TARGET_BRANCH="${TARGET_BRANCH#refs/heads/}" PUSH_TOKEN="${{ steps.run_base.outputs.push_token }}" REMOTE_URL="https://x-access-token:${PUSH_TOKEN}@github.com/${{ github.repository }}" diff --git a/tests/workflows/test_no_untrusted_interpolation.py b/tests/workflows/test_no_untrusted_interpolation.py new file mode 100644 index 000000000..f62037b35 --- /dev/null +++ b/tests/workflows/test_no_untrusted_interpolation.py @@ -0,0 +1,147 @@ +"""Regression guard for free-text Actions values embedded in scripts. + +Workflow expressions are evaluated before a shell or github-script body runs. +The listed values are free-form workflow-dispatch or runner inputs, so placing +them directly in a ``run:``/``with.script:`` body can turn quotes or shell +metacharacters into source code. They must cross that boundary through a +step-level ``env:`` value instead. +""" + +from __future__ import annotations + +import re +from collections.abc import Iterable +from pathlib import Path + +import pytest +import yaml + +ROOT = Path(__file__).resolve().parents[2] +WORKFLOW_GLOBS = (".github/workflows/*.yml", ".github/workflows/*.yaml") + +# Keep this deliberately small. Other expressions require a file-by-file +# constrained-value review; expanding this set is not a substitute for that +# review. +UNTRUSTED_EXPRESSIONS = frozenset({"inputs.commit_message", "inputs.codex_args", "inputs.repos"}) + + +def _workflow_paths() -> Iterable[Path]: + for pattern in WORKFLOW_GLOBS: + yield from sorted(ROOT.glob(pattern)) + + +def _script_values(path: Path) -> Iterable[tuple[str, str]]: + data = yaml.safe_load(path.read_text(encoding="utf-8")) or {} + for job_name, job in (data.get("jobs") or {}).items(): + if not isinstance(job, dict): + continue + for index, step in enumerate(job.get("steps") or []): + if not isinstance(step, dict): + continue + run = step.get("run") + if isinstance(run, str): + yield f"{job_name}/step-{index}/run", run + script = (step.get("with") or {}).get("script") + if isinstance(script, str): + yield f"{job_name}/step-{index}/with.script", script + + +def _actions_expression_bodies(script: str) -> Iterable[str]: + """Yield Actions expression bodies without ending quoted brace literals early.""" + + start = 0 + while (opening := script.find("${{", start)) != -1: + index = opening + 3 + quote: str | None = None + escaped = False + while index < len(script) - 1: + character = script[index] + if quote: + if escaped: + escaped = False + elif character == "\\": + escaped = True + elif character == quote: + quote = None + elif character in {"'", '"'}: + quote = character + elif script[index : index + 2] == "}}": + yield script[opening + 3 : index] + start = index + 2 + break + index += 1 + else: + start = opening + 3 + + +def _references_untrusted_input(body: str, expression: str) -> bool: + """Recognize equivalent property and bracket references in an expression.""" + + _, property_name = expression.split(".", maxsplit=1) + return bool( + re.search( + rf"\binputs\s*(?:\.\s*{re.escape(property_name)}\b|\[\s*['\"]{re.escape(property_name)}['\"]\s*\])", + body, + ) + ) + + +def _untrusted_references(script: str) -> list[str]: + """Return free-text inputs referenced anywhere in Actions expressions.""" + + expression_bodies = list(_actions_expression_bodies(script)) + return sorted( + expression + for expression in UNTRUSTED_EXPRESSIONS + if any(_references_untrusted_input(body, expression) for body in expression_bodies) + ) + + +def test_no_untrusted_expressions_in_script_bodies() -> None: + """Free-text inputs must not be interpolated into shell or JS source.""" + violations: list[str] = [] + for workflow in _workflow_paths(): + for location, script in _script_values(workflow): + for expression in _untrusted_references(script): + violations.append(f"{workflow.relative_to(ROOT)}:{location}: {expression}") + assert not violations, ( + "Pass untrusted workflow values through step env and consume the env " + "variable in the script:\n" + "\n".join(violations) + ) + + +@pytest.mark.parametrize( + ("body", "expression", "expected"), + [ + ("inputs.commit_message", "inputs.commit_message", True), + ("inputs['commit_message']", "inputs.commit_message", True), + ("inputs.repos || 'all'", "inputs.repos", True), + ("inputs['repos']", "inputs.repos", True), + ("inputs.codex_args", "inputs.codex_args", True), + ("inputs.safe_field", "inputs.commit_message", False), + ], +) +def test_references_untrusted_input(body: str, expression: str, expected: bool) -> None: + assert _references_untrusted_input(body, expression) is expected + + +@pytest.mark.parametrize("expression", sorted(UNTRUSTED_EXPRESSIONS)) +def test_untrusted_expression_guard_detects_listed_inputs(expression: str) -> None: + """Each listed field must be detectable via _untrusted_references.""" + + property_name = expression.split(".", 1)[1] + dot_form = f"echo ${{{{ inputs.{property_name} }}}}" + bracket_form = f"echo ${{{{ inputs['{property_name}'] }}}}" + assert expression in _untrusted_references(dot_form) + assert expression in _untrusted_references(bracket_form) + + +def test_untrusted_expression_guard_matches_default_and_wrapper_forms() -> None: + assert _untrusted_references("echo ${{ inputs.repos || 'all' }}") == ["inputs.repos"] + assert _untrusted_references("const v = '${{ format('{0}', inputs.codex_args) }}';") == [ + "inputs.codex_args" + ] + assert _untrusted_references("echo ${{ inputs['repos'] }}") == ["inputs.repos"] + assert _untrusted_references("${{ format('{{prefix}} {0}', inputs.codex_args) }}") == [ + "inputs.codex_args" + ]