fix(security): harden dispatch inputs against shell injection - #2242
Merged
HongmingWang-Rabbit merged 1 commit intoApr 29, 2026
Merged
Conversation
e2e-staging-canvas had a single global concurrency group:
concurrency:
group: e2e-staging-canvas
cancel-in-progress: false
That meant the entire repo shared one running + one pending slot. When a
staging push queued behind an in-flight run and a third entrant (a PR
run, a follow-on push) entered the group, the staging push got
cancelled. auto-promote-staging then saw `completed/cancelled` for a
required gate and refused to advance main.
Observed 2026-04-28 23:51-23:53: staging tip 3f99fed's e2e-staging-
canvas push run was cancelled within 2:20 of starting because a PR run
on a follow-on branch entered the group. Auto-promote-staging fired 8+
times after that, all skipped because canvas was still in the cancelled
state. The chain stayed stuck until the cancelled run was manually
re-dispatched.
e2e-api had a softer version of the same bug — `group: e2e-api-${{
github.ref }}`. Per-ref isolates push events from PR events, so this
specific scenario didn't hit it, but back-to-back pushes to staging at
SHA-A and SHA-B share refs/heads/staging and would still cancel SHA-A's
queued run when SHA-B enters.
Both workflows now use per-SHA grouping. The single-global-group's
original intent was to throttle parallel E2E provisions, but each E2E
run already isolates its state via fresh-org-per-run, and parallel
infrastructure cost at our scale (~$0.001/min × 10min × 2) is rounding
error compared to a stuck pipeline.
Per-SHA still dedupes accidental double-triggers for the SAME SHA.
It does not cancel obsolete-PR-version runs on force-push — that wasted
CI is acceptable given the alternative is losing staging-tip data that
auto-promote-staging depends on.
Other gate workflows: ci.yml uses `cancel-in-progress: true` which is
correct for unit tests (intentional cancellation on supersede). codeql.yml
is per-ref like e2e-api was; same fix probably applies if the same
deadlock pattern is observed there, but no incident yet so deferring.
HongmingWang-Rabbit
requested a review
from hongmingwang-moleculeai
as a code owner
April 29, 2026 00:20
HongmingWang-Rabbit
enabled auto-merge
April 29, 2026 00:20
HongmingWang-Rabbit
deleted the
fix/dispatch-input-shell-injection-hardening
branch
April 29, 2026 00:35
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
1 participant
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.
Summary
Surfaced during today's comprehensive review. Two workflows interpolated
github.event.inputs.*directly intorun:script bodies — a dispatcher with repo-write could inject shell via crafted input.Vulnerable spots
auto-promote-on-e2e.yml — Compute short sha step:
FULL=\"${{ github.event.inputs.sha }}\"Input
\"; rm -rf /; #becomesFULL=\"\"; rm -rf /; #\"→ executes.sweep-cf-orphans.yml — Run sweeper step:
if [ \"${{ github.event.inputs.dry_run || 'false' }}\" = \"true\" ]; thenInput
true\"; rm -rf /; #becomesif [ \"true\"; rm -rf /; #\" = \"true\" ]; then→ executes.Threat model
Contained — workflow_dispatch requires repo-write. But the standard mitigation is mechanical and matches what 5 other workflows in the repo already do correctly (auto-promote-staging, e2e-staging-saas, publish-canvas-image, sweep-stale-e2e-orgs, sweep-cf-orphans line 78).
Fix
Pass dispatch inputs through `env:` and reference as quoted shell variables. `env:` indirection sends the value through bash's variable system rather than template substitution; the quoted `"$INPUT_SHA"` prevents word-splitting.
Test plan