fix(federation): address code review findings for FED-011

- Fix race window: store pendingHmacSecret AFTER remote peer confirms,
  not before (admin endpoint + auto-rotation worker)
- Add hex validation on newSecret at /peer/rotate endpoint
- Use pending-secret-aware signing in initial sync worker
- Add test for corrupt state (pendingHmacSecret set, secretRotationAt null)

packages/server/src/routes/federation.ts

@@ -416,7 +416,7 @@ export async function federationRoutes(app: FastifyInstance): Promise<void> {
 
       // 2. Validate request body
       const { newSecret } = request.body ?? {};
-      if (!newSecret || typeof newSecret !== 'string' || newSecret.length !== 64) {
+      if (!newSecret || typeof newSecret !== 'string' || newSecret.length !== 64 || !/^[0-9a-f]+$/.test(newSecret)) {
         return reply.code(400).send({ error: 'newSecret must be a 64-character hex string', statusCode: 400 });
       }
 
@@ -534,15 +534,6 @@ export async function federationRoutes(app: FastifyInstance): Promise<void> {
         });
       }
 
-      // Store pending secret locally BEFORE posting to peer (per audit recommendation)
-      db.update(schema.federationPeers)
-        .set({
-          pendingHmacSecret: newSecret,
-          secretRotationAt: Date.now(),
-        })
-        .where(eq(schema.federationPeers.id, peer.id))
-        .run();
-
       // Send rotation request to peer, signed with the CURRENT (old) secret
       try {
         const rotateBody = JSON.stringify({ newSecret });
@@ -556,12 +547,6 @@ export async function federationRoutes(app: FastifyInstance): Promise<void> {
         });
 
         if (!response.ok) {
-          // Rollback — clear pending secret
-          db.update(schema.federationPeers)
-            .set({ pendingHmacSecret: null, secretRotationAt: null })
-            .where(eq(schema.federationPeers.id, peer.id))
-            .run();
-
           let errorMessage = `Remote instance rejected rotation (HTTP ${response.status})`;
           try {
             const body = await response.json() as { error?: string };
@@ -571,16 +556,19 @@ export async function federationRoutes(app: FastifyInstance): Promise<void> {
           return reply.code(502).send({ error: errorMessage, statusCode: 502 });
         }
 
+        // Store pending secret locally AFTER remote peer confirms acceptance
+        db.update(schema.federationPeers)
+          .set({
+            pendingHmacSecret: newSecret,
+            secretRotationAt: Date.now(),
+          })
+          .where(eq(schema.federationPeers.id, peer.id))
+          .run();
+
         console.log(`[federation] Secret rotation initiated with peer ${peer.origin}`);
 
         return reply.code(200).send({ success: true, gracePeriodMs: 900_000 });
       } catch (err: unknown) {
-        // Rollback — clear pending secret
-        db.update(schema.federationPeers)
-          .set({ pendingHmacSecret: null, secretRotationAt: null })
-          .where(eq(schema.federationPeers.id, peer.id))
-          .run();
-
         const message = err instanceof Error ? err.message : 'Unknown error';
         if (err instanceof DOMException && err.name === 'TimeoutError') {
           return reply.code(504).send({

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

@@ -67,4 +67,14 @@ describe('verifyPeerSignature', () => {
     const sig = signRequest(body, primarySecret, timestamp, null);
     expect(verifyPeerSignature(body, sig, timestamp, null, makePeer())).toBe(true);
   });
+
+  it('does not try pending secret when secretRotationAt is null', () => {
+    const timestamp = Date.now();
+    const sig = signRequest(body, pendingSecret, timestamp, nonce);
+    const peer = makePeer({
+      pendingHmacSecret: pendingSecret,
+      secretRotationAt: null,
+    });
+    expect(verifyPeerSignature(body, sig, timestamp, nonce, peer)).toBe(false);
+  });
 });

packages/server/src/utils/federationWorker.ts

@@ -733,15 +733,6 @@ async function processHealthCheckTick(): Promise<void> {
     // Time to rotate
     const newSecret = generateHmacSecret();
 
-    // Store pending locally first
-    db.update(schema.federationPeers)
-      .set({
-        pendingHmacSecret: newSecret,
-        secretRotationAt: Date.now(),
-      })
-      .where(eq(schema.federationPeers.id, peer.id))
-      .run();
-
     try {
       const rotateBody = JSON.stringify({ newSecret });
       const headers = buildFederationHeaders(rotateBody, peer.hmacSecret, ourOrigin);
@@ -754,21 +745,19 @@ async function processHealthCheckTick(): Promise<void> {
       });
 
       if (response.ok) {
-        console.log(`[federation-worker] Auto-rotation initiated with peer ${peer.origin}`);
-      } else {
-        // Rollback
+        // Store pending locally AFTER remote peer confirms acceptance
         db.update(schema.federationPeers)
-          .set({ pendingHmacSecret: null, secretRotationAt: null })
+          .set({
+            pendingHmacSecret: newSecret,
+            secretRotationAt: Date.now(),
+          })
           .where(eq(schema.federationPeers.id, peer.id))
           .run();
+        console.log(`[federation-worker] Auto-rotation initiated with peer ${peer.origin}`);
+      } else {
         console.warn(`[federation-worker] Auto-rotation rejected by peer ${peer.origin} (HTTP ${response.status})`);
       }
     } catch (err) {
-      // Rollback
-      db.update(schema.federationPeers)
-        .set({ pendingHmacSecret: null, secretRotationAt: null })
-        .where(eq(schema.federationPeers.id, peer.id))
-        .run();
       const message = err instanceof Error ? err.message : 'Unknown error';
       console.warn(`[federation-worker] Auto-rotation failed for peer ${peer.origin}: ${message}`);
     }
@@ -854,6 +843,9 @@ async function runInitialSyncForNewPeers(): Promise<void> {
   const ourOrigin = getOurOrigin();
 
   for (const peer of unsyncedPeers) {
+    const signingSecret = (peer.pendingHmacSecret && peer.secretRotationAt)
+      ? peer.pendingHmacSecret
+      : peer.hmacSecret;
     try {
       console.log(`[federation-worker] Running initial sync with ${peer.origin}...`);
       let sinceTimestamp = 0;
@@ -862,7 +854,7 @@ async function runInitialSyncForNewPeers(): Promise<void> {
       // Paginate through all events from the peer
       while (true) {
         const body = JSON.stringify({ sinceTimestamp, limit: 100 });
-        const headers = buildFederationHeaders(body, peer.hmacSecret, ourOrigin);
+        const headers = buildFederationHeaders(body, signingSecret, ourOrigin);
 
         const response = await fetch(`${peer.origin}/api/federation/sync`, {
           method: 'POST',
@@ -893,7 +885,7 @@ async function runInitialSyncForNewPeers(): Promise<void> {
       let friendSinceTimestamp = 0;
       while (true) {
         const friendBody = JSON.stringify({ sinceTimestamp: friendSinceTimestamp, contextType: 'friend', limit: 100 });
-        const friendHeaders = buildFederationHeaders(friendBody, peer.hmacSecret, ourOrigin);
+        const friendHeaders = buildFederationHeaders(friendBody, signingSecret, ourOrigin);
 
         const friendResponse = await fetch(`${peer.origin}/api/federation/sync`, {
           method: 'POST',