Skip to content

fix: prevent command injection in nightly workflow @W-24289114@ - #2938

Merged
iowillhoit merged 4 commits into
mainfrom
wr/fixNightlyWorkflowInjection
Sep 24, 2026
Merged

iowillhoit merged 4 commits into
mainfrom
wr/fixNightlyWorkflowInjection

Conversation

@WillieRuemmele

Copy link
Copy Markdown
Contributor

Summary

  • Move user-controlled inputs.only from direct ${{ }} expression expansion in run: blocks to env: blocks, so the value is treated as shell variable data — not inline script text that bash re-parses for metacharacters
  • Add an input validation step that rejects only values containing characters outside the npm package name charset (defense-in-depth)
  • Add comments explaining why $ONLY_FLAG is intentionally unquoted (word-splitting required for --only <value> to become two separate CLI arguments)

Vulnerability

The workflow_dispatch input only (a free-form string) was interpolated directly into run: steps via ${{ inputs.only }}. GitHub expression expansion happens before the shell parses the generated script, so shell metacharacters in the input (;, |, `, $()) become live syntax. The same job exports SVC_CLI_BOT_GITHUB_TOKEN as GITHUB_TOKEN, creating a command injection → secret exfiltration path for any actor with workflow_dispatch access.

Work Item

@W-24289114@: [Frontier Strike][Core] salesforcecli/cli nightly workflow command injection can expose service bot token

Proof of Work

  • YAML syntax: validated (js-yaml parse — clean)
  • Grep audit: confirmed no remaining ${{ inputs.* }} in any run: block across all workflows (one other instance in promote.yml is a boolean input — safe)
  • Lint: clean (pre-commit hook passed)
  • Commitlint: clean

Validation

  • Per-task adversarial review: 0 blockers, 1 warning (unquoted var — addressed with comment), 1 nit (comma-only input — sf-release handles gracefully)
  • Integration: lint and commitlint pass
  • Regex validation reviewed for newline bypass, glob expansion, format() specifier injection — all mitigated

Test plan

  • Trigger nightly workflow manually with a valid only value (e.g. @salesforce/core) — should succeed
  • Trigger with empty only — should succeed (same as cron path)
  • Trigger with shell metacharacters (e.g. foo;echo pwned) — should fail at validation step

Move user-controlled `inputs.only` from direct GitHub expression
expansion in `run:` blocks to `env:` blocks, where the value becomes
a shell variable instead of inline script text. Add input validation
to reject values containing shell metacharacters.
iowillhoit
iowillhoit previously approved these changes Sep 24, 2026
if: ${{ inputs.only != '' }}
shell: bash
run: |
if [[ ! "$INPUT_ONLY" =~ ^[@a-zA-Z0-9/_,.~-]+$ ]]; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could remove the ~ here. No packages passed to --only will have a ~

@iowillhoit
iowillhoit merged commit 28ad916 into main Sep 24, 2026
37 checks passed
@iowillhoit
iowillhoit deleted the wr/fixNightlyWorkflowInjection branch September 24, 2026 19:10
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.

2 participants