Harden CI: bind verify-published-wheel's dispatch input to env - #5744
Conversation
`verify-published-wheel.yml` expanded the `workflow_dispatch` `version`
input straight into the body of a `shell: python` step. A `${{ }}`
expansion is pasted into the program text before the interpreter starts,
so the input was program source rather than data.
Bind it to `INPUT_VERSION` and read it with `os.environ` instead, the
same shape #5673 used for auto-deploy-cloud. The resolved value is then
checked against a bare-version pattern before it is written to
GITHUB_OUTPUT, so the downstream step cannot inherit a newline (which
would forge a second step output) or a shell metacharacter.
The verify step reads that output, so it is bound to env too and quoted
as `"$VERSION"`. It gets an explicit `shell: bash` because the job also
runs on windows-latest, where the default shell is pwsh and `$VERSION`
would read as an unset PowerShell variable rather than the environment
variable.
The workflow self-tests on any PR touching it, so all three platforms
exercise this on the PR itself.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HfPzC2zA4eb21Gr66skCo7
|
![8090-software-factory[bot]](https://avatars.githubusercontent.com/in/1847834?s=60&v=4){kind=small}
| # 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. | ||
| env: |
There was a problem hiding this comment.
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.
|
Blocked on
At the same head ( The findingDrift Bot reports that the hardening pattern — binding the dispatch input to That is a gap in the blueprint, not a defect in the diff. The finding does not dispute that the workflow expanded a Why this is not fixed in this PRTwo reasons, in order:
What unblocks itThe This PR is otherwise mergeable and green. It joins #5305 (red since 28 Aug), #5382 and #5733 in the same state — four hardening changes now held by the same blueprint gap rather than by four separate ones. Generated by Claude Code |
|
blocked on author decision — skipping (auto-mergeability sweep) Generated by Claude Code |
|
| # 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. | ||
| env: |
There was a problem hiding this comment.
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.
|
✨ auto-fixed: merged latest main into branch to keep it up to date Generated by Claude Code |
|
| # 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. | ||
| env: |
There was a problem hiding this comment.
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.
|
✨ auto-fixed: merged latest main into branch (was BEHIND; no conflicts) Generated by Claude Code |
|
| @@ -73,16 +73,35 @@ jobs: | |||
| - name: Resolve version | |||
There was a problem hiding this comment.
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.
|
Automated sweep note — E2E Gate blocked by Drift Bot The E2E Gate ( What to do:
If Drift Bot is flagging something spurious (e.g. a doc that was intentionally superseded), a This PR is otherwise not in a DIRTY or BEHIND state from a git perspective; the only blocker is the Drift Bot check. Generated by Claude Code |
…ance
Drift Bot's finding was right: the blueprint's WorkflowIntegrityGuard did not
document checking workflow_dispatch input handling for template injection,
even though SecurityAuditScanner scans for script injection. Scanning without
gating produces a report nobody is obliged to act on.
The Release Verification and Merge Gating blueprint now carries the contract
(an input reaches a step through env:, never a ${{ }} expansion in the step
body; a value written to GITHUB_OUTPUT is validated first; the guard covers
the pattern) and an ADR recording why the class is hardened rather than each
occurrence: #5673 fixed this construct in auto-deploy-cloud.yml, it recurred
here, and nothing failed when it did.
Placed near the top of that record, per FLYWHEEL 0c.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xb6A5G74JiMe3zHFs1JZEP
|
| @@ -73,16 +73,43 @@ jobs: | |||
| - name: Resolve version | |||
There was a problem hiding this comment.
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.
Drift Bot's second finding was sharper than the first: the blueprint now
asserted the guard covers workflow_dispatch template injection, but the
WorkflowIntegrityGuard component's responsibilities did not, so the record
claimed a coverage that did not exist.
It does now. tests/test_ci_workflow_invocations_are_real.py rejects any
${{ }} expansion of an actor-influenced context -- github.event.inputs.*,
inputs.*, github.head_ref, the issue/PR/comment/review/discussion payloads --
inside a run: step body. Scope is derived from the workflow files, so a new
workflow is covered without editing the guard. Proven red against the exact
construct this PR removes and green after it.
And the guard is now in a CI file list. It was in none. This repo runs
explicit file lists, not `pytest tests/`, so the guard written for ADR-005
-- 'a CI step that cannot fail is worse than no CI step' -- had never once
executed. That is the defect it exists to catch, on itself.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xb6A5G74JiMe3zHFs1JZEP
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
2 participants
verify-published-wheel.ymlexpanded itsworkflow_dispatchversioninput straight into the body of ashell: pythonstep:A
${{ }}expansion is pasted into the program text before the interpreter starts, so the input arrived as program source rather than as data. This is the template-injection class that zizmor flags and that Scorecard reports as DangerousWorkflow. Triggering it needs write access (workflow_dispatchis not open to outsiders), so this is defence in depth rather than an exposed hole — but it is the same shape as the one #5673 closed inauto-deploy-cloud.yml, and the fix is the same three lines.What changed
1. The input is bound to
env, not expanded into the script.INPUT_VERSIONis read withos.environ.get(...), so the value is data the program reads instead of text the interpreter compiles.2. The resolved version is validated before it reaches
GITHUB_OUTPUT. This step is the taint root for the whole job — every later step readssteps.ver.outputs.version— so what leaves it is checked against a bare-version pattern. That stops a value carrying a newline (which would forge a second step output) or shell metacharacters from propagating.3. The verify step binds that output to
envand quotes it.--version "$VERSION"instead of a raw${{ }}expansion.4. That step gets an explicit
shell: bash. The job matrix includeswindows-latest, where the default shell is pwsh and$VERSIONwould silently read as an unset PowerShell variable rather than the environment variable — an empty--versioninstead of a failure.conformance-heartbeat.ymlandrelease-canary.ymlboth already runshell: bashsteps unconditionally on their Windows legs, so this is established here.The
concurrency:group still interpolates the input, deliberately: a concurrency group is a name, not a program, and nothing evaluates it.Verification
The embedded resolver was run directly against the real step body:
0.12.844version=0.12.8441.0.0rc1version=1.0.0rc11!2.0.0+local.1(epoch + local)GITHUB_OUTPUT$(id)Blank input (the path a PR self-test takes) still resolves latest from PyPI — checked live, returning
0.12.844.Every workflow file parses (
yaml.safe_loadover.github/workflows/*.yml), andtests/test_workflow_yaml_valid.pypasses: 542 passed, 336 skipped.This workflow is path-scoped to self-test on any PR that touches it, so this PR exercises the change on Ubuntu, macOS and Windows before it merges.
No-PRD: CI-only change,
.github/is exempt perscripts/check_product_record.py.🤖 Generated with Claude Code
https://claude.ai/code/session_01HfPzC2zA4eb21Gr66skCo7
Generated by Claude Code
The class, not the instance (added after Drift Bot)
#5673 fixed this exact construct in
auto-deploy-cloud.yml. It came back here, because nothing wrote the rule down and nothing failed when it returned. Two occurrences is a pattern, so this PR now also closes the pattern:The rule is recorded. "A workflow input is data, never program text" is a section on the Release Verification and Merge Gating blueprint, with three System Contracts (an input reaches a step through
env:; a value written toGITHUB_OUTPUTis validated first;WorkflowIntegrityGuardcovers the pattern) and an ADR for why the class is hardened rather than each occurrence.The rule is enforced.
tests/test_ci_workflow_invocations_are_real.pynow rejects any${{ }}expansion of an actor-influenced context —github.event.inputs.*,inputs.*,github.head_ref, the issue / pull-request / comment / review / discussion payloads — inside arun:step body. Scope is derived from the workflow files, so a new workflow is covered without touching the guard. Proven red against the exact construct this PR removes:and green after the fix.
SecurityAuditScanneralready scans for script injection but is explicitly non-gating — scanning without gating produces a report nobody is obliged to act on, which is how the same shape reached a second workflow.And the guard now runs. It was in no CI job at all. This repo runs explicit file lists, not
pytest tests/, so the guard written for ADR-005 — "a CI step that cannot fail is worse than no CI step" — had never once executed, which is precisely the defect it exists to catch, on itself. It is now named inci.yml'sSyntax & Lintjob: 7 passed.