Skip to content

ci: prod down migration with steps>=total bypasses confirmation gate (down -all equivalent) #68

Description

@cristim

Problem

PR LeanerCloud/cloud-commitments-cli#1216 added an explicit-positive-steps guard plus confirm=rollback-prod for production down migrations. Both gates pass for steps=99999999, but golang-migrate interprets any down N where N exceeds the available rollback count as "roll back everything that remains" rather than erroring out. So an operator who types a deliberately large step count plus the correct prod confirmation effectively performs down -all, which is the exact failure mode LeanerCloud/cloud-commitments-cli#1216 was scoped to prevent.

The threat is realistic: a tired operator faced with "the rollback I want is somewhere in the last dozen migrations" may type 100 to "leave room", not realising 100 sweeps the whole schema. The current confirmation gate ("type rollback-prod") confirms intent to roll back prod, but not intent to roll back all of prod.

Reproduction

.github/workflows/database-migration.yml after LeanerCloud/cloud-commitments-cli#1216:

# validate
if ! [[ "$STEPS" =~ ^[1-9][0-9]*$ ]]; then IS_SAFE=false; fi
if [[ "$ENVIRONMENT" == "prod" && "$CONFIRM" != "rollback-prod" ]]; then IS_SAFE=false; fi

# migrate-aws / migrate-gcp / migrate-azure (defense-in-depth)
if ! [[ "${{ inputs.steps }}" =~ ^[1-9][0-9]*$ ]]; then exit 1; fi
migrate -path ... -database "$DB_URL" down ${{ inputs.steps }}

Invocation:

gh workflow run database-migration.yml \
  -f cloud=aws -f environment=prod -f direction=down \
  -f steps=99999999 -f confirm=rollback-prod

Both guards pass, the migrate CLI rolls back all available down migrations, the schema is dropped.

Fix options

  1. Cap steps at a sane maximum (e.g. 20) in the validate step. Anything larger fails IS_SAFE. Simple but the cap is arbitrary and may become stale.
  2. Count *.down.sql files in the migrate- job and refuse steps >= TOTAL* (or refuse steps > TOTAL - SOME_RESERVE). Tighter, but still doesn't catch "roll back everything down to migration 1" intent.
  3. Require the operator to type the target version into a new target_version input instead of a relative step count, and let validate diff current - target to compute the implied step count. The confirmation string then meaningfully reflects what is being dropped. Highest assurance, biggest change.
  4. Tighten the confirmation string to include the step count: e.g. require confirm=rollback-prod-<N> where N must equal inputs.steps. Trivial change, forces the operator to re-type the destructive magnitude.

Option (4) is the smallest change that closes the gap and matches the existing cleanup-staging pattern (typed string that encodes intent). Option (3) is the principled answer if there is appetite.

Related

Acceptance criteria

  • A run with environment=prod direction=down steps=999999 and the right confirmation either fails validate (option 1/2/3) or requires an extra encoded acknowledgement (option 4).
  • The legitimate steps=1 confirm=rollback-prod flow still passes.
  • The dev/staging path is not affected (or, if it is, the change is intentional and documented).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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