mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-02 03:35:09 +02:00
fix(server): tab-ownership gate keys on tabPolicy, not isWrite
Browser-skill spawns hit `403: Tab not owned by your agent` on every first run because the gate at server.ts:639 fired for any non-root write, regardless of the token's tabPolicy. The bundled hackernews-frontpage reference skill failed identically. Every /skillify-generated skill failed identically. The user's natural tabs have no claimed owner — by design — so any skill driving them via `goto` (a write) was 403'd. The intent in skill-token.ts:79 was always correct: `tabPolicy: 'shared'` with the comment "skill scripts may switch tabs as needed." The enforcement just ignored it. Two surgical changes: browser-manager.ts:checkTabAccess — gate now keys on options.ownOnly only. Shared-policy tokens (skill spawns, default scoped clients) get permissive access — root-equivalent for the tab gate. Own-only tokens (pair-agent over the ngrok tunnel) still require ownership for every read and write. isWrite stays in the signature for callers that want to log or branch elsewhere; it no longer gates the decision. server.ts:639 — gate predicate narrowed from (WRITE_COMMANDS.has(command) || tokenInfo.tabPolicy === 'own-only') to just tokenInfo.tabPolicy === 'own-only' The 'newtab' exemption stays. Shared tokens skip the gate entirely; own-only tokens still hit it. Comment block above the gate updated to document the new predicate intent. Pair-agent isolation is intact. Tunnel tokens still default to tabPolicy: 'own-only', still must `newtab` first to get a tab they can drive, still can't dispatch any of the 23 commands outside the tunnel allowlist. The capability gate (scope checks) and rate limits already constrain what local scoped clients can do; tab ownership was never a security boundary for them — only for pair-agent. This release makes the enforcement match the original design intent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -694,14 +694,32 @@ export class BrowserManager {
|
||||
|
||||
/**
|
||||
* Check if a client can access a tab.
|
||||
* If ownOnly or isWrite is true, requires ownership.
|
||||
* Otherwise (reads), allow by default.
|
||||
*
|
||||
* Two policies, distinguished by `options.ownOnly`:
|
||||
*
|
||||
* - **own-only (pair-agent over tunnel):** the strict mode. Token must own
|
||||
* the target tab for any access (reads or writes). Unowned user tabs
|
||||
* and tabs owned by other clients are off-limits. Remote agents must
|
||||
* `newtab` first to get a tab they can drive.
|
||||
*
|
||||
* - **shared (local skill spawns, default scoped tokens):** permissive on
|
||||
* tab access. The token can read/write any tab — capability is gated
|
||||
* elsewhere (scope checks at /command, rate limits, the dual-listener
|
||||
* allowlist for tunnel-bound traffic). Tab ownership is not a security
|
||||
* boundary for shared tokens; it only matters for pair-agent isolation.
|
||||
* This matches the contract documented in `skill-token.ts:79`
|
||||
* ("skill scripts may switch tabs as needed").
|
||||
*
|
||||
* Root is unconstrained.
|
||||
*
|
||||
* `isWrite` is preserved in the signature for callers that want to log or
|
||||
* branch on it elsewhere, but the access decision itself only depends on
|
||||
* `ownOnly` + ownership map state.
|
||||
*/
|
||||
checkTabAccess(tabId: number, clientId: string, options: { isWrite?: boolean; ownOnly?: boolean } = {}): boolean {
|
||||
if (clientId === 'root') return true;
|
||||
const owner = this.tabOwnership.get(tabId);
|
||||
if (options.ownOnly || options.isWrite) {
|
||||
if (!owner) return false;
|
||||
if (options.ownOnly) {
|
||||
const owner = this.tabOwnership.get(tabId);
|
||||
return owner === clientId;
|
||||
}
|
||||
return true;
|
||||
|
||||
+10
-4
@@ -634,11 +634,17 @@ async function handleCommandInternal(
|
||||
}
|
||||
}
|
||||
|
||||
// ─── Tab ownership check (for scoped tokens) ──────────────
|
||||
// Skip for newtab — it creates a new tab, doesn't access an existing one.
|
||||
if (command !== 'newtab' && tokenInfo && tokenInfo.clientId !== 'root' && (WRITE_COMMANDS.has(command) || tokenInfo.tabPolicy === 'own-only')) {
|
||||
// ─── Tab ownership check (own-only tokens / pair-agent isolation) ──
|
||||
//
|
||||
// Only `own-only` tokens (pair-agent over tunnel) are bound to their own
|
||||
// tabs. `shared` tokens — the default for skill spawns and local scoped
|
||||
// clients — can drive any tab; the capability gate (scope checks above)
|
||||
// and rate limits already constrain what they can do.
|
||||
//
|
||||
// Skip for `newtab` — it creates a tab rather than accessing one.
|
||||
if (command !== 'newtab' && tokenInfo && tokenInfo.clientId !== 'root' && tokenInfo.tabPolicy === 'own-only') {
|
||||
const targetTab = tabId ?? browserManager.getActiveTabId();
|
||||
if (!browserManager.checkTabAccess(targetTab, tokenInfo.clientId, { isWrite: WRITE_COMMANDS.has(command), ownOnly: tokenInfo.tabPolicy === 'own-only' })) {
|
||||
if (!browserManager.checkTabAccess(targetTab, tokenInfo.clientId, { isWrite: WRITE_COMMANDS.has(command), ownOnly: true })) {
|
||||
return {
|
||||
status: 403, json: true,
|
||||
result: JSON.stringify({
|
||||
|
||||
Reference in New Issue
Block a user