fix(federation): close re-attach final-review findings — client/server domain normalization, merge attachment repoint, empty-domain guard, test hardening

packages/server/src/routes/auth.attachProof.test.ts

@@ -112,6 +112,19 @@ describe('POST /api/auth/attach-proof', () => {
     expect(res.statusCode).toBe(400);
   });
 
+  it('rejects a targetDomain that normalizes to empty and inserts no row', async () => {
+    // "https://" passes the pre-normalization length check but collapses to ""
+    // after protocol/slash stripping — must 400, never persist an inert row.
+    const res = await app.inject({
+      method: 'POST',
+      url: '/api/auth/attach-proof',
+      headers: { authorization: `Bearer ${signJwt({ userId: 'native-1', username: 'youruser' })}` },
+      payload: { targetDomain: 'https://' },
+    });
+    expect(res.statusCode).toBe(400);
+    expect(testDb.select().from(schema.federationAttachProofs).all()).toHaveLength(0);
+  });
+
   it('rejects unauthenticated requests', async () => {
     const res = await app.inject({ method: 'POST', url: '/api/auth/attach-proof', payload: { targetDomain: 'nova.ddns.net' } });
     expect(res.statusCode).toBe(401);

packages/server/src/routes/auth.ts

@@ -504,6 +504,12 @@ export async function authRoutes(app: FastifyInstance): Promise<void> {
       return reply.code(400).send({ error: 'targetDomain is required (string)', statusCode: 400 });
     }
     const targetDomain = rawTarget.trim().toLowerCase().replace(/^https?:\/\//, '').replace(/\/+$/, '');
+    // Re-check emptiness AFTER normalization: inputs like "https://" or "/"
+    // pass the pre-normalization guard but collapse to "" — never persist an
+    // inert target_domain='' proof row.
+    if (targetDomain.length === 0) {
+      return reply.code(400).send({ error: 'targetDomain is required (string)', statusCode: 400 });
+    }
 
     // Native accounts only — a federated/replicated account has no authority
     // to mint proofs for this domain's identities.

packages/server/src/routes/federation.reattach.test.ts

@@ -226,6 +226,13 @@ describe('POST /api/users/@me/reattach — stub merge', () => {
     testDb.insert(schema.dmMessages).values({
       id: 'm-stub', dmChannelId: 'ch-1', userId: 'stub-new', content: 'from new incarnation', createdAt: 3,
     }).run();
+    // An attachment the stub uploaded onto its DM message — uploader_id is a
+    // plain text column (no FK), so it must be repointed explicitly or attribution
+    // dangles at the deleted stub's id.
+    testDb.insert(schema.attachments).values({
+      id: 'att-stub', dmMessageId: 'm-stub', uploaderId: 'stub-new',
+      filename: 'f.webp', originalName: 'f.webp', mimetype: 'image/webp', size: 100, createdAt: 3,
+    }).run();
     testDb.insert(schema.friends).values([
       { userId: 'alice', friendId: 'detached-1', createdAt: 1 },
       { userId: 'alice', friendId: 'stub-new', createdAt: 2 },
@@ -240,6 +247,9 @@ describe('POST /api/users/@me/reattach — stub merge', () => {
     // Message repointed.
     const msg = testDb.select().from(schema.dmMessages).where(eq(schema.dmMessages.id, 'm-stub')).get()!;
     expect(msg.userId).toBe('detached-1');
+    // Attachment attribution repointed off the deleted stub.
+    const att = testDb.select().from(schema.attachments).where(eq(schema.attachments.id, 'att-stub')).get()!;
+    expect(att.uploaderId).toBe('detached-1');
     // Membership deduped (detached row already a member).
     const members = testDb.select().from(schema.dmMembers).all().filter(m => m.dmChannelId === 'ch-1');
     expect(members.map(m => m.userId).sort()).toEqual(['alice', 'detached-1']);

packages/server/src/routes/federation.ts

@@ -2959,6 +2959,11 @@ export async function federationRoutes(app: FastifyInstance): Promise<void> {
         // dm_messages / messages (RESTRICT FK, no unique on user_id → straight repoint).
         rawDb.prepare(`UPDATE dm_messages SET user_id = ? WHERE user_id = ?`).run(targetId, stubId);
         rawDb.prepare(`UPDATE messages SET user_id = ? WHERE user_id = ?`).run(targetId, stubId);
+        // attachments.uploader_id (plain text column, NO FK, no unique → straight
+        // repoint). A replicated stub that uploaded a DM/channel attachment would
+        // otherwise leave uploader_id dangling at the deleted stub's id — broken
+        // attribution.
+        rawDb.prepare(`UPDATE attachments SET uploader_id = ? WHERE uploader_id = ?`).run(targetId, stubId);
         // dm_reactions (dedupe on dm_message_id+emoji per user).
         rawDb.prepare(`DELETE FROM dm_reactions WHERE user_id = ? AND EXISTS (SELECT 1 FROM dm_reactions r2 WHERE r2.user_id = ? AND r2.dm_message_id = dm_reactions.dm_message_id AND r2.emoji = dm_reactions.emoji)`).run(stubId, targetId);
         rawDb.prepare(`UPDATE dm_reactions SET user_id = ? WHERE user_id = ?`).run(targetId, stubId);

packages/server/src/utils/federationAttach.test.ts

@@ -37,6 +37,30 @@ describe('verifyAttachProofWithPeer', () => {
     expect(await verifyAttachProofWithPeer(PEER, 'a'.repeat(64))).toEqual({ valid: false });
   });
 
+  it('treats a PRESENT-but-INVALID signature as valid:false (signature must verify against the peer secret)', async () => {
+    // A 200 response with a well-formed signature header that was computed with
+    // the WRONG secret — it must NOT verify against the peer's real secret. This
+    // hardens the core "never trust unauthenticated bodies" gate: a malicious or
+    // misconfigured peer that returns valid:true with a bogus signature is rejected.
+    const wrongSignedResponse = (bodyObj: object): Response => {
+      const body = JSON.stringify(bodyObj);
+      const ts = Date.now();
+      const nonce = 'resp-nonce';
+      const sig = signRequest(body, 'c'.repeat(64) /* wrong secret */, ts, nonce);
+      return new Response(body, {
+        status: 200,
+        headers: {
+          'x-federation-signature': `sha256=${sig}`,
+          'x-federation-timestamp': String(ts),
+          'x-federation-nonce': nonce,
+        },
+      });
+    };
+    vi.stubGlobal('fetch', vi.fn(async () => wrongSignedResponse({ valid: true, homeUserId: 'h1', username: 'youruser' })));
+    const { verifyAttachProofWithPeer } = await import('./federationAttach.js');
+    expect(await verifyAttachProofWithPeer(PEER, 'a'.repeat(64))).toEqual({ valid: false });
+  });
+
   it('network error → valid:false', async () => {
     vi.stubGlobal('fetch', vi.fn(async () => { throw new Error('ECONNREFUSED'); }));
     const { verifyAttachProofWithPeer } = await import('./federationAttach.js');
@@ -56,9 +80,15 @@ describe('fetchHomeProfileByHomeId', () => {
     expect(result?.profile.avatar).toBe('a.webp');
   });
 
-  it('found:false or network error → null', async () => {
+  it('found:false → null', async () => {
     vi.stubGlobal('fetch', vi.fn(async () => new Response(JSON.stringify({ found: false }), { status: 200 })));
     const { fetchHomeProfileByHomeId } = await import('./federationAttach.js');
     expect(await fetchHomeProfileByHomeId(PEER, 'h1')).toBeNull();
   });
+
+  it('network error → null', async () => {
+    vi.stubGlobal('fetch', vi.fn(async () => { throw new Error('ECONNREFUSED'); }));
+    const { fetchHomeProfileByHomeId } = await import('./federationAttach.js');
+    expect(await fetchHomeProfileByHomeId(PEER, 'h1')).toBeNull();
+  });
 });