From 001ba59be0f7552949cdbf54ec447ee27c3bfa36 Mon Sep 17 00:00:00 2001
From: Garry Tan <garrytan@gmail.com>
Date: Mon, 6 Apr 2026 00:34:32 -0700
Subject: [PATCH] refactor: checkTabAccess uses options object, add own-only
 tab policy
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Refactors checkTabAccess(tabId, clientId, isWrite) to use an options
object { isWrite?, ownOnly? }. Adds tabPolicy === 'own-only' support
in the server command dispatch — scoped tokens with this policy are
restricted to their own tabs for all commands, not just writes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 browse/src/browser-manager.ts     | 14 ++++++++------
 browse/src/server.ts              |  4 ++--
 browse/test/tab-isolation.test.ts | 12 ++++++------
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts
index 2470c046..8dd103e1 100644
--- a/browse/src/browser-manager.ts
+++ b/browse/src/browser-manager.ts
@@ -630,15 +630,17 @@ export class BrowserManager {
 
   /**
    * Check if a client can access a tab.
-   * Read access is always allowed. Write access requires ownership.
-   * Unowned tabs are root-only for writes.
+   * If ownOnly or isWrite is true, requires ownership.
+   * Otherwise (reads), allow by default.
    */
-  checkTabAccess(tabId: number, clientId: string, isWrite: boolean): boolean {
+  checkTabAccess(tabId: number, clientId: string, options: { isWrite?: boolean; ownOnly?: boolean } = {}): boolean {
     if (clientId === 'root') return true;
-    if (!isWrite) return true;
     const owner = this.tabOwnership.get(tabId);
-    if (!owner) return false; // unowned = root-only for writes
-    return owner === clientId;
+    if (options.ownOnly || options.isWrite) {
+      if (!owner) return false;
+      return owner === clientId;
+    }
+    return true;
   }
 
   /** Transfer tab ownership to a different client. */
diff --git a/browse/src/server.ts b/browse/src/server.ts
index 8ffb1b13..7f320017 100644
--- a/browse/src/server.ts
+++ b/browse/src/server.ts
@@ -906,9 +906,9 @@ async function handleCommandInternal(
   }
 
   // ─── Tab ownership check (for scoped tokens) ──────────────
-  if (tokenInfo && tokenInfo.clientId !== 'root' && WRITE_COMMANDS.has(command)) {
+  if (tokenInfo && tokenInfo.clientId !== 'root' && (WRITE_COMMANDS.has(command) || tokenInfo.tabPolicy === 'own-only')) {
     const targetTab = tabId ?? browserManager.getActiveTabId();
-    if (!browserManager.checkTabAccess(targetTab, tokenInfo.clientId, true)) {
+    if (!browserManager.checkTabAccess(targetTab, tokenInfo.clientId, { isWrite: WRITE_COMMANDS.has(command), ownOnly: tokenInfo.tabPolicy === 'own-only' })) {
       return {
         status: 403, json: true,
         result: JSON.stringify({
diff --git a/browse/test/tab-isolation.test.ts b/browse/test/tab-isolation.test.ts
index 0a6469d7..367d4d49 100644
--- a/browse/test/tab-isolation.test.ts
+++ b/browse/test/tab-isolation.test.ts
@@ -28,19 +28,19 @@ describe('Tab Isolation', () => {
 
   describe('checkTabAccess', () => {
     it('root can always access any tab (read)', () => {
-      expect(bm.checkTabAccess(1, 'root', false)).toBe(true);
+      expect(bm.checkTabAccess(1, 'root', { isWrite: false })).toBe(true);
     });
 
     it('root can always access any tab (write)', () => {
-      expect(bm.checkTabAccess(1, 'root', true)).toBe(true);
+      expect(bm.checkTabAccess(1, 'root', { isWrite: true })).toBe(true);
     });
 
     it('any agent can read an unowned tab', () => {
-      expect(bm.checkTabAccess(1, 'agent-1', false)).toBe(true);
+      expect(bm.checkTabAccess(1, 'agent-1', { isWrite: false })).toBe(true);
     });
 
     it('scoped agent cannot write to unowned tab', () => {
-      expect(bm.checkTabAccess(1, 'agent-1', true)).toBe(false);
+      expect(bm.checkTabAccess(1, 'agent-1', { isWrite: true })).toBe(false);
     });
 
     it('scoped agent can read another agent tab', () => {
@@ -49,12 +49,12 @@ describe('Tab Isolation', () => {
       // with a known owner via the internal state
       // We'll use transferTab which only checks pages map... let's test checkTabAccess directly
       // checkTabAccess reads from tabOwnership map, which is empty here
-      expect(bm.checkTabAccess(1, 'agent-2', false)).toBe(true);
+      expect(bm.checkTabAccess(1, 'agent-2', { isWrite: false })).toBe(true);
     });
 
     it('scoped agent cannot write to another agent tab', () => {
       // With no ownership set, this is an unowned tab -> denied
-      expect(bm.checkTabAccess(1, 'agent-2', true)).toBe(false);
+      expect(bm.checkTabAccess(1, 'agent-2', { isWrite: true })).toBe(false);
     });
   });