Harden CI: bind desktop-artifacts' version into env, not into the script - #5913
Merged
Conversation
The four packaging steps in desktop-artifacts.yml each read the resolved
version by expanding `${{ steps.ver.outputs.version }}` directly inside a
`run:` block. That substitution is textual: the runner splices the value
into the script body before pwsh or bash ever parses it, so the value is
evaluated as code rather than read as data. Every such site is one zizmor
`template-injection` finding, and this workflow held nine of them -- the
largest single concentration in the repository.
Each step now receives the value as a step-level `env:` var and reads it
back as a variable ($Env:APP_VERSION in pwsh, "$APP_VERSION" in bash),
which is the same env-binding fix already applied to auto-deploy-cloud
(#5673), verify-published-wheel (#5744) and the smoke-test matrix (#5363).
The interpolation disappears from the script text; the packaging logic,
the artifact names and the fixed-name stable-URL copies are unchanged.
In the NSIS step the value is bound once into `$version` and the three
later uses read that, replacing five separate expansions. PowerShell
member-access ambiguity is avoided by writing `${version}` where a literal
`.` follows the reference.
Scope note: `permissions:` is untouched. This workflow is one of the six
that need write scopes -- its `release` job elevates to `contents: write`
to attach installers to the tag's Release -- and that elevation, plus the
least-privilege `contents: read` top level, is exactly as it was.
Verified: every workflow file still parses, and the repository's workflow
guards pass (tests/test_workflow_yaml_valid.py +
tests/test_ci_workflow_invocations_are_real.py: 594 passed, 369 skipped).
No `${{ }}` expansion remains inside any `run:` block in this file.
No-PRD: CI-only change under .github/, no product code touched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WGkmcn95GPNqMaK6dAnVNw
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The four packaging steps in
desktop-artifacts.ymleach read the resolved version by expanding${{ steps.ver.outputs.version }}directly inside arun:block. That substitution is textual — the runner splices the value into the script body before pwsh or bash ever parses it, so the value is evaluated as code rather than read as data.Each of those sites is one zizmor
template-injectionfinding, and this workflow held nine of them, the largest single concentration in the repository (24 across all workflows;release-on-merge.ymlhas the other 10 and is left for its own PR).The fix
Each step now receives the value as a step-level
env:var and reads it back as a variable —$Env:APP_VERSIONin pwsh,"$APP_VERSION"in bash. This is the same env-binding fix already applied toauto-deploy-cloud(#5673),verify-published-wheel(#5744) and the smoke-test matrix (#5363).The interpolation disappears from the script text. The packaging logic, the artifact names and the fixed-name stable-URL copies (
ClawMetry-windows.zip,ClawMetry-windows-setup.exe,clawmetry-linux.tar.gz) are unchanged.Steps touched, all four in the Windows and Linux build jobs:
In the NSIS step the value is bound once into
$versionand the three later uses read that, collapsing five separate expansions into one binding. Where a literal.follows the reference (${version}.0,ClawMetry-Setup-${version}.exe) it is written braced, so there is no PowerShell member-access ambiguity for a reader to have to reason about.Permissions are untouched
This workflow is one of the six that legitimately need write scopes: its
releasejob elevates tocontents: writesoaction-gh-releasecan attach installers to the tag's Release. That elevation, and the least-privilegecontents: readtop level it sits under, are exactly as they were — this PR adds nopermissions:line and removes none.git diffover the file contains no permissions change.Verification
python3 -c "import yaml,glob; [yaml.safe_load(open(f)) for f in glob.glob('.github/workflows/*.yml')]"tests/test_workflow_yaml_valid.py+tests/test_ci_workflow_invocations_are_real.py→ 594 passed, 369 skipped${{ }}expansion remains inside anyrun:block in this file (re-scanned after the edit; count went 9 → 0)Scope
One concern, one workflow file, 25 insertions / 7 deletions. Under
.github/, so exempt from the product-record gate.No-PRD: CI-only change under
.github/, no product code touched.🤖 Generated with Claude Code
https://claude.ai/code/session_01WGkmcn95GPNqMaK6dAnVNw
Generated by Claude Code