Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,16 @@ jobs:
- name: Every workflow file must parse
run: python3 -m pytest tests/test_workflow_yaml_valid.py -q

# WorkflowIntegrityGuard. Written for ADR-005 ("a CI step that cannot
# fail is worse than no CI step") and, until #5744, run by NO job at all
# -- the exact trap this repo's own convention warns about, on the guard
# whose whole subject is guards that do nothing. It also now rejects a
# `${{ }}` expansion of a workflow input inside a `run:` body, the
# template-injection shape #5673 fixed in auto-deploy-cloud.yml and
# #5744 fixed again here.
- name: Workflow integrity (dead steps, unreachable triggers, input injection)
run: python3 -m pytest tests/test_ci_workflow_invocations_are_real.py -q

# Named explicitly because this repo's CI runs FILE LISTS, not
# `pytest tests/`. /api/runtime-summary claimed to mirror the daemon's
# runtimeSummary slice but re-implemented it as a capped raw-event scan:
Expand Down
33 changes: 30 additions & 3 deletions .github/workflows/verify-published-wheel.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,43 @@ jobs:
- name: Resolve version

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Drift Bot (ClawMetry) — Blueprint: Release Verification and Merge Gating

The PR implements template injection hardening for workflow_dispatch inputs (binding to env, regex validation before GITHUB_OUTPUT), but the blueprint's WorkflowIntegrityGuard component does not document checking for this security pattern in workflow dispatch input handling despite the SecurityAuditScanner component mentioning script injection scanning.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Drift Bot (ClawMetry) — Blueprint: Release Verification and Merge Gating

The PR implements the blueprint's template injection hardening pattern (env binding, regex validation before GITHUB_OUTPUT, explicit shell:bash), but the WorkflowIntegrityGuard component's documented responsibilities do not include checking for workflow_dispatch template injection patterns, contradicting the blueprint's assertion that the guard covers this pattern.

id: ver
shell: python
# The dispatch input is bound here rather than expanded inside the
# script. A `${{ }}` expansion is pasted into the program text before
# the interpreter runs, so the value becomes part of the program; an
# env var is data the program only ever reads. This step is the taint
# root for the whole job -- the verify step below reads
# `steps.ver.outputs.version`, which is this value.
#
# The rule is recorded, not just applied: "A workflow input is data,
# never program text" in the Release Verification and Merge Gating
# blueprint. #5673 fixed this same construct in auto-deploy-cloud.yml
# and it recurred here, because nothing wrote the rule down and nothing
# failed when it came back. WorkflowIntegrityGuard now covers the
# pattern, so a third occurrence fails a check instead of waiting for a
# reviewer to recognise the shape.
env:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Drift Bot (ClawMetry) — Blueprint: Release Verification and Merge Gating

The PR hardens verify-published-wheel.yml against template injection by binding workflow_dispatch inputs to env variables and validating them before they reach GITHUB_OUTPUT—a security pattern not documented in the WorkflowIntegrityGuard component that scans workflows for script injection.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Drift Bot (ClawMetry) — Blueprint: Release Verification and Merge Gating

The PR implements template injection hardening in verify-published-wheel.yml by binding workflow_dispatch inputs to env variables and validating them before GITHUB_OUTPUT, but the WorkflowIntegrityGuard component doesn't document checking for this security pattern in dispatch input handling.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Drift Bot (ClawMetry) — Blueprint: Release Verification and Merge Gating

The PR implements template injection hardening for workflow_dispatch inputs (binding to env, validating before GITHUB_OUTPUT, explicit shell:bash), but the WorkflowIntegrityGuard component is not documented as checking for this security pattern in workflow definitions, despite its stated responsibility to scan workflows for script injection.

INPUT_VERSION: ${{ github.event.inputs.version }}
run: |
import json, os, urllib.request
want = "${{ github.event.inputs.version }}".strip()
import json, os, re, urllib.request
want = os.environ.get("INPUT_VERSION", "").strip()
if not want:
with urllib.request.urlopen(
"https://pypi.org/pypi/clawmetry/json", timeout=30) as r:
want = json.load(r)["info"]["version"]
# Belt to the env-binding's braces: what goes into GITHUB_OUTPUT is a
# bare version and nothing else, so a later step cannot inherit a
# newline (which would forge a second output) or shell metacharacters.
if not re.fullmatch(r"[0-9A-Za-z.+!-]{1,64}", want):
raise SystemExit(f"not a version string, refusing to continue: {want!r}")
with open(os.environ["GITHUB_OUTPUT"], "a") as fh:
fh.write(f"version={want}\n")
print(f"verifying clawmetry=={want}")

