fix(users): store avatars as bytea in DB + login hydration & badge-403 polish

apps/api/migrations/2026-06-11-j-avatar-bytea-columns.sql

@@ -0,0 +1,37 @@
+-- Move user avatars from the filesystem (/data/avatars/<id>.<ext>) into the
+-- database as a bytea blob on the users row. The old filesystem path depended
+-- on a writable `api_data` volume owned by the API's runtime uid; when that
+-- volume was root-owned the upload failed with EACCES and a 500. Storing the
+-- bytes in Postgres removes the volume dependency and works across replicas.
+--
+-- Columns:
+--   avatar_data       bytea  — the raw image bytes (PNG/JPEG/WebP, ≤ 5 MB).
+--                              Postgres TOASTs values over ~2 KB out-of-line,
+--                              so the base row stays lean; size-only reads use
+--                              octet_length() to avoid pulling the blob.
+--   avatar_mime       text   — sniffed content type (image/png|jpeg|webp).
+--   avatar_updated_at timestamptz — bump on each write; powers a cheap weak
+--                              ETag (size + mtime) without hashing the bytes.
+
+ALTER TABLE users ADD COLUMN IF NOT EXISTS avatar_data bytea;
+ALTER TABLE users ADD COLUMN IF NOT EXISTS avatar_mime text;
+ALTER TABLE users ADD COLUMN IF NOT EXISTS avatar_updated_at timestamptz;
+
+-- A pre-existing INTERNAL avatar_url ('/api/v1/users/<id>/avatar') pointed at
+-- a filesystem-backed avatar that this migration does NOT carry over (the
+-- bytes stay on the old volume). Clear those now-dangling URLs so the UI falls
+-- back to initials instead of a broken image; affected users simply re-upload.
+-- External URLs (the pre-upload era let users set arbitrary avatar URLs) are
+-- deliberately left alone — they still resolve.
+-- Always log the count (even 0) so the run leaves a forensic trail.
+DO $$
+DECLARE
+  n integer;
+BEGIN
+  UPDATE users
+     SET avatar_url = NULL
+   WHERE avatar_url LIKE '/api/v1/users/%/avatar'
+     AND avatar_data IS NULL;
+  GET DIAGNOSTICS n = ROW_COUNT;
+  RAISE WARNING 'cleared % dangling filesystem avatar_url(s) during bytea migration', n;
+END $$;

apps/api/src/__tests__/integration/avatar-bytea-roundtrip.integration.test.ts

