fix: sanitize user data (#3552)

extensions/whoami.test.ts

@@ -206,6 +206,84 @@ describe('whoami extension — handleWhoami', () => {
         expect(JSON.stringify(body)).not.toContain('bootstrap-secret');
     });
 
+    it('never exposes phone, card fingerprint or signup identity', async () => {
+        const user = await seedUser();
+        await server.stores.user.update(user.id as number, {
+            phone: '+15551234567',
+            card_fingerprint: 'fp_ABC123',
+            requires_phone_verification: false,
+            requires_card_verification: false,
+        });
+        // Guard against a vacuous assertion below: the columns really do hold
+        // the values we then expect never to see on the wire.
+        const stored = await server.stores.user.getById(user.id as number, {
+            cached: false,
+            force: true,
+        });
+        expect(stored?.phone).toBe('+15551234567');
+        expect(stored?.card_fingerprint).toBe('fp_ABC123');
+
+        for (const actor of [
+            { user: { uuid: user.uuid, id: user.id as number } },
+            {
+                user: { uuid: user.uuid, id: user.id as number },
+                app: { uid: 'app-test-actor' },
+            },
+        ]) {
+            const { res, captured } = makeRes();
+            await runWithContext({ actor }, () =>
+                handleWhoami(makeReq(), res),
+            );
+
+            const body = captured.body as Record<string, unknown>;
+            expect(body.phone).toBeUndefined();
+            expect(body.card_fingerprint).toBeUndefined();
+            expect(body.password).toBeUndefined();
+            expect(body.otp_secret).toBeUndefined();
+            expect(body.signup_ip).toBeUndefined();
+            // Not just absent as a key — the values must not appear anywhere
+            // in the payload (nested under metadata, taskbar items, …).
+            const serialized = JSON.stringify(body);
+            expect(serialized).not.toContain('5551234567');
+            expect(serialized).not.toContain('fp_ABC123');
+            // The verification flags, which the GUI acts on, still ship.
+            expect(body).toHaveProperty('requires_phone_verification');
+            expect(body).toHaveProperty('requires_card_verification');
+        }
+    });
+
+    it('scrubs sensitive keys added to metadata, and leaves the cached row intact', async () => {
+        const user = await seedUser();
+        await server.stores.user.updateMetadata(user.id as number, {
+            tmp_password: 'bootstrap-secret',
+            billing: { card_fingerprint: 'fp_NESTED', tier: 'pro' },
+        });
+
+        const { res, captured } = makeRes();
+        await runWithContext(
+            { actor: { user: { uuid: user.uuid, id: user.id as number } } },
+            () => handleWhoami(makeReq(), res),
+        );
+
+        const body = captured.body as Record<string, unknown>;
+        const metadata = body.metadata as Record<string, unknown>;
+        expect(metadata.tmp_password).toBeUndefined();
+        // Nested sensitive keys are removed too; siblings survive.
+        expect(metadata.billing).toEqual({ tier: 'pro' });
+        expect(JSON.stringify(body)).not.toContain('fp_NESTED');
+
+        // The scrub works on a copy — server-side state is untouched.
+        const fresh = await server.stores.user.getById(user.id as number, {
+            cached: false,
+            force: true,
+        });
+        expect(fresh?.metadata?.tmp_password).toBe('bootstrap-secret');
+        expect(
+            (fresh?.metadata?.billing as Record<string, unknown>)
+                ?.card_fingerprint,
+        ).toBe('fp_NESTED');
+    });
+
     it('marks the user as oidc_only when password is null', async () => {
         const slug = Math.random().toString(36).slice(2, 8);
         const oidcUser = await server.stores.user.create({

extensions/whoami.ts

@@ -33,6 +33,68 @@ const CLIENT_VISIBLE_FEATURE_FLAGS: ReadonlySet<string> = new Set([
     'prompt_user_when_navigation_away_from_puter',
 ]);
 
+// Keys that must never leave the server, whoever put them on the response.
+// `details` is an explicit pick, but the `whoami.details` event hands
+// listeners the full UserRow next to the object they may write to, and
+// `metadata` is a free-form blob — so any of these can arrive on the
+// response without an edit to the pick above. The scrub runs last, over the
+// whole payload, and is the one place that decides what "sensitive" means.
+//
+// Credentials and single-use tokens, the payment/phone identifiers used for
+// verification (`card_fingerprint` is the Stripe fingerprint, stable per
+// card number), the network identity recorded at signup, and internal
+// anti-abuse bookkeeping. `requires_phone_verification` /
+// `requires_card_verification` stay: they are the flags the GUI acts on, and
+// they carry no identifier.
+const SENSITIVE_KEYS: ReadonlySet<string> = new Set([
+    'password',
+    'tmp_password',
+    'pass_recovery_token',
+    'email_confirm_code',
+    'email_confirm_token',
+    'change_email_confirm_token',
+    'otp_secret',
+    'otp_recovery_codes',
+    'card_fingerprint',
+    'phone',
+    'clean_email',
+    'signup_ip',
+    'signup_ip_forwarded',
+    'signup_user_agent',
+    'signup_origin',
+    'signup_server',
+    'audit_metadata',
+]);
+
+// Depth-limited, cycle-safe walk deleting every SENSITIVE_KEYS entry it finds
+// at any level (`metadata` and `taskbar_items` are both nested structures).
+const scrubSensitive = (
+    value: unknown,
+    seen: Set<object> = new Set(),
+    depth = 0,
+): void => {
+    if (depth > 8 || value === null || typeof value !== 'object') return;
+    if (seen.has(value as object)) return;
+    seen.add(value as object);
+
+    if (Array.isArray(value)) {
+        for (const entry of value) scrubSensitive(entry, seen, depth + 1);
+        return;
+    }
+
+    for (const key of Object.keys(value as Record<string, unknown>)) {
+        if (SENSITIVE_KEYS.has(key)) {
+            delete (value as Record<string, unknown>)[key];
+            continue;
+        }
+        scrubSensitive(
+            (value as Record<string, unknown>)[key],
+            seen,
+            depth + 1,
+        );
+    }
+};
+
 export const handleWhoami = async (
     req: Request,
     res: Response,
@@ -74,8 +136,12 @@ export const handleWhoami = async (
         }
     }
 
-    const metadata = user.metadata ? { ...user.metadata } : user.metadata;
-    if (metadata) delete metadata.tmp_password;
+    // Deep-copied (it is decoded JSON) so the scrub below edits the response
+    // and not the cached UserRow. Sensitive keys inside it — tmp_password and
+    // anything else on the denylist — are removed by scrubSensitive.
+    const metadata = user.metadata
+        ? structuredClone(user.metadata)
+        : user.metadata;
 
     const details: Record<string, unknown> = {
         username: user.username,
@@ -84,7 +150,9 @@ export const handleWhoami = async (
         unconfirmed_email: user.email,
         email_confirmed: user.email_confirmed || user.username === 'admin',
         requires_email_confirmation: user.requires_email_confirmation,
-        phone: user.phone,
+        // The phone number itself is deliberately absent: nothing on the
+        // client reads it, and it is PII that would otherwise be handed to
+        // every app actor. Only the verification flag ships.
         requires_phone_verification: user.requires_phone_verification,
         requires_card_verification: user.requires_card_verification,
         desktop_bg_url: user.desktop_bg_url,
@@ -196,6 +264,9 @@ export const handleWhoami = async (
         delete subscription.offering.price_id;
     }
 
+    // Last word on what ships, after every listener has had its say.
+    scrubSensitive(details);
+
     res.json(details);
 };