diff --git a/.github/scripts/check_release_pins.py b/.github/scripts/check_release_pins.py index ab46058..295ab43 100644 --- a/.github/scripts/check_release_pins.py +++ b/.github/scripts/check_release_pins.py @@ -52,7 +52,7 @@ import sys from dataclasses import dataclass, field from pathlib import Path -from typing import Any +from typing import Any, Iterator try: # pragma: no cover - exercised only on Python < 3.11 import tomllib @@ -1293,6 +1293,83 @@ def _python_versions(text: str) -> set[str]: return {scalar.value for key, scalar in _scalars(document) if key == "python-version"} +#: Shells whose ``-c`` argument is itself a script (issue #184). +_NESTED_SHELLS = frozenset({"sh", "bash", "dash", "zsh", "ksh", "ash"}) +_NESTED_DEPTH = 4 + + +_SHELL_OPTIONS_WITH_VALUE = frozenset({"--rcfile", "--init-file"}) + + +def _shell_c_script(segment: list[str], position: int) -> str | None: + """The ``-c`` script of the shell at ``segment[position]``, else None. + + Follows bash option parsing: single-letter bundles (``-eo pipefail -c``, + ``-ec``, ``+o``), long options (``--noprofile``; ``--rcfile FILE`` and + ``--init-file FILE`` take a value), ``--`` ends the options, and the + first operand after a ``c`` flag is the script (``bash -c -- 'x'``). + A ``c`` flag without a following operand raises :class:`ValueError` + (fail-closed, review of #184/#185). + """ + wants_script = False + options_done = False + at = position + 1 + while at < len(segment): + word = segment[at] + if not options_done: + if word == "--": + options_done = True + at += 1 + continue + if word in _SHELL_OPTIONS_WITH_VALUE: + at += 2 + continue + if word.startswith("--"): + at += 1 + continue + if word[:1] in {"-", "+"} and len(word) > 1: + letters = word[1:] + wants_script = wants_script or "c" in letters + at += 2 if ("o" in letters or "O" in letters) else 1 + continue + if wants_script: + return word + return None + if wants_script: + raise ValueError("shell -c without an inspectable script") + return None + + +def _command_segments(tokens: list[str], depth: int = 0) -> Iterator[list[str]]: + """Every command segment of ``tokens``, descending into nested scripts. + + ``bash -c 'twine --verbose upload dist/*'`` is ONE shlex token for the + script, so the tool/verb check would never see ``twine`` (issue #184). The + argument of a shell's ``-c`` (also combined flags such as ``-ec``) and of + ``eval`` is tokenised again and its segments yielded as well. A nested + script that cannot be tokenised raises :class:`ValueError`, which the + caller already treats as publishing; nesting deeper than + :data:`_NESTED_DEPTH` raises it too, so depth cannot hide a command. + """ + for segment in _segments(tokens): + yield segment + words = [word.rsplit("/", 1)[-1] for word in segment] + inner: list[str] = [] + for position, word in enumerate(words): + if word == "eval": + inner.append(" ".join(segment[position + 1 :])) + break + if word in _NESTED_SHELLS: + script = _shell_c_script(segment, position) + if script is not None: + inner.append(script) + break + for script in inner: + if depth >= _NESTED_DEPTH: + raise ValueError("nested shell scripts too deep to inspect") + yield from _command_segments(_tokens(script), depth + 1) + + def publish_evidence(text: str) -> tuple[list[str], str | None]: """Why a workflow counts as publishing to PyPI, and a read error if any. @@ -1334,11 +1411,11 @@ def publish_evidence(text: str) -> tuple[list[str], str | None]: ): continue try: - tokens = _tokens(line) + segments = list(_command_segments(_tokens(line))) except ValueError: evidence.append(f"line {number}: cannot tokenise {line.strip()!r}") continue - for segment in _segments(tokens): + for segment in segments: words = [word.rsplit("/", 1)[-1] for word in segment] for position, word in enumerate(words): verb = _PUBLISH_TOOLS.get(word) @@ -1357,6 +1434,115 @@ def publish_evidence(text: str) -> tuple[list[str], str | None]: return evidence, (uninspectable[0] if uninspectable else None) +#: Jobs allowed to request an OIDC token (``id-token: write``), as +#: ``(workflow file, job id)``. The PyPI credential boundary is the Trusted +#: Publisher (workflow file + GitHub environment ``pypi`` + that environment's +#: deployment rules); this allowlist makes any *new* OIDC grant a visible gate +#: failure instead of relying on recognising publish commands in shell text, +#: which cannot be complete (``curl`` to the upload API, wrappers, ``eval``). +OIDC_ALLOWLIST: frozenset[tuple[str, str]] = frozenset( + {("pypi.yml", "publish"), ("scorecard.yml", "analysis")} +) +#: Allowlisted jobs in a release workflow must deploy to this environment, so +#: the OIDC ``environment`` claim matches the Trusted Publisher binding. +PUBLISH_ENVIRONMENT = "pypi" +_PUBLISH_SECRET_RE = re.compile( + r"secrets\s*(?:\.\s*|\[\s*['\"])[A-Za-z0-9_]*(?:PYPI|TWINE|PUBLISH)" + r"|toJSON\(\s*secrets\s*\)", + re.IGNORECASE, +) + + +def _grants_id_token(permissions: Any) -> bool | None: + """True/False for a ``permissions`` node; None if its shape is unknown.""" + if permissions is None: + return False + if isinstance(permissions, YamlScalar): + value = permissions.value.strip() + if value == "write-all": + return True + if value in {"read-all", ""}: + return False + return None + if isinstance(permissions, dict): + # Scope keys compared case-insensitively: fail-closed if GitHub + # ever accepts `ID-Token` (review of #185). + grant = next( + (value for key, value in permissions.items() if str(key).lower() == "id-token"), + None, + ) + if grant is None: + return False + if isinstance(grant, YamlScalar) and grant.value.strip() in {"none", "read"}: + return False + if isinstance(grant, YamlScalar) and grant.value.strip() == "write": + return True + return None + return None + + +def oidc_violations(name: str, text: str) -> list[str]: + """Every OIDC grant outside :data:`OIDC_ALLOWLIST`, fail-closed on doubt.""" + shown = f".github/workflows/{name}" + errors = [ + f"{shown}: references a publish credential secret ({match.group(0)!r}); " + "PyPI uploads must use Trusted Publishing" + for match in _PUBLISH_SECRET_RE.finditer(text) + ] + try: + document = load_workflow_yaml(text) + except WorkflowYamlError as exc: + return [*errors, f"{shown}: cannot check OIDC grants ({exc})"] + top = _grants_id_token(document.get("permissions")) + if top is not False: + errors.append( + f"{shown}: workflow-level permissions grant or may grant id-token: write; " + "grant it per job, and only to an allowlisted job" + ) + jobs = document.get("jobs") + if not isinstance(jobs, dict): + return [*errors, f"{shown}: 'jobs' is not a mapping"] + for job_id, job in jobs.items(): + if not isinstance(job, dict): + errors.append(f"{shown}: job {job_id!r} is not a mapping") + continue + secrets_node = job.get("secrets") + if isinstance(secrets_node, YamlScalar) and secrets_node.value.strip() == "inherit": + errors.append( + f"{shown}: job {job_id!r} passes `secrets: inherit`; name the secrets " + "a called workflow needs" + ) + grant = _grants_id_token(job.get("permissions")) + if grant is None: + errors.append(f"{shown}: job {job_id!r} has permissions this gate cannot read") + continue + if not grant: + continue + if (name, job_id) not in OIDC_ALLOWLIST: + errors.append( + f"{shown}: job {job_id!r} requests id-token: write but is not in " + "OIDC_ALLOWLIST" + ) + continue + if not isinstance(job.get("permissions"), dict): + errors.append( + f"{shown}: allowlisted job {job_id!r} must list its permissions " + "explicitly, not `write-all`" + ) + if name in RELEASE_WORKFLOWS: + environment = job.get("environment") + env_name = ( + environment.get("name") if isinstance(environment, dict) else environment + ) + if not (isinstance(env_name, YamlScalar) and env_name.value == PUBLISH_ENVIRONMENT): + errors.append( + f"{shown}: job {job_id!r} holds id-token: write without " + f"environment {PUBLISH_ENVIRONMENT!r}; the Trusted Publisher " + "binding and its deployment rules would not apply" + ) + return errors + + def check_workflow(path: Path, root: Path) -> list[str]: """All contract violations of one release workflow.""" text = path.read_text(encoding="utf-8") @@ -1405,6 +1591,8 @@ def check(root: Path = ROOT) -> list[str]: errors.extend(check_workflow(path, root)) if workflows.is_dir(): for path in sorted(workflows.iterdir()): + if path.suffix in {".yml", ".yaml"}: + errors.extend(oidc_violations(path.name, path.read_text(encoding="utf-8"))) if path.suffix not in {".yml", ".yaml"} or path.name in RELEASE_WORKFLOWS: continue evidence, unreadable = publish_evidence(path.read_text(encoding="utf-8")) diff --git a/tests/test_release_gate_oidc_allowlist.py b/tests/test_release_gate_oidc_allowlist.py new file mode 100644 index 0000000..77133ba --- /dev/null +++ b/tests/test_release_gate_oidc_allowlist.py @@ -0,0 +1,171 @@ +"""OIDC allowlist of the release gate (LiouScope #184, Claude cross-check 2026-09-30). + +Publisher detection on shell text cannot be complete (curl to the upload API, +wrapper scripts, eval). The gate therefore also pins WHICH jobs may request an +OIDC token; PyPI's Trusted Publisher binding (workflow file + environment +``pypi`` + its deployment rules) is the credential boundary itself. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / ".github" / "scripts")) +import check_release_pins as gate + +ROOT = Path(__file__).resolve().parents[1] + + +def _wf(job: str) -> str: + return "name: x\non: push\npermissions: {}\njobs:\n" + job + + +def test_repository_workflows_pass() -> None: + for path in sorted((ROOT / ".github" / "workflows").glob("*.yml")): + assert gate.oidc_violations(path.name, path.read_text(encoding="utf-8")) == [] + + +@pytest.mark.parametrize( + "job", + [ + " sneak:\n runs-on: u\n permissions:\n id-token: write\n steps:\n - run: ./release.sh\n", + " sneak:\n runs-on: u\n permissions: write-all\n steps:\n - run: echo\n", + ], +) +def test_unlisted_job_with_oidc_fails(job: str) -> None: + errors = gate.oidc_violations("other.yml", _wf(job)) + assert any("not in OIDC_ALLOWLIST" in e for e in errors) + + +def test_workflow_level_grant_fails() -> None: + text = "name: x\non: push\npermissions:\n id-token: write\njobs:\n a:\n runs-on: u\n steps:\n - run: echo\n" + assert any("workflow-level" in e for e in gate.oidc_violations("other.yml", text)) + + +def test_unreadable_permissions_fail_closed() -> None: + job = " a:\n runs-on: u\n permissions: ${{ fromJSON(vars.P) }}\n steps:\n - run: echo\n" + errors = gate.oidc_violations("other.yml", _wf(job)) + assert errors, "an expression-valued permissions node must not pass silently" + + +def test_publish_job_without_environment_fails() -> None: + job = " publish:\n runs-on: u\n permissions:\n id-token: write\n steps:\n - run: echo\n" + errors = gate.oidc_violations("pypi.yml", _wf(job)) + assert any("without environment 'pypi'" in e for e in errors) + + +def test_publish_job_with_other_environment_fails() -> None: + job = ( + " publish:\n runs-on: u\n environment: staging\n" + " permissions:\n id-token: write\n steps:\n - run: echo\n" + ) + assert gate.oidc_violations("pypi.yml", _wf(job)) + + +def test_publish_secret_reference_fails() -> None: + job = " a:\n runs-on: u\n steps:\n - run: twine upload -p ${{ secrets.PYPI_API_TOKEN }} dist/*\n" + assert any( + "publish credential secret" in e for e in gate.oidc_violations("other.yml", _wf(job)) + ) + + +def test_read_and_none_grants_pass() -> None: + job = " a:\n runs-on: u\n permissions:\n id-token: none\n contents: read\n steps:\n - run: echo\n" + assert gate.oidc_violations("other.yml", _wf(job)) == [] + + +def test_check_wires_the_allowlist(tmp_path: Path) -> None: + wf = tmp_path / ".github" / "workflows" + wf.mkdir(parents=True) + (wf / "pypi.yml").write_text( + (ROOT / ".github/workflows/pypi.yml").read_text(encoding="utf-8"), encoding="utf-8" + ) + (wf / "evil.yml").write_text( + _wf( + " x:\n runs-on: u\n permissions:\n id-token: write\n steps:\n - run: curl -F content=@d https://upload.pypi.org/legacy/\n" + ), + encoding="utf-8", + ) + errors = gate.check(tmp_path) + assert any("evil.yml" in e and "OIDC_ALLOWLIST" in e for e in errors) + + +def _workflow(run: str) -> str: + return f"jobs:\n x:\n runs-on: ubuntu-latest\n steps:\n - run: {run}\n" + + +@pytest.mark.parametrize( + "run", + [ + "bash -c 'twine --verbose upload dist/*'", + "sh -ec 'twine --verbose upload dist/*'", + "bash -o pipefail -c 'cd dist && twine --verbose upload *'", + "/bin/bash --noprofile -c 'twine --verbose upload dist/*'", + "eval 'twine --verbose upload dist/*'", + "bash -c \"sh -c 'twine --verbose upload dist/*'\"", + "bash -eo pipefail -c 'twine --verbose upload dist/*'", + "bash --noprofile --norc -eo pipefail -c 'twine --verbose upload dist/*'", + "bash -c -- 'twine --verbose upload dist/*'", + "bash --rcfile /dev/null -c 'twine --verbose upload dist/*'", + "bash --init-file /dev/null -c 'twine --verbose upload dist/*'", + "bash -c -o pipefail 'twine --verbose upload dist/*'", + ], +) +def test_nested_shell_publisher_is_evidence(run: str) -> None: + """Issue #184: the publisher inside a nested script is still seen.""" + evidence, unreadable = gate.publish_evidence(_workflow(run)) + assert unreadable is None + assert any("twine" in item and "upload" in item for item in evidence), evidence + + +def test_nested_shell_without_publisher_is_clean() -> None: + evidence, unreadable = gate.publish_evidence(_workflow("bash -c 'twine check dist/*'")) + assert (evidence, unreadable) == ([], None) + + +def test_untokenisable_nested_script_counts_as_publishing() -> None: + evidence, _ = gate.publish_evidence(_workflow("bash -c \"twine --verbose upload 'dist\"")) + assert evidence + + +def test_shell_c_without_script_counts_as_publishing() -> None: + evidence, _ = gate.publish_evidence(_workflow("bash -c")) + assert evidence == [] # no tool/verb on the line: pre-filter skips it + evidence, _ = gate.publish_evidence(_workflow("twine --verbose upload; bash -eo pipefail -c")) + assert any("cannot tokenise" in item for item in evidence), evidence + + +def test_shell_script_file_is_not_a_nested_script() -> None: + evidence, unreadable = gate.publish_evidence(_workflow("bash scripts/upload_docs.sh")) + assert (evidence, unreadable) == ([], None) + + +@pytest.mark.parametrize( + "secret", + [ + "${{ secrets['PYPI_API_TOKEN'] }}", + '${{ secrets["TWINE_PASSWORD"] }}', + "${{ toJSON(secrets) }}", + ], +) +def test_publish_secret_access_forms_fail(secret: str) -> None: + text = f"jobs:\n x:\n runs-on: ubuntu-latest\n steps:\n - run: echo {secret}\n" + assert gate.oidc_violations("other.yml", text) + + +def test_secrets_inherit_fails() -> None: + text = "jobs:\n x:\n uses: ./.github/workflows/ci.yml\n secrets: inherit\n" + assert any("secrets: inherit" in e for e in gate.oidc_violations("other.yml", text)) + + +def test_id_token_key_is_case_insensitive() -> None: + text = "jobs:\n x:\n permissions:\n ID-Token: write\n runs-on: ubuntu-latest\n" + assert any("OIDC_ALLOWLIST" in e for e in gate.oidc_violations("other.yml", text)) + + +def test_allowlisted_job_with_write_all_fails() -> None: + text = "jobs:\n analysis:\n permissions: write-all\n runs-on: ubuntu-latest\n" + assert any("explicitly" in e for e in gate.oidc_violations("scorecard.yml", text))