diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index bd380ce3..f5a3121d 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -694,14 +694,32 @@ export class BrowserManager { /** * Check if a client can access a tab. - * If ownOnly or isWrite is true, requires ownership. - * Otherwise (reads), allow by default. + * + * Two policies, distinguished by `options.ownOnly`: + * + * - **own-only (pair-agent over tunnel):** the strict mode. Token must own + * the target tab for any access (reads or writes). Unowned user tabs + * and tabs owned by other clients are off-limits. Remote agents must + * `newtab` first to get a tab they can drive. + * + * - **shared (local skill spawns, default scoped tokens):** permissive on + * tab access. The token can read/write any tab — capability is gated + * elsewhere (scope checks at /command, rate limits, the dual-listener + * allowlist for tunnel-bound traffic). Tab ownership is not a security + * boundary for shared tokens; it only matters for pair-agent isolation. + * This matches the contract documented in `skill-token.ts:79` + * ("skill scripts may switch tabs as needed"). + * + * Root is unconstrained. + * + * `isWrite` is preserved in the signature for callers that want to log or + * branch on it elsewhere, but the access decision itself only depends on + * `ownOnly` + ownership map state. */ checkTabAccess(tabId: number, clientId: string, options: { isWrite?: boolean; ownOnly?: boolean } = {}): boolean { if (clientId === 'root') return true; - const owner = this.tabOwnership.get(tabId); - if (options.ownOnly || options.isWrite) { - if (!owner) return false; + if (options.ownOnly) { + const owner = this.tabOwnership.get(tabId); return owner === clientId; } return true; diff --git a/browse/src/server.ts b/browse/src/server.ts index 02060812..042616e7 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -634,11 +634,17 @@ async function handleCommandInternal( } } - // ─── Tab ownership check (for scoped tokens) ────────────── - // Skip for newtab — it creates a new tab, doesn't access an existing one. - if (command !== 'newtab' && tokenInfo && tokenInfo.clientId !== 'root' && (WRITE_COMMANDS.has(command) || tokenInfo.tabPolicy === 'own-only')) { + // ─── Tab ownership check (own-only tokens / pair-agent isolation) ── + // + // Only `own-only` tokens (pair-agent over tunnel) are bound to their own + // tabs. `shared` tokens — the default for skill spawns and local scoped + // clients — can drive any tab; the capability gate (scope checks above) + // and rate limits already constrain what they can do. + // + // Skip for `newtab` — it creates a tab rather than accessing one. + if (command !== 'newtab' && tokenInfo && tokenInfo.clientId !== 'root' && tokenInfo.tabPolicy === 'own-only') { const targetTab = tabId ?? browserManager.getActiveTabId(); - if (!browserManager.checkTabAccess(targetTab, tokenInfo.clientId, { isWrite: WRITE_COMMANDS.has(command), ownOnly: tokenInfo.tabPolicy === 'own-only' })) { + if (!browserManager.checkTabAccess(targetTab, tokenInfo.clientId, { isWrite: WRITE_COMMANDS.has(command), ownOnly: true })) { return { status: 403, json: true, result: JSON.stringify({