Skip to content

Harden CI: bind auto-deploy-cloud's shell values to env, not into the script - #5673

Merged
vivekchand merged 2 commits into
mainfrom
harden/auto-deploy-cloud-shell-env
Sep 8, 2026
Merged

Harden CI: bind auto-deploy-cloud's shell values to env, not into the script#5673
vivekchand merged 2 commits into
mainfrom
harden/auto-deploy-cloud-shell-env

Conversation

@vivekchand

Copy link
Copy Markdown
Owner

What this changes

auto-deploy-cloud.yml expanded ${{ inputs.version }} — and the step output derived from it — directly inside run: blocks. A ${{ }} expansion is substituted into the script before the shell starts, so the value arrives as program text rather than as data. An env: binding is a lookup the shell only ever reads.

That value is the taint root for the whole job: every later step reads steps.oss_version.outputs.version, which is this value, so it reached sed -i, a git commit -m, and a gh pr create title and body.

It also reached a python3 -c snippet as '$V'.split('.') — pasted into the source text, where a quote in the value would end the string literal and leave the remainder to be parsed as Python.

Both are now bound through env: and read as ordinary quoted shell variables, and the version comparison reads os.environ instead of interpolated source.

This is the same fix pattern as #5295 (ref out of run: blocks), #5298 (setup-openclaw inputs through env) and #5363 (bind matrix.source before the shell). This workflow was the one left holding it.

Why it is safe on the release path

This job is the OSS→cloud pin, so the bar for "no behaviour change" is high:

  • The comparison returns the same verdict on the same inputs. Checked directly: 0.12.834 > 0.12.833 → yes, 0.12.833 > 0.12.834 → no, 0.12.75 > 0.12.755 → no (the substring trap the surrounding comments call out).
  • A non-numeric version still fails. int() rejected it before and rejects it now — the guard's existing behaviour, not something this PR introduces.
  • permissions: is untouched. It stays contents: read. Every cross-repo step here authenticates as secrets.CLOUD_REPO_PAT, which workflow permissions do not govern, and the block's existing comment says exactly that. Nothing was elevated or narrowed.
  • Triggers, inputs, outputs and step order are unchanged.

Verification

  • All 36 workflow files parse (yaml.safe_load over .github/workflows/*.yml).
  • Zero ${{ }} expansions remain inside any run: block in this file — re-ran the scan that found the original.
  • tests/test_release_dispatches_cloud_deploy.py and tests/test_ci_workflow_invocations_are_real.py15 tests, all passing. These are the two guards that assert against this workflow.
  • scripts/check_action_refs.py passes (16 distinct refs, all SHA-pinned).
  • scripts/check_product_record.py passes: "no product-visible code changed".

No-PRD: CI-only hardening under .github/, an exempt path. No product behaviour changes — the job's triggers, inputs, outputs and permissions are identical.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J6pLSRDuyy4zvQoGPFNxwc


Generated by Claude Code

… script

The cloud auto-deploy job expanded `${{ inputs.version }}` and the step
output derived from it directly inside `run:` blocks. A `${{ }}` expansion
is substituted before the shell starts, so the value arrives as program
text rather than as data. Every later step in the job reads that same
value, so it reached `sed -i`, a `git commit -m`, and a `gh pr create`
title and body.

The version also reached a `python3 -c` snippet as `'$V'.split('.')` --
pasted into the source, where a quote in the value would have ended the
literal and left the remainder to run as Python.

Both are now bound through `env:` and read as ordinary shell variables,
and the Python comparison reads `os.environ` instead of interpolated
source. Same fix pattern as #5295, #5298 and #5363; this workflow was the
one left holding it.

No behaviour change: the comparison returns the same verdict on the same
inputs (0.12.834 > 0.12.833 yes, 0.12.833 > 0.12.834 no, 0.12.75 >
0.12.755 no), a non-numeric version is still rejected by `int()` as
before, and the `permissions:` block is untouched -- it stays
`contents: read`, since every cross-repo step authenticates as
CLOUD_REPO_PAT.

Verified: all 36 workflows parse; zero `${{ }}` expansions remain in any
`run:` block in this file; tests/test_release_dispatches_cloud_deploy.py
and tests/test_ci_workflow_invocations_are_real.py pass (15 tests);
scripts/check_action_refs.py and scripts/check_product_record.py pass.

No-PRD: CI-only hardening under .github/, an exempt path. No product
behaviour changes -- the job's inputs, outputs, triggers and permissions
are identical.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J6pLSRDuyy4zvQoGPFNxwc
@8090-software-factory

Copy link
Copy Markdown

✅ Drift Bot (ClawMetry): no drift detected

Drift Bot analyzed the changed files against this project's blueprints and requirements and found no drift.

@8090-software-factory

Copy link
Copy Markdown

✅ Drift Bot (ClawMetry): no drift detected

Drift Bot analyzed the changed files against this project's blueprints and requirements and found no drift.

Copy link
Copy Markdown
Owner Author

✨ auto-fixed: merged origin/main into branch — was BEHIND (base at e9bf21a, main at 94954ca)


Generated by Claude Code

@vivekchand
vivekchand merged commit 9b98bb2 into main Sep 8, 2026
38 of 39 checks passed
vivekchand added a commit that referenced this pull request Sep 10, 2026
…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
vivekchand added a commit that referenced this pull request Sep 10, 2026
* Harden CI: bind verify-published-wheel's dispatch input to env

`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

* Record the rule this PR applies, so the class is guarded not the instance

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

* Make WorkflowIntegrityGuard actually cover input injection, and run it

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

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant