fix(pairing): reject reserved clientId 'root' at all token writers

'root' is the sentinel checkScope/checkDomain/checkRate and the server
command gate use for the omnipotent caller, so a scoped token carrying it
bypasses every enforcement path. Add ReservedClientIdError + a shared
assertValidClientId; createToken/createSetupKey throw, restoreRegistry
skips-and-logs (a corrupt state file must not brick boot). /pair and /token
surface it as a named 400, and the CLI fast-fails --client root.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

browse/src/cli.ts

@@ -1225,6 +1225,14 @@ async function tunnelAgents(): Promise<number> {
  * opposite of the user's intent. And `control` never rides in via --restrict:
  * browser-wide destructive ops stay behind the explicit --control flag. */
 function validatePairAgentFlags(args: string[]): void {
+  // `root` is the sentinel that bypasses all scope/domain/rate/tab enforcement;
+  // naming an agent that way would silently un-sandbox it. Reject client-side
+  // before hitting the daemon (the server rejects it too).
+  const client = parseFlag(args, '--client');
+  if (client !== undefined && client.trim().toLowerCase() === 'root') {
+    console.error("[browse] --client 'root' is reserved — it would bypass all scope enforcement. Choose another name.");
+    process.exit(1);
+  }
   // hasFlag/parseFlag are exact-token matches, so `--restrict=read` would
   // sail past every check below and silently grant FULL access.
   if (args.some(a => a.startsWith('--restrict='))) {

browse/src/server.ts

@@ -32,7 +32,7 @@ import {
   checkRate, createToken, createSetupKey, exchangeSetupKey, revokeToken,
   listTokens, recordCommand,
   isRootToken, checkConnectRateLimit, type TokenInfo, type ScopeCategory,
-  DEFAULT_PAIR_SCOPES, InvalidScopeError,
+  DEFAULT_PAIR_SCOPES, InvalidScopeError, ReservedClientIdError,
 } from './token-registry';
 import { validateTempPath } from './path-security';
 import { resolveConfig, ensureStateDir, readVersionHash, resolveChromiumProfile, cleanSingletonLocks, isPairAgentEnabled } from './config';
@@ -2318,9 +2318,9 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
             agent: session.clientId,
           }), { status: 200, headers: { 'Content-Type': 'application/json' } });
         } catch (err) {
-          // Name the caller's typo (bad scope, negative rateLimit) instead of
-          // hiding it behind the generic body error.
-          if (err instanceof InvalidScopeError) {
+          // Name the caller's typo (bad scope, negative rateLimit, reserved
+          // clientId) instead of hiding it behind the generic body error.
+          if (err instanceof InvalidScopeError || err instanceof ReservedClientIdError) {
             return new Response(JSON.stringify({ error: err.message }), {
               status: 400, headers: { 'Content-Type': 'application/json' },
             });
@@ -2442,9 +2442,9 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
             server_url: `http://127.0.0.1:${browsePort}`,
           }), { status: 200, headers: { 'Content-Type': 'application/json' } });
         } catch (err) {
-          // Name the caller's typo (bad scope, negative rateLimit) instead of
-          // hiding it behind the generic body error.
-          if (err instanceof InvalidScopeError) {
+          // Name the caller's typo (bad scope, negative rateLimit, reserved
+          // clientId) instead of hiding it behind the generic body error.
+          if (err instanceof InvalidScopeError || err instanceof ReservedClientIdError) {
             return new Response(JSON.stringify({ error: err.message }), {
               status: 400, headers: { 'Content-Type': 'application/json' },
             });

browse/src/token-registry.ts

@@ -116,6 +116,26 @@ function assertValidTokenOptions(scopes: readonly string[], rateLimit: number):
   if (rateLimit < 0) throw new InvalidScopeError('rateLimit must be >= 0');
 }
 
+/**
+ * Typed error for a reserved or malformed clientId. `root` is the sentinel that
+ * checkScope/checkDomain/checkRate and the server command gate use to mean "the
+ * omnipotent root caller" (validateToken:360), so a scoped token carrying it
+ * would bypass every enforcement path. Empty/non-string ids collapse distinct
+ * agents together and break revoke-by-clientId. Request-path writers throw;
+ * restoreRegistry skips-and-logs so one bad state-file entry can't drop later
+ * sessions or brick boot.
+ */
+export class ReservedClientIdError extends Error {}
+
+export function assertValidClientId(clientId: unknown): asserts clientId is string {
+  if (typeof clientId !== 'string' || clientId.trim() === '') {
+    throw new ReservedClientIdError('clientId must be a non-empty string');
+  }
+  if (clientId.trim().toLowerCase() === 'root') {
+    throw new ReservedClientIdError("clientId 'root' is reserved");
+  }
+}
+
 // ─── Types ──────────────────────────────────────────────────────
 
 export interface TokenInfo {
@@ -234,6 +254,7 @@ export function createToken(opts: CreateTokenOptions): TokenInfo {
   } = opts;
 
   // Validate inputs
+  assertValidClientId(clientId);
   assertValidTokenOptions(scopes, rateLimit);
   if (expiresSeconds !== null && expiresSeconds !== undefined && expiresSeconds < 0) {
     throw new Error('expiresSeconds must be >= 0 or null');
@@ -276,6 +297,9 @@ export function createToken(opts: CreateTokenOptions): TokenInfo {
  * Setup keys expire in 5 minutes and can only be exchanged once.
  */
 export function createSetupKey(opts: Omit<CreateTokenOptions, 'clientId'> & { clientId?: string }): TokenInfo {
+  // Only validate when a clientId is supplied; an omitted one gets a safe
+  // generated `remote-<ts>` default below.
+  if (opts.clientId !== undefined) assertValidClientId(opts.clientId);
   const scopes = opts.scopes || ['read', 'write'];
   // ?? not ||: rateLimit 0 is documented as "unlimited" and must survive.
   const rateLimit = opts.rateLimit ?? 10;
@@ -535,6 +559,16 @@ export function restoreRegistry(state: TokenRegistryState): void {
     // Skip expired tokens
     if (data.expiresAt && new Date(data.expiresAt) < now) continue;
 
+    // Skip-and-log rather than throw: a hand-edited or corrupt state file must
+    // not brick boot or drop every later valid session. A persisted clientId
+    // 'root' would otherwise inject a token that bypasses all scope checks.
+    try {
+      assertValidClientId(clientId);
+    } catch (err) {
+      console.warn(`[browse] restoreRegistry: skipping invalid clientId ${JSON.stringify(clientId)}: ${err instanceof Error ? err.message : String(err)}`);
+      continue;
+    }
+
     tokens.set(data.token, {
       ...data,
       clientId,

browse/test/pair-agent-e2e.test.ts

@@ -291,6 +291,33 @@ describe('pair-agent flow end-to-end (HTTP only, no ngrok)', () => {
     expect(body.hint).not.toContain('--admin');
   });
 
+  // ─── D2: reserved clientId is rejected with a named 400 ───────────────
+
+  test('POST /pair with clientId "root" returns 400 naming the reservation', async () => {
+    const resp = await fetch(`${daemon.baseUrl}/pair`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${daemon.token}` },
+      body: JSON.stringify({ clientId: 'root' }),
+    });
+    expect(resp.status).toBe(400);
+    const body = await resp.json() as any;
+    // The reservation is named, NOT hidden behind the generic "Invalid request body".
+    expect(body.error).toContain('root');
+    expect(body.error).not.toBe('Invalid request body');
+  });
+
+  test('POST /token with clientId "root" returns 400 naming the reservation', async () => {
+    const resp = await fetch(`${daemon.baseUrl}/token`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${daemon.token}` },
+      body: JSON.stringify({ clientId: 'root' }),
+    });
+    expect(resp.status).toBe(400);
+    const body = await resp.json() as any;
+    expect(body.error).toContain('root');
+    expect(body.error).not.toBe('Invalid request body');
+  });
+
   // ─── Revocation e2e: revoke-all + the /agents verification surface ────
 
   test('DELETE /token revokes session AND setup keys; agent leaves /agents; token 401s; re-connect fails', async () => {

browse/test/token-registry.test.ts

@@ -6,7 +6,7 @@ import {
   revokeToken, rotateRoot, listTokens, recordCommand,
   serializeRegistry, restoreRegistry, checkConnectRateLimit,
   SCOPE_READ, SCOPE_WRITE, SCOPE_ADMIN, SCOPE_CONTROL, SCOPE_META,
-  DEFAULT_PAIR_SCOPES, InvalidScopeError,
+  DEFAULT_PAIR_SCOPES, InvalidScopeError, ReservedClientIdError,
   __resetRegistry,
 } from '../src/token-registry';
 
@@ -19,6 +19,39 @@ describe('token-registry', () => {
     initRegistry('root-token-for-tests');
   });
 
+  // D2: `root` is the sentinel checkScope/checkDomain/checkRate use for the
+  // omnipotent caller; a scoped token carrying it bypasses all enforcement.
+  describe('reserved clientId (D2)', () => {
+    it('createToken rejects clientId "root" and lookalikes', () => {
+      expect(() => createToken({ clientId: 'root' })).toThrow(ReservedClientIdError);
+      expect(() => createToken({ clientId: 'ROOT' })).toThrow(ReservedClientIdError);
+      expect(() => createToken({ clientId: '  root  ' })).toThrow(ReservedClientIdError);
+    });
+
+    it('createToken rejects empty / whitespace clientId', () => {
+      expect(() => createToken({ clientId: '' })).toThrow(ReservedClientIdError);
+      expect(() => createToken({ clientId: '   ' })).toThrow(ReservedClientIdError);
+    });
+
+    it('createSetupKey rejects clientId "root" but allows an omitted one', () => {
+      expect(() => createSetupKey({ clientId: 'root' })).toThrow(ReservedClientIdError);
+      // Omitted clientId gets a safe generated default, not a throw.
+      const key = createSetupKey({});
+      expect(key.clientId.startsWith('remote-')).toBe(true);
+    });
+
+    it('restoreRegistry skips a persisted "root" entry instead of injecting a bypass token', () => {
+      restoreRegistry({ agents: {
+        root: { token: 'gsk_sess_evil', type: 'session', scopes: ['read', 'write', 'admin', 'meta', 'control'], tabPolicy: 'shared', rateLimit: 0, expiresAt: null, createdAt: new Date().toISOString() } as any,
+        good: { token: 'gsk_sess_good', type: 'session', scopes: ['read'], tabPolicy: 'own-only', rateLimit: 10, expiresAt: null, createdAt: new Date().toISOString() } as any,
+      } });
+      // The evil root entry is dropped; the valid one still restores.
+      expect(validateToken('gsk_sess_evil')).toBeNull();
+      const good = validateToken('gsk_sess_good');
+      expect(good?.clientId).toBe('good');
+    });
+  });
+
   describe('root token', () => {
     it('identifies root token correctly', () => {
       expect(isRootToken('root-token-for-tests')).toBe(true);