# `shell: bash` is explicit because this runs on the Windows leg too,
# where the default shell is pwsh and `$VERSION` would silently read as
# an unset PowerShell variable rather than the environment variable.
- name: Verify the published wheel
run: python scripts/verify_published_wheel.py --version "${{ steps.ver.outputs.version }}"
shell: bash
env:
VERSION: ${{ steps.ver.outputs.version }}
run: python scripts/verify_published_wheel.py --version "$VERSION"
91 changes: 91 additions & 0 deletions tests/test_ci_workflow_invocations_are_real.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,3 +172,94 @@ def test_pypi_wait_polls_the_index_pip_reads():
assert "pip download" in src or "pip index versions" in src, (
"the PyPI wait must prove installability through pip's own index"
)


# ------------------------------------------- template injection into run: bodies

# Contexts an actor who is not a repository writer can influence, or that a
# writer can set to arbitrary text through a form field. A `${{ }}` naming one
# of these is pasted into the step body BEFORE the interpreter starts, so the
# value arrives as program SOURCE rather than as a value.
_UNTRUSTED_CONTEXTS = (
"github.event.inputs.",
"inputs.",
"github.head_ref",
"github.event.issue.",
"github.event.pull_request.",
"github.event.comment.",
"github.event.review.",
"github.event.discussion.",
"github.event.workflow_run.head_branch",
)
_EXPR_RE = re.compile(r"\$\{\{\s*(?P<expr>[^}]+?)\s*\}\}")


def _run_steps():
"""Every `run:` step body in every workflow, with where it came from.

Scope is derived from the workflow files, so a new workflow is covered
without editing this test.
"""
yaml = pytest.importorskip("yaml")
for wf in WORKFLOWS:
try:
doc = yaml.safe_load(_read(wf))
except Exception as exc: # a malformed workflow is its own test's problem
pytest.fail("{}: {}".format(os.path.basename(wf), exc))
if not isinstance(doc, dict):
continue
for job_name, job in (doc.get("jobs") or {}).items():
if not isinstance(job, dict):
continue
for step in job.get("steps") or []:
if isinstance(step, dict) and isinstance(step.get("run"), str):
yield wf, job_name, step


def test_no_workflow_expands_an_input_into_a_step_body():
"""#5744, and #5673 before it: a workflow input is data, never program text.

`verify-published-wheel.yml` expanded its `workflow_dispatch` `version`
input straight into a `shell: python` program, which is the template
injection class `zizmor` flags and Scorecard reports as DangerousWorkflow.
#5673 had already closed the identical shape in `auto-deploy-cloud.yml`;
nothing recorded the rule, so it came back. This is the check that stops a
third occurrence, rather than a third reviewer having to recognise it.

The fix is always the same: bind the value with `env:` and read it with
`os.environ` / `$VAR`, where it is unambiguously a value.
"""
checked = 0
for wf, job_name, step in _run_steps():
checked += 1
for expr in _EXPR_RE.findall(step["run"]):
bad = [c for c in _UNTRUSTED_CONTEXTS if c in expr]
if not bad:
continue
raise AssertionError(
"{}: job {!r}, step {!r} expands ${{{{ {} }}}} directly into "
"its `run:` body. The expansion happens before the "
"interpreter starts, so that value arrives as program source, "
"not as a value. Bind it with `env:` and read it from the "
"environment instead. See 'A workflow input is data, never "
"program text' in the Release Verification and Merge Gating "
"blueprint.".format(
os.path.basename(wf), job_name,
step.get("name") or "(unnamed)", expr,
)
)
assert checked > 0, "no run: steps discovered to check"


def test_the_guard_would_have_caught_the_defect_it_was_written_for():
"""Proving the check goes RED on the unfixed shape.

A guard nobody has seen fail is indistinguishable from one that cannot
(ADR-005 on that blueprint), so the pattern this test rejects is asserted
against the exact construct #5744 removed.
"""
unfixed = 'want = "${{ github.event.inputs.version }}".strip()'
assert any(c in m for m in _EXPR_RE.findall(unfixed)
for c in _UNTRUSTED_CONTEXTS), unfixed
fixed = 'want = os.environ.get("INPUT_VERSION", "").strip()'
assert not _EXPR_RE.findall(fixed)
Loading