mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-14 17:05:28 +02:00
v1.68.3.0 fix(pairing): re-pair to narrow revokes the old grant on the spot
POST /pair minted a new setup key but never touched the agent's live session, so re-pairing --client X --restrict read while X was connected (or whose 5-min key expired unexchanged) left the original full-access session, eval included, alive up to 24h. A reducing re-pair (fewer scopes, tighter domains, lower rate, stricter tab policy) now revokes the live session and releases its tabs before minting the new key (grantReducesAccess + revokeClientFully; superseded in the response). Non-reducing re-pairs keep the session and only drop stale PENDING setup keys, so a broaden/refresh never strands a working agent and a narrowing re-pair issued before the agent connects can't leave the old broad key exchangeable. Revoke happens before mint (revokeToken deletes all of a client's tokens). CLI prints a version-skew-safe supersede notice and warns when a re-pair-shaped call omits --client. Docs + CHANGELOG + VERSION. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
65b967c53a
commit
6fe3e67736
@@ -1313,8 +1313,21 @@ async function handlePairAgent(state: ServerState, args: string[]): Promise<void
|
||||
scopes: string[];
|
||||
tunnel_url: string | null;
|
||||
server_url: string;
|
||||
superseded?: { tokens_deleted: number; tabs_released: number };
|
||||
};
|
||||
|
||||
// Version-skew safe: only speak when the daemon actually superseded a live
|
||||
// session (old daemons omit the field, so a new CLI never claims a false one).
|
||||
if (pairData.superseded && pairData.superseded.tokens_deleted > 0) {
|
||||
console.log(`[browse] Superseded the previous session for "${clientName}" (${pairData.superseded.tokens_deleted} token(s), ${pairData.superseded.tabs_released} tab(s) released). The agent must reconnect with the new key.`);
|
||||
}
|
||||
// A re-pair narrows/changes an EXISTING agent only when it reuses that agent's
|
||||
// --client name. Without one, this mints a brand-new agent and the old grant
|
||||
// lives on — warn when the intent looks like a re-pair.
|
||||
if (!parseFlag(args, '--client') && (restrict || domains)) {
|
||||
console.warn(`[browse] No --client given: this pairs a NEW agent and does NOT narrow an existing one. To change an agent's access, re-pair with its --client name (see 'browse tunnel agents').`);
|
||||
}
|
||||
|
||||
// Determine the URL to use
|
||||
let serverUrl: string;
|
||||
if (pairData.tunnel_url) {
|
||||
|
||||
+32
-1
@@ -32,7 +32,8 @@ import {
|
||||
checkRate, createToken, createSetupKey, exchangeSetupKey, revokeToken,
|
||||
listTokens, recordCommand,
|
||||
isRootToken, checkConnectRateLimit, type TokenInfo, type ScopeCategory,
|
||||
DEFAULT_PAIR_SCOPES, InvalidScopeError, ReservedClientIdError,
|
||||
DEFAULT_PAIR_SCOPES, InvalidScopeError, ReservedClientIdError, assertValidClientId,
|
||||
revokeSetupKeys, getClientSession, grantReducesAccess,
|
||||
} from './token-registry';
|
||||
import { validateTempPath } from './path-security';
|
||||
import { resolveConfig, ensureStateDir, readVersionHash, resolveChromiumProfile, cleanSingletonLocks, isPairAgentEnabled } from './config';
|
||||
@@ -2397,6 +2398,9 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
|
||||
}
|
||||
try {
|
||||
const pairBody = await req.json() as any;
|
||||
// Reject a reserved/invalid clientId up front (createSetupKey enforces
|
||||
// it too, but this makes the 400 unambiguous and skips the teardown).
|
||||
if (pairBody.clientId !== undefined) assertValidClientId(pairBody.clientId);
|
||||
// Default: DEFAULT_PAIR_SCOPES (full page access). The trust boundary
|
||||
// is the pairing ceremony itself, not the scope. --control adds
|
||||
// browser-wide destructive commands (stop, restart, disconnect).
|
||||
@@ -2411,6 +2415,32 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
|
||||
const scopes = pairBody.control || pairBody.admin
|
||||
? [...DEFAULT_PAIR_SCOPES, 'control' as const]
|
||||
: ((pairBody.scopes || [...DEFAULT_PAIR_SCOPES]) as ScopeCategory[]);
|
||||
// D1: a re-pair supersedes prior grants. ALWAYS drop stale setup keys
|
||||
// so a superseded broad key can never be exchanged — this closes the
|
||||
// shadow-key hole where a narrowing re-pair before the agent connects
|
||||
// would otherwise leave the old broad key live. Revoke the live
|
||||
// SESSION only when the new grant actually reduces access, so a
|
||||
// broaden/refresh never strands a working agent mid-task. Compare
|
||||
// against the resolved grant (not raw pairBody) so dropping 'control'
|
||||
// or a default re-pair is classified correctly. Revoke runs BEFORE
|
||||
// createSetupKey — revokeToken deletes all of a clientId's tokens, so
|
||||
// minting first would nuke the fresh key.
|
||||
const grant = {
|
||||
scopes: [...scopes] as ScopeCategory[],
|
||||
domains: pairBody.domains as string[] | undefined,
|
||||
rateLimit: pairBody.rateLimit ?? 10,
|
||||
tabPolicy: 'own-only' as const,
|
||||
};
|
||||
const priorSession = pairBody.clientId ? getClientSession(pairBody.clientId) : null;
|
||||
let superseded: { tokens_deleted: number; tabs_released: number } | undefined;
|
||||
if (priorSession && grantReducesAccess(priorSession, grant)) {
|
||||
const tokensDeleted = revokeToken(pairBody.clientId);
|
||||
const tabsReleased = browserManager.releaseClientTabs(pairBody.clientId).length;
|
||||
superseded = { tokens_deleted: tokensDeleted, tabs_released: tabsReleased };
|
||||
console.log(`[browse] Superseded ${tokensDeleted} token(s), released ${tabsReleased} tab(s) for reducing re-pair: ${pairBody.clientId}`);
|
||||
} else if (pairBody.clientId) {
|
||||
revokeSetupKeys(pairBody.clientId);
|
||||
}
|
||||
const setupKey = createSetupKey({
|
||||
clientId: pairBody.clientId,
|
||||
scopes: [...scopes],
|
||||
@@ -2445,6 +2475,7 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
|
||||
scopes: setupKey.scopes,
|
||||
tunnel_url: verifiedTunnelUrl,
|
||||
server_url: `http://127.0.0.1:${browsePort}`,
|
||||
...(superseded ? { superseded } : {}),
|
||||
}), { status: 200, headers: { 'Content-Type': 'application/json' } });
|
||||
} catch (err) {
|
||||
// Name the caller's typo (bad scope, negative rateLimit, reserved
|
||||
|
||||
@@ -495,6 +495,110 @@ export function revokeToken(clientId: string): number {
|
||||
return deleted;
|
||||
}
|
||||
|
||||
/**
|
||||
* Revoke the PENDING (unspent) setup keys for a client, leaving any live
|
||||
* session AND spent keys untouched. A re-pair always drops pending keys so a
|
||||
* superseded broad key can never be exchanged — this closes the shadow-key
|
||||
* hole (a reducing re-pair before the agent connects would otherwise leave the
|
||||
* old broad key live) without touching the spent key that #2646 keeps for
|
||||
* idempotent re-exchange on a tunnel drop. Returns the number deleted.
|
||||
*/
|
||||
export function revokeSetupKeys(clientId: string): number {
|
||||
let deleted = 0;
|
||||
for (const [token, info] of tokens) {
|
||||
// usesRemaining !== 0 = still exchangeable (pending). Spent keys (0) are
|
||||
// harmless: their session is either kept here or revoked on the reduce path.
|
||||
if (info.clientId === clientId && info.type === 'setup' && info.usesRemaining !== 0) {
|
||||
tokens.delete(token);
|
||||
deleted++;
|
||||
}
|
||||
}
|
||||
return deleted;
|
||||
}
|
||||
|
||||
/** The live (non-expired) session token for a client, if any. */
|
||||
export function getClientSession(clientId: string): TokenInfo | null {
|
||||
const now = new Date();
|
||||
for (const info of tokens.values()) {
|
||||
if (info.clientId !== clientId || info.type !== 'session') continue;
|
||||
if (info.expiresAt && new Date(info.expiresAt) < now) continue;
|
||||
return info;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/** The effective grant a re-pair is requesting, resolved to concrete values. */
|
||||
export interface ResolvedGrant {
|
||||
scopes: ScopeCategory[];
|
||||
domains?: string[];
|
||||
rateLimit: number;
|
||||
tabPolicy: 'own-only' | 'shared';
|
||||
}
|
||||
|
||||
/**
|
||||
* Does `grant` remove any capability the live `prior` session holds? Drives the
|
||||
* /pair supersede decision: a reducing re-pair revokes the old session
|
||||
* immediately (the narrowing must not wait for a reconnect that may never
|
||||
* happen); a broaden/refresh leaves it working (no outage). Fails toward
|
||||
* revocation on an unprovable domain superset — a spurious revoke costs one
|
||||
* reconnect, a missed one leaves wide access live.
|
||||
*/
|
||||
export function grantReducesAccess(prior: TokenInfo, grant: ResolvedGrant): boolean {
|
||||
return scopesReduced(prior.scopes, grant.scopes)
|
||||
|| domainsReduced(prior.domains, grant.domains)
|
||||
|| rateReduced(prior.rateLimit, grant.rateLimit)
|
||||
|| tabPolicyReduced(prior.tabPolicy, grant.tabPolicy);
|
||||
}
|
||||
|
||||
function scopesReduced(prior: ScopeCategory[], next: ScopeCategory[]): boolean {
|
||||
// Any scope the prior held that the new grant omits (also catches dropping 'control').
|
||||
return prior.some(s => !next.includes(s));
|
||||
}
|
||||
|
||||
function domainsReduced(prior: string[] | undefined, next: string[] | undefined): boolean {
|
||||
const priorUnrestricted = !prior || prior.length === 0;
|
||||
const nextUnrestricted = !next || next.length === 0;
|
||||
if (priorUnrestricted) return !nextUnrestricted; // universe → restricted = reduce
|
||||
if (nextUnrestricted) return false; // restricted → universe = broaden
|
||||
// Both restricted: reduced if any host the prior allowlist admits is no longer
|
||||
// admitted by the new one. Approximate over patterns — prior is covered iff
|
||||
// every prior pattern is covered by some next pattern; anything unprovable
|
||||
// counts as reduced (fail toward revocation).
|
||||
return prior!.some(p => !next!.some(n => domainGlobCovers(n, p)));
|
||||
}
|
||||
|
||||
/** Does allowlist pattern `wide` admit every host that `narrow` admits? Mirrors
|
||||
* matchDomainGlob's suffix/exact rules. */
|
||||
function domainGlobCovers(wide: string, narrow: string): boolean {
|
||||
if (wide === narrow) return true;
|
||||
const wideGlob = wide.startsWith('*.');
|
||||
if (wideGlob) {
|
||||
const wideSuffix = wide.slice(1); // ".example.com"
|
||||
const wideApex = wide.slice(2); // "example.com"
|
||||
if (!narrow.startsWith('*.')) {
|
||||
// narrow is an exact host; covered iff the wide glob matches it.
|
||||
return narrow === wideApex || narrow.endsWith(wideSuffix);
|
||||
}
|
||||
// narrow is also a glob; its apex must fall under the wide suffix.
|
||||
const narrowApex = narrow.slice(2);
|
||||
return narrowApex === wideApex || narrowApex.endsWith(wideSuffix);
|
||||
}
|
||||
// wide is an exact host: covers only the identical host (handled by === above).
|
||||
return false;
|
||||
}
|
||||
|
||||
function rateReduced(prior: number, next: number): boolean {
|
||||
const priorUnlimited = prior <= 0; // 0 = unlimited
|
||||
const nextUnlimited = next <= 0;
|
||||
if (priorUnlimited) return !nextUnlimited; // unlimited → capped = reduce
|
||||
if (nextUnlimited) return false; // capped → unlimited = broaden
|
||||
return next < prior; // both capped: a lower cap = reduce
|
||||
}
|
||||
|
||||
function tabPolicyReduced(prior: 'own-only' | 'shared', next: 'own-only' | 'shared'): boolean {
|
||||
return prior === 'shared' && next === 'own-only';
|
||||
}
|
||||
|
||||
/**
|
||||
* Rotate the root token. All scoped tokens are invalidated.
|
||||
* Returns the new root token.
|
||||
|
||||
@@ -318,6 +318,61 @@ describe('pair-agent flow end-to-end (HTTP only, no ngrok)', () => {
|
||||
expect(body.error).not.toBe('Invalid request body');
|
||||
});
|
||||
|
||||
// ─── D1: a reducing re-pair supersedes the prior grant immediately ────
|
||||
|
||||
const pairAs = async (body: any) => (await (await fetch(`${daemon.baseUrl}/pair`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${daemon.token}` },
|
||||
body: JSON.stringify(body),
|
||||
})).json()) as any;
|
||||
const connectKey = async (setup_key: string) => {
|
||||
const r = await fetch(`${daemon.baseUrl}/connect`, {
|
||||
method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ setup_key }),
|
||||
});
|
||||
return { status: r.status, body: await r.json().catch(() => ({})) as any };
|
||||
};
|
||||
const statusWith = (token: string) => fetch(`${daemon.baseUrl}/command`, {
|
||||
method: 'POST', headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}` },
|
||||
body: JSON.stringify({ command: 'status', args: [] }),
|
||||
});
|
||||
|
||||
test('reducing re-pair revokes the prior session immediately, without exchanging the new key', async () => {
|
||||
const { setup_key: k1 } = await pairAs({ clientId: 'reduce-me' }); // broad
|
||||
const { body: c1 } = await connectKey(k1);
|
||||
const s1 = c1.token as string;
|
||||
expect((await statusWith(s1)).status).not.toBe(401); // works
|
||||
// Narrow WITHOUT exchanging the new key — this is the whole bug.
|
||||
const rp = await pairAs({ clientId: 'reduce-me', scopes: ['read'] });
|
||||
expect(rp.superseded?.tokens_deleted).toBeGreaterThanOrEqual(1);
|
||||
expect((await statusWith(s1)).status).toBe(401); // old session revoked
|
||||
// The new narrow key still works and yields the reduced scope.
|
||||
const c2 = await connectKey(rp.setup_key);
|
||||
expect(c2.status).toBe(200);
|
||||
expect(c2.body.scopes).toEqual(['read']);
|
||||
expect((await statusWith(c2.body.token)).status).not.toBe(401);
|
||||
});
|
||||
|
||||
test('reducing re-pair BEFORE connect kills the stale broad setup key; only the narrow key works', async () => {
|
||||
const { setup_key: broad } = await pairAs({ clientId: 'shadow' }); // never connected
|
||||
const rp = await pairAs({ clientId: 'shadow', scopes: ['read'] }); // narrowing re-pair
|
||||
expect(rp.superseded).toBeUndefined(); // no live session existed
|
||||
expect((await connectKey(broad)).status).toBe(401); // stale broad key dead
|
||||
const c = await connectKey(rp.setup_key);
|
||||
expect(c.status).toBe(200);
|
||||
expect(c.body.scopes).toEqual(['read']); // narrow key survives
|
||||
});
|
||||
|
||||
test('broadening re-pair does NOT revoke the working session (no outage)', async () => {
|
||||
const first = await pairAs({ clientId: 'broaden', scopes: ['read'] });
|
||||
expect(first.superseded).toBeUndefined(); // first pair supersedes nothing
|
||||
const { body: c } = await connectKey(first.setup_key);
|
||||
const s = c.token as string;
|
||||
expect((await statusWith(s)).status).not.toBe(401);
|
||||
const rp = await pairAs({ clientId: 'broaden', scopes: ['read', 'write'] }); // broaden
|
||||
expect(rp.superseded).toBeUndefined(); // session not superseded
|
||||
expect((await statusWith(s)).status).not.toBe(401); // still working
|
||||
});
|
||||
|
||||
// ─── 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 () => {
|
||||
|
||||
@@ -7,6 +7,8 @@ import {
|
||||
serializeRegistry, restoreRegistry, checkConnectRateLimit,
|
||||
SCOPE_READ, SCOPE_WRITE, SCOPE_ADMIN, SCOPE_CONTROL, SCOPE_META,
|
||||
DEFAULT_PAIR_SCOPES, InvalidScopeError, ReservedClientIdError,
|
||||
revokeSetupKeys, getClientSession, grantReducesAccess,
|
||||
type TokenInfo, type ResolvedGrant,
|
||||
__resetRegistry,
|
||||
} from '../src/token-registry';
|
||||
|
||||
@@ -40,6 +42,16 @@ describe('token-registry', () => {
|
||||
expect(key.clientId.startsWith('remote-')).toBe(true);
|
||||
});
|
||||
|
||||
it('revokeSetupKeys drops only PENDING keys, keeping the spent key and the session', () => {
|
||||
const k1 = createSetupKey({ clientId: 'x' }); // pending
|
||||
exchangeSetupKey(k1.token); // k1 now spent + a session exists
|
||||
createSetupKey({ clientId: 'x' }); // pending k2
|
||||
expect(revokeSetupKeys('x')).toBe(1); // only the pending k2
|
||||
expect(getClientSession('x')).not.toBeNull(); // session kept
|
||||
// The spent key survives for idempotent re-exchange (#2646).
|
||||
expect(exchangeSetupKey(k1.token)).not.toBeNull();
|
||||
});
|
||||
|
||||
it('restoreRegistry skips a persisted "root" entry instead of injecting a bypass token', () => {
|
||||
restoreRegistry({ agents: {
|
||||
root: { token: 'gsk_sess_evil', type: 'session', scopes: ['read', 'write', 'admin', 'meta', 'control'], tabPolicy: 'shared', rateLimit: 0, expiresAt: null, createdAt: new Date().toISOString() } as any,
|
||||
@@ -52,6 +64,49 @@ describe('token-registry', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// D1: drives the /pair supersede decision. Direction matters — dropping an
|
||||
// allowlisted domain is the reduction, not adding one; 0 = unlimited rate.
|
||||
describe('grantReducesAccess (D1)', () => {
|
||||
const prior = (o: Partial<TokenInfo> = {}): TokenInfo => ({
|
||||
token: 't', clientId: 'c', type: 'session',
|
||||
scopes: ['read', 'write', 'admin', 'meta'], tabPolicy: 'own-only',
|
||||
rateLimit: 10, expiresAt: null, createdAt: '', commandCount: 0, ...o,
|
||||
});
|
||||
const grant = (o: Partial<ResolvedGrant> = {}): ResolvedGrant => ({
|
||||
scopes: ['read', 'write', 'admin', 'meta'], rateLimit: 10, tabPolicy: 'own-only', ...o,
|
||||
});
|
||||
|
||||
it('scopes: drop → reduce; add/equal → not; dropping control → reduce', () => {
|
||||
expect(grantReducesAccess(prior({ scopes: ['read', 'write', 'admin', 'meta'] }), grant({ scopes: ['read'] }))).toBe(true);
|
||||
expect(grantReducesAccess(prior({ scopes: ['read'] }), grant({ scopes: ['read', 'write'] }))).toBe(false);
|
||||
expect(grantReducesAccess(prior({ scopes: ['read'] }), grant({ scopes: ['read'] }))).toBe(false);
|
||||
expect(grantReducesAccess(prior({ scopes: ['read', 'control'] }), grant({ scopes: ['read'] }))).toBe(true);
|
||||
});
|
||||
|
||||
it('domains: drop → reduce; add/equal → not; unrestricted→restricted → reduce; glob narrowing → reduce', () => {
|
||||
expect(grantReducesAccess(prior({ domains: ['a.com', 'b.com'] }), grant({ domains: ['a.com'] }))).toBe(true);
|
||||
expect(grantReducesAccess(prior({ domains: ['a.com'] }), grant({ domains: ['a.com', 'b.com'] }))).toBe(false);
|
||||
expect(grantReducesAccess(prior({ domains: ['a.com'] }), grant({ domains: ['a.com'] }))).toBe(false);
|
||||
expect(grantReducesAccess(prior({ domains: undefined }), grant({ domains: ['a.com'] }))).toBe(true);
|
||||
expect(grantReducesAccess(prior({ domains: ['a.com'] }), grant({ domains: undefined }))).toBe(false);
|
||||
expect(grantReducesAccess(prior({ domains: ['*.example.com'] }), grant({ domains: ['*.com'] }))).toBe(false); // widen
|
||||
expect(grantReducesAccess(prior({ domains: ['*.com'] }), grant({ domains: ['*.example.com'] }))).toBe(true); // narrow
|
||||
});
|
||||
|
||||
it('rate (0 = unlimited): unlimited→capped → reduce; lower cap → reduce; higher/equal → not', () => {
|
||||
expect(grantReducesAccess(prior({ rateLimit: 0 }), grant({ rateLimit: 10 }))).toBe(true);
|
||||
expect(grantReducesAccess(prior({ rateLimit: 10 }), grant({ rateLimit: 5 }))).toBe(true);
|
||||
expect(grantReducesAccess(prior({ rateLimit: 5 }), grant({ rateLimit: 10 }))).toBe(false);
|
||||
expect(grantReducesAccess(prior({ rateLimit: 10 }), grant({ rateLimit: 10 }))).toBe(false);
|
||||
expect(grantReducesAccess(prior({ rateLimit: 10 }), grant({ rateLimit: 0 }))).toBe(false); // → unlimited = broaden
|
||||
});
|
||||
|
||||
it('tabPolicy: shared → own-only → reduce; the reverse → not', () => {
|
||||
expect(grantReducesAccess(prior({ tabPolicy: 'shared' }), grant({ tabPolicy: 'own-only' }))).toBe(true);
|
||||
expect(grantReducesAccess(prior({ tabPolicy: 'own-only' }), grant({ tabPolicy: 'shared' }))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('root token', () => {
|
||||
it('identifies root token correctly', () => {
|
||||
expect(isRootToken('root-token-for-tests')).toBe(true);
|
||||
|
||||
Reference in New Issue
Block a user