feat(dashboard-api): supabase auth users sync background runner

[ben-fornefeld]

Motivation

We're migrating our Database and need to de-couple current database triggers between auth and core schema. This PR allows having a sync between these two schema, that supports separate db configurations respectively.

[cursor]

PR Summary

Medium Risk
Introduces new DB triggers and a background worker that writes into the auth database, so misconfiguration or migration/queue issues could cause missed user syncs or unintended deletions/updates. Deployment risk is moderate due to new environment variables and infra templating changes affecting dashboard-api runtime behavior.

Overview
Adds an optional dashboard-api background worker (River + Supabase DB client) that reacts to auth.users insert/update/delete via new Supabase-side triggers and keeps public.users in the auth DB up to date (upsert email or delete). Deployment config is extended to support a separate SUPABASE_DB_CONNECTION_STRING and a feature flag (ENABLE_AUTH_USER_SYNC_BACKGROUND_WORKER), and database tooling is updated with new sqlc-generated Supabase queries, new auth DB queries (UpsertPublicUser/DeletePublicUser), and end-to-end tests around the sync behavior; several services also bump pgx to v5.9.1.

^{Reviewed by Cursor Bugbot for commit fbb8835. Bugbot is set up for automated code reviews on this repo. Configure here.}

[claude]

The partial index only covers dead_lettered_at IS NULL AND locked_at IS NULL, so every unlocked non-dead-lettered row satisfies it — including rows with a future next_attempt_at. The claim query then discards those rows in a post-index filter. Under sustained retry load (many rows waiting with next_attempt_at > now()), the index returns a large candidate set that must be filtered in-memory before the LIMIT is applied. Adding next_attempt_at as the leading column — ON auth.user_sync_queue (next_attempt_at, id) WHERE dead_lettered_at IS NULL AND locked_at IS NULL — lets PostgreSQL range-scan only rows that are actually ready to be claimed.

[claude]

Items in a batch are processed sequentially. Each item does 2–3 synchronous DB round-trips (GetAuthUser + UpsertPublicUser/DeletePublicUser + Ack/Retry), so batch throughput is proportional to batchSize × DB latency. With the default BatchSize=50 and typical Postgres latency, a full batch can take several seconds — longer than PollInterval=2s. If sustained write volume is expected, consider processing items in a small goroutine pool.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3f56979748

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Ensure auth queue migration is executed by db-migrator

The queue/table trigger migration was added under packages/db/pkg/auth/migrations, but the deployed migrator workflow only runs ./migrations from packages/db/scripts/migrator.go (and the db Dockerfile only copies db/migrations). That means the standard prestart migration job will never create public.user_sync_queue or the new triggers, so enabling the sync worker will fail at runtime when it tries to claim from a non-existent table.

Useful? React with 👍 / 👎.

[ben-fornefeld]

we want to execute this manually + it should not be tied to main db migrations since authDB will be separate going forward

[claude]

🔴 The SQL query selects email from auth.users without null handling, and the generated AuthUser.Email field is a non-nullable string; Supabase supports phone-only and anonymous auth where email is NULL, so pgx returns a scan error (not pgx.ErrNoRows) for those users. In reconcile(), that error triggers the generic retry path, causing indefinite retries until the item is dead-lettered and the user is never synced to public.users. Fix by changing the query to COALESCE(email, '') or by mapping Email to *string and skipping the upsert when absent.

Extended reasoning...

What the bug is and how it manifests

The file packages/db/pkg/auth/sql_queries/supabase_auth_user_sync/get_auth_user.sql (lines 1-4) contains:

-- name: GetAuthUserByID :one
SELECT id, email
FROM auth.users
WHERE id = sqlc.arg(user_id)::uuid;

The generated code in get_auth_user.sql.go scans the result into AuthUser, whose Email field is declared as a non-nullable string in models.go. In pgx v5, scanning a SQL NULL into a plain Go string returns a scan error — it does not return pgx.ErrNoRows.

The specific code path that triggers it

In processor.go, reconcile() calls p.store.GetAuthUser(ctx, item.UserID). It checks errors.Is(err, pgx.ErrNoRows) to decide whether to delete the public user. A NULL-email scan error is not pgx.ErrNoRows, so it falls into the generic if err != nil { return "", fmt.Errorf(...) } branch, which propagates back to process(). Since the item is not yet at maxAttempts, process() calls store.Retry(...), scheduling the item for another attempt with exponential backoff.

Why existing code does not prevent it

The reconcile() function only has two exit paths for errors from GetAuthUser: the pgx.ErrNoRows special case (leading to a delete) and a generic retry path. There is no handling for scan errors caused by NULL columns, and no defensive COALESCE or nullable type in the generated SQL/struct.

Impact

Any Supabase deployment using phone-only or anonymous authentication will have users with auth.users.email = NULL. Every such user that enters the sync queue will be retried up to MaxAttempts (default 20) times with growing backoffs and then dead-lettered. Those users are permanently excluded from public.users, causing any feature that relies on public.users to silently omit them.

How to fix it

The simplest fix is to add COALESCE in the SQL query:

SELECT id, COALESCE(email, '') AS email
FROM auth.users
WHERE id = sqlc.arg(user_id)::uuid;

Alternatively, change the sqlc config so email maps to *string, update AuthUser.Email to *string, and skip the upsert (or pass an empty string) when Email is nil.

Step-by-step proof

1. A Supabase phone-only user signs up. The auth.users row has id = <uuid>, email = NULL.
2. The sync_insert_auth_users_to_public_users_trigger fires, inserting (user_id, 'upsert') into public.user_sync_queue.
3. The worker calls ClaimBatch, picks up the item, increments attempt_count to 1.
4. processor.process() -> processOnce() -> reconcile() -> store.GetAuthUser(ctx, userID).
5. GetAuthUserByID executes SELECT id, email FROM auth.users WHERE id = $1. The row exists so pgx does not return pgx.ErrNoRows. Instead row.Scan(&i.ID, &i.Email) attempts to scan NULL into string and returns a pgx scan error.
6. Back in reconcile(), errors.Is(err, pgx.ErrNoRows) is false. The error is returned as fmt.Errorf("get auth.users %s: %w", ...).
7. process() sees item.AttemptCount (1) < maxAttempts (20), so it calls store.Retry(...) with a 5s backoff.
8. Steps 3-7 repeat until attempt_count >= 20, at which point DeadLetter is called.
9. The user is now dead-lettered and will never be synced to public.users.

[ben-fornefeld]

fine, we require emails for auth

[claude]

🟡 The %{ for key, val in env } loop in dashboard-api.hcl renders env var values directly inside double-quoted HCL strings without escaping. A value containing a double-quote produces syntactically invalid HCL (Nomad job submission fails), and a value containing ${ causes Terraform templatefile() to attempt expression evaluation at render time (plan-time error). DASHBOARD_API_ENV_VARS is documented as accepting arbitrary operator-supplied values, making these characters plausible; fix by escaping double-quotes in values or passing extra env vars via a Nomad template stanza instead.

Extended reasoning...

What the bug is and how it manifests

In iac/modules/job-dashboard-api/jobs/dashboard-api.hcl lines 86-88, a Terraform templatefile() directive renders arbitrary env var values directly inside double-quoted HCL strings:

%{ for key, val in env }
${ key }   = "${ val }"
%{ endfor }

Two distinct injection vectors exist. First, if val contains a double-quote character, the rendered output becomes syntactically invalid HCL (e.g. MY_VAR = "conn"string" breaks the HCL parser), causing Nomad to reject the job spec at submit time. Second, if val contains ${, Terraform's templatefile() function interprets it as an expression interpolation during template rendering and produces a plan-time error before the job is ever submitted.

The specific code path that triggers it

The value flows through: DASHBOARD_API_ENV_VARS env var -> parsed as map(string) in iac/provider-gcp/variables.tf -> passed to module.nomad as dashboard_api_env_vars -> passed to module.dashboard_api as env -> filtered in local.env in main.tf (removes null/empty only) -> passed to templatefile() -> rendered at line 87 with no escaping.

Why existing code does not prevent it

The locals block in main.tf only filters out null/empty values (if value != null && value != ""); it does not escape or sanitize values. Terraform's templatefile() does not automatically escape template variable contents when they are interpolated. The HCL string delimiters are hard-coded double quotes with no escaping mechanism.

What the impact would be

For a double-quote injection (e.g. a PostgreSQL DSN like postgresql://user:p"ass@host/db), the Nomad job submission fails at terraform apply time with an HCL parse error -- the deployment is blocked. For a ${ injection (e.g. a JWT secret or template string containing ${), Terraform fails during plan or apply with an evaluation error. Neither failure is silent, but both are surprising and difficult to debug without knowing the root cause.

How to fix it

The safest fix is to use replace(val, "\"", "\\\"") to escape double-quotes within the current templatefile() approach, or pass extra env vars through a Nomad template stanza which has proper escaping semantics. The ${ vector can be addressed by replacing ${ with $${ (Terraform's escape sequence for a literal ${).

Step-by-step proof

1. Operator sets DASHBOARD_API_ENV_VARS='{"DB_URL":"postgresql://user:p\"ass@host/db"}' in their env file.
2. Terraform receives var.dashboard_api_env_vars = {DB_URL = "postgresql://user:p\"ass@host/db"}.
3. local.env passes it through unchanged (non-null, non-empty).
4. templatefile() renders line 87 as: DB_URL = "postgresql://user:p"ass@host/db" -- the embedded " terminates the HCL string prematurely.
5. Nomad's job spec parser receives invalid HCL and rejects the job with a parse error.
6. Alternatively, for a value containing ${SOME_VAR}: templatefile() evaluates ${SOME_VAR} as a Terraform expression, returning an undefined variable error during terraform apply.

Note: the identical ${ key } = "${ value }" pattern exists in job-api and job-orchestrator for their job_env_vars, but those maps contain hardcoded infrastructure constants (VOLUME_TOKEN_ISSUER, etc.) unlikely to contain special characters. The new DASHBOARD_API_ENV_VARS is explicitly documented as a freeform map for arbitrary operator-supplied values, removing that safety assumption.

[cursor]

Down migration drops schema without CASCADE, will fail

Medium Severity

The down migration uses DROP SCHEMA IF EXISTS auth_custom without CASCADE, but the schema will contain River library tables (created by the programmatic RunRiverMigrations call between goose migration steps 1 and 3). Since goose has no knowledge of the interleaved River migrations, running the down migration will fail with a "schema is not empty" error because auth_custom.river_job and other River tables still exist.

 

^{Reviewed by Cursor Bugbot for commit 4862139. Configure here.}

[ben-fornefeld]

desired, we have other services potentially depend on the schema and don't want to cascade

[cursor]

Stopping already-stopped River client causes spurious shutdown error

Low Severity

When the River client stops unexpectedly, waitForServiceStop returns via riverStoppedCh. Then shutdownService calls riverClient.Stop(ctx) on the already-stopped client, which returns ErrClientAlreadyStopped. This causes shutdownService to return an error and the process exits with code 1 with a misleading "shutdown River client" error, even though the HTTP server shutdown succeeded. The shutdownService function lacks a guard for the already-stopped case.

Additional Locations (1)

• packages/dashboard-api/main.go#L353-L356

 

^{Reviewed by Cursor Bugbot for commit 29bbf59. Configure here.}

[ben-fornefeld]

invalid, the channel returns nil either way

[dobrac]

this should stay in the dashboard-api.hcl config

[dobrac]

can we remove this altogether and just use the newly added TF_vars env variables?

[dobrac]

we should be able to not add this anymore now

[dobrac]

not needed anymore

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit fbb8835. Configure here.}

[cursor]

River client started with wrong context for lifecycle

Medium Severity

StartAuthUserSyncWorker accepts separate setupCtx and runCtx parameters, but main.go passes the same base ctx for both. The runCtx controls the River client's lifecycle via riverClient.Start(runCtx). Since this is the base context (only cancelled by defer cancel() when run() returns), the riverClient.Stopped() channel used in waitForServiceStop will effectively never fire on its own — River won't self-stop because its context is never cancelled independently. If signalCtx were passed as runCtx, River would begin graceful shutdown as soon as a signal arrives, in parallel with the explicit shutdownService call, improving shutdown responsiveness.

Additional Locations (1)

• packages/dashboard-api/internal/backgroundworker/river.go#L43-L44

 

^{Reviewed by Cursor Bugbot for commit fbb8835. Configure here.}