From d2a4ea0b6af79cb9e16d707c3477c7f1285c23ac Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 25 Apr 2026 13:08:17 -0700 Subject: [PATCH] test(browse): CDP allowlist + two-tier mutex 13 tests: - Allowlist linter: every entry has 4 required fields, no duplicates, justification length > 20 chars - Deny-list verification: dangerous methods (Runtime.evaluate, Page.navigate, Network.getResponseBody, Browser.close, Target.attachToTarget, etc.) are NOT allowed (Codex T2 categories 4-7) - Per-tab mutex serializes ops on same tab - Per-tab mutex allows parallel ops across different tabs - Global lock blocks tab locks; tab locks block global lock - Acquire timeout yields CDPMutexAcquireTimeout (no silent hang) - Timeout error names the tab id and the timeout budget Also extends Network.disable justification to satisfy linter. Co-Authored-By: Claude Opus 4.7 (1M context) --- browse/src/cdp-allowlist.ts | 2 +- browse/test/cdp-allowlist.test.ts | 80 +++++++++++++++++++++ browse/test/cdp-mutex.test.ts | 113 ++++++++++++++++++++++++++++++ 3 files changed, 194 insertions(+), 1 deletion(-) create mode 100644 browse/test/cdp-allowlist.test.ts create mode 100644 browse/test/cdp-mutex.test.ts diff --git a/browse/src/cdp-allowlist.ts b/browse/src/cdp-allowlist.ts index 82c73689..b9c3a953 100644 --- a/browse/src/cdp-allowlist.ts +++ b/browse/src/cdp-allowlist.ts @@ -185,7 +185,7 @@ export const CDP_ALLOWLIST: ReadonlyArray = Object.freeze([ method: 'disable', scope: 'tab', output: 'trusted', - justification: 'Domain disable.', + justification: 'Domain disable; mirrors Network.enable for cleanup symmetry.', }, // NOTE: Network.getResponseBody, Network.getCookies, Network.replayXHR, // Network.loadNetworkResource are INTENTIONALLY NOT allowed (Codex T2 cat 7). diff --git a/browse/test/cdp-allowlist.test.ts b/browse/test/cdp-allowlist.test.ts new file mode 100644 index 00000000..8c80b2cd --- /dev/null +++ b/browse/test/cdp-allowlist.test.ts @@ -0,0 +1,80 @@ +import { describe, it, expect } from 'bun:test'; +import { CDP_ALLOWLIST, lookupCdpMethod, isCdpMethodAllowed } from '../src/cdp-allowlist'; + +describe('CDP allowlist (T2: deny-default)', () => { + it('every entry has all 4 required fields', () => { + for (const entry of CDP_ALLOWLIST) { + expect(entry.domain).toBeTruthy(); + expect(entry.method).toBeTruthy(); + expect(['tab', 'browser']).toContain(entry.scope); + expect(['trusted', 'untrusted']).toContain(entry.output); + expect(entry.justification).toBeTruthy(); + expect(entry.justification.length).toBeGreaterThan(20); // not a placeholder + } + }); + + it('no duplicate (domain.method) entries', () => { + const seen = new Set(); + for (const e of CDP_ALLOWLIST) { + const key = `${e.domain}.${e.method}`; + expect(seen.has(key)).toBe(false); + seen.add(key); + } + }); + + it('lookupCdpMethod returns the entry for allowed methods', () => { + const e = lookupCdpMethod('Accessibility.getFullAXTree'); + expect(e).not.toBeNull(); + expect(e!.scope).toBe('tab'); + expect(e!.output).toBe('untrusted'); + }); + + it('isCdpMethodAllowed returns false for dangerous methods that must NOT be allowed (Codex T2)', () => { + // Code execution surfaces — would be RCE if allowed + expect(isCdpMethodAllowed('Runtime.evaluate')).toBe(false); + expect(isCdpMethodAllowed('Runtime.callFunctionOn')).toBe(false); + expect(isCdpMethodAllowed('Runtime.compileScript')).toBe(false); + expect(isCdpMethodAllowed('Runtime.runScript')).toBe(false); + expect(isCdpMethodAllowed('Debugger.evaluateOnCallFrame')).toBe(false); + expect(isCdpMethodAllowed('Page.addScriptToEvaluateOnNewDocument')).toBe(false); + expect(isCdpMethodAllowed('Page.createIsolatedWorld')).toBe(false); + + // Navigation — must use $B goto so URL blocklist applies + expect(isCdpMethodAllowed('Page.navigate')).toBe(false); + expect(isCdpMethodAllowed('Page.navigateToHistoryEntry')).toBe(false); + + // Exfil surfaces + expect(isCdpMethodAllowed('Network.getResponseBody')).toBe(false); + expect(isCdpMethodAllowed('Network.getCookies')).toBe(false); + expect(isCdpMethodAllowed('Network.replayXHR')).toBe(false); + expect(isCdpMethodAllowed('Network.loadNetworkResource')).toBe(false); + expect(isCdpMethodAllowed('Storage.getCookies')).toBe(false); + expect(isCdpMethodAllowed('Fetch.fulfillRequest')).toBe(false); + + // Browser/process-level mutators + expect(isCdpMethodAllowed('Browser.close')).toBe(false); + expect(isCdpMethodAllowed('Browser.crash')).toBe(false); + expect(isCdpMethodAllowed('Target.attachToTarget')).toBe(false); + expect(isCdpMethodAllowed('Target.createTarget')).toBe(false); + expect(isCdpMethodAllowed('Target.setAutoAttach')).toBe(false); + expect(isCdpMethodAllowed('Target.exposeDevToolsProtocol')).toBe(false); + + // Read-only methods we never added + expect(isCdpMethodAllowed('Bogus.unknown')).toBe(false); + }); + + it('isCdpMethodAllowed returns true for the small read-only safe set', () => { + expect(isCdpMethodAllowed('Accessibility.getFullAXTree')).toBe(true); + expect(isCdpMethodAllowed('DOM.getBoxModel')).toBe(true); + expect(isCdpMethodAllowed('Performance.getMetrics')).toBe(true); + expect(isCdpMethodAllowed('Page.captureScreenshot')).toBe(true); + }); + + it('untrusted-output methods cover the read-everything-attacker-controlled cases', () => { + // Anything that reads attacker-controlled strings (DOM/AX/CSS selectors) + // should be tagged untrusted so the envelope wraps the result. + const untrustedMethods = CDP_ALLOWLIST.filter((e) => e.output === 'untrusted').map((e) => `${e.domain}.${e.method}`); + expect(untrustedMethods).toContain('Accessibility.getFullAXTree'); + expect(untrustedMethods).toContain('CSS.getMatchedStylesForNode'); + }); +}); diff --git a/browse/test/cdp-mutex.test.ts b/browse/test/cdp-mutex.test.ts new file mode 100644 index 00000000..259ad1dc --- /dev/null +++ b/browse/test/cdp-mutex.test.ts @@ -0,0 +1,113 @@ +import { describe, it, expect } from 'bun:test'; +import { BrowserManager } from '../src/browser-manager'; + +describe('Two-tier CDP mutex (Codex T7)', () => { + it('per-tab acquire returns a release fn that unlocks subsequent acquires', async () => { + const bm = new BrowserManager(); + const release = await bm.acquireTabLock(1, 1000); + expect(typeof release).toBe('function'); + release(); + // Second acquire on same tab must succeed quickly. + const release2 = await bm.acquireTabLock(1, 100); + release2(); + }); + + it('per-tab serializes operations on the same tab', async () => { + const bm = new BrowserManager(); + const events: string[] = []; + async function op(label: string, holdMs: number) { + const release = await bm.acquireTabLock(1, 5000); + events.push(`${label}:start`); + await new Promise((r) => setTimeout(r, holdMs)); + events.push(`${label}:end`); + release(); + } + await Promise.all([op('A', 80), op('B', 10), op('C', 10)]); + // A's start happens before A's end, then B starts, then B ends, then C. + // Strict A→B→C ordering with no interleaving. + expect(events).toEqual(['A:start', 'A:end', 'B:start', 'B:end', 'C:start', 'C:end']); + }); + + it('cross-tab tab locks DO run in parallel (no serialization)', async () => { + const bm = new BrowserManager(); + const events: string[] = []; + async function op(tabId: number, label: string, holdMs: number) { + const release = await bm.acquireTabLock(tabId, 5000); + events.push(`${label}:start`); + await new Promise((r) => setTimeout(r, holdMs)); + events.push(`${label}:end`); + release(); + } + await Promise.all([op(1, 'tab1', 50), op(2, 'tab2', 50)]); + // Both start before either ends — interleaved. + const startsBeforeAnyEnd = events.slice(0, 2).every((e) => e.endsWith(':start')); + expect(startsBeforeAnyEnd).toBe(true); + }); + + it('global lock blocks all tab locks; tab locks block global lock', async () => { + const bm = new BrowserManager(); + const events: string[] = []; + + async function tabOp(tabId: number, label: string, holdMs: number) { + const release = await bm.acquireTabLock(tabId, 5000); + events.push(`${label}:start`); + await new Promise((r) => setTimeout(r, holdMs)); + events.push(`${label}:end`); + release(); + } + async function globalOp(label: string, holdMs: number) { + const release = await bm.acquireGlobalCdpLock(5000); + events.push(`${label}:start`); + await new Promise((r) => setTimeout(r, holdMs)); + events.push(`${label}:end`); + release(); + } + + // Tab1 starts first (holds 80ms). Global queues behind. Tab2 queues behind global. + const tab1 = tabOp(1, 'tab1', 80); + await new Promise((r) => setTimeout(r, 10)); // ensure tab1 started first + const global = globalOp('global', 30); + const tab2 = tabOp(2, 'tab2', 10); + await Promise.all([tab1, global, tab2]); + + // tab1 must end before global starts (global waits for tab1) + const tab1End = events.indexOf('tab1:end'); + const globalStart = events.indexOf('global:start'); + expect(tab1End).toBeGreaterThan(-1); + expect(globalStart).toBeGreaterThan(tab1End); + + // global must end before tab2 starts (tab2 was queued after global) + const globalEnd = events.indexOf('global:end'); + const tab2Start = events.indexOf('tab2:start'); + expect(tab2Start).toBeGreaterThan(globalEnd); + }); + + it('acquire timeout fires CDPMutexAcquireTimeout (no silent hang)', async () => { + const bm = new BrowserManager(); + // Hold the tab lock indefinitely for this test. + const heldRelease = await bm.acquireTabLock(1, 1000); + // Try to acquire with a tiny timeout — must throw. + await expect(bm.acquireTabLock(1, 50)).rejects.toThrow(/CDPMutexAcquireTimeout/); + heldRelease(); + }); + + it('acquire timeout error names the tab id', async () => { + const bm = new BrowserManager(); + const heldRelease = await bm.acquireTabLock(7, 1000); + try { + await bm.acquireTabLock(7, 30); + throw new Error('should have thrown'); + } catch (e: any) { + expect(e.message).toContain('tab 7'); + expect(e.message).toContain('30ms'); + } + heldRelease(); + }); + + it('global lock acquire timeout fires CDPMutexAcquireTimeout', async () => { + const bm = new BrowserManager(); + const heldRelease = await bm.acquireGlobalCdpLock(1000); + await expect(bm.acquireGlobalCdpLock(30)).rejects.toThrow(/CDPMutexAcquireTimeout/); + heldRelease(); + }); +});