mirror of
https://github.com/Tencent/BrowserSkill.git
synced 2026-09-10 13:01:23 +08:00
fix(extension): never return a borrowed tab into another session's Agent Window
When a borrowed tab's original window is gone, tab_return's fallback
picker called getLastFocused({windowTypes:["normal"]}) and only
excluded the *current* session's Agent Window. Agent Windows are
created with type:"normal", so the picker could move a returned user
tab into *another* session's Agent Window. That session could then
write to the tab (it passes enforceAgentWindow), and the tab would be
destroyed when that session stops and closes its window — a
cross-session sandbox break plus silent user-data loss.
Thread an isAgentWindowId predicate (derived from SessionManager in
the production handlers, mirroring the existing filter in
borrow-confirmation's listConfirmationCandidates) through
returnBorrowedTab -> chooseFallbackWindow so every live Agent Window
is skipped as a fallback target.
Adds two regression tests (both fail on the unpatched code).
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -613,6 +613,65 @@ describe("handleTabReturn", () => {
|
||||
expect(winSpies.create).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("does not fall back into another session's Agent Window (creates a new window instead)", async () => {
|
||||
// Regression: a returned borrowed tab whose original window is gone
|
||||
// must never be parked in *another* session's Agent Window. Agent
|
||||
// Windows are `type: "normal"`, so getLastFocused({windowTypes:
|
||||
// ["normal"]}) can return session bb22's window (200). Without the
|
||||
// isAgentWindowId guard the tab would be moved into window 200,
|
||||
// letting bb22 write to it and destroying it when bb22 stops.
|
||||
const sm = new SessionManager({ agentWindow: fakeAgentWindow([100, 200]) });
|
||||
const ctx = await sm.start("aa11");
|
||||
await sm.start("bb22"); // owns Agent Window 200
|
||||
ctx.borrowedTabs.set(7, { tabId: 7, originalWindowId: 300, originalIndex: 4 });
|
||||
const state: FakeTabState = {
|
||||
tabs: new Map([[7, { id: 7, windowId: 100 } as chrome.tabs.Tab]]),
|
||||
nextTabId: 50,
|
||||
windowsClosed: new Set([300]),
|
||||
};
|
||||
const { api, spies } = makeTabMutationApi(state);
|
||||
const { api: windowsApi, spies: winSpies } = makeWindowsApi(state, {
|
||||
lastFocused: 200, // session bb22's Agent Window
|
||||
createWindowId: 777,
|
||||
});
|
||||
const res = await handleTabReturn(
|
||||
sm,
|
||||
{ session_id: "aa11", tab_id: 7 },
|
||||
{ tabs: api, windows: windowsApi },
|
||||
);
|
||||
if ("code" in res) throw new Error(`unexpected error: ${JSON.stringify(res)}`);
|
||||
expect(res.returned_to_window_id).toBe(777);
|
||||
expect(res.fallback).toBe(true);
|
||||
expect(winSpies.create).toHaveBeenCalledOnce();
|
||||
expect(spies.move).not.toHaveBeenCalledWith(7, expect.objectContaining({ windowId: 200 }));
|
||||
});
|
||||
|
||||
it("returnBorrowedTab honours an explicit isAgentWindowId predicate", async () => {
|
||||
const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) });
|
||||
const ctx = await sm.start("aa11");
|
||||
ctx.borrowedTabs.set(7, { tabId: 7, originalWindowId: 300, originalIndex: 4 });
|
||||
const state: FakeTabState = {
|
||||
tabs: new Map([[7, { id: 7, windowId: 100 } as chrome.tabs.Tab]]),
|
||||
nextTabId: 50,
|
||||
windowsClosed: new Set([300]),
|
||||
};
|
||||
const { api, spies } = makeTabMutationApi(state);
|
||||
const { api: windowsApi, spies: winSpies } = makeWindowsApi(state, {
|
||||
lastFocused: 555,
|
||||
createWindowId: 777,
|
||||
});
|
||||
const res = await returnBorrowedTab(ctx, 7, {
|
||||
tabs: api,
|
||||
windows: windowsApi,
|
||||
isAgentWindowId: (windowId) => windowId === 555,
|
||||
});
|
||||
if ("code" in res) throw new Error(`unexpected error: ${JSON.stringify(res)}`);
|
||||
expect(res.toWindowId).toBe(777);
|
||||
expect(res.fallback).toBe(true);
|
||||
expect(winSpies.create).toHaveBeenCalledOnce();
|
||||
expect(spies.move).not.toHaveBeenCalledWith(7, expect.objectContaining({ windowId: 555 }));
|
||||
});
|
||||
|
||||
it("returns not_found when the tab is not borrowed by this session", async () => {
|
||||
const sm = new SessionManager({ agentWindow: fakeAgentWindow([100]) });
|
||||
await sm.start("aa11");
|
||||
|
||||
@@ -113,7 +113,13 @@ export async function handleSessionStop(
|
||||
const borrowedIds = Array.from(ctx.borrowedTabs.keys());
|
||||
for (const tabId of borrowedIds) {
|
||||
try {
|
||||
const outcome = await returnBorrowedTab(ctx, tabId, deps.tabManagement ?? {});
|
||||
const tabManagement = {
|
||||
...(deps.tabManagement ?? {}),
|
||||
isAgentWindowId:
|
||||
deps.tabManagement?.isAgentWindowId ??
|
||||
((windowId: number) => manager.findByWindowId(windowId) !== null),
|
||||
};
|
||||
const outcome = await returnBorrowedTab(ctx, tabId, tabManagement);
|
||||
if (typeof outcome === "object" && "code" in outcome) {
|
||||
console.warn(`[bsk session_stop] auto-return failed for tab ${tabId}`, outcome);
|
||||
returnFailures?.push({
|
||||
|
||||
@@ -282,6 +282,16 @@ export interface TabManagementDeps {
|
||||
approveBorrow?: BorrowConfirmationApprover;
|
||||
/** Clears Agent-scoped overlays after a borrowed tab is returned. */
|
||||
agentOverlayReset?: AgentOverlayResetApi;
|
||||
/**
|
||||
* Reports whether `windowId` is any live session's Agent Window.
|
||||
* `tab_return`'s fallback window picker uses this to avoid moving a
|
||||
* returned borrowed tab into *another* session's Agent Window (Agent
|
||||
* Windows are created with `type: "normal"`, so they otherwise match
|
||||
* the `getLastFocused({ windowTypes: ["normal"] })` query). Production
|
||||
* callers derive this from the `SessionManager`; the `() => false`
|
||||
* default only preserves legacy behaviour for direct/test callers.
|
||||
*/
|
||||
isAgentWindowId?: (windowId: number) => boolean;
|
||||
}
|
||||
|
||||
function getTabsApi(deps: TabManagementDeps): TabMutationApi {
|
||||
@@ -292,6 +302,10 @@ function getWindowsApi(deps: TabManagementDeps): ChromeWindowsApi {
|
||||
return deps.windows ?? chromeWindowsApi;
|
||||
}
|
||||
|
||||
function getIsAgentWindowId(deps: TabManagementDeps): (windowId: number) => boolean {
|
||||
return deps.isAgentWindowId ?? (() => false);
|
||||
}
|
||||
|
||||
function getAgentOverlayResetApi(deps: TabManagementDeps): AgentOverlayResetApi {
|
||||
return deps.agentOverlayReset ?? chromeAgentOverlayResetApi;
|
||||
}
|
||||
@@ -818,12 +832,20 @@ function describeError(err: unknown): string {
|
||||
async function chooseFallbackWindow(
|
||||
ctx: SessionContext,
|
||||
windowsApi: ChromeWindowsApi,
|
||||
isAgentWindowId: (windowId: number) => boolean,
|
||||
): Promise<{ windowId: number; index: number } | RpcError> {
|
||||
let lastFocusedError: unknown;
|
||||
try {
|
||||
const last = await windowsApi.getLastFocused({ windowTypes: ["normal"] });
|
||||
const lastId = typeof last?.id === "number" ? last.id : null;
|
||||
if (lastId !== null && lastId !== ctx.agentWindowId) {
|
||||
// Never relocate a user tab into *any* session's Agent Window: this
|
||||
// session's is excluded explicitly, and other sessions' are excluded
|
||||
// via `isAgentWindowId` (Agent Windows are `type: "normal"`, so they
|
||||
// otherwise survive the `windowTypes: ["normal"]` filter). Parking a
|
||||
// returned tab in another session's Agent Window would let that
|
||||
// session write to it and, worse, see it destroyed when that session
|
||||
// stops and closes its window.
|
||||
if (lastId !== null && lastId !== ctx.agentWindowId && !isAgentWindowId(lastId)) {
|
||||
return { windowId: lastId, index: -1 };
|
||||
}
|
||||
} catch (err) {
|
||||
@@ -888,6 +910,7 @@ export async function returnBorrowedTab(
|
||||
}
|
||||
const tabsApi = getTabsApi(deps);
|
||||
const windowsApi = getWindowsApi(deps);
|
||||
const isAgentWindowId = getIsAgentWindowId(deps);
|
||||
|
||||
let targetWindowId = entry.originalWindowId;
|
||||
let targetIndex = entry.originalIndex;
|
||||
@@ -903,7 +926,7 @@ export async function returnBorrowedTab(
|
||||
}
|
||||
if (!originalAlive) {
|
||||
fallback = true;
|
||||
const target = await chooseFallbackWindow(ctx, windowsApi);
|
||||
const target = await chooseFallbackWindow(ctx, windowsApi, isAgentWindowId);
|
||||
if ("code" in target) return target;
|
||||
targetWindowId = target.windowId;
|
||||
targetIndex = target.index;
|
||||
@@ -925,7 +948,7 @@ export async function returnBorrowedTab(
|
||||
};
|
||||
} catch (err) {
|
||||
if (!fallback) {
|
||||
const target = await chooseFallbackWindow(ctx, windowsApi);
|
||||
const target = await chooseFallbackWindow(ctx, windowsApi, isAgentWindowId);
|
||||
if ("code" in target) {
|
||||
return {
|
||||
code: "cdp_failed",
|
||||
@@ -979,7 +1002,11 @@ export async function handleTabReturn(
|
||||
message: `tab_return: tab ${params.tab_id} is not borrowed by this session`,
|
||||
};
|
||||
}
|
||||
const outcome = await returnBorrowedTab(ctx, params.tab_id, deps);
|
||||
const outcome = await returnBorrowedTab(ctx, params.tab_id, {
|
||||
...deps,
|
||||
isAgentWindowId:
|
||||
deps.isAgentWindowId ?? ((windowId) => manager.findByWindowId(windowId) !== null),
|
||||
});
|
||||
if (isRpcError(outcome)) return outcome;
|
||||
ctx.borrowedTabs.delete(params.tab_id);
|
||||
const result: TabReturnResult = {
|
||||
|
||||
Reference in New Issue
Block a user