@@ -0,0 +1,125 @@
+/**
+ * Avatar bytea storage — real-driver round-trip + RLS enforcement.
+ *
+ * Migration under test: 2026-06-11-j-avatar-bytea-columns.sql
+ * Service under test:   services/avatarStorage.ts (DB-backed rewrite)
+ *
+ * The route tests in routes/users.test.ts mock the avatarStorage I/O behind an
+ * in-memory Map, so nothing there proves that:
+ *   1. postgres.js round-trips `bytea` byte-exactly as a Node Buffer (the
+ *      schema's customType assumes Buffer in/out), and
+ *   2. the UPDATE/SELECT on one's own `users` row actually passes the table's
+ *      RLS policies as the unprivileged breeze_app role — the exact failure
+ *      mode custom_field_definitions hit (#1257): policies looked fine in
+ *      metadata, real writes 42501'd.
+ * These tests run through the REAL driver inside withDbAccessContext.
+ */
+import './setup';
+import { describe, expect, it } from 'vitest';
+import { eq } from 'drizzle-orm';
+import { db, withDbAccessContext, type DbAccessContext } from '../../db';
+import { users } from '../../db/schema';
+import { createPartner, createUser } from './db-utils';
+import { deleteAvatar, readAvatarBuffer, statAvatar, writeAvatar } from '../../services/avatarStorage';
+
+// Valid PNG magic + every byte value 0..255 — catches any encoding/escaping
+// mangling in the bytea path (e.g. the text::bytea class of bug from #994).
+const PNG_BYTES = Buffer.concat([
+  Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]),
+  Buffer.from(Array.from({ length: 256 }, (_, i) => i)),
+]);
+
+const JPEG_BYTES = Buffer.concat([
+  Buffer.from([0xff, 0xd8, 0xff, 0xe0]),
+  Buffer.alloc(64, 0x42),
+]);
+
+/** Request-shaped context for an MSP-staff user acting as themselves. */
+function selfContext(partnerId: string, userId: string): DbAccessContext {
+  return {
+    scope: 'partner',
+    orgId: null,
+    accessibleOrgIds: [],
+    accessiblePartnerIds: [partnerId],
+    userId,
+  };
+}
+
+describe('avatar bytea round-trip (real driver, RLS enforced)', () => {
+  it('write → stat → read → delete round-trips byte-exactly in the user\'s own context', async () => {
+    const partner = await createPartner();
+    const user = await createUser({ partnerId: partner.id });
+    const ctx = selfContext(partner.id, user.id);
+
+    await withDbAccessContext(ctx, async () => {
+      const written = await writeAvatar(user.id, 'image/png', PNG_BYTES);
+      expect(written).not.toBeNull();
+      expect(written!.avatarUrl).toBe(`/api/v1/users/${user.id}/avatar`);
+      expect(written!.size).toBe(PNG_BYTES.length);
+
+      // statAvatar computes size via octet_length — must match without
+      // transferring the blob.
+      const stat = await statAvatar(user.id);
+      expect(stat).not.toBeNull();
+      expect(stat!.mime).toBe('image/png');
+      expect(stat!.size).toBe(PNG_BYTES.length);
+      expect(stat!.mtimeMs).toBeGreaterThan(0);
+
+      const opened = await readAvatarBuffer(user.id);
+      expect(opened).not.toBeNull();
+      expect(Buffer.isBuffer(opened!.buffer)).toBe(true);
+      expect(opened!.buffer.equals(PNG_BYTES)).toBe(true);
+      expect(opened!.mime).toBe('image/png');
+
+      // avatar_url landed on the row in the same UPDATE.
+      const [row] = await db.select({ avatarUrl: users.avatarUrl }).from(users).where(eq(users.id, user.id)).limit(1);
+      expect(row?.avatarUrl).toBe(`/api/v1/users/${user.id}/avatar`);
+
+      expect(await deleteAvatar(user.id)).toBe(true);
+      expect(await statAvatar(user.id)).toBeNull();
+      const [after] = await db.select({ avatarUrl: users.avatarUrl }).from(users).where(eq(users.id, user.id)).limit(1);
+      expect(after?.avatarUrl).toBeNull();
+    });
+  });
+
+  it('overwriting replaces bytes and mime in place', async () => {
+    const partner = await createPartner();
+    const user = await createUser({ partnerId: partner.id });
+    const ctx = selfContext(partner.id, user.id);
+
+    await withDbAccessContext(ctx, async () => {
+      await writeAvatar(user.id, 'image/png', PNG_BYTES);
+      await writeAvatar(user.id, 'image/jpeg', JPEG_BYTES);
+
+      const opened = await readAvatarBuffer(user.id);
+      expect(opened!.mime).toBe('image/jpeg');
+      expect(opened!.buffer.equals(JPEG_BYTES)).toBe(true);
+    });
+  });
+
+  it('RLS fails closed: a foreign-partner context sees no avatar and cannot write one', async () => {
+    const partnerA = await createPartner();
+    const partnerB = await createPartner();
+    const userA = await createUser({ partnerId: partnerA.id });
+    const userB = await createUser({ partnerId: partnerB.id });
+
+    await withDbAccessContext(selfContext(partnerA.id, userA.id), async () => {
+      await writeAvatar(userA.id, 'image/png', PNG_BYTES);
+    });
+
+    // userB (different partner) can neither see userA's avatar nor write over
+    // it — the UPDATE matches no visible row and writeAvatar reports null.
+    await withDbAccessContext(selfContext(partnerB.id, userB.id), async () => {
+      expect(await statAvatar(userA.id)).toBeNull();
+      expect(await readAvatarBuffer(userA.id)).toBeNull();
+      expect(await writeAvatar(userA.id, 'image/jpeg', JPEG_BYTES)).toBeNull();
+    });
+
+    // userA's avatar is untouched.
+    await withDbAccessContext(selfContext(partnerA.id, userA.id), async () => {
+      const opened = await readAvatarBuffer(userA.id);
+      expect(opened!.mime).toBe('image/png');
+      expect(opened!.buffer.equals(PNG_BYTES)).toBe(true);
+    });
+  });
+});

apps/api/src/db/schema/users.ts

@@ -1,6 +1,17 @@
-import { pgTable, uuid, varchar, text, timestamp, boolean, jsonb, pgEnum } from 'drizzle-orm/pg-core';
+import { pgTable, uuid, varchar, text, timestamp, boolean, jsonb, pgEnum, customType } from 'drizzle-orm/pg-core';
 import { partners, organizations } from './orgs';
 
