pluto
PLUTO-202
BUILD (dispatch-ready, Elazar-launched per bs-mqpzo3ly470 — do NOT act between sessions): Pluto admin "Cambiar email de acceso" server action (canonical change-login-Gmail, decision bs-mqq2v3r1802). New action in src/lib/admin-actions.ts, gate withCap(manageUsers) NOT fullAccess (reportes-trap, mem pluto-view-as-architecture). Input targetUserId+newEmail. In ONE txn w/ setArchiveContext (users is trigger-archived): (1) isAllowlistedEmail(new); (2) dedup-reject if canonicalizeEmail(new) belongs to another live user (id!=target, deletedAt IS NULL); (3) UPDATE users.email=new (emailCanonical GENERATED auto-regens, mem pluto-stored-generated-column-no-recompute) + SET users.authId=NULL so new Google identity re-links next login; (4) cleanup stale auth.users row for OLD identity via service-role admin deleteUser so old account cannot log back in; (5) userAuditLog email delta old->new; (6) optional fire welcome email to new address. Re-login on new gmail = proof-of-control (auth/callback matches by canonicalizeEmail(users.email)==canon(auth email), stamps new authId, same row -> roles/practicas intact). UI: "Cambiar email de acceso" in admin-user-detail.tsx with AlertDialog confirm (account-takeover-shaped). Touches auth/identity -> pre-impl audit design-ping + pre-push audit diff review per push-to-done.
Done normal
cpcoder-pluto-cc
Activity
-
pm-pluto-cc
-
coder-pluto-cc
-
PRE-IMPLEMENTATION DESIGN NOTE (coder-pluto-cc, read at acc9d0f / v2.23.140). Four of the six steps in the title are wrong against the CURRENT code. Measured, not reasoned - file:line for each. INVARIANT NOTE (what the change touches) - Callers that reach it: ONE admin surface (admin-user-detail.tsx) behind requireCapForAction('manageUsers'). manageUsers not fullAccess is correct and load-bearing: fullAccess is the reportes trap (navigation.ts:27 routes reportes on fullAccess), and view-as impersonation is already gated on manageUsers (auth-actions.ts:26 VIEW_AS_ADMIN_CAP) so this does not widen the admin surface. - Attacker-controlled inputs: newEmail only (operator-typed). targetUserId is operator-chosen from the admin list. NEITHER is mailbox-selected, which is the containment pmmaster named: the row is picked by the ADMIN, never by whoever proves control of an address. - BUT the post-change re-login DOES select the row by canonical email: auth-autolink.ts:84-90 resolves the target from the email alone. So the window between the UPDATE and the first successful re-login is one in which anyone controlling the new address binds to that row with its roles and practicas. Containment is exactly three clauses: isAllowlistedEmail (admission), the dedup reject, and auto-link's exactly-one-eligible + both-anchors-NULL predicate. Flipping any one of them un-contains a full account takeover, which is why this is Class-A. FINDING 1 - the spec strands the user (BLOCKING). Step (3) clears authId only. auth-autolink.ts:102 rejects the bind with reason 'target-bound-different-sub' when users.googleSub still holds the OLD identity's sub. The re-login then routes to /solicitar-acceso and the account cannot be recovered through the UI at all. The action MUST clear BOTH authId and googleSub (uq_users_googleSub_live is the partial live index, schema.md:1480). FINDING 2 - step (4) is unnecessary AND has no primitive. There is no service-role Supabase client anywhere in src (only the SSR anon createClient, supabase/server.ts:4) and zero SERVICE_ROLE references in the tree, so 'admin deleteUser' would mean introducing a service-role key into the app runtime for a cleanup step. It is also not needed for security: after the change no live row holds the OLD canonical email, so the old Google account hits callback/route.ts:134 -> findUnlinkedUserByEmail -> no-eligible-match -> /solicitar-acceso, creating nothing and granting nothing. The old identity is locked out by users.email being the SSOT, not by deleting the auth row. RECOMMEND: drop step (4), keep the service-role key out of the runtime, and note the orphaned auth.users row as hygiene, not exposure. FINDING 3 - step (5) would RAISE. userAuditLog no longer exists; it is userAuditLogLegacy (schema.md:1353) and it carries trg_userAuditLogLegacy_forbidWrite on INSERT/UPDATE/DELETE (schema.md:1372-1374). The email delta is already captured by fn_archive_users into usersArchive, attributed by the setArchiveContext the txn already sets. RECOMMEND: delete step (5); the archive row IS the audit record. (Separately: schema.md:2061 still says 'Audit: userAuditLog table' in hand-written prose - stale, not my row.) FINDING 4 - the row's stated proof-of-control rail is the wrong one. The title says callback matches canonicalizeEmail(users.email)==canon(auth email) and stamps the new authId. That rail (callback/route.ts:248-276) only runs when authId ALREADY matches, and it REJECTS on mismatch (loginRejectedEmailMismatch + operator alert). With authId cleared, the flow takes the length===0 branch and the actual binder is autoLinkExactEmailAtLogin, which has its own CAS, FOR UPDATE lock and named-unique handling. Proof-of-control still holds, but the acceptance test must exercise the auto-link path, not the mismatch path. NO DDL: users.email, emailCanonical (STORED generated), authId, googleSub all exist. No db-pluto dependency. PROPOSED SHAPE after the four corrections: requireCapForAction('manageUsers'); one txn; setArchiveContext(client, actor, 'admin:changeLoginEmail:<...>'); isAllowlistedEmail(newEmail); reject if canonicalizeEmail(newEmail) belongs to another live row (id<>target AND deletedAt IS NULL); UPDATE users SET email=$2, authId=NULL, googleSub=NULL WHERE id=$1 AND deletedAt IS NULL; AlertDialog confirm in the UI; optional notice to the new address. Tests assert the FORBIDDEN direction: googleSub IS NULL after the change (the mutant that only clears authId must fail), and the dedup reject fires on a canonical collision that is not a raw-string collision (dotted gmail). Design-ping sent to audit-general-ca (no audit-pluto-* seat in the roster; audit-general-ca live, poll_age 3s at ask time). Not implementing until it answers.
-
AUDIT BLOCK RECONCILED - design revised to a CONSOLIDATION, not a new parallel action (coder-pluto-cc, audit-general-ca review tag audit-general-ca-muwotnuhxodj). audit-general-ca BLOCKed on an existing path my design note missed: updateUserEmail at admin-actions.ts:1267, called from admin-user-detail.tsx:309. My grep was for 'cambiar email|changeEmail|changeLoginEmail' and missed it on the name. The BLOCK is upheld; two of its particulars are wrong and the correction changes what gets built. WHAT updateUserEmail ACTUALLY DOES (read at acc9d0f): - gate withCap('manageUsers') - SAME gate, no widening. - line 1291: REJECTS when target.authId IS NOT NULL ('Solo se puede cambiar el email de usuarios que no han ingresado al sistema'). That is not a permission, it is the design: this action is the never-logged-in case ONLY. - lines 1296-1304: canonical dedup IS present and IS correct - canonicalizeEmail both sides, id != target, deletedAt IS NULL. Audit read it as absent; it is not. - NO allowlist check. Audit is right: an operator can set an address outside the admission allowlist. Real gap. - NO googleSub check. Audit is right, and this is the interesting one (below). - writes canonicalizeEmail(email) INTO users.email, so a dotted/+tag gmail loses its display form. Pre-existing, cosmetic, named not fixed here. THE googleSub HOLE IS CURRENTLY UNREACHABLE BY APP CODE, AND THAT IS THE PROBLEM. updateUserEmail treats 'authId IS NULL' as 'never logged in'. That equivalence is held up by convention across all five googleSub write sites - auth-autolink.ts:183, admin-batch.ts:139 and :853, admin-actions.ts:2247 - every one of which sets authId and googleSub TOGETHER and none of which ever writes authId = NULL (admin-batch.ts:132 says so explicitly). NO CONSTRAINT enforces it. So a migration, a manual repair, or any future path that nulls authId alone makes the hole reachable with nothing raising. A row with googleSub set and authId NULL would, after an email change, be unbindable by the new address (auth-autolink.ts:102 target-bound-different-sub) and unreachable by the old Google account (its canonical email no longer matches any row) - a silently stranded account. UNMEASURED: this seat has no Pluto DSN, so I cannot run SELECT count(*) FROM users WHERE googleSub IS NOT NULL AND authId IS NULL AND deletedAt IS NULL. That count decides live defect vs latent one; it does not change the fix. Asked db-pluto-cc. SUPABASE BOUNDARY (audit's pre-implementation ask, measured myself): ZERO supabase.from / .rpc / .storage call sites in src. The only client beyond the SSR server client is createBrowserClient in src/lib/supabase/client.ts, used for the OAuth sign-in trigger only (login-client.tsx, solicitar-acceso/page.tsx). All data access is pg through src/db/pool.ts behind app-side gates, so there is no JWT-authorized Data API / Storage / RLS path that an unrevoked old Auth session could use. Boundary explicit, as asked. REVISED PLAN - consolidate rather than add. ONE action supersedes updateUserEmail and its single call site: 1. withCap('manageUsers') unchanged. 2. isAllowlistedEmail(newEmail) - NEW, closes the allowlist gap on the path that already exists. 3. live target SELECT ... FOR UPDATE; no authId-IS-NULL precondition (lifting that precondition is what PLUTO-202 IS). 4. canonical dedup excluding self - kept as written, it was already right. 5. UPDATE users SET email, authId = NULL, googleSub = NULL - BOTH anchors, which also makes the googleSub hole unreachable by construction instead of by convention. 6. setArchiveContext coded reason; usersArchive IS the audit record; no userAuditLogLegacy write (it carries forbidWrite triggers). 7. UI: AlertDialog confirm replacing the inline save at admin-user-detail.tsx:309. SCOPE NOTE audit required and I am adopting verbatim: this is NOT Auth-session revocation. The old auth.users account and its refresh capability survive; what the change revokes is APP authorization, because callback and role resolution both resolve through users. Dropping auth.admin.deleteUser stays, and the orphaned Auth row is hygiene, not exposure. Acceptance asserts the FORBIDDEN direction: googleSub IS NULL after the change (a mutant clearing only authId must fail), the allowlist reject fires, and the dedup reject fires on a canonical-only collision (dotted gmail) that is not a raw-string collision.
-
Prod census (db-pluto-ca, 2026-10-06 10:17 ART, coder_pluto_cc_ro via direct DSN, default_transaction_read_only=on): 0 active users with googleSub IS NOT NULL AND authId IS NULL. Classifies existing exposure only - it did not gate this fix. Zero stranded rows today, so no follow-up row is opened; the invariant stays unenforced by the schema and is closed by construction in changeLoginEmail (both anchors cleared unconditionally) rather than by that count staying zero.
-
SHIPPED 65951c0 / v2.24.0 (gitpush v5.5, --minor, 5 files: src/lib/admin-actions.ts, src/components/admin-user-detail.tsx, src/lib/friction-codes.ts, src/lib/change-login-email.test.ts [new], package.json). VERDICT (ledger record, not on the commit): PASS:00652f43 from audit-general-ca, tag audit-general-ca-muwpt3nux8vi, Class-A pre-push SOURCE verdict on the frozen patch it independently rehashed. Explicitly NOT a production-behaviour or deploy PASS. Reviewer: audit-general-ca (no audit-pluto-* seat exists; picked by lane from a live roster read, poll_age 2s). WHAT SHIPPED: updateUserEmail replaced by changeLoginEmail. Allowlist admission -> FOR UPDATE live target -> canonical no-op reject -> canonical dedup -> ONE UPDATE setting email + authId NULL + googleSub NULL -> archive context admin:changeLoginEmail:user:<id>. Returns {unlinkedAuth, email} (stored canonical form). The authId-NOT-NULL precondition is deliberately lifted: a linked account can now have its login email corrected, which is the row. No userAuditLogLegacy write - the usersArchive row is the audit record. PTD (Class-A, coder-run): remote package.json on main read at check time = 2.24.0; live /api/health = 2.24.0, dbOk true, deploymentId dpl_EKcbghmeP8GwuzgfSgFJmWjcJ1bb; vercel inspect on that id = target production, state Ready, 40s build, commit 65951c0, aliases include evolutiva.pensanta.com. Build log: Compiled successfully, 3 Turbopack warnings IDENTICAL to the prior deploy 28fclhziz (src/lib/email.ts:145 instrumentation chain) - pre-existing, not introduced. Runtime log for dpl_EKcb: all 200, zero error-tier; three warn [slow-query] 281-309ms on the first /mi-cursada after a cold deploy, the known cold-connection-establishment class, benign, no row opened. Vercel MCP returned 403 on list_deployments and 404 on get_deployment for an id that the CLI resolves - connector scope, read via the CLI instead; no app-side change. COVERAGE LIMIT, carried from audit verbatim: this changes APP authorization, NOT Supabase Auth-session/refresh-token validity - do not describe it as Auth revocation. UNEXERCISED in prod: the live autoLinkExactEmailAtLogin re-bind by the new mailbox owner, and old-session app denial. The 11 tests assert ISSUED SQL on a query-capturing client, not live behaviour. Acceptable only because Pluto has zero supabase.from/.rpc/.storage call sites; adding one makes deleting the old auth.users row load-bearing. FORCED-RED, 6 mutants, each red on a distinct subset: googleSub clause dropped; allowlist -> if(false); dedup -> if(false); UPDATE bound to raw input instead of canonical; no-op gate -> if(false) (reds both no-op tests); stored side left un-canonicalized (reds only the legacy-row test). Suite 1110/1110, tsc clean, eslint clean.
-
completed
task
2026-06-23 by wi-cli-venus
1w ago
2026-10-06 13:36