fix(ci): close shell-injection vector via head.ref in create-release.yml (BUNYIP-236) #263
No reviewers
Labels
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
psa-systems/bunyip!263
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/BUNYIP-236-create-release-injection"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Audit finding from BUNYIP-205 roll-up (medium, exploitable, mirror of PMS-500 which already shipped on mokosh-server). The previous shape interpolated
${{ github.event.pull_request.head.ref }}directly into the nushell script body:Forgejo Actions expand
${{ ... }}into the script TEXT before the shell sees it, so a PR branch namedrelease/v1.0.0"; ^rm -rf /; #would inject literal shell into nushell. The job-levelif:requires the ref to start withrelease/v, but the smuggled-suffix branch above satisfies that check. The workflow holdscontents: writeplus a PAT with write:repository scope, so a successful injection could mint rogue releases, push to main, or exfiltrate the token.Two layers of defence:
Move
head.refinto an env mapping (PR_BRANCH) and read$env.PR_BRANCHinside the script. The runner binds the value before shell parsing, so it is data, not code. Mirror of the PMS-500 fix.Validate
$env.PR_BRANCHagainst strict semver^release/v[0-9]+\.[0-9]+\.[0-9]+$before any downstream command uses it. Belt-and-braces: every downstream use (git describe, git log, http post) now takes the value as a data arg via the env mapping, so this second check guards against a future revert undoing layer #1.No regression to the release flow: a real release/vX.Y.Z branch passes the regex and the downstream commands run identically.
Closes the corresponding checkbox on BUNYIP-205.
#BUNYIP-236
Audit finding from BUNYIP-205 roll-up (medium, exploitable, mirror of PMS-500 which already shipped on mokosh-server). The previous shape interpolated `${{ github.event.pull_request.head.ref }}` directly into the nushell script body: let branch = "${{ github.event.pull_request.head.ref }}" Forgejo Actions expand `${{ ... }}` into the script TEXT before the shell sees it, so a PR branch named `release/v1.0.0"; ^rm -rf /; #` would inject literal shell into nushell. The job-level `if:` requires the ref to start with `release/v`, but the smuggled-suffix branch above satisfies that check. The workflow holds `contents: write` plus a PAT with write:repository scope, so a successful injection could mint rogue releases, push to main, or exfiltrate the token. Two layers of defence: 1. Move `head.ref` into an env mapping (`PR_BRANCH`) and read `$env.PR_BRANCH` inside the script. The runner binds the value before shell parsing, so it is data, not code. Mirror of the PMS-500 fix. 2. Validate `$env.PR_BRANCH` against strict semver `^release/v[0-9]+\.[0-9]+\.[0-9]+$` before any downstream command uses it. Belt-and-braces: every downstream use (git describe, git log, http post) now takes the value as a data arg via the env mapping, so this second check guards against a future revert undoing layer #1. No regression to the release flow: a real release/vX.Y.Z branch passes the regex and the downstream commands run identically. Closes the corresponding checkbox on BUNYIP-205. #BUNYIP-236