fix(browse): adversarial-review hardening — 6 findings fixed, regression-pinned

Pre-push adversarial review (4 lenses, refute-style verification: 13 raw
findings, 7 refuted, 6 confirmed) caught these; each fix carries a pin:

1. --restrict=read (equals form) sailed past validatePairAgentFlags —
   hasFlag/parseFlag are exact-token matches — so the user asked for a
   read-only sandbox and silently got FULL access: the exact failure mode
   this branch claims to close. The equals form is now a hard error before
   any server work.
2. handleTunnel trimmed the agent name but clientIds are stored verbatim,
   so a space-padded agent was unrevocable by the documented kill switch
   (trimmed DELETE 404'd while the grant stayed live). Names now pass
   through verbatim; the live-daemon test revokes ' padded'.
3. The sole pin for "CLI always sends explicit scopes" passed vacuously on
   a simulated revert: toContain('DEFAULT_PAIR_SCOPES') was satisfied by a
   comment. The tripwire now matches the code shape with a regex and bans
   the conditional spread formatting-insensitively.
4. The rewritten 403 scope hint was unpinned — new e2e asserts it names
   --restrict and --control and never --admin.
5. tunnelRevoke's verify-failure and HTTP-error branches and tunnelAgents'
   unreadable-list branch had no coverage — three stub-daemon pins added
   (an unreadable list must never render as "No paired agents").
6. CHANGELOG claimed "40+ new test cases"; the honest count is 35.

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

CHANGELOG.md

@@ -42,7 +42,7 @@ Revoke means revoked: one command deletes the session and every setup key, print
 - `DELETE /token/:id` decodes percent-encoded client ids, so names with spaces round-trip from the CLI.
 
 ### For contributors
-- 40+ new test cases: revoke-all regression shapes, a subprocess CLI harness with stub daemons pinning the version-skew net ("Revocation incomplete" on a lying daemon), e2e scope-contract pins, and source tripwires for `DEFAULT_PAIR_SCOPES` and the decode path.
+- 35 new test cases: revoke-all regression shapes, a subprocess CLI harness with stub daemons pinning the version-skew net ("Revocation incomplete" on a lying daemon) and every CLI error branch, e2e scope-contract and 403-hint pins, and code-shape tripwires for `DEFAULT_PAIR_SCOPES` and the decode path.
 
 ## [1.68.1.0] - 2026-08-18
 

browse/src/cli.ts

@@ -1225,6 +1225,12 @@ 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 {
+  // 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='))) {
+    console.error('[browse] --restrict takes a space-separated value: --restrict read or --restrict "read,write". The --restrict=... form is not supported.');
+    process.exit(1);
+  }
   if (!hasFlag(args, '--restrict')) return;
   const restrict = parseFlag(args, '--restrict');
   if (!restrict || !restrict.trim() || restrict.startsWith('--')) {
@@ -1244,8 +1250,10 @@ function validatePairAgentFlags(args: string[]): void {
 
 async function handleTunnel(args: string[]): Promise<never> {
   const sub = args[0];
-  if (sub === 'revoke' && args.length === 2 && args[1].trim()) {
-    process.exit(await tunnelRevoke(args[1].trim()));
+  // The name passes through VERBATIM: clientIds are stored untrimmed, so a
+  // space-padded name must stay revocable (encodeURIComponent handles it).
+  if (sub === 'revoke' && args.length === 2 && args[1]) {
+    process.exit(await tunnelRevoke(args[1]));
   }
   if (sub === 'agents' && args.length === 1) {
     process.exit(await tunnelAgents());

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

@@ -264,6 +264,33 @@ describe('pair-agent flow end-to-end (HTTP only, no ngrok)', () => {
     expect(body.error).toContain('control');
   });
 
+  test('scope-denied 403 hint points at --restrict/--control, never --admin', async () => {
+    // Regression: the old hint said "re-pair with --admin", which is a legacy
+    // alias for --control — following it over-granted browser-wide control.
+    const pairResp = await fetch(`${daemon.baseUrl}/pair`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${daemon.token}` },
+      body: JSON.stringify({ clientId: 'hint-agent', scopes: ['read'] }),
+    });
+    const { setup_key } = await pairResp.json() as any;
+    const connectResp = await fetch(`${daemon.baseUrl}/connect`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({ setup_key }),
+    });
+    const { token } = await connectResp.json() as any;
+    const resp = await fetch(`${daemon.baseUrl}/command`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}` },
+      body: JSON.stringify({ command: 'goto', args: ['https://example.com'] }),
+    });
+    expect(resp.status).toBe(403);
+    const body = await resp.json() as any;
+    expect(body.hint).toContain('--restrict');
+    expect(body.hint).toContain('--control');
+    expect(body.hint).not.toContain('--admin');
+  });
+
   // ─── 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/server-auth.test.ts

@@ -420,8 +420,10 @@ describe('Pair scope defaults and revocation surface', () => {
     const pairBlock = sliceBetween(SERVER_SRC, "url.pathname === '/pair'", "url.pathname === '/tunnel/start'");
     expect(pairBlock).toContain('DEFAULT_PAIR_SCOPES');
     const cliBlock = sliceBetween(CLI_SRC, 'async function handlePairAgent', 'Determine the URL to use');
-    expect(cliBlock).toContain('DEFAULT_PAIR_SCOPES');
-    expect(cliBlock).not.toContain('...(restrict ? { scopes');
+    // Match the CODE shape, not a comment: a bare toContain('DEFAULT_PAIR_SCOPES')
+    // is satisfied by the explanatory comment and passes vacuously on a revert.
+    expect(cliBlock).toMatch(/scopes:\s*restrict\s*\?[\s\S]{0,200}?:\s*\[\.\.\.DEFAULT_PAIR_SCOPES\]/);
+    expect(cliBlock).not.toMatch(/\.\.\.\(restrict\s*\?/);
   });
 
   // control is the only scope behind an explicit flag; a scopes list must

browse/test/tunnel-revoke-cli.test.ts

@@ -114,6 +114,22 @@ describe('pair-agent scope-flag validation (pre-server)', () => {
     }
   }, 30_000);
 
+  test('--restrict=read (equals form) → exit 1, NO daemon spawned', async () => {
+    // Regression: hasFlag/parseFlag are exact-token matches, so the equals
+    // form sailed past validatePairAgentFlags AND handlePairAgent's parse —
+    // the user asked for a read-only sandbox and silently got FULL access.
+    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-restrict-eq-'));
+    const stateFile = path.join(tmpDir, 'browse.json');
+    try {
+      const result = await runCli(['pair-agent', '--restrict=read'], baseEnv(stateFile));
+      expect(result.code).toBe(1);
+      expect(result.stderr).toContain('--restrict takes a space-separated value');
+      expect(fs.existsSync(stateFile)).toBe(false);
+    } finally {
+      fs.rmSync(tmpDir, { recursive: true, force: true });
+    }
+  }, 30_000);
+
   test('--restrict swallowing the next flag → exit 1, NO daemon spawned', async () => {
     const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-restrict-flag-'));
     const stateFile = path.join(tmpDir, 'browse.json');
@@ -248,6 +264,19 @@ describe('tunnel against a live daemon (HTTP only, no browser)', () => {
       const empty = await runCli(['tunnel', 'agents'], baseEnv(stateFile));
       expect(empty.code).toBe(0);
       expect(empty.stdout).toContain('No paired agents.');
+
+      // Regression: handleTunnel used to trim() the name, but clientIds are
+      // stored verbatim — a space-padded agent became unrevocable by the
+      // documented kill switch (trimmed DELETE 404'd while the grant lived).
+      const padPair = await fetch(`${baseUrl}/pair`, {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${rootToken}` },
+        body: JSON.stringify({ clientId: ' padded' }),
+      });
+      expect(padPair.status).toBe(200);
+      const padRevoke = await runCli(['tunnel', 'revoke', ' padded'], baseEnv(stateFile));
+      expect(padRevoke.code).toBe(0);
+      expect(padRevoke.stdout).toContain('Verified: not in the active agent list.');
     } finally {
       try { daemon.kill('SIGKILL'); } catch { /* already gone */ }
       fs.rmSync(tmpDir, { recursive: true, force: true });
@@ -289,6 +318,91 @@ describe('tunnel against a lying or unreachable daemon (stub harness)', () => {
     }
   }, 30_000);
 
+  test('DELETE succeeds but /agents errors → "could not verify", exit 1 (never a silent success)', async () => {
+    // Regression pin: a revoke whose verification read fails must NOT report
+    // clean success — the whole point of the re-read is proving the deletion.
+    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-tunnel-noverify-'));
+    const stateFile = path.join(tmpDir, 'browse.json');
+    const stub = Bun.serve({
+      hostname: '127.0.0.1',
+      port: 0,
+      fetch(req) {
+        const url = new URL(req.url);
+        if (req.method === 'DELETE' && url.pathname.startsWith('/token/')) {
+          return Response.json({ revoked: 'mallory', tokens_deleted: 1 });
+        }
+        if (url.pathname === '/agents') {
+          return new Response('boom', { status: 500 });
+        }
+        return Response.json({ status: 'healthy' });
+      },
+    });
+    try {
+      writeStateFile(stateFile, process.pid, stub.port);
+      const result = await runCli(['tunnel', 'revoke', 'mallory'], baseEnv(stateFile));
+      expect(result.code).toBe(1);
+      expect(result.stderr).toContain('Revoked, but could not verify against the agent list.');
+    } finally {
+      stub.stop(true);
+      fs.rmSync(tmpDir, { recursive: true, force: true });
+    }
+  }, 30_000);
+
+  test('DELETE returns 500 → "Revoke failed" with the body error, exit 1', async () => {
+    // Regression pin: a non-404 HTTP failure must surface the daemon's error,
+    // not fall through to a success print.
+    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-tunnel-500-'));
+    const stateFile = path.join(tmpDir, 'browse.json');
+    const stub = Bun.serve({
+      hostname: '127.0.0.1',
+      port: 0,
+      fetch(req) {
+        const url = new URL(req.url);
+        if (req.method === 'DELETE' && url.pathname.startsWith('/token/')) {
+          return Response.json({ error: 'registry exploded' }, { status: 500 });
+        }
+        return Response.json({ status: 'healthy' });
+      },
+    });
+    try {
+      writeStateFile(stateFile, process.pid, stub.port);
+      const result = await runCli(['tunnel', 'revoke', 'mallory'], baseEnv(stateFile));
+      expect(result.code).toBe(1);
+      expect(result.stderr).toContain('Revoke failed: registry exploded');
+    } finally {
+      stub.stop(true);
+      fs.rmSync(tmpDir, { recursive: true, force: true });
+    }
+  }, 30_000);
+
+  test('tunnel agents with a broken /agents → "Could not read the agent list", exit 1', async () => {
+    // Regression pin: a 500 from /agents must not render as "No paired
+    // agents." — an unreadable list is not an empty list.
+    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-tunnel-agents500-'));
+    const stateFile = path.join(tmpDir, 'browse.json');
+    const stub = Bun.serve({
+      hostname: '127.0.0.1',
+      port: 0,
+      fetch(req) {
+        const url = new URL(req.url);
+        if (url.pathname === '/agents') {
+          return new Response('boom', { status: 500 });
+        }
+        return Response.json({ status: 'healthy' });
+      },
+    });
+    try {
+      writeStateFile(stateFile, process.pid, stub.port);
+      const result = await runCli(['tunnel', 'agents'], baseEnv(stateFile));
+      expect(result.code).toBe(1);
+      expect(result.stderr).toContain('Could not read the agent list');
+      expect(result.stdout).not.toContain('No paired agents.');
+    } finally {
+      stub.stop(true);
+      fs.rmSync(tmpDir, { recursive: true, force: true });
+    }
+  }, 30_000);
+
   test('alive pid but unreachable port → "Could not reach daemon", exit 1 (NOT "no daemon")', async () => {
     const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-tunnel-unreach-'));
     const stateFile = path.join(tmpDir, 'browse.json');