fix(federation): use null fallback for audit log FK cols when token row missing

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

apps/gateway/src/__tests__/integration/federation-m2-e2e.integration.test.ts

@@ -0,0 +1,243 @@
+/**
+ * Federation M2 E2E test — peer-add enrollment flow (FED-M2-10).
+ *
+ * Covers MILESTONES.md acceptance test #6:
+ *   "`peer add <url>` on Server A yields an `active` peer record with a valid cert + key"
+ *
+ * This test simulates two gateways using a single bootstrapped NestJS app:
+ *   - "Server A": the admin API that generates a keypair and stores the cert
+ *   - "Server B": the enrollment endpoint that signs the CSR
+ *   Both share the same DB + Step-CA in the test environment.
+ *
+ * Prerequisites:
+ *   docker compose -f docker-compose.federated.yml --profile federated up -d
+ *
+ * Run:
+ *   FEDERATED_INTEGRATION=1 STEP_CA_AVAILABLE=1 \
+ *   STEP_CA_URL=https://localhost:9000 \
+ *   STEP_CA_PROVISIONER_KEY_JSON="$(docker exec $(docker ps -qf name=step-ca) cat /home/step/secrets/mosaic-fed.json)" \
+ *   STEP_CA_ROOT_CERT_PATH=/tmp/step-ca-root.crt \
+ *   pnpm --filter @mosaicstack/gateway test \
+ *     src/__tests__/integration/federation-m2-e2e.integration.test.ts
+ *
+ * Obtaining Step-CA credentials:
+ *   # Extract provisioner key from running container:
+ *   #   docker exec $(docker ps -qf name=step-ca) cat /home/step/secrets/mosaic-fed.json
+ *   # Copy root cert from container:
+ *   #   docker cp $(docker ps -qf name=step-ca):/home/step/certs/root_ca.crt /tmp/step-ca-root.crt
+ *   # Then: export STEP_CA_ROOT_CERT_PATH=/tmp/step-ca-root.crt
+ *
+ * Skipped unless both FEDERATED_INTEGRATION=1 and STEP_CA_AVAILABLE=1 are set.
+ */
+
+import * as crypto from 'node:crypto';
+import { afterAll, beforeAll, describe, expect, it } from 'vitest';
+import { Test } from '@nestjs/testing';
+import { ValidationPipe } from '@nestjs/common';
+import { FastifyAdapter, type NestFastifyApplication } from '@nestjs/platform-fastify';
+import supertest from 'supertest';
+import {
+  createDb,
+  type Db,
+  type DbHandle,
+  federationPeers,
+  federationGrants,
+  federationEnrollmentTokens,
+  inArray,
+  eq,
+} from '@mosaicstack/db';
+import * as schema from '@mosaicstack/db';
+import { DB } from '../../database/database.module.js';
+import { AdminGuard } from '../../admin/admin.guard.js';
+import { FederationModule } from '../../federation/federation.module.js';
+import { GrantsService } from '../../federation/grants.service.js';
+import { EnrollmentService } from '../../federation/enrollment.service.js';
+
+const run = process.env['FEDERATED_INTEGRATION'] === '1';
+const stepCaRun =
+  run &&
+  process.env['STEP_CA_AVAILABLE'] === '1' &&
+  !!process.env['STEP_CA_URL'] &&
+  !!process.env['STEP_CA_PROVISIONER_KEY_JSON'] &&
+  !!process.env['STEP_CA_ROOT_CERT_PATH'];
+
+const PG_URL = 'postgresql://mosaic:mosaic@localhost:5433/mosaic';
+
+const RUN_ID = crypto.randomUUID();
+
+describe.skipIf(!stepCaRun)('federation M2 E2E — peer add enrollment flow', () => {
+  let handle: DbHandle;
+  let db: Db;
+  let app: NestFastifyApplication;
+  let agent: ReturnType<typeof supertest>;
+  let grantsService: GrantsService;
+  let enrollmentService: EnrollmentService;
+
+  const createdTokenGrantIds: string[] = [];
+  const createdGrantIds: string[] = [];
+  const createdPeerIds: string[] = [];
+  const createdUserIds: string[] = [];
+
+  beforeAll(async () => {
+    process.env['BETTER_AUTH_SECRET'] ??= 'test-e2e-sealing-key';
+
+    handle = createDb(PG_URL);
+    db = handle.db;
+
+    const moduleRef = await Test.createTestingModule({
+      imports: [FederationModule],
+      providers: [{ provide: DB, useValue: db }],
+    })
+      .overrideGuard(AdminGuard)
+      .useValue({ canActivate: () => true })
+      .compile();
+
+    app = moduleRef.createNestApplication<NestFastifyApplication>(new FastifyAdapter());
+    app.useGlobalPipes(new ValidationPipe({ whitelist: true, transform: true }));
+    await app.init();
+    await app.getHttpAdapter().getInstance().ready();
+
+    agent = supertest(app.getHttpServer());
+
+    grantsService = moduleRef.get(GrantsService);
+    enrollmentService = moduleRef.get(EnrollmentService);
+  }, 30_000);
+
+  afterAll(async () => {
+    if (db && createdTokenGrantIds.length > 0) {
+      await db
+        .delete(federationEnrollmentTokens)
+        .where(inArray(federationEnrollmentTokens.grantId, createdTokenGrantIds))
+        .catch((e: unknown) => console.error('[federation-m2-e2e cleanup]', e));
+    }
+    if (db && createdGrantIds.length > 0) {
+      await db
+        .delete(federationGrants)
+        .where(inArray(federationGrants.id, createdGrantIds))
+        .catch((e: unknown) => console.error('[federation-m2-e2e cleanup]', e));
+    }
+    if (db && createdPeerIds.length > 0) {
+      await db
+        .delete(federationPeers)
+        .where(inArray(federationPeers.id, createdPeerIds))
+        .catch((e: unknown) => console.error('[federation-m2-e2e cleanup]', e));
+    }
+    if (db && createdUserIds.length > 0) {
+      await db
+        .delete(schema.users)
+        .where(inArray(schema.users.id, createdUserIds))
+        .catch((e: unknown) => console.error('[federation-m2-e2e cleanup]', e));
+    }
+    if (app)
+      await app.close().catch((e: unknown) => console.error('[federation-m2-e2e cleanup]', e));
+    if (handle)
+      await handle.close().catch((e: unknown) => console.error('[federation-m2-e2e cleanup]', e));
+  });
+
+  // -------------------------------------------------------------------------
+  // #6 — peer add: keypair → enrollment → cert storage → active peer record
+  // -------------------------------------------------------------------------
+  it('#6 — peer add flow: keypair → enrollment → cert storage → active peer record', async () => {
+    // Create a subject user to satisfy FK on federation_grants.subject_user_id
+    const userId = crypto.randomUUID();
+    await db
+      .insert(schema.users)
+      .values({
+        id: userId,
+        name: `e2e-user-${RUN_ID}`,
+        email: `e2e-${RUN_ID}@federation-test.invalid`,
+        emailVerified: false,
+      })
+      .onConflictDoNothing();
+    createdUserIds.push(userId);
+
+    // ── Step A: "Server B" setup ─────────────────────────────────────────
+    // Server B admin creates a grant and generates an enrollment token to
+    // share out-of-band with Server A's operator.
+
+    // Insert a placeholder peer on "Server B" to satisfy the grant FK
+    const serverBPeerId = crypto.randomUUID();
+    await db
+      .insert(federationPeers)
+      .values({
+        id: serverBPeerId,
+        commonName: `server-b-peer-${RUN_ID}`,
+        displayName: 'Server B Placeholder',
+        certPem: '-----BEGIN CERTIFICATE-----\nMOCK\n-----END CERTIFICATE-----\n',
+        certSerial: `serial-b-${serverBPeerId}`,
+        certNotAfter: new Date(Date.now() + 365 * 24 * 60 * 60 * 1000),
+        state: 'pending',
+      })
+      .onConflictDoNothing();
+    createdPeerIds.push(serverBPeerId);
+
+    const grant = await grantsService.createGrant({
+      subjectUserId: userId,
+      scope: { resources: ['tasks'], excluded_resources: [], max_rows_per_query: 100 },
+      peerId: serverBPeerId,
+    });
+    createdGrantIds.push(grant.id);
+    createdTokenGrantIds.push(grant.id);
+
+    const { token } = await enrollmentService.createToken({
+      grantId: grant.id,
+      peerId: serverBPeerId,
+      ttlSeconds: 900,
+    });
+
+    // ── Step B: "Server A" generates keypair ─────────────────────────────
+    const keypairRes = await agent
+      .post('/api/admin/federation/peers/keypair')
+      .send({
+        commonName: `e2e-peer-${RUN_ID.slice(0, 8)}`,
+        displayName: 'E2E Test Peer',
+        endpointUrl: 'https://test.invalid',
+      })
+      .set('Content-Type', 'application/json');
+
+    expect(keypairRes.status).toBe(201);
+    const { peerId, csrPem } = keypairRes.body as { peerId: string; csrPem: string };
+    expect(typeof peerId).toBe('string');
+    expect(csrPem).toContain('-----BEGIN CERTIFICATE REQUEST-----');
+    createdPeerIds.push(peerId);
+
+    // ── Step C: Enrollment (simulates Server A sending CSR to Server B) ──
+    const enrollRes = await agent
+      .post(`/api/federation/enrollment/${token}`)
+      .send({ csrPem })
+      .set('Content-Type', 'application/json');
+
+    expect(enrollRes.status).toBe(200);
+    const { certPem, certChainPem } = enrollRes.body as {
+      certPem: string;
+      certChainPem: string;
+    };
+    expect(certPem).toContain('-----BEGIN CERTIFICATE-----');
+    expect(certChainPem).toContain('-----BEGIN CERTIFICATE-----');
+
+    // ── Step D: "Server A" stores the cert ───────────────────────────────
+    const storeRes = await agent
+      .patch(`/api/admin/federation/peers/${peerId}/cert`)
+      .send({ certPem })
+      .set('Content-Type', 'application/json');
+
+    expect(storeRes.status).toBe(200);
+
+    // ── Step E: Verify peer record in DB ─────────────────────────────────
+    const [peer] = await db
+      .select()
+      .from(federationPeers)
+      .where(eq(federationPeers.id, peerId))
+      .limit(1);
+
+    expect(peer).toBeDefined();
+    expect(peer?.state).toBe('active');
+    expect(peer?.certPem).toContain('-----BEGIN CERTIFICATE-----');
+    expect(typeof peer?.certSerial).toBe('string');
+    expect((peer?.certSerial ?? '').length).toBeGreaterThan(0);
+    // clientKeyPem is a sealed ciphertext — must not be a raw PEM
+    expect(peer?.clientKeyPem?.startsWith('-----BEGIN')).toBe(false);
+    // certNotAfter must be in the future
+    expect(peer?.certNotAfter?.getTime()).toBeGreaterThan(Date.now());
+  }, 60_000);
+});

apps/gateway/src/federation/ca.service.ts

@@ -35,7 +35,7 @@ import * as crypto from 'node:crypto';
 import * as fs from 'node:fs';
 import * as https from 'node:https';
 import { SignJWT, importJWK } from 'jose';
-import { Pkcs10CertificateRequest } from '@peculiar/x509';
+import { Pkcs10CertificateRequest, X509Certificate } from '@peculiar/x509';
 import type { IssueCertRequestDto } from './ca.dto.js';
 import { IssuedCertDto } from './ca.dto.js';
 
@@ -624,6 +624,51 @@ export class CaService {
 
     const serialNumber = extractSerial(response.crt);
 
+    // CRIT-1: Verify the issued certificate contains both Mosaic OID extensions
+    // with the correct values. Step-CA's federation.tpl encodes each as an ASN.1
+    // UTF8String TLV: tag 0x0C + 1-byte length + UUID bytes. We skip 2 bytes
+    // (tag + length) to extract the raw UUID string.
+    const issuedCert = new X509Certificate(response.crt);
+    const decoder = new TextDecoder();
+
+    const grantIdExt = issuedCert.getExtension('1.3.6.1.4.1.99999.1');
+    if (!grantIdExt) {
+      throw new CaServiceError(
+        'Issued certificate is missing required Mosaic OID: mosaic_grant_id',
+        'The Step-CA federation.tpl template did not embed OID 1.3.6.1.4.1.99999.1. Check the provisioner template configuration.',
+        undefined,
+        'OID_MISSING',
+      );
+    }
+    const grantIdInCert = decoder.decode(grantIdExt.value.slice(2));
+    if (grantIdInCert !== req.grantId) {
+      throw new CaServiceError(
+        `Issued certificate mosaic_grant_id mismatch: expected ${req.grantId}, got ${grantIdInCert}`,
+        'The Step-CA issued a certificate with a different grant ID than requested. This may indicate a provisioner misconfiguration or a MITM.',
+        undefined,
+        'OID_MISMATCH',
+      );
+    }
+
+    const subjectUserIdExt = issuedCert.getExtension('1.3.6.1.4.1.99999.2');
+    if (!subjectUserIdExt) {
+      throw new CaServiceError(
+        'Issued certificate is missing required Mosaic OID: mosaic_subject_user_id',
+        'The Step-CA federation.tpl template did not embed OID 1.3.6.1.4.1.99999.2. Check the provisioner template configuration.',
+        undefined,
+        'OID_MISSING',
+      );
+    }
+    const subjectUserIdInCert = decoder.decode(subjectUserIdExt.value.slice(2));
+    if (subjectUserIdInCert !== req.subjectUserId) {
+      throw new CaServiceError(
+        `Issued certificate mosaic_subject_user_id mismatch: expected ${req.subjectUserId}, got ${subjectUserIdInCert}`,
+        'The Step-CA issued a certificate with a different subject user ID than requested. This may indicate a provisioner misconfiguration or a MITM.',
+        undefined,
+        'OID_MISMATCH',
+      );
+    }
+
     this.logger.log(`Certificate issued — serial=${serialNumber} grantId=${req.grantId}`);
 
     const result = new IssuedCertDto();

apps/gateway/src/federation/enrollment.service.ts

@@ -14,6 +14,7 @@
 
 import {
   BadRequestException,
+  ConflictException,
   GoneException,
   Inject,
   Injectable,
@@ -66,6 +67,21 @@ export class EnrollmentService {
    */
   async createToken(dto: CreateEnrollmentTokenDto): Promise<EnrollmentTokenResult> {
     const ttl = Math.min(dto.ttlSeconds, 900);
+
+    // MED-3: Verify the grantId ↔ peerId binding — prevents attacker from
+    // cross-wiring grants to attacker-controlled peers.
+    const [grant] = await this.db
+      .select({ peerId: federationGrants.peerId })
+      .from(federationGrants)
+      .where(eq(federationGrants.id, dto.grantId))
+      .limit(1);
+    if (!grant) {
+      throw new NotFoundException(`Grant ${dto.grantId} not found`);
+    }
+    if (grant.peerId !== dto.peerId) {
+      throw new BadRequestException(`peerId does not match the grant's registered peer`);
+    }
+
     const token = crypto.randomBytes(32).toString('hex');
     const expiresAt = new Date(Date.now() + ttl * 1000);
 
@@ -99,132 +115,167 @@ export class EnrollmentService {
    *  8. Return { certPem, certChainPem }
    */
   async redeem(token: string, csrPem: string): Promise<RedeemResult> {
-    // 1. Fetch token row
-    const [row] = await this.db
-      .select()
-      .from(federationEnrollmentTokens)
-      .where(eq(federationEnrollmentTokens.token, token))
-      .limit(1);
+    // HIGH-5: Track outcome so we can write a failure audit row on any error.
+    let outcome: 'allowed' | 'denied' = 'denied';
+    // row may be undefined if the token is not found — used defensively in catch.
+    let row: typeof federationEnrollmentTokens.$inferSelect | undefined;
 
-    if (!row) {
-      throw new NotFoundException('Enrollment token not found');
-    }
-
-    // 2. Already used?
-    if (row.usedAt !== null) {
-      throw new GoneException('Enrollment token has already been used');
-    }
-
-    // 3. Expired?
-    if (row.expiresAt < new Date()) {
-      throw new GoneException('Enrollment token has expired');
-    }
-
-    // 4. Load grant and verify it is still pending
-    let grant;
     try {
-      grant = await this.grantsService.getGrant(row.grantId);
+      // 1. Fetch token row
+      const [fetchedRow] = await this.db
+        .select()
+        .from(federationEnrollmentTokens)
+        .where(eq(federationEnrollmentTokens.token, token))
+        .limit(1);
+
+      if (!fetchedRow) {
+        throw new NotFoundException('Enrollment token not found');
+      }
+      row = fetchedRow;
+
+      // 2. Already used?
+      if (row.usedAt !== null) {
+        throw new GoneException('Enrollment token has already been used');
+      }
+
+      // 3. Expired?
+      if (row.expiresAt < new Date()) {
+        throw new GoneException('Enrollment token has expired');
+      }
+
+      // 4. Load grant and verify it is still pending
+      let grant;
+      try {
+        grant = await this.grantsService.getGrant(row.grantId);
+      } catch (err) {
+        if (err instanceof FederationScopeError) {
+          throw new BadRequestException(err.message);
+        }
+        throw err;
+      }
+
+      if (grant.status !== 'pending') {
+        throw new GoneException(
+          `Grant ${row.grantId} is no longer pending (status: ${grant.status})`,
+        );
+      }
+
+      // 5. Atomically claim the token BEFORE cert issuance to prevent double-minting.
+      // WHERE used_at IS NULL ensures only one concurrent request wins.
+      // Using .returning() works on both node-postgres and PGlite without rowCount inspection.
+      const claimed = await this.db
+        .update(federationEnrollmentTokens)
+        .set({ usedAt: sql`NOW()` })
+        .where(
+          and(
+            eq(federationEnrollmentTokens.token, token),
+            isNull(federationEnrollmentTokens.usedAt),
+          ),
+        )
+        .returning({ token: federationEnrollmentTokens.token });
+
+      if (claimed.length === 0) {
+        throw new GoneException('Enrollment token has already been used (concurrent request)');
+      }
+
+      // 6. Issue certificate via CaService (network call — outside any transaction).
+      // If this throws, the token is already consumed. The grant stays pending.
+      // Admin must revoke the grant and create a new one.
+      let issued;
+      try {
+        issued = await this.caService.issueCert({
+          csrPem,
+          grantId: row.grantId,
+          subjectUserId: grant.subjectUserId,
+          ttlSeconds: 300,
+        });
+      } catch (err) {
+        // HIGH-4: Log only the first 8 hex chars of the token for correlation — never log the full token.
+        this.logger.error(
+          `issueCert failed after token ${token.slice(0, 8)}... was claimed — grant ${row.grantId} is stranded pending`,
+          err instanceof Error ? err.stack : String(err),
+        );
+        if (err instanceof FederationScopeError) {
+          throw new BadRequestException((err as Error).message);
+        }
+        throw err;
+      }
+
+      // 7. Atomically activate grant, update peer record, and write audit log.
+      const certNotAfter = this.extractCertNotAfter(issued.certPem);
+      await this.db.transaction(async (tx) => {
+        // CRIT-2: Guard activation with WHERE status='pending' to prevent double-activation.
+        const [activated] = await tx
+          .update(federationGrants)
+          .set({ status: 'active' })
+          .where(and(eq(federationGrants.id, row!.grantId), eq(federationGrants.status, 'pending')))
+          .returning({ id: federationGrants.id });
+        if (!activated) {
+          throw new ConflictException(
+            `Grant ${row!.grantId} is no longer pending — cannot activate`,
+          );
+        }
+
+        // CRIT-2: Guard peer update with WHERE state='pending'.
+        await tx
+          .update(federationPeers)
+          .set({
+            certPem: issued.certPem,
+            certSerial: issued.serialNumber,
+            certNotAfter,
+            state: 'active',
+          })
+          .where(and(eq(federationPeers.id, row!.peerId), eq(federationPeers.state, 'pending')));
+
+        await tx.insert(federationAuditLog).values({
+          requestId: crypto.randomUUID(),
+          peerId: row!.peerId,
+          grantId: row!.grantId,
+          verb: 'enrollment',
+          resource: 'federation_grant',
+          statusCode: 200,
+          outcome: 'allowed',
+        });
+      });
+
+      this.logger.log(
+        `Enrollment complete — peerId=${row.peerId} grantId=${row.grantId} serial=${issued.serialNumber}`,
+      );
+
+      outcome = 'allowed';
+
+      // 8. Return cert material
+      return {
+        certPem: issued.certPem,
+        certChainPem: issued.certChainPem,
+      };
     } catch (err) {
-      if (err instanceof FederationScopeError) {
-        throw new BadRequestException(err.message);
+      // HIGH-5: Best-effort audit write on failure — do not let this throw.
+      if (outcome === 'denied') {
+        await this.db
+          .insert(federationAuditLog)
+          .values({
+            requestId: crypto.randomUUID(),
+            peerId: row?.peerId ?? null,
+            grantId: row?.grantId ?? null,
+            verb: 'enrollment',
+            resource: 'federation_grant',
+            statusCode:
+              err instanceof GoneException ? 410 : err instanceof NotFoundException ? 404 : 500,
+            outcome: 'denied',
+          })
+          .catch(() => {});
       }
       throw err;
     }
-
-    if (grant.status !== 'pending') {
-      throw new GoneException(
-        `Grant ${row.grantId} is no longer pending (status: ${grant.status})`,
-      );
-    }
-
-    // 5. Atomically claim the token BEFORE cert issuance to prevent double-minting.
-    // WHERE used_at IS NULL ensures only one concurrent request wins.
-    // Using .returning() works on both node-postgres and PGlite without rowCount inspection.
-    const claimed = await this.db
-      .update(federationEnrollmentTokens)
-      .set({ usedAt: sql`NOW()` })
-      .where(
-        and(eq(federationEnrollmentTokens.token, token), isNull(federationEnrollmentTokens.usedAt)),
-      )
-      .returning({ token: federationEnrollmentTokens.token });
-
-    if (claimed.length === 0) {
-      throw new GoneException('Enrollment token has already been used (concurrent request)');
-    }
-
-    // 6. Issue certificate via CaService (network call — outside any transaction).
-    // If this throws, the token is already consumed. The grant stays pending.
-    // Admin must revoke the grant and create a new one.
-    let issued;
-    try {
-      issued = await this.caService.issueCert({
-        csrPem,
-        grantId: row.grantId,
-        subjectUserId: grant.subjectUserId,
-        ttlSeconds: 300,
-      });
-    } catch (err) {
-      this.logger.error(
-        `issueCert failed after token ${token} was claimed — grant ${row.grantId} is stranded pending`,
-        err instanceof Error ? err.stack : String(err),
-      );
-      if (err instanceof FederationScopeError) {
-        throw new BadRequestException((err as Error).message);
-      }
-      throw err;
-    }
-
-    // 7. Atomically activate grant, update peer record, and write audit log.
-    const certNotAfter = this.extractCertNotAfter(issued.certPem);
-    await this.db.transaction(async (tx) => {
-      await tx
-        .update(federationGrants)
-        .set({ status: 'active' })
-        .where(eq(federationGrants.id, row.grantId));
-
-      await tx
-        .update(federationPeers)
-        .set({
-          certPem: issued.certPem,
-          certSerial: issued.serialNumber,
-          certNotAfter,
-          state: 'active',
-        })
-        .where(eq(federationPeers.id, row.peerId));
-
-      await tx.insert(federationAuditLog).values({
-        requestId: crypto.randomUUID(),
-        peerId: row.peerId,
-        grantId: row.grantId,
-        verb: 'enrollment',
-        resource: 'federation_grant',
-        statusCode: 200,
-        outcome: 'allowed',
-      });
-    });
-
-    this.logger.log(
-      `Enrollment complete — peerId=${row.peerId} grantId=${row.grantId} serial=${issued.serialNumber}`,
-    );
-
-    // 8. Return cert material
-    return {
-      certPem: issued.certPem,
-      certChainPem: issued.certChainPem,
-    };
   }
 
   /**
    * Extract the notAfter date from a PEM certificate.
-   * Falls back to 90 days from now if parsing fails.
+   * HIGH-2: No silent fallback — a cert that cannot be parsed should fail loud.
    */
   private extractCertNotAfter(certPem: string): Date {
-    try {
-      const cert = new X509Certificate(certPem);
-      return new Date(cert.validTo);
-    } catch {
-      // Fallback: 90 days from now
-      return new Date(Date.now() + 90 * 24 * 60 * 60 * 1000);
-    }
+    const cert = new X509Certificate(certPem);
+    return new Date(cert.validTo);
   }
 }