Skip to content

sec(ci): database-migration.yml interpolates inputs into run blocks behind a post-interpolation guard #1647

Description

@cristim

Follow-up from #1542 / PR #1641. Same defect class, different file. Verified against origin/main @ 3e9660d06.

Revised after closer analysis. An earlier version of this body implied the injection could be reached from the ungated validate / summary jobs. That is wrong and is corrected below: the raw interpolation lives in the environment-bound migrate-* jobs, and validate handles its inputs correctly. The blast radius is therefore smaller than #1542's, which is why this is p1/high rather than p0/critical. See "Why not p0" and the escalation trigger.

Where

.github/workflows/database-migration.yml

  • :27permissions: id-token: write at workflow level
  • :212, :216, :217, :221, :225, :226${{ inputs.steps }} interpolated raw into a run: block (AWS job); GCP repeats it at :290-:302, Azure likewise
  • :59-70workflow_call inputs cloud, environment, direction typed string
  • :77 (validate), :371 (summary) — no environment: binding

What

The value is pasted into the shell source before bash parses it:

migrate -path ${{ env.MIGRATIONS_PATH }} -database "$DB_URL" up ${{ inputs.steps }}

The guard intended to constrain it is itself post-interpolation, so it validates nothing — the same flaw #1542 had with image_tag:

if ! [[ "${{ inputs.steps }}" =~ ^[1-9][0-9]*$ ]]; then

The direction=up path has no steps guard at all. validate's "Safety checks" step (:89) does the right thing — every input arrives via env: and is referenced quoted ("$DIRECTION", "$STEPS", "$CONFIRM"), so that job is not injectable, and its ^[1-9][0-9]*$ check is effective. But that check sits inside if [[ "$DIRECTION" == "down" ]] (:107). For direction=up, steps is only compared against "0", so any other value passes validate untouched and lands raw at :217.

What currently limits this

  • On workflow_dispatch, cloud / environment / direction are type: choice, which GitHub validates server-side against the declared options. Not injectable.
  • confirm is type: string but only ever reaches a run: block through env: (:95, used quoted at :119). Not injectable.
  • steps is type: number. Whether GitHub enforces that on every dispatch path — the REST API in particular, not just the UI form — is the crux, and I could not confirm it. GitHub's input-types changelog states the value is validated before the run starts, which suggests it is enforced on github.com. The concrete reports of type: number accepting arbitrary strings (go-gitea/gitea#33652) are against Gitea's reimplementation, not GitHub. I did not attempt an empirical test, because the only way to test it here would be dispatching a real migration workflow against a real database.

So this is a latent injection whose exploitability depends on a platform guarantee the repo should not be relying on, rather than a demonstrated one.

Why not p0

Unlike #1542 and #1649, the raw interpolation here sits in a job that is gated: migrate-aws (:163) declares environment: aws-db-${{ inputs.environment }} and additionally gates on needs.validate.outputs.is_safe == 'true'. So reaching it requires passing the environment's protection rules (where configured), whereas #1542's verify-image and #1649's prepare are ungated and credentialed. The ungated validate and summary jobs here do hold workflow-level id-token: write, but contain no injectable interpolation — that half is #1591's concern.

Escalate to p0/critical if anyone confirms type: number is not enforced on the REST dispatch path, or if a workflow_call caller is added (see below). Either turns this into a directly exploitable path.

Latent workflow_call surface

Nothing calls this workflow today — grep -rn 'database-migration' .github/workflows/ returns only the file itself. But its workflow_call inputs cloud, environment and direction are type: string with no allowlist, and workflow_call gets no server-side choice enforcement. inputs.direction is interpolated raw at :211 and :290. The moment another workflow calls this one, those become free-text paths into a run: block.

Fix direction

Mirror PR #1641:

  1. Move every inputs.* and step output into env: and reference the quoted shell variable, so the ^[1-9][0-9]*$ guard runs against data instead of already-parsed syntax. validate already demonstrates the correct pattern in this same file.
  2. Validate steps for direction=up too, not only down.
  3. Drop id-token: write to job level, granting it only to the environment-bound migrate-* jobs.
  4. Validate the workflow_call string inputs against an explicit allowlist at the top of validate.

Related: #1542 / PR #1641, #1649 (same class, p0), #1591 (environment bindings), #1646 (no workflow linter, which is why this went unnoticed).

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions