Skip to content

auth_user.name secretly duplicates DJs' legal names — derive it from handle/username at one choke point #2296

Description

@jakebromberg

Tracks 2b–2d + 3b of plans/dj-name-pii-safeguards.md (the plan itself ships in #2294).

The structural problem

WXYC collects DJs' full legal names as a legal requirement. That collection is correct and stays. The failure class is conflation.

Commit a0cd1979 (2025-12-31) collapsed the old djs table's deliberate two-column design — dj_name (the on-air handle) and real_name (the PII) — into auth_user. From that point, dj-site provisioning fills auth_user.name with realName || username (RosterTable.tsx:92, and again in ImportCSVModal.tsx), landing on POST /auth/admin/provision-user, which stores the client-supplied value verbatim — with name sitting in the route's required-fields validation, so a caller cannot opt out of supplying one.

So auth_user.name — better-auth's innocuously-named, NOT NULL, universally-read display field — has been a hidden second copy of the legal name for eight months, and every innocent-looking .name read has been a potential PII leak to a public surface. BS#1286/#1288, BS#1393 and BS#2281 are all downstream of that single structural fact, which is why fixing them one at a time has not converged.

It is worse than a duplicate: it is a duplicate that is written once and never maintained. Roster edits (AccountEditForm.tsx) call better-auth's authClient.admin.updateUser directly against the auth service — that request never passes through any Backend-Service call site — and update realName/djName without touching name. Onboarding (complete-onboarding.ts) likewise. The hidden copy also goes stale, so it is neither correct nor safe.

End state

auth_user.name := on-air handle, else username. Public-safe by construction: after the unscrew, even an accidental .name read leaks nothing. real_name becomes the sole carrier of the legal name, which is what the schema meant before the collapse.

Design: one choke point, not N call sites

databaseHooks.user.create.before / update.before in shared/authentication/src/auth.definition.ts (extending the databaseHooks.user block that already exists there), delegating to two pure, unit-testable helpers in a new shared/authentication/src/derive-user-display-name.ts.

The choke point is the whole point. This one site covers provision-user.ts, create-default-user.ts, create-auto-dj-user.ts, complete-onboarding.tsand dj-site's direct admin.updateUser roster path, which no Backend-Service call-site helper can reach at all. The rejected alternative (an admin-route-scoped hook plus per-call-site derivation) would re-scatter the invariant across N writer sites, which is precisely the disease.

  • create.before: the full payload is present, so the chain is a straight priority order — resolveDjDisplayName(djName) ?? username ?? <supplied name>. The supplied-name terminal is deliberate: it is how the literal 'Anonymous' (better-auth's anonymous plugin) and 'Auto DJ' (create-auto-dj-user.ts) survive the hook unclobbered, since neither has a resolvable handle or a username.

  • update.before is payload-only, and that is forced rather than chosen. Verified against better-auth's db/with-hooks.mjs at the lockfile-resolved version (1.6.26): internalAdapter.updateUser(userId, data) builds where: [{ field: 'id', value: userId }] itself and calls updateWithHooks(data, where, 'user', …); the hook only ever receives toRun(data, context) and never sees where. There is no row fetch available inside an update.before hook, by construction — so the hook can only ever answer "does this update's own djName resolve to a usable handle?", never "what is this user's handle right now?". complete-onboarding.ts calls the identical internalAdapter.updateUser path from outside any better-auth endpoint, so a request-context fallback is unavailable there too.

  • The { data: … } return contract is load-bearing, not style. The same file shows createWithHooks/updateWithHooks merging only a returned value satisfying typeof result === 'object' && 'data' in result (actualData = { ...actualData, ...result.data }). A mutated data argument, or a bare object, is silently discarded — the hook appears to run while the write proceeds with the original payload — and returning false aborts the write entirely. undefined is therefore the only correct no-op return. The helpers mirror capSessionUpdateAgainstDeviceFlow's existing contract for exactly this reason: an unpinned return shape here fails as a silent no-op, the worst possible failure mode for a PII guard.

  • Two update shapes are deliberately left untouched rather than guessed at: handle-clear (djName present but blank), and username-only rename (no djName key in the payload — a username-only payload cannot reveal whether the user currently has a handle, so deriving from it risks clobbering a live handle). Both leave name at its prior value, which post-backfill is an earlier handle or an earlier username: cosmetic staleness, structurally non-PII either way.

Provision route stops trusting the client

name leaves the required-fields array in apps/auth/app.ts and becomes optional on ProvisionUserInput; provisionUser() derives the stored value itself. That is belt-and-suspenders with the hook on purpose: whether the hook merge runs before better-auth's adapter enforces the core schema's required: true on name is unverified against the resolved version, and supplying the value at the call site removes the ordering dependency entirely. A still-supplied name is accepted-and-ignored for the deploy overlap window, so dj-site can drop the field in its own PR without a lockstep deploy.

Backfill job, opening with a machine-enforced gate

jobs/auth-user-name-backfill/ — a one-shot workspace, dry-run by default, --execute to write, rewriting ~139 existing rows to the same derivation, computed in TypeScript through the canonical resolveDjDisplayName helper and never re-derived in SQL.

Its first act is a precondition gate that runs the preserve-first predicate

(real_name IS NULL OR trim(real_name) = '')
  AND NOT is_anonymous
  AND name NOT IN ('Anonymous', 'Auto DJ')
  AND name IS DISTINCT FROM username

against every row and aborts non-zero — in dry-run and execute mode alike — if any row matches. A matching row holds its only copy of a legal name in auth_user.name; rewriting it before that value has been copied to real_name loses the legal name outright, which is the one unrecoverable failure in this whole program. The gate makes run order irrelevant instead of trusting an operator to sequence the manual step correctly.

A Type check: auth-user-name-backfill step joins .github/workflows/test.yml (the BS#2009 precedent): the root typecheck excludes jobs/** and tsup --minify is transpile-only, so without that step the one job carrying the gate could ship uncompiled-but-green.

Sentinel wire spec

tests/integration/dj-real-name-sentinel.spec.js is the mechanism that would have caught a0cd1979. It seeds an auth_user row shaped exactly like the conflation this issue exists to unscrew — real_name and name both holding a sentinel legal name — plus a show and flowsheet entries, then whole-body string-matches JSON.stringify(res.body) across GET /flowsheet, /flowsheet/latest, /flowsheet/range and /flowsheet/search (including a dj: operator query) and asserts the sentinel appears in no response body.

Whole-body matching rather than a keys-only shape assertion, so it catches fields that do not exist yet — the read path is where every prior incident actually surfaced. Every it carries a positive control asserting the seeded handle is present, so a passing "no sentinel" assertion cannot be quietly vacuous from the row falling out of query scope.

The step that is deliberately NOT automated

The preserve-first copy (name → real_name, for rows whose only legal-name copy is in name) stays reviewed manual SQL with the audit SELECT evidence attached, per the org data-safety rule. It is human-gated on purpose: it is the one irreversible action here, and losing a legal name is unrecoverable. The backfill job must not be --executed until it has run. The precondition gate enforces that mechanically, but the sequencing decision remains a person's.

Acceptance criteria

  • create.before/update.before derive name for every writer path — including a dj-site-shaped direct admin.updateUser djName edit — with 'Anonymous'/'Auto DJ' preserved and handle-clear leaving name untouched; hook-invocation shape re-verified against the lockfile-resolved better-auth version.
  • Provision succeeds with no name in the request body (required-fields array and ProvisionUserInput both edited), and a supplied name is ignored rather than stored.
  • The backfill's precondition gate aborts non-zero while any preserve-first row remains, in both dry-run and execute mode.
  • The backfill derives through resolveDjDisplayName, skips anonymous and 'Auto DJ' rows, and leaves rows with neither handle nor username unchanged.
  • The sentinel spec passes, and demonstrably fails against a deliberately leaky mutation (proven once during implementation, not assumed).
  • Type check: auth-user-name-backfill runs in CI.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    resiliencePrevents prod regressions or surfaces them earliersecurityAuthentication, authorization, CORS, credentials, or secrets risk (warrants security review)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions