fix(ci): require explicit steps and prod confirmation for down migrations

[cristim]

Problem

Closes #1173 (review finding INF-03, P2).

In .github/workflows/database-migration.yml, the steps dispatch input defaults to 0 ("0 = all"). In all three cloud jobs, direction=down plus steps=0 ran migrate ... down -all, dropping the entire schema. The validate job's prod+down branch commented "we don't auto-fail, but require manual confirmation" yet only echoed a warning and still set is_safe=true; no confirmation input existed anywhere in the workflow. An operator picking direction=down and leaving steps at its default would destroy every table in the target database, including prod.

Fix

• The validate job now rejects direction=down unless steps is an explicit positive integer (^[1-9][0-9]*$), and fails the job (exit 1) instead of silently skipping downstream jobs, so a blocked run is visibly red.
• Prod down migrations additionally require typing rollback-prod in a new confirm dispatch input (same pattern as cleanup-staging's typed destroy confirmation). The phantom-guard comment is gone; the guard is real.
• Defense in depth: the down -all branch is removed from all three migrate jobs (AWS/GCP/Azure); they fail loud if steps is missing or non-positive, so even a future validate regression cannot reach down -all.
• The free-form confirm input (and the other inputs used by the new checks) are passed to the shell via env: rather than inline interpolation.
• workflow_call (which defines neither steps nor confirm) is unaffected for up and now fails validation for down, which is the safe behavior; no caller in the repo uses this workflow today.

Test evidence

• actionlint on the modified workflow: clean.
• Local simulation of the validate-step and migrate-job guard logic with the real failing inputs (environment=prod, direction=down, steps=0, empty confirm): the pre-fix logic reproduces the bug (is_safe=true, job would run down -all); the post-fix logic sets is_safe=false and the job guard blocks. Legitimate flows still pass: staging down with steps=1, prod down with steps=2 + confirm=rollback-prod, prod up with steps=0 (all), and workflow_call up with empty steps. A wrong confirm string is rejected.
• No in-repo test harness exists for workflow YAML, so the regression check is the simulation above rather than a committed test; the workflow itself is dispatch-only and cannot be exercised by CI on this PR.

Summary by CodeRabbit

• Chores
  • Enhanced database migration rollback safety with new confirmation requirement for production
  • Improved validation with early failure on safety check violations
  • Updated rollback behavior with explicit step validation

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 40 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3a60b07b-b7bc-4480-a3dc-048fc3e26f9c

📥 Commits

Reviewing files that changed from the base of the PR and between 0880a3b and 644bd59.

📒 Files selected for processing (2)

• .github/workflows/README.md
• .github/workflows/database-migration.yml

📝 Walkthrough

Walkthrough

The database-migration workflow is hardened against accidental production schema destruction by requiring explicit authorization for rollback operations. A new confirm input and positive steps validation prevent the dangerous default behavior where direction=down with steps=0 would run migrate down -all. The validate job now enforces safety checks early and consistently across AWS, GCP, and Azure execution paths.

Changes

Database Migration Rollback Safety

Layer / File(s)	Summary
Input declarations and safety check wiring .github/workflows/database-migration.yml	Adds confirm input (required for production rollbacks) and refines steps description to clarify that down migrations require a positive value. Wires inputs.environment, inputs.direction, inputs.steps, and inputs.confirm as environment variables for safety check logic.
Safety validation logic and early exit .github/workflows/database-migration.yml	Down migrations now require steps to be a positive integer; production down migrations additionally require confirm to equal rollback-prod. Workflow immediately exits with failure if safety check does not pass, instead of proceeding to planning and execution.
AWS, GCP, and Azure rollback enforcement .github/workflows/database-migration.yml	All three cloud providers remove the "down all" code path and defensively validate steps again before execution. Down operations now always run migrate down ${{ inputs.steps }} with the supplied step count.

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested Labels

urgency/this-sprint

Poem

A rollback's a risk, a step to the past,
But down-all-at-once? That's a footgun too fast!
With confirm and with guards on each cloud-provider road,
No prod will burn bright when the migrations get bold. 🐰✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: enforcing explicit steps and production confirmation for down migrations to prevent dangerous defaults.
Linked Issues check	✅ Passed	The PR fully addresses the objectives from #1173: invalidates steps=0 for direction=down, requires explicit positive steps or confirmation, implements prod safeguard with typed confirmation input, and adds defense-in-depth.
Out of Scope Changes check	✅ Passed	All changes are scoped to enforcing safer down-migration behavior in the database-migration workflow; no unrelated modifications are present.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/inf-03-migration-down-guard

---

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

Adversarial-review pass

Reviewed against the threat model (down migrations are destructive; the prior code echoed a warning while still setting is_safe=true), the listed risk surfaces, and the memory entries feedback_migration_full_restore.md / project_migration_dirty_recovery.md / project_migration_number_collisions.md.

In-scope fix pushed (force-push on fix/inf-03-migration-down-guard, base main)

• Rebase onto main to clear the migration-number collision. Before rebase the merge base was 2d4d6ae63, which carried the 000074_audit_actor_stamps + 000074_repair_partial_migration_058_067 duplicate that main resolved in fix(db): renumber audit_actor_stamps migration to clear 000074 collision #1261 (renumbered 074 -> 077). That's the Check for conflicting migration numbers failure, the duplicate-migration error in Integration Tests, and the same in E2E Tests. pre-commit's end-of-file-fixer hiccup on terraform/environments/azure/ci-cd-permissions/sp.tf auto-resolves on the second attempt. The remaining red checks (Lint Code errcheck, Security Scanning npm-audit on frontend/) are pre-existing on main and orthogonal to this PR; see fix(lint): properly handle all errcheck violations from golangci-lint v2 #1265 for the errcheck cleanup track. actionlint is clean on the rebased file.

Confirmed clean

• Validate gate simulation, 13 scenarios run end-to-end against the new bash block (/tmp/claude/pr1216/sim.sh):
  • prod down steps=0 no confirm -> is_safe=false (the original INF-03 repro is now blocked).
  • prod down steps=1 missing/wrong confirm -> is_safe=false.
  • prod down steps=1 confirm=rollback-prod -> is_safe=true.
  • dev/staging down steps=0 / steps=-1 / leading-zero 01 / 1.5 / 1e9 -> all rejected by ^[1-9][0-9]*$.
  • prod up steps=0 -> safe (unchanged).
  • workflow_call mode (STEPS and CONFIRM undefined): down rejected, up accepted, matching the PR description.
• No injection surface. inputs.environment/inputs.direction are type: choice from a closed list. inputs.steps is type: number in dispatch and undefined in workflow_call (renders empty), so the inline ${{ inputs.steps }} in the migrate-* jobs has no injection path even though only the validate step uses env: indirection.
• Defense in depth holds. Each cloud's migrate job re-checks the regex before invoking migrate down; the old down -all branch is gone in all three jobs.
• Pin to actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c (v6.4.0) is consistent with the rest of the repo's SHA pinning and matches the in-use major (no downgrade), per feedback_sha_pin_current_major.md.
• workflow_call callers: none in the repo today, and the new gate fails closed for down while leaving up working, so this is safe to merge.

Out-of-scope follow-up filed

• ci: prod down migration with steps>=total bypasses confirmation gate (down -all equivalent) #1321 -- steps=99999999 confirm=rollback-prod still slips through. golang-migrate treats down N where N exceeds the available count as "roll back everything that remains", so a tired operator typing a deliberately large step count plus the correct prod confirmation effectively performs down -all -- the exact failure mode this PR was scoped to prevent. Four mitigations listed in the issue; option 4 (confirm=rollback-prod-<N> where the typed N must equal inputs.steps) is the smallest patch and matches the existing typed-string pattern. Filed separately because option 3 (target_version input + computed step delta) is a UX redesign.

Risk surfaces from the brief, checked

• "Explicit steps" enforcement: regex ^[1-9][0-9]*$ in validate + same regex re-checked in each migrate-* job. Both gates must pass.
• "Prod" detection: closed-set type: choice on environment (dev/staging/prod). No env-var or branch-name heuristic, so misclassification is not possible at the workflow boundary.
• Bypass paths: workflow_dispatch validates; workflow_call fails closed on down; migrate CLI direct execution from a runbook is outside the workflow's reach but also outside its scope. (Repository-dispatch is not configured for this workflow.)
• feedback_migration_full_restore.md (paranoia about IF NOT EXISTS): not applicable -- this PR gates the trigger, it does not change the migration SQL.
• project_migration_dirty_recovery.md: this workflow does not detect or surface dirty state after a half-applied down. If a down fails partway, the operator still needs CUDLY_FORCE_MIGRATION_VERSION=<current> out of band. Worth a separate follow-up if we want the workflow to print recovery guidance on failure; not blocking here.
• Audit trail: GitHub captures who/when/what for workflow_dispatch runs; the PR adds the typed confirmation to that record. Slack notification is a nice-to-have, not blocking.
• Reversibility: orthogonal to this PR -- even a single-step down can drop a column whose data was being written between up and down. Out of scope here; mitigated by the new typed-confirmation gate.

Re-pinging CR.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

@coderabbitai review

[cristim]

Merge-prep bot: rebased onto main (73166c2). Gates: build=0 (no Go touched), gocyclo=n/a, lint=n/a. Workflow change verified: validate job now exits 1 on bad inputs, down-all branch removed from all 3 migrate jobs (AWS/GCP/Azure), setup-go SHA-pinned to v6.4.0, required-check job names unchanged. No existing deploy jobs broken.

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

Merged to main (rebased onto current main, CLEAN + all CI green; closes #1173). CI hardening: validate steps now exit 1 on bad inputs, destructive down-all removed from 3 migrate jobs (guarded), setup-go SHA-pinned; required-check names unchanged. The earlier Integration failure was a shared testcontainer-quota infra flake (migrations passed to v83), cleared on re-run.