+// Postgres `bytea` mapped to a Node Buffer. postgres.js returns bytea columns
+// as Buffers and accepts Buffers/Uint8Arrays on write, so this is a thin
+// pass-through. Postgres TOASTs values over ~2 KB out-of-line, so wide blobs
+// don't bloat the base row — but SELECTs naming the column still pay the full
+// read; size-only checks should use octet_length() instead.
+export const bytea = customType<{ data: Buffer; driverData: Buffer }>({
+  dataType() {
+    return 'bytea';
+  },
+});
+
 export const userStatusEnum = pgEnum('user_status', ['active', 'invited', 'disabled']);
 export const roleScopeEnum = pgEnum('role_scope', ['system', 'partner', 'organization']);
 export const orgAccessEnum = pgEnum('org_access', ['all', 'selected', 'none']);
@@ -30,7 +41,16 @@ export const users = pgTable('users', {
   // other reason (compromise, off-boarding, manual admin action) and unsuspend
   // must leave them alone. See #917 (L-5).
   disabledReason: text('disabled_reason'),
+  // avatarUrl holds the internal serving URL (`/api/v1/users/<id>/avatar`) when
+  // an avatar exists, NULL otherwise. The bytes live in avatarData (bytea) on
+  // this same row — stored in the DB rather than a filesystem volume so uploads
+  // work across replicas and don't depend on volume permissions (#1059).
   avatarUrl: text('avatar_url'),
+  avatarData: bytea('avatar_data'),
+  avatarMime: text('avatar_mime'),
+  // timestamptz in the migration — withTimezone must match or db:check-drift
+  // flags it (the sibling users timestamps are legacy timestamp-without-tz).
+  avatarUpdatedAt: timestamp('avatar_updated_at', { withTimezone: true }),
   lastLoginAt: timestamp('last_login_at'),
   passwordChangedAt: timestamp('password_changed_at'),
   setupCompletedAt: timestamp('setup_completed_at'),

apps/api/src/routes/auth/login.test.ts

@@ -195,4 +195,38 @@ describe('POST /login — IP allowlist', () => {
     expect(await res.json()).toMatchObject({ error: 'Invalid email or password' });
     expect(createTokenPair).not.toHaveBeenCalled();
   });
+
+  // The web auth store is seeded from THIS payload on password login; the
+  // sidebar gates platform-admin-only nav (deletion requests) on the flag.
+  // If it ever drops out of the payload, platform admins silently lose that
+  // nav (the /users/me copy only reaches the store on a later refresh).
+  it('includes isPlatformAdmin in the success payload', async () => {
+    vi.mocked(db.select).mockReturnValue(selectChain([{
+      id: 'user-1',
+      email: 'admin@msp.com',
+      name: 'Admin User',
+      passwordHash: 'password-hash',
+      status: 'active',
+      mfaEnabled: false,
+      mfaSecret: null,
+      mfaMethod: null,
+      phoneNumber: null,
+      avatarUrl: null,
+      isPlatformAdmin: true,
+    }]) as any);
+
+    const res = await postLogin({ email: 'admin@msp.com', password: 'correct-horse' });
+
+    expect(res.status).toBe(200);
+    const body = await res.json() as { user: { isPlatformAdmin?: boolean } };
+    expect(body.user.isPlatformAdmin).toBe(true);
+  });
+
+  it('coerces a missing isPlatformAdmin to false in the success payload', async () => {
+    const res = await postLogin({ email: 'admin@msp.com', password: 'correct-horse' });
+
+    expect(res.status).toBe(200);
+    const body = await res.json() as { user: { isPlatformAdmin?: boolean } };
+    expect(body.user.isPlatformAdmin).toBe(false);
+  });
 });

apps/api/src/routes/auth/login.ts

@@ -505,7 +505,11 @@ loginRoutes.post('/login', cfAccessLoginMiddleware, zValidator('json', loginSche
       email: user.email,
       name: user.name,
       mfaEnabled: ENABLE_2FA ? user.mfaEnabled : false,
-      avatarUrl: user.avatarUrl
+      avatarUrl: user.avatarUrl,
+      // The web sidebar gates platform-admin-only nav (and its badge fetch) on
+      // this flag from the auth store, which is seeded from THIS payload on
+      // password login — omit it and platform admins lose that nav entirely.
+      isPlatformAdmin: user.isPlatformAdmin === true
     },
     tokens: toPublicTokens(tokens),
     mfaRequired: false,

apps/api/src/routes/auth/mfa.ts

@@ -252,7 +252,12 @@ mfaRoutes.post('/mfa/verify', zValidator('json', mfaVerifySchema), async (c) =>
         id: user.id,
         email: user.email,
         name: user.name,
-        mfaEnabled: true
+        mfaEnabled: true,
+        avatarUrl: user.avatarUrl,
+        // Mirrors the password-login payload — the auth store is seeded from
+        // whichever of the two completes the login, and the sidebar gates
+        // platform-admin-only nav on this flag.
+        isPlatformAdmin: user.isPlatformAdmin === true
       },
       tokens: toPublicTokens(tokens),
       mfaRequired: false,