diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 6a0050a7d..60c478842 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -1056,6 +1056,25 @@ export class BrowserManager { this.tabOwnership.set(tabId, toClientId); } + /** + * Release all tab ownership held by a client (called on revoke / reducing + * re-pair). Deletes the ownership entries so an own-only client re-pairing + * under the same name can no longer inherit the revoked agent's + * authenticated tabs (checkTabAccess gates own-only on owner === clientId). + * Tabs are NOT closed — leaving the page open is the local human's call, not + * the daemon's; the residual is a root-only tab. Returns the released ids. + */ + releaseClientTabs(clientId: string): number[] { + const released: number[] = []; + for (const [tabId, owner] of this.tabOwnership) { + if (owner === clientId) { + this.tabOwnership.delete(tabId); + released.push(tabId); + } + } + return released; + } + async getTabListWithTitles(): Promise> { const tabs: Array<{ id: number; url: string; title: string; active: boolean }> = []; for (const [id, page] of this.pages) { diff --git a/browse/src/server.ts b/browse/src/server.ts index 92ecdeebb..1a6cb0ce9 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -2348,13 +2348,18 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { }); } const revoked = revokeToken(clientId); - if (!revoked) { + // Release tabs UNCONDITIONALLY: ownership outlives the token (it clears + // only on tab close), so a client whose token already expired can still + // own tabs. Gating release on a revoke hit would orphan that ownership + // and let a same-name re-pair inherit an authenticated tab. + const tabsReleased = browserManager.releaseClientTabs(clientId).length; + if (!revoked && tabsReleased === 0) { return new Response(JSON.stringify({ error: `Agent "${clientId}" not found` }), { status: 404, headers: { 'Content-Type': 'application/json' }, }); } - console.log(`[browse] Revoked ${revoked} token(s) for: ${clientId}`); - return new Response(JSON.stringify({ revoked: clientId, tokens_deleted: revoked }), { + console.log(`[browse] Revoked ${revoked} token(s), released ${tabsReleased} tab(s) for: ${clientId}`); + return new Response(JSON.stringify({ revoked: clientId, tokens_deleted: revoked, tabs_released: tabsReleased }), { status: 200, headers: { 'Content-Type': 'application/json' }, }); } diff --git a/browse/test/pair-agent-e2e.test.ts b/browse/test/pair-agent-e2e.test.ts index 36aec9e58..4e5fce023 100644 --- a/browse/test/pair-agent-e2e.test.ts +++ b/browse/test/pair-agent-e2e.test.ts @@ -318,6 +318,35 @@ describe('pair-agent flow end-to-end (HTTP only, no ngrok)', () => { expect(body.error).not.toBe('Invalid request body'); }); + // ─── D3: DELETE /token releases tabs unconditionally; 404 only when empty ─ + + test('DELETE /token returns tabs_released and 404 only when nothing to revoke or release', async () => { + const pairResp = await fetch(`${daemon.baseUrl}/pair`, { + method: 'POST', + headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${daemon.token}` }, + body: JSON.stringify({ clientId: 'd3-agent' }), + }); + const { setup_key } = await pairResp.json() as any; + await fetch(`${daemon.baseUrl}/connect`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ setup_key }), + }); + const del = await fetch(`${daemon.baseUrl}/token/d3-agent`, { + method: 'DELETE', headers: { Authorization: `Bearer ${daemon.token}` }, + }); + expect(del.status).toBe(200); + const body = await del.json() as any; + expect(body.tokens_deleted).toBeGreaterThanOrEqual(1); + // Headless-skip daemon owns no real tabs, but the field is always present. + expect(body.tabs_released).toBe(0); + // Nothing to revoke AND nothing to release → 404. + const del2 = await fetch(`${daemon.baseUrl}/token/nonexistent-xyz`, { + method: 'DELETE', headers: { Authorization: `Bearer ${daemon.token}` }, + }); + expect(del2.status).toBe(404); + }); + // ─── 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 () => { diff --git a/browse/test/tab-isolation.test.ts b/browse/test/tab-isolation.test.ts index b995bb4e3..698dd1ac8 100644 --- a/browse/test/tab-isolation.test.ts +++ b/browse/test/tab-isolation.test.ts @@ -99,6 +99,35 @@ describe('Tab Isolation', () => { expect(() => bm.transferTab(999, 'agent-1')).toThrow('Tab 999 not found'); }); }); + + // D3: revocation must release tab ownership, or a same-name re-pair inherits + // the revoked agent's authenticated tabs (own-only access keys on + // owner === clientId). Prime the ownership map directly — newTab needs a + // real browser (see the file header note on private-map injection). + describe('releaseClientTabs (D3)', () => { + it('deletes only the target client\'s ownership and returns the released ids', () => { + const own = (bm as any).tabOwnership as Map; + own.set(1, 'codex'); own.set(2, 'codex'); own.set(3, 'other'); + const released = bm.releaseClientTabs('codex').sort((a, b) => a - b); + expect(released).toEqual([1, 2]); + expect(bm.getTabOwner(1)).toBeNull(); + expect(bm.getTabOwner(2)).toBeNull(); + expect(bm.getTabOwner(3)).toBe('other'); + }); + + it('after release, own-only access to the freed tab is denied even for the same name', () => { + const own = (bm as any).tabOwnership as Map; + own.set(1, 'codex'); + expect(bm.checkTabAccess(1, 'codex', { ownOnly: true })).toBe(true); // owns it + bm.releaseClientTabs('codex'); + // Ownership gone → a re-paired 'codex' can no longer read/write tab 1. + expect(bm.checkTabAccess(1, 'codex', { ownOnly: true })).toBe(false); + }); + + it('is a no-op on a client that owns nothing', () => { + expect(bm.releaseClientTabs('nobody')).toEqual([]); + }); + }); }); // Test the instruction block generator