From a4a6769a6dc7f19d18f59f0df6a917ec5f9f7d74 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Thu, 23 Apr 2026 22:30:09 -0500 Subject: [PATCH] fix(federation/client): pin Step-CA root, fix lockfile, harden cache test CRIT-1: regenerate pnpm-lock.yaml so apps/gateway resolves undici@7.24.6 (prior PR pushed package.json without lockfile update; CI failed with ERR_PNPM_OUTDATED_LOCKFILE). Incidentally cleans 57 lines of stale peer-dep entries. CRIT-2: cache-hit test no longer swallows resolveEntry errors. Calls the private method directly twice and asserts identity equality plus a single DB select, removing the silent-failure path the prior assertion allowed. HIGH-1: mTLS Agent now pins Step-CA root via STEP_CA_ROOT_CERT_PATH. Without the env var resolveEntry throws PEER_MISCONFIGURED, refusing to dial peers against the public trust store. PEM is read once and cached on the service instance. Co-Authored-By: Claude Opus 4.7 --- .../federation-client.service.spec.ts | 59 ++++++++++------- .../client/federation-client.service.ts | 46 ++++++++++++- pnpm-lock.yaml | 64 ++----------------- 3 files changed, 87 insertions(+), 82 deletions(-) diff --git a/apps/gateway/src/federation/client/__tests__/federation-client.service.spec.ts b/apps/gateway/src/federation/client/__tests__/federation-client.service.spec.ts index 3f92a33..8837885 100644 --- a/apps/gateway/src/federation/client/__tests__/federation-client.service.spec.ts +++ b/apps/gateway/src/federation/client/__tests__/federation-client.service.spec.ts @@ -22,9 +22,12 @@ */ import 'reflect-metadata'; -import { describe, it, expect, vi, beforeEach, afterEach, beforeAll } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach, beforeAll, afterAll } from 'vitest'; import { MockAgent, setGlobalDispatcher, getGlobalDispatcher } from 'undici'; import type { Dispatcher } from 'undici'; +import { writeFileSync, unlinkSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import type { Db } from '@mosaicstack/db'; import { FederationClientService, FederationClientError } from '../federation-client.service.js'; import { sealClientKey } from '../../peer-key.util.js'; @@ -57,6 +60,15 @@ const TEST_CERT_SERIAL = 'ABCDEF1234567890'; let SEALED_KEY: string; +// Path to a stub Step-CA root cert file written in beforeAll. The cert is never +// actually used to negotiate TLS in unit tests (MockAgent + spy on resolveEntry +// short-circuit the network), but loadStepCaRoot() requires the file to exist. +const STUB_CA_PEM_PATH = join(tmpdir(), 'federation-client-spec-ca.pem'); +const STUB_CA_PEM = `-----BEGIN CERTIFICATE----- +MIIBdummyCAforFederationClientSpecOnly== +-----END CERTIFICATE----- +`; + // --------------------------------------------------------------------------- // Peer row factory // --------------------------------------------------------------------------- @@ -167,17 +179,28 @@ beforeAll(() => { process.env['BETTER_AUTH_SECRET'] = saved; } } + writeFileSync(STUB_CA_PEM_PATH, STUB_CA_PEM, 'utf8'); +}); + +afterAll(() => { + try { + unlinkSync(STUB_CA_PEM_PATH); + } catch { + // best-effort cleanup + } }); beforeEach(() => { originalDispatcher = getGlobalDispatcher(); process.env['BETTER_AUTH_SECRET'] = TEST_SECRET; + process.env['STEP_CA_ROOT_CERT_PATH'] = STUB_CA_PEM_PATH; }); afterEach(() => { setGlobalDispatcher(originalDispatcher); vi.restoreAllMocks(); delete process.env['BETTER_AUTH_SECRET']; + delete process.env['STEP_CA_ROOT_CERT_PATH']; }); // --------------------------------------------------------------------------- @@ -401,34 +424,22 @@ describe('FederationClientService', () => { describe('cache behaviour', () => { it('hits cache on second call — only one DB lookup happens', async () => { + // Verify cache by calling the private resolveEntry directly twice and + // asserting the DB was queried only once. This avoids the HTTP layer, + // which would require either a real network or per-peer Agent rewiring + // that the cache invariant doesn't depend on. const db = makeDb(); - const { mockAgent, pool } = makeMockAgent(); - - // Use real resolveEntry — let it build cache from DB - const svc = new FederationClientService(db); - - // First capabilities call — will build + cache entry - pool - .intercept({ path: '/api/federation/v1/capabilities', method: 'GET' }) - .reply(200, CAP_BODY, { headers: { 'content-type': 'application/json' } }) - .times(2); // allow two HTTP calls (one per call below) - - // Spy on the private resolveEntry to count DB calls via the DB select spy const selectSpy = vi.spyOn(db, 'select'); + const svc = new FederationClientService(db); + const resolveEntry = ( + svc as unknown as { resolveEntry: (peerId: string) => Promise } + ).resolveEntry.bind(svc); - // First call - await svc.capabilities(PEER_ID).catch(() => { - // May fail with PEER_MISCONFIGURED if key unseal fails in test — that's OK - // for this specific test which only cares about select spy count - }); + const first = await resolveEntry(PEER_ID); + const second = await resolveEntry(PEER_ID); - // Second call — should use cache - await svc.capabilities(PEER_ID).catch(() => {}); - - // DB select should have been called at most once (cache hit on second call) + expect(first).toBe(second); expect(selectSpy).toHaveBeenCalledTimes(1); - - await mockAgent.close(); }); it('flushPeer() invalidates cache — next call re-reads DB', async () => { diff --git a/apps/gateway/src/federation/client/federation-client.service.ts b/apps/gateway/src/federation/client/federation-client.service.ts index 798ab51..1efd803 100644 --- a/apps/gateway/src/federation/client/federation-client.service.ts +++ b/apps/gateway/src/federation/client/federation-client.service.ts @@ -34,6 +34,7 @@ */ import { Injectable, Inject, Logger } from '@nestjs/common'; +import { readFileSync } from 'node:fs'; import { Agent, fetch as undiciFetch } from 'undici'; import type { Dispatcher } from 'undici'; import { z } from 'zod'; @@ -123,6 +124,14 @@ export class FederationClientService { */ private readonly cache = new Map(); + /** + * Step-CA root cert PEM, loaded once from `STEP_CA_ROOT_CERT_PATH`. + * Used as the trust anchor for peer server certificates so federation TLS is + * pinned to our PKI, not the public trust store. Lazily loaded on first use + * so unit tests that don't exercise the agent path can run without the env var. + */ + private cachedCaPem: string | null = null; + constructor(@Inject(DB) private readonly db: Db) {} // ------------------------------------------------------------------------- @@ -219,6 +228,38 @@ export class FederationClientService { // Internal helpers // ------------------------------------------------------------------------- + /** + * Load and cache the Step-CA root cert PEM from `STEP_CA_ROOT_CERT_PATH`. + * Throws `FederationClientError` if the env var is unset or the file cannot + * be read — mTLS to a peer without a pinned trust anchor would silently + * fall back to the public trust store. + */ + private loadStepCaRoot(): string { + if (this.cachedCaPem !== null) { + return this.cachedCaPem; + } + const path = process.env['STEP_CA_ROOT_CERT_PATH']; + if (!path) { + throw new FederationClientError({ + code: 'PEER_MISCONFIGURED', + message: 'STEP_CA_ROOT_CERT_PATH is not set; refusing to dial peer without pinned CA trust', + peerId: '', + }); + } + try { + const pem = readFileSync(path, 'utf8'); + this.cachedCaPem = pem; + return pem; + } catch (err) { + throw new FederationClientError({ + code: 'PEER_MISCONFIGURED', + message: `Failed to read STEP_CA_ROOT_CERT_PATH (${path})`, + peerId: '', + cause: err, + }); + } + } + /** * Resolve the cache entry for a peer, reading DB on miss. * @@ -275,11 +316,14 @@ export class FederationClientService { }); } - // Build mTLS agent + // Build mTLS agent — pin trust to Step-CA root so we never accept + // a peer cert signed by a public CA (defense against MITM with a + // publicly-trusted DV cert for the peer's hostname). const agent = new Agent({ connect: { cert: peer.certPem, key: privateKeyPem, + ca: this.loadStepCaRoot(), // rejectUnauthorized: true is the undici default for HTTPS }, }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 793f3ed..7c5055c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -179,6 +179,9 @@ importers: socket.io: specifier: ^4.8.0 version: 4.8.3 + undici: + specifier: ^7.24.6 + version: 7.24.6 uuid: specifier: ^11.0.0 version: 11.1.0 @@ -713,10 +716,10 @@ importers: dependencies: '@mariozechner/pi-agent-core': specifier: ^0.63.1 - version: 0.63.2(@modelcontextprotocol/sdk@1.28.0(zod@4.3.6))(ws@8.20.0)(zod@3.25.76) + version: 0.63.2(@modelcontextprotocol/sdk@1.28.0(zod@4.3.6))(ws@8.20.0)(zod@4.3.6) '@mariozechner/pi-ai': specifier: ^0.63.1 - version: 0.63.2(@modelcontextprotocol/sdk@1.28.0(zod@4.3.6))(ws@8.20.0)(zod@3.25.76) + version: 0.63.2(@modelcontextprotocol/sdk@1.28.0(zod@4.3.6))(ws@8.20.0)(zod@4.3.6) '@sinclair/typebox': specifier: ^0.34.41 version: 0.34.48 @@ -6993,10 +6996,6 @@ packages: resolution: {integrity: sha512-gBLkYIlEnSp8pFbT64yFgGE6UIB9tAkhukC23PmMDCe5Nd+cRqKxSjw5y54MK2AZMgZfJWMaNE4nYUHgi1XEOw==} engines: {node: '>=18.17'} - undici@7.24.3: - resolution: {integrity: sha512-eJdUmK/Wrx2d+mnWWmwwLRyA7OQCkLap60sk3dOK4ViZR7DKwwptwuIvFBg2HaiP9ESaEdhtpSymQPvytpmkCA==} - engines: {node: '>=20.18.1'} - undici@7.24.6: resolution: {integrity: sha512-Xi4agocCbRzt0yYMZGMA6ApD7gvtUFaxm4ZmeacWI4cZxaF6C+8I8QfofC20NAePiB/IcvZmzkJ7XPa471AEtA==} engines: {node: '>=20.18.1'} @@ -7329,12 +7328,6 @@ snapshots: '@jridgewell/gen-mapping': 0.3.13 '@jridgewell/trace-mapping': 0.3.31 - '@anthropic-ai/sdk@0.73.0(zod@3.25.76)': - dependencies: - json-schema-to-ts: 3.1.1 - optionalDependencies: - zod: 3.25.76 - '@anthropic-ai/sdk@0.73.0(zod@4.3.6)': dependencies: json-schema-to-ts: 3.1.1 @@ -8676,18 +8669,6 @@ snapshots: - ws - zod - '@mariozechner/pi-agent-core@0.63.2(@modelcontextprotocol/sdk@1.28.0(zod@4.3.6))(ws@8.20.0)(zod@3.25.76)': - dependencies: - '@mariozechner/pi-ai': 0.63.2(@modelcontextprotocol/sdk@1.28.0(zod@4.3.6))(ws@8.20.0)(zod@3.25.76) - transitivePeerDependencies: - - '@modelcontextprotocol/sdk' - - aws-crt - - bufferutil - - supports-color - - utf-8-validate - - ws - - zod - '@mariozechner/pi-agent-core@0.63.2(@modelcontextprotocol/sdk@1.28.0(zod@4.3.6))(ws@8.20.0)(zod@4.3.6)': dependencies: '@mariozechner/pi-ai': 0.63.2(@modelcontextprotocol/sdk@1.28.0(zod@4.3.6))(ws@8.20.0)(zod@4.3.6) @@ -8736,30 +8717,6 @@ snapshots: - ws - zod - '@mariozechner/pi-ai@0.63.2(@modelcontextprotocol/sdk@1.28.0(zod@4.3.6))(ws@8.20.0)(zod@3.25.76)': - dependencies: - '@anthropic-ai/sdk': 0.73.0(zod@3.25.76) - '@aws-sdk/client-bedrock-runtime': 3.1008.0 - '@google/genai': 1.45.0(@modelcontextprotocol/sdk@1.28.0(zod@4.3.6)) - '@mistralai/mistralai': 1.14.1 - '@sinclair/typebox': 0.34.48 - ajv: 8.18.0 - ajv-formats: 3.0.1(ajv@8.18.0) - chalk: 5.6.2 - openai: 6.26.0(ws@8.20.0)(zod@3.25.76) - partial-json: 0.1.7 - proxy-agent: 6.5.0 - undici: 7.24.3 - zod-to-json-schema: 3.25.1(zod@3.25.76) - transitivePeerDependencies: - - '@modelcontextprotocol/sdk' - - aws-crt - - bufferutil - - supports-color - - utf-8-validate - - ws - - zod - '@mariozechner/pi-ai@0.63.2(@modelcontextprotocol/sdk@1.28.0(zod@4.3.6))(ws@8.20.0)(zod@4.3.6)': dependencies: '@anthropic-ai/sdk': 0.73.0(zod@4.3.6) @@ -8773,7 +8730,7 @@ snapshots: openai: 6.26.0(ws@8.20.0)(zod@4.3.6) partial-json: 0.1.7 proxy-agent: 6.5.0 - undici: 7.24.3 + undici: 7.24.6 zod-to-json-schema: 3.25.1(zod@4.3.6) transitivePeerDependencies: - '@modelcontextprotocol/sdk' @@ -12632,7 +12589,7 @@ snapshots: saxes: 6.0.0 symbol-tree: 3.2.4 tough-cookie: 6.0.1 - undici: 7.24.3 + undici: 7.24.6 w3c-xmlserializer: 5.0.0 webidl-conversions: 8.0.1 whatwg-mimetype: 5.0.0 @@ -13352,11 +13309,6 @@ snapshots: dependencies: mimic-function: 5.0.1 - openai@6.26.0(ws@8.20.0)(zod@3.25.76): - optionalDependencies: - ws: 8.20.0 - zod: 3.25.76 - openai@6.26.0(ws@8.20.0)(zod@4.3.6): optionalDependencies: ws: 8.20.0 @@ -14488,8 +14440,6 @@ snapshots: undici@6.21.3: {} - undici@7.24.3: {} - undici@7.24.6: {} unhomoglyph@1.0.6: {}