From 7a1c373fc54e6febfb46120ec9e1b931492783f1 Mon Sep 17 00:00:00 2001 From: marcohost33-maker Date: Thu, 1 Oct 2026 21:50:54 +0200 Subject: [PATCH 1/2] fix(release-gate): see nested shell publishers, allowlist OIDC grants (#184) Two layers against the #184 bypass (`bash -c 'twine --verbose upload'` in an unlisted workflow was classified as "no publisher"): 1. Credential boundary: every job granting `id-token: write` must be in OIDC_ALLOWLIST; allowlisted release jobs must deploy to environment `pypi`; workflow-level grants, unreadable permissions and publish-secret references fail closed. Shell text can never be parsed completely (curl, wrappers), so the OIDC grant is the boundary the gate actually controls. 2. Defence in depth: publish_evidence descends into the script argument of sh/bash/dash/zsh/ksh/ash -c (combined flags, -o opt, --long) and of eval, bounded depth; an untokenisable or too-deep nested script counts as publishing. Base patch: 007acc2 Claude answer 2026-09-30 (liouscope_184_oidc-allowlist). Nested descent + tests added here. Discrimination: disabling the descent turns all 7 nested tests red; full suite 1618 passed (1 pre-existing env failure: stale installed dist 0.5.0 vs 0.6.0.dev0 in test_version_single_source). Co-Authored-By: Claude Opus 5.5 --- .github/scripts/check_release_pins.py | 144 +++++++++++++++++++++- tests/test_release_gate_oidc_allowlist.py | 125 +++++++++++++++++++ 2 files changed, 266 insertions(+), 3 deletions(-) create mode 100644 tests/test_release_gate_oidc_allowlist.py diff --git a/.github/scripts/check_release_pins.py b/.github/scripts/check_release_pins.py index ab46058..8253e67 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,53 @@ 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 + + +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: + flag_at = position + 1 + while flag_at < len(segment) - 1: + flag = segment[flag_at] + if flag in {"-o", "+o", "-O", "+O"}: + flag_at += 2 + continue + if flag.startswith("--"): + flag_at += 1 + continue + if not flag.startswith(("-", "+")): + break + if "c" in flag[1:]: + inner.append(segment[flag_at + 1]) + break + flag_at += 1 + 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 +1381,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 +1404,95 @@ 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\.[A-Za-z0-9_]*(PYPI|TWINE|PUBLISH)", 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): + grant = permissions.get("id-token") + 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 + 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 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 +1541,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..fbd1b41 --- /dev/null +++ b/tests/test_release_gate_oidc_allowlist.py @@ -0,0 +1,125 @@ +"""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/*'\"", + ], +) +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 From c5ea22574774845410d2109686e903d47fa621bb Mon Sep 17 00:00:00 2001 From: marcohost33-maker Date: Thu, 1 Oct 2026 22:19:28 +0200 Subject: [PATCH 2/2] fix(release-gate): bash option parsing for nested -c scripts, secret/permission forms (#185 review) Aegis round on 7a1c373 (P2-1): the nested-shell descent missed `bash -eo pipefail -c`, GitHub's own `bash --noprofile --norc -eo pipefail`, `bash -c -- '...'`, `--rcfile/--init-file FILE -c`. `_shell_c_script` now follows bash option parsing (o-bundles consume their value, value-taking long options, `--` ends options, operand after a c flag is the script); a c flag without an operand raises -> counted as publishing. Also (P3-2/P3-3): publish secrets via `secrets['..']`/`secrets[".."]` and `toJSON(secrets)`, `secrets: inherit` on a job, `id-token` scope key matched case-insensitively, allowlisted jobs must list permissions explicitly. Tests: release-gate files 103 passed; against 7a1c373 13 of the new tests fail (the script-file control passes on both). Gate on repo: exit 0. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/check_release_pins.py | 84 ++++++++++++++++++----- tests/test_release_gate_oidc_allowlist.py | 46 +++++++++++++ 2 files changed, 113 insertions(+), 17 deletions(-) diff --git a/.github/scripts/check_release_pins.py b/.github/scripts/check_release_pins.py index 8253e67..295ab43 100644 --- a/.github/scripts/check_release_pins.py +++ b/.github/scripts/check_release_pins.py @@ -1298,6 +1298,48 @@ def _python_versions(text: str) -> set[str]: _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. @@ -1318,21 +1360,9 @@ def _command_segments(tokens: list[str], depth: int = 0) -> Iterator[list[str]]: inner.append(" ".join(segment[position + 1 :])) break if word in _NESTED_SHELLS: - flag_at = position + 1 - while flag_at < len(segment) - 1: - flag = segment[flag_at] - if flag in {"-o", "+o", "-O", "+O"}: - flag_at += 2 - continue - if flag.startswith("--"): - flag_at += 1 - continue - if not flag.startswith(("-", "+")): - break - if "c" in flag[1:]: - inner.append(segment[flag_at + 1]) - break - flag_at += 1 + script = _shell_c_script(segment, position) + if script is not None: + inner.append(script) break for script in inner: if depth >= _NESTED_DEPTH: @@ -1416,7 +1446,11 @@ def publish_evidence(text: str) -> tuple[list[str], str | None]: #: 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\.[A-Za-z0-9_]*(PYPI|TWINE|PUBLISH)", re.IGNORECASE) +_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: @@ -1431,7 +1465,12 @@ def _grants_id_token(permissions: Any) -> bool | None: return False return None if isinstance(permissions, dict): - grant = permissions.get("id-token") + # 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"}: @@ -1467,6 +1506,12 @@ def oidc_violations(name: str, text: str) -> list[str]: 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") @@ -1479,6 +1524,11 @@ def oidc_violations(name: str, text: str) -> list[str]: "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 = ( diff --git a/tests/test_release_gate_oidc_allowlist.py b/tests/test_release_gate_oidc_allowlist.py index fbd1b41..77133ba 100644 --- a/tests/test_release_gate_oidc_allowlist.py +++ b/tests/test_release_gate_oidc_allowlist.py @@ -106,6 +106,12 @@ def _workflow(run: str) -> str: "/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: @@ -123,3 +129,43 @@ def test_nested_shell_without_publisher_is_clean() -> 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))