Skip to content

Route step outputs through the environment in publish workflows - #394

Merged
TennyZhuang merged 1 commit into
mainfrom
security/workflow-version-output-interpolation
Jul 30, 2026
Merged

Route step outputs through the environment in publish workflows#394
TennyZhuang merged 1 commit into
mainfrom
security/workflow-version-output-interpolation

Conversation

@TennyZhuang

Copy link
Copy Markdown
Contributor

Problem

#393 closed the workflow_dispatch input, but @sentinel identified a second feed into the same sink: step outputs interpolated with ${{ }} inside run:.

echo "| Version | \`${{ steps.version.outputs.value }}\` |"

${{ }} is substituted as script text before bash parses the line, so a value containing $(...) executes inside the double quotes. These values derive from repository content — package.json for the npm packages, the packed tarball path for the CLI — and workflow_dispatch runs the workflow and the repository content of the selected ref, which any write-access collaborator can push. The version equality check does not help: both sides come from the same package.json.

The original scan was itself incomplete

#393 claimed 12 expansions reached a shell, all constrained. That count came from a scanner that only matched block scalars (run: |) and skipped single-line run:. Corrected, the real number was 16, and the four it had never inspected included:

  • publish-cli.yml:95,101npm publish "${{ steps.pack.outputs.path }}", where the path comes from a find -name 'botiverse-hands-cli-*.tgz' -print -quit glob, so a committed file whose name contains $(...) is selected and executed
  • publish-android-sdk.yml:56,73 — both gradle -PVERSION_NAME= calls, in a workflow previously considered fixed

Two separate lessons, recorded because they generalise: an exhaustive search proves coverage, not classification#393 also misjudged 6 found sites as safe because they were "repo-controlled", which is not a boundary when dispatch selects the ref. And a search is only as exhaustive as the pattern it uses.

Changes

All 10 shell-reaching, non-type-constrained expansions now pass through env:, matching the shape already used for REQUESTED_VERSION:

File Sites
publish-cli.yml 2 × PACKAGE_PATH, 1 × RESOLVED_VERSION
publish-node.yml 1 × RESOLVED_VERSION
publish-electron-sdk.yml 1 × RESOLVED_VERSION
publish-ohos-sdk.yml 3 × RESOLVED_VERSION (validate step + summary)
publish-android-sdk.yml 2 × RESOLVED_VERSION

The android gradle arguments are also now quoted; they were bare before. The version there is semver-validated, so this changes no behaviour — it is defence in depth, not a fix.

No publish semantics change: same resolution order, same equality checks, same outputs and summaries.

Verification

Rescan after the change reports 6 expansions reaching a shell, all Actions-typed and not free text:

  • inputs.dry_run × 4 — type: boolean
  • inputs.containers_rollout × 2 — type: choice, options none|immediate

All 8 workflow files parse, with job and step counts unchanged.

🤖 Generated with Claude Code

Follow-up to #393, which fixed the workflow_dispatch input but left a
second feed into the same sink: step outputs interpolated with ${{ }}
inside run:. Those are substituted as script text before bash parses the
line, and the values derive from repository content, which any
write-access collaborator can change on a branch that workflow_dispatch
can select. The version equality check does not help, because both sides
come from the same package.json.

Also fixes four sites the original scan never inspected: it only matched
block scalars (run: |) and skipped single-line run:. That includes
publish-cli's two npm publish invocations, where the packed path comes
from a find(1) glob, and both publish-android-sdk gradle calls.

The android gradle arguments are now quoted as well; they were bare
before.

Remaining ${{ }} reaching a shell: 6, all Actions-typed (dry_run
boolean x4, containers_rollout choice x2).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Rhea Rafferty <hands-rhea@mail.build>
@TennyZhuang
TennyZhuang merged commit 493a338 into main Jul 30, 2026
8 checks passed
@TennyZhuang
TennyZhuang deleted the security/workflow-version-output-interpolation branch July 30, 2026 14:02
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