Skip to content

Harden validation-run against branch-name script injection#443

Open
arpitjain099 wants to merge 1 commit into
GSA:developfrom
arpitjain099:chore/harden-expression-injection
Open

Harden validation-run against branch-name script injection#443
arpitjain099 wants to merge 1 commit into
GSA:developfrom
arpitjain099:chore/harden-expression-injection

Conversation

@arpitjain099
Copy link
Copy Markdown

Jira ticket

External contribution, no DIGITAL ticket. Happy to file one if the team prefers to track it.

Purpose

.github/workflows/validation-run.yml passes github.head_ref, github.base_ref, and the extracted branch output into the RoboValidate steps by interpolating ${{ }} directly into the run: script, for example:

ROBO_VALIDATE_BRANCH_NAME=${{ steps.extract_branch.outputs.branch }} ROBO_VALIDATE_TARGET_BRANCH="${{ github.base_ref }}" ROBO_VALIDATE_CURRENT_BRANCH="${{ github.head_ref }}" vendor/bin/robo validate:all

The runner substitutes those expressions into the shell command before bash runs, and the branch name on a pull request is contributor-controlled. A branch named with shell metacharacters could therefore execute arbitrary commands in the validation job. The extract_branch output is derived from the head ref, so it carries the same risk.

This change binds each of those values to step env: entries and references them as quoted shell variables ("$CURRENT_BRANCH"). Environment variables are not re-parsed by the shell, which closes the injection path. This is the mitigation in GitHub's security-hardening guidance: https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#understanding-the-risk-of-script-injections

The same pattern is what CodeQL (actions/expression-injection) and zizmor report on these lines.

Deployment and testing

No behavior change. The robo validate:all invocations receive the same ROBO_VALIDATE_* values as before, just sourced from the step environment instead of inlined into the script. YAML parses cleanly.

Checklist for the Developer

  • No merge conflicts exist with the target branch.
  • Commit is signed (per CONTRIBUTING git configuration requirement).

The validation steps build ROBO_VALIDATE_* arguments by interpolating
github.head_ref, github.base_ref, and the extracted branch name (which
itself comes from the head ref) straight into the run script. Because
${{ }} is expanded into the script before bash executes it, a pull
request from a branch with a specially crafted name could run commands
in the job.

Bind those values to step-level env entries and pass them as quoted
shell variables. The robo validate invocation is unchanged.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
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