fix(pairing): harden re-pair per adversarial review

Adversarial review of the diff found four issues, now fixed:
- Validate the requested grant BEFORE the supersede revoke: a reducing
  re-pair with a bad scope/rate no longer destroys the live session and
  then fails to mint a replacement (assertValidTokenOptions runs up front).
- A re-pair with no live session releases tabs orphaned by an expired
  incarnation, closing the tab-inheritance gap /pair had (DELETE /token
  already released unconditionally).
- Test the DELETE /token revoked=0/tabs>0 path and the /pair orphaned-tab
  release at the handler level (HTTP e2e can't, headless owns no tabs).
- Test the CLI --client root fast-fail; fix its null-guard (parseFlag
  returns null when --client is absent).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-08-21 15:15:13 -07:00
committed by Garry Tan
co-authored by Claude Fable 5
parent 6fe3e67736
commit d86edf1f8f
7 changed files with 85 additions and 5 deletions
+18
View File
@@ -373,6 +373,24 @@ describe('pair-agent flow end-to-end (HTTP only, no ngrok)', () => {
expect((await statusWith(s)).status).not.toBe(401); // still working
});
test('a reducing re-pair with an INVALID scope 400s and leaves the live session intact', async () => {
// Regression: the supersede revoke must run AFTER validation. A scope typo
// (--restrict red) on a narrowing re-pair must not destroy the session and
// then fail to mint a replacement — the agent would be knocked offline.
const { setup_key: k } = await pairAs({ clientId: 'validate-me' });
const { body: c } = await connectKey(k);
const s = c.token as string;
expect((await statusWith(s)).status).not.toBe(401);
const resp = await fetch(`${daemon.baseUrl}/pair`, {
method: 'POST', headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${daemon.token}` },
body: JSON.stringify({ clientId: 'validate-me', scopes: ['red'] }),
});
expect(resp.status).toBe(400);
expect((await resp.json() as any).error).toContain('red');
// The working session survives the validation error (not revoked).
expect((await statusWith(s)).status).not.toBe(401);
});
// ─── 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 () => {
+35
View File
@@ -370,6 +370,41 @@ describe('buildFetchHandler factory contract', () => {
initRegistry('first-token-pad-to-16-chars');
expect(() => initRegistry('second-token-pad-to-16-chars')).toThrow(/already initialized/i);
});
// D3 handler gating: tab ownership outlives token expiry, so DELETE /token
// must release tabs and 200 even when no token remains (revoked=0, tabs>0).
// Drives the handler into that exact state — HTTP e2e can't (headless-skip
// owns no real tabs), so the revoke-gated-release regression would pass there.
test('13. DELETE /token releases orphaned tabs and 200s when no token remains', async () => {
const cfg = makeMinimalConfig();
(cfg.browserManager as any).tabOwnership.set(7, 'ghost'); // owned, no token
const handle = buildFetchHandler(cfg);
const req = new Request('http://127.0.0.1/token/ghost', {
method: 'DELETE', headers: { Authorization: `Bearer ${cfg.authToken}` },
});
const resp = await handle.fetchLocal(req, null);
expect(resp.status).toBe(200);
const body = await resp.json() as { tokens_deleted: number; tabs_released: number };
expect(body.tokens_deleted).toBe(0);
expect(body.tabs_released).toBe(1);
expect(cfg.browserManager.getTabOwner(7)).toBeNull();
});
// D1 F2: a re-pair with no LIVE session must release tabs orphaned by an
// expired incarnation, or the new session inherits its authenticated pages.
test('14. /pair releases tabs orphaned by an expired session (no inheritance)', async () => {
const cfg = makeMinimalConfig();
(cfg.browserManager as any).tabOwnership.set(9, 'ghost'); // orphaned, no live session
const handle = buildFetchHandler(cfg);
const req = new Request('http://127.0.0.1/pair', {
method: 'POST',
headers: { Authorization: `Bearer ${cfg.authToken}`, 'Content-Type': 'application/json' },
body: JSON.stringify({ clientId: 'ghost', scopes: ['read'] }),
});
const resp = await handle.fetchLocal(req, null);
expect(resp.status).toBe(200);
expect(cfg.browserManager.getTabOwner(9)).toBeNull();
});
});
// ─── Idle timer + onDisconnect dual-instance fix (v1.42.3.0) ──────────
+15
View File
@@ -155,6 +155,21 @@ describe('pair-agent scope-flag validation (pre-server)', () => {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
}, 30_000);
test('--client root → exit 1 reserved-name error, NO daemon spawned', async () => {
// `root` is the sentinel that bypasses every scope/domain/rate/tab check;
// the CLI must reject it pre-server (the daemon also 400s it as defense-in-depth).
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-client-root-'));
const stateFile = path.join(tmpDir, 'browse.json');
try {
const result = await runCli(['pair-agent', '--client', 'root'], baseEnv(stateFile));
expect(result.code).toBe(1);
expect(result.stderr).toContain('reserved');
expect(fs.existsSync(stateFile)).toBe(false);
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
}, 30_000);
});
describe('tunnel against no daemon (#2254 — never boot one)', () => {