mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-01 08:59:50 +02:00
fix(pairing): release tab ownership on revoke
tabOwnership cleared only on tab close, so after DELETE /token a same-name re-pair inherited the revoked agent's authenticated tabs (own-only access keys on owner === clientId). Add BrowserManager.releaseClientTabs and run it unconditionally in DELETE /token (ownership outlives the token, so an expired-token client can still own tabs); 404 only when both nothing was revoked and nothing released. Response now carries tabs_released. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
783504d7ea
commit
65b967c53a
4 files changed
+85
-3
No files matched your search
@@ -1056,6 +1056,25 @@ export class BrowserManager {
|
|||||||
this.tabOwnership.set(tabId, toClientId);
|
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<Array<{ id: number; url: string; title: string; active: boolean }>> {
|
async getTabListWithTitles(): Promise<Array<{ id: number; url: string; title: string; active: boolean }>> {
|
||||||
const tabs: Array<{ id: number; url: string; title: string; active: boolean }> = [];
|
const tabs: Array<{ id: number; url: string; title: string; active: boolean }> = [];
|
||||||
for (const [id, page] of this.pages) {
|
for (const [id, page] of this.pages) {
|
||||||
|
|||||||
@@ -2348,13 +2348,18 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
const revoked = revokeToken(clientId);
|
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` }), {
|
return new Response(JSON.stringify({ error: `Agent "${clientId}" not found` }), {
|
||||||
status: 404, headers: { 'Content-Type': 'application/json' },
|
status: 404, headers: { 'Content-Type': 'application/json' },
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
console.log(`[browse] Revoked ${revoked} token(s) for: ${clientId}`);
|
console.log(`[browse] Revoked ${revoked} token(s), released ${tabsReleased} tab(s) for: ${clientId}`);
|
||||||
return new Response(JSON.stringify({ revoked: clientId, tokens_deleted: revoked }), {
|
return new Response(JSON.stringify({ revoked: clientId, tokens_deleted: revoked, tabs_released: tabsReleased }), {
|
||||||
status: 200, headers: { 'Content-Type': 'application/json' },
|
status: 200, headers: { 'Content-Type': 'application/json' },
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -318,6 +318,35 @@ describe('pair-agent flow end-to-end (HTTP only, no ngrok)', () => {
|
|||||||
expect(body.error).not.toBe('Invalid request body');
|
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 ────
|
// ─── 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 () => {
|
test('DELETE /token revokes session AND setup keys; agent leaves /agents; token 401s; re-connect fails', async () => {
|
||||||
|
|||||||
@@ -99,6 +99,35 @@ describe('Tab Isolation', () => {
|
|||||||
expect(() => bm.transferTab(999, 'agent-1')).toThrow('Tab 999 not found');
|
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<number, string>;
|
||||||
|
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<number, string>;
|
||||||
|
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
|
// Test the instruction block generator
|
||||||
|
|||||||
Reference in new issue
Block a user