From 5b4ea675f3a1869235e190df7f30caa21ecb3115 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 10 Jun 2026 17:43:09 -0700 Subject: [PATCH 1/2] fix(ci): require explicit steps and prod confirmation for down migrations direction=down with the default steps=0 ran 'migrate down -all' in all three cloud jobs, dropping the entire schema, and the validate job's prod guard only echoed a warning while still setting is_safe=true, promising a confirmation step that did not exist. - validate now rejects direction=down unless steps is an explicit positive integer, and fails the job instead of skipping silently - prod down migrations additionally require typing 'rollback-prod' in the new 'confirm' dispatch input - the three migrate jobs no longer have a 'down -all' branch; they fail loud if steps is missing or non-positive (defense in depth) - free-form inputs are passed to the shell via env vars Validated with actionlint and a local simulation of the validate logic against the failing scenario (prod, down, steps=0, no confirm): the pre-fix logic yields is_safe=true, the post-fix logic fails validation, and legitimate up/down flows still pass. Closes #1173 --- .github/workflows/database-migration.yml | 71 +++++++++++++++++------- 1 file changed, 51 insertions(+), 20 deletions(-) diff --git a/.github/workflows/database-migration.yml b/.github/workflows/database-migration.yml index 0a6463e9d..dd7c5c12e 100644 --- a/.github/workflows/database-migration.yml +++ b/.github/workflows/database-migration.yml @@ -47,10 +47,15 @@ on: options: [up, down] default: up steps: - description: 'Number of migrations to apply/rollback (0 = all)' + description: 'Number of migrations to apply/rollback (0 = all; for direction=down an explicit positive value is required)' required: false type: number default: 0 + confirm: + description: 'Type "rollback-prod" to confirm a down migration on prod (ignored otherwise)' + required: false + type: string + default: '' workflow_call: inputs: cloud: @@ -83,6 +88,11 @@ jobs: - name: Safety checks id: check + env: + ENVIRONMENT: ${{ inputs.environment }} + DIRECTION: ${{ inputs.direction }} + STEPS: ${{ inputs.steps }} + CONFIRM: ${{ inputs.confirm }} run: | IS_SAFE=true @@ -92,11 +102,25 @@ jobs: IS_SAFE=false fi - # Warn on production down migrations - if [[ "${{ inputs.environment }}" == "prod" ]] && [[ "${{ inputs.direction }}" == "down" ]]; then - echo "⚠️ WARNING: Attempting to rollback migrations on PRODUCTION" - echo "This operation is destructive and may cause data loss!" - # For production, we don't auto-fail, but require manual confirmation + # Down migrations must specify an explicit positive step count. + # steps=0 (the default) would run 'migrate down -all' and drop the + # entire schema, so it is rejected here for direction=down. + if [[ "$DIRECTION" == "down" ]]; then + if ! [[ "$STEPS" =~ ^[1-9][0-9]*$ ]]; then + echo "❌ direction=down requires an explicit positive 'steps' value (got '${STEPS:-}')." + echo "Rolling back ALL migrations at once is not supported by this workflow." + IS_SAFE=false + fi + + # Production down migrations additionally require typed confirmation. + if [[ "$ENVIRONMENT" == "prod" ]]; then + echo "⚠️ WARNING: Attempting to rollback migrations on PRODUCTION" + echo "This operation is destructive and may cause data loss!" + if [[ "$CONFIRM" != "rollback-prod" ]]; then + echo "❌ Production rollback requires typing 'rollback-prod' in the 'confirm' input." + IS_SAFE=false + fi + fi fi # Check migration files @@ -116,6 +140,11 @@ jobs: echo "is_safe=$IS_SAFE" >> $GITHUB_OUTPUT + if [[ "$IS_SAFE" != "true" ]]; then + echo "Validation failed; refusing to run migrations." + exit 1 + fi + - name: Display migration plan run: | echo "## Database Migration Plan" >> $GITHUB_STEP_SUMMARY @@ -188,13 +217,13 @@ jobs: migrate -path ${{ env.MIGRATIONS_PATH }} -database "$DB_URL" up ${{ inputs.steps }} fi else - if [[ "${{ inputs.steps }}" == "0" ]]; then - echo "Rolling back all migrations..." - migrate -path ${{ env.MIGRATIONS_PATH }} -database "$DB_URL" down -all - else - echo "Rolling back ${{ inputs.steps }} migration(s)..." - migrate -path ${{ env.MIGRATIONS_PATH }} -database "$DB_URL" down ${{ inputs.steps }} + # Defense in depth: validate already rejects this, but never run 'down -all'. + if ! [[ "${{ inputs.steps }}" =~ ^[1-9][0-9]*$ ]]; then + echo "❌ Refusing to roll back without an explicit positive 'steps' value." + exit 1 fi + echo "Rolling back ${{ inputs.steps }} migration(s)..." + migrate -path ${{ env.MIGRATIONS_PATH }} -database "$DB_URL" down ${{ inputs.steps }} fi - name: Get migration version @@ -265,11 +294,12 @@ jobs: migrate -path ${{ env.MIGRATIONS_PATH }} -database "$DB_URL" up ${{ inputs.steps }} fi else - if [[ "${{ inputs.steps }}" == "0" ]]; then - migrate -path ${{ env.MIGRATIONS_PATH }} -database "$DB_URL" down -all - else - migrate -path ${{ env.MIGRATIONS_PATH }} -database "$DB_URL" down ${{ inputs.steps }} + # Defense in depth: validate already rejects this, but never run 'down -all'. + if ! [[ "${{ inputs.steps }}" =~ ^[1-9][0-9]*$ ]]; then + echo "❌ Refusing to roll back without an explicit positive 'steps' value." + exit 1 fi + migrate -path ${{ env.MIGRATIONS_PATH }} -database "$DB_URL" down ${{ inputs.steps }} fi # Run Azure migrations @@ -329,11 +359,12 @@ jobs: migrate -path ${{ env.MIGRATIONS_PATH }} -database "$DB_URL" up ${{ inputs.steps }} fi else - if [[ "${{ inputs.steps }}" == "0" ]]; then - migrate -path ${{ env.MIGRATIONS_PATH }} -database "$DB_URL" down -all - else - migrate -path ${{ env.MIGRATIONS_PATH }} -database "$DB_URL" down ${{ inputs.steps }} + # Defense in depth: validate already rejects this, but never run 'down -all'. + if ! [[ "${{ inputs.steps }}" =~ ^[1-9][0-9]*$ ]]; then + echo "❌ Refusing to roll back without an explicit positive 'steps' value." + exit 1 fi + migrate -path ${{ env.MIGRATIONS_PATH }} -database "$DB_URL" down ${{ inputs.steps }} fi # Summary From 644bd59f059ed67ed8951d8b6857fd92bef8092d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 19 Jun 2026 16:50:35 +0200 Subject: [PATCH 2/2] fix(ci): pin setup-go SHA and update migration README Pin actions/setup-go to SHA 4a3601...c1c06c (v6.4.0) in all three migrate jobs (AWS, GCP, Azure); the pre-existing @v5 tag was unpinned, inconsistent with every other workflow in the repo. Update .github/workflows/README.md to accurately describe the down- migration guards introduced in this PR: replace the stale "warns on prod down migrations" bullet with the actual fail-closed behavior (explicit steps required, rollback-prod confirmation for prod), and add a prod-down example command showing the confirm input. --- .github/workflows/README.md | 17 +++++++++++++---- .github/workflows/database-migration.yml | 6 +++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 98116b640..bfc5acff8 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -342,6 +342,14 @@ gh workflow run database-migration.yml \ -f direction=down \ -f steps=2 +# Rollback 1 migration on AWS prod (requires typed confirmation) +gh workflow run database-migration.yml \ + -f cloud=aws \ + -f environment=prod \ + -f direction=down \ + -f steps=1 \ + -f confirm=rollback-prod + # Apply to all clouds gh workflow run database-migration.yml \ -f cloud=all \ @@ -351,10 +359,11 @@ gh workflow run database-migration.yml \ ### Safety Features -- **Validation** - Checks migration files exist -- **Production Warnings** - Warns on prod down migrations -- **Audit Trail** - Records all migrations -- **Step Control** - Apply specific number of migrations +- **Validation** - Checks migration files exist before running +- **Explicit steps required** - `direction=down` requires an explicit positive `steps` value; `steps=0` (the default, which would run `down -all` and drop the entire schema) is rejected +- **Production confirmation** - `direction=down` on `environment=prod` additionally requires typing `rollback-prod` in the `confirm` input; omitting or mistyping it blocks the run +- **Defense in depth** - each migrate job independently re-validates the positive-steps constraint, so a future validate regression cannot reach `down -all` +- **Audit Trail** - Records all migrations in the step summary --- diff --git a/.github/workflows/database-migration.yml b/.github/workflows/database-migration.yml index dd7c5c12e..29b39b479 100644 --- a/.github/workflows/database-migration.yml +++ b/.github/workflows/database-migration.yml @@ -177,7 +177,7 @@ jobs: persist-credentials: false - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version-file: go.mod @@ -253,7 +253,7 @@ jobs: persist-credentials: false - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version-file: go.mod @@ -320,7 +320,7 @@ jobs: persist-credentials: false - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version-file: go.mod