fix(ocap-kernel): harden relay management, tighten validation, fix docs

Address review findings across the bounded relay management feature:
tighten RelayEntryStruct to reject NaN/Infinity/negative/empty values,
unify read/write validation paths, harden initRemoteIdentity return
value, add defensive cap validation, prevent mnemonic leaking to
platform services, simplify RPC handler and options passing, fix
inaccurate SES/hardened-object comments and stale JSDoc references,
and add missing test coverage for edge cases.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: sirtimid

packages/ocap-kernel/CHANGELOG.md

@@ -7,14 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-### Fixed
-
-- Deserialize CapData rejections in `Kernel.queueMessage` so vat errors surface as plain `Error` objects to all callers ([#928](https://github.com/MetaMask/ocap-kernel/pull/928))
-
 ### Changed
 
 - Bound relay hints in OCAP URLs to a maximum of 3 and cap the relay pool at 20 entries with eviction of oldest non-bootstrap relays ([#929](https://github.com/MetaMask/ocap-kernel/pull/929))
 
+### Fixed
+
+- Deserialize CapData rejections in `Kernel.queueMessage` so vat errors surface as plain `Error` objects to all callers ([#928](https://github.com/MetaMask/ocap-kernel/pull/928))
+
 ## [0.7.0]
 
 ### Added

packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts

@@ -581,7 +581,8 @@ describe('remote-comms', () => {
       identity.addKnownRelays(['/dns4/relay.example/tcp/443/wss/p2p-circuit']);
       const firstSeen = mockKernelStore.getRelayEntries()[0]?.lastSeen ?? 0;
 
-      // SES lockdown prevents mocking Date.now; use a real delay instead
+      // SES lockdown freezes Date, preventing vi.useFakeTimers(); use a
+      // real delay to guarantee a different Date.now() value.
       await new Promise((resolve) => {
         setTimeout(resolve, 50);
       });
@@ -591,6 +592,19 @@ describe('remote-comms', () => {
       expect(secondSeen).toBeGreaterThan(firstSeen);
     });
 
+    it('addKnownRelays does not add duplicate entries on re-observation', async () => {
+      const { identity } = await initRemoteIdentity(mockKernelStore);
+
+      const relay = '/dns4/relay.example/tcp/443/wss/p2p-circuit';
+      identity.addKnownRelays([relay]);
+      expect(mockKernelStore.getRelayEntries()).toHaveLength(1);
+
+      // Re-add the same relay — entry count must stay at 1
+      identity.addKnownRelays([relay, relay]);
+      expect(mockKernelStore.getRelayEntries()).toHaveLength(1);
+      expect(mockKernelStore.getRelayEntries()[0]?.addr).toBe(relay);
+    });
+
     it('init clears bootstrap flag on relays removed from the bootstrap set', async () => {
       await initRemoteIdentity(mockKernelStore, {
         relays: ['/dns4/relayA.example/tcp/443/wss/p2p-circuit'],
@@ -775,6 +789,53 @@ describe('remote-comms', () => {
       );
     });
 
+    it('init truncates pool when bootstrap count exceeds maxKnownRelays', async () => {
+      const bootstrapRelays = Array.from(
+        { length: 5 },
+        (_, i) => `/dns4/bootstrap${i}.example/tcp/443/wss/p2p-circuit`,
+      );
+      await initRemoteIdentity(mockKernelStore, {
+        relays: bootstrapRelays,
+        maxKnownRelays: 3,
+      });
+
+      const entries = mockKernelStore.getRelayEntries();
+      // Pool must be capped at maxKnownRelays even when all entries are bootstrap
+      expect(entries).toHaveLength(3);
+      // All surviving entries should still be marked as bootstrap
+      expect(entries.every((entry) => entry.isBootstrap)).toBe(true);
+    });
+
+    it('issueOcapURL includes all relays when count equals maxUrlRelayHints', async () => {
+      const relays = Array.from(
+        { length: DEFAULT_MAX_URL_RELAY_HINTS },
+        (_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`,
+      );
+      const { identity } = await initRemoteIdentity(mockKernelStore, {
+        relays,
+      });
+      const ocapURL = await identity.issueOcapURL('ko1' as KRef);
+      const { hints } = parseOcapURL(ocapURL);
+      expect(hints).toHaveLength(DEFAULT_MAX_URL_RELAY_HINTS);
+      expect(hints).toStrictEqual(relays);
+    });
+
+    it.each([
+      ['maxUrlRelayHints', { maxUrlRelayHints: 0 }],
+      ['maxUrlRelayHints', { maxUrlRelayHints: 1.5 }],
+      ['maxUrlRelayHints', { maxUrlRelayHints: -1 }],
+      ['maxKnownRelays', { maxKnownRelays: 0 }],
+      ['maxKnownRelays', { maxKnownRelays: 1.5 }],
+      ['maxKnownRelays', { maxKnownRelays: -1 }],
+    ] as const)(
+      'throws when %s is not a positive integer (%o)',
+      async (_label, opts) => {
+        await expect(initRemoteIdentity(mockKernelStore, opts)).rejects.toThrow(
+          /must be positive integers/u,
+        );
+      },
+    );
+
     it('throws with mnemonic when identity already exists', async () => {
       mockKernelStore.setRemoteIdentityValue('peerId', 'existing-peer-id');
       mockKernelStore.setRemoteIdentityValue(

packages/ocap-kernel/src/remotes/kernel/remote-comms.ts

@@ -39,8 +39,10 @@ export const DEFAULT_MAX_URL_RELAY_HINTS = 3;
 export const DEFAULT_MAX_KNOWN_RELAYS = 20;
 
 /**
- * Enforce the relay pool cap by keeping all bootstrap entries and the newest
- * non-bootstrap entries, up to `cap`.
+ * Enforce the relay pool cap by prioritizing bootstrap entries, then the
+ * newest non-bootstrap entries, up to `cap`. If bootstrap entries alone
+ * exceed the cap, the result is truncated to `cap` (some bootstrap entries
+ * may be dropped).
  *
  * @param entries - The full set of relay entries.
  * @param cap - The maximum pool size.
@@ -118,14 +120,14 @@ async function generateKeyInfo(seedString?: string): Promise<[string, string]> {
  * operations) without starting any network communications.
  *
  * @param kernelStore - The kernel store, for storing persistent key info.
- * @param options - Options for identity initialization.
- * @param options.relays - Bootstrap relay addresses. These are merged into
+ * @param [options] - Options for identity initialization.
+ * @param [options.relays] - Bootstrap relay addresses. These are merged into
  *   the relay pool, marked as bootstrap (prioritized during eviction and URL
  *   selection), and persisted. Relays previously marked as bootstrap that are
  *   no longer in this list have their bootstrap flag cleared.
- * @param options.mnemonic - BIP39 mnemonic for seed recovery.
- * @param options.maxUrlRelayHints - Cap on relay hints per OCAP URL.
- * @param options.maxKnownRelays - Cap on the stored relay pool.
+ * @param [options.mnemonic] - BIP39 mnemonic for seed recovery.
+ * @param [options.maxUrlRelayHints] - Cap on relay hints per OCAP URL.
+ * @param [options.maxKnownRelays] - Cap on the stored relay pool.
  * @param logger - The logger to use.
  * @param keySeed - Optional seed for key generation.
  * @returns the identity object, the key seed, and known relays.
@@ -155,8 +157,20 @@ export async function initRemoteIdentity(
     options?.maxUrlRelayHints ?? DEFAULT_MAX_URL_RELAY_HINTS;
   const maxKnownRelays = options?.maxKnownRelays ?? DEFAULT_MAX_KNOWN_RELAYS;
 
+  if (
+    !Number.isInteger(maxUrlRelayHints) ||
+    maxUrlRelayHints < 1 ||
+    !Number.isInteger(maxKnownRelays) ||
+    maxKnownRelays < 1
+  ) {
+    throw Error(
+      `maxUrlRelayHints (${maxUrlRelayHints}) and maxKnownRelays (${maxKnownRelays}) must be positive integers`,
+    );
+  }
+
   if (relays.length > 0) {
-    // Date.now() is available under SES lockdown (on the intrinsics allow-list).
+    // Date.now() works here because this code runs in the start compartment,
+    // which retains the original Date constructor (%InitialDate%).
     const now = Date.now();
     const bootstrapSet = new Set(relays);
 
@@ -174,7 +188,7 @@ export async function initRemoteIdentity(
       byAddr.set(addr, { addr, lastSeen: now, isBootstrap: true });
     }
     // Clear bootstrap flag on entries no longer in the current bootstrap set
-    // (create new objects to avoid mutating potentially-hardened entries)
+    // (create new objects to follow immutability conventions)
     for (const [addr, entry] of byAddr.entries()) {
       if (entry.isBootstrap && !bootstrapSet.has(addr)) {
         byAddr.set(addr, { ...entry, isBootstrap: false });
@@ -269,15 +283,17 @@ export async function initRemoteIdentity(
   /**
    * Add relay addresses to the kernel's known relay pool.
    * Deduplicates, updates lastSeen on re-observation, and enforces
-   * {@link MAX_KNOWN_RELAYS} by evicting the oldest non-bootstrap entries.
+   * the configured `maxKnownRelays` cap by evicting the oldest
+   * non-bootstrap entries.
    *
    * @param newRelays - Relay multiaddrs to add.
    */
   function addKnownRelays(newRelays: string[]): void {
     if (newRelays.length === 0) {
       return;
     }
-    // Date.now() is available under SES lockdown (on the intrinsics allow-list).
+    // Date.now() works here because this code runs in the start compartment,
+    // which retains the original Date constructor (%InitialDate%).
     const now = Date.now();
     const existing = kernelStore.getRelayEntries();
     const byAddr = new Map(existing.map((entry) => [entry.addr, entry]));
@@ -286,8 +302,8 @@ export async function initRemoteIdentity(
     for (const addr of newRelays) {
       const entry = byAddr.get(addr);
       if (entry) {
-        // Update lastSeen on re-observation (create new object to avoid
-        // mutating potentially-hardened deserialized entries)
+        // Update lastSeen on re-observation (create new object to follow
+        // immutability conventions)
         if (entry.lastSeen !== now) {
           byAddr.set(addr, { ...entry, lastSeen: now });
           changed = true;
@@ -339,11 +355,16 @@ export async function initRemoteIdentity(
     return kref;
   }
 
-  return {
-    identity: { getPeerId, issueOcapURL, redeemLocalOcapURL, addKnownRelays },
+  return harden({
+    identity: harden({
+      getPeerId,
+      issueOcapURL,
+      redeemLocalOcapURL,
+      addKnownRelays,
+    }),
     keySeed,
     knownRelays: kernelStore.getKnownRelayAddresses(),
-  };
+  });
 }
 
 /**
@@ -352,7 +373,7 @@ export async function initRemoteIdentity(
  * @param kernelStore - The kernel store, for storing persistent key info.
  * @param platformServices - The platform services, for accessing network I/O
  *   operations that are not available within the web worker that the kernel runs in.
- * @param remoteMessageHandler - Handler to process received inbound communcations.
+ * @param remoteMessageHandler - Handler to process received inbound communications.
  * @param options - Options for remote communications initialization.
  * @param logger - The logger to use.
  * @param keySeed - Optional seed for libp2p key generation.
@@ -373,16 +394,17 @@ export async function initRemoteComms(
   incarnationId?: string,
   onIncarnationChange?: OnIncarnationChange,
 ): Promise<RemoteComms> {
-  const { relays = [], mnemonic, maxUrlRelayHints, maxKnownRelays } = options;
+  const {
+    relays = [],
+    mnemonic,
+    maxUrlRelayHints,
+    maxKnownRelays,
+    ...platformOptions
+  } = options;
 
   const result = await initRemoteIdentity(
     kernelStore,
-    {
-      relays,
-      ...(mnemonic === undefined ? {} : { mnemonic }),
-      maxUrlRelayHints,
-      maxKnownRelays,
-    },
+    { relays, mnemonic, maxUrlRelayHints, maxKnownRelays },
     logger,
     keySeed,
   );
@@ -395,9 +417,11 @@ export async function initRemoteComms(
   // called before initializeRemoteComms to capture the pre-restart timestamp.
   const wakeDetected = kernelStore.detectWake();
 
+  // Omit mnemonic (sensitive key material) from the options passed to
+  // platform services, which only needs network-level configuration.
   await platformServices.initializeRemoteComms(
     result.keySeed,
-    { ...options, relays: knownRelays },
+    { ...platformOptions, relays: knownRelays },
     remoteMessageHandler,
     onRemoteGiveUp,
     incarnationId,

packages/ocap-kernel/src/remotes/types.ts

@@ -148,8 +148,10 @@ export type RemoteCommsOptions = {
   maxUrlRelayHints?: number | undefined;
   /**
    * Maximum number of relay entries stored in the kernel's relay pool
-   * (default: 20). The pool retains all bootstrap relays; when the cap is
-   * reached, the oldest non-bootstrap (learned) entries are evicted first.
+   * (default: 20). Bootstrap relays are prioritized during eviction; when
+   * the cap is reached, the oldest non-bootstrap (learned) entries are
+   * evicted first. If bootstrap relays alone exceed the cap, the pool is
+   * truncated to the cap.
    */
   maxKnownRelays?: number | undefined;
   /**

packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.ts

@@ -59,28 +59,13 @@ export const initRemoteCommsHandler: Handler<
   ...initRemoteCommsSpec,
   hooks: { kernel: true },
   implementation: async ({ kernel }, params): Promise<null> => {
-    const options: RemoteCommsOptions = {};
-    if (params.relays !== undefined) {
-      options.relays = params.relays;
-    }
-    if (params.directListenAddresses !== undefined) {
-      options.directListenAddresses = params.directListenAddresses;
-    }
-    if (params.maxRetryAttempts !== undefined) {
-      options.maxRetryAttempts = params.maxRetryAttempts;
-    }
-    if (params.maxQueue !== undefined) {
-      options.maxQueue = params.maxQueue;
-    }
-    if (params.maxUrlRelayHints !== undefined) {
-      options.maxUrlRelayHints = params.maxUrlRelayHints;
-    }
-    if (params.maxKnownRelays !== undefined) {
-      options.maxKnownRelays = params.maxKnownRelays;
-    }
-    if (params.allowedWsHosts !== undefined) {
-      options.allowedWsHosts = params.allowedWsHosts;
-    }
+    // Build options from only the defined RPC params. Superstruct has
+    // already validated the shape; stripping undefined keeps the bag sparse.
+    // Note: sensitive fields like `mnemonic` and internal fields like
+    // `directTransports` are intentionally excluded from the RPC struct.
+    const options: RemoteCommsOptions = Object.fromEntries(
+      Object.entries(params).filter(([, value]) => value !== undefined),
+    );
     await kernel.initRemoteComms(options);
     return null;
   },

packages/ocap-kernel/src/store/methods/relay.test.ts

@@ -91,9 +91,13 @@ describe('getRelayMethods', () => {
         'knownRelays',
         JSON.stringify([{ addr: 'ok', lastSeen: 'bad', isBootstrap: false }]),
       );
-      expect(() => methods.getRelayEntries()).toThrow(
-        'knownRelays entries must have addr, lastSeen, isBootstrap',
-      );
+      expect(() => methods.getRelayEntries()).toThrow(/Invalid stored relay/u);
+    });
+
+    it('returns empty array when stored value is an empty JSON array', () => {
+      const { kv, methods } = makeCtx();
+      kv.set('knownRelays', '[]');
+      expect(methods.getRelayEntries()).toStrictEqual([]);
     });
   });
 
@@ -106,6 +110,24 @@ describe('getRelayMethods', () => {
         ]),
       ).toThrow(/Invalid relay entry/u);
     });
+
+    it.each([
+      ['empty addr', { addr: '', lastSeen: 0, isBootstrap: false }],
+      [
+        'negative lastSeen',
+        { addr: 'relay1', lastSeen: -1, isBootstrap: false },
+      ],
+      ['NaN lastSeen', { addr: 'relay1', lastSeen: NaN, isBootstrap: false }],
+      [
+        'Infinity lastSeen',
+        { addr: 'relay1', lastSeen: Infinity, isBootstrap: false },
+      ],
+    ])('rejects entry with %s', (_label, entry) => {
+      const { methods } = makeCtx();
+      expect(() => methods.setRelayEntries([entry])).toThrow(
+        /Invalid relay entry/u,
+      );
+    });
   });
 
   describe('getKnownRelayAddresses', () => {

packages/ocap-kernel/src/store/methods/relay.ts

@@ -23,7 +23,8 @@ export function getRelayMethods(ctx: {
   const { kv, logger } = ctx;
 
   /**
-   * Parse a JSON string, wrapping errors with a contextual message.
+   * Parse the knownRelays JSON string from storage, wrapping errors with
+   * a contextual message.
    *
    * @param raw - The raw JSON string to parse.
    * @returns The parsed value.
@@ -36,6 +37,7 @@ export function getRelayMethods(ctx: {
         `Failed to parse knownRelays from store (value may be corrupted): ${
           error instanceof Error ? error.message : String(error)
         }`,
+        { cause: error },
       );
     }
   }
@@ -78,12 +80,7 @@ export function getRelayMethods(ctx: {
     // Validate each entry against the RelayEntry schema (entries deserialized
     // from storage may have been written by an older version or corrupted)
     for (const entry of parsed) {
-      (typeof entry === 'object' &&
-        entry !== null &&
-        typeof (entry as RelayEntry).addr === 'string' &&
-        typeof (entry as RelayEntry).lastSeen === 'number' &&
-        typeof (entry as RelayEntry).isBootstrap === 'boolean') ||
-        Fail`knownRelays entries must have addr, lastSeen, isBootstrap`;
+      assert(entry, RelayEntryStruct, 'Invalid stored relay entry');
     }
     return parsed as RelayEntry[];
   }