Skip to content
This repository was archived by the owner on Aug 19, 2026. It is now read-only.

fix(setup): serialize one-time setup completion to prevent owner-account race - #639

Open
dnviti wants to merge 1 commit into
mainfrom
codex/propose-fix-for-setup-race-vulnerability
Open

fix(setup): serialize one-time setup completion to prevent owner-account race#639
dnviti wants to merge 1 commit into
mainfrom
codex/propose-fix-for-setup-race-vulnerability

Conversation

@dnviti

@dnviti dnviti commented May 26, 2026

Copy link
Copy Markdown
Owner

Motivation

  • The unauthenticated first-run POST /api/setup/complete flow could be raced: parallel callers could all observe zero users and each create an OWNER account and tenant.
  • The root cause was a non-atomic pre-check performed outside the DB transaction and expensive async work before entering the transaction, leaving a large race window.

Description

  • Added a transaction-scoped PostgreSQL advisory lock using pg_advisory_xact_lock in createSetupAdmin to serialize concurrent setup completion attempts.
  • Added an in-transaction re-check isSetupRequiredTx that validates setupCompleted and user existence inside the same transaction and returns a 409 (requestError) when setup is already completed.
  • Kept the existing atomic creation of the initial admin user, AppConfig upserts, audit log insertion, and commit behavior unchanged except for the added lock and recheck.
  • Modified file: backend/internal/setup/complete.go.

Testing

  • Ran go test ./backend/internal/setup/... which reported no test files for the package (smoke check completed).
  • Ran go test ./backend/cmd/control-plane-api/... which succeeded (ok).

Codex Task

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the unauthenticated first-run setup completion flow against concurrent/raced requests by serializing setup completion within the database transaction and re-checking setup state under that serialization.

Changes:

  • Added a transaction-scoped PostgreSQL advisory lock (pg_advisory_xact_lock) in createSetupAdmin to serialize concurrent setup completion attempts.
  • Added isSetupRequiredTx to re-check setupCompleted and user existence inside the same transaction, returning 409 Conflict when setup is already completed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +256 to +258
// Serialize one-time setup completion attempts for this database.
if _, err := tx.Exec(ctx, `SELECT pg_advisory_xact_lock($1)`, int64(84572133)); err != nil {
return "", fmt.Errorf("acquire setup lock: %w", err)
Comment on lines +256 to +259
// Serialize one-time setup completion attempts for this database.
if _, err := tx.Exec(ctx, `SELECT pg_advisory_xact_lock($1)`, int64(84572133)); err != nil {
return "", fmt.Errorf("acquire setup lock: %w", err)
}
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants