From d5de9b0154fb93a84e18ff0f878df995ee39666f Mon Sep 17 00:00:00 2001 From: Ljy-0827 Date: Fri, 21 Aug 2026 11:14:51 +0800 Subject: [PATCH] feat(bsk): fix upload function that cannot bypass layout --- .../src/tools/__tests__/dispatcher.test.ts | 88 +++++ .../src/tools/__tests__/file-transfer.test.ts | 322 +++++++++-------- apps/extension/src/tools/dispatcher.ts | 2 + apps/extension/src/tools/download-capture.ts | 211 +++++++++++ apps/extension/src/tools/download.ts | 149 +++----- apps/extension/src/tools/file-chooser.ts | 265 -------------- .../src/tools/file-input-transaction.ts | 339 ++++++++++++++++++ apps/extension/src/tools/interaction.ts | 2 +- apps/extension/src/tools/upload.ts | 34 +- apps/extension/src/transport/types.ts | 11 +- crates/bsk-cli/skill/SKILL.md | 15 +- crates/bsk-cli/src/cli/download.rs | 4 +- crates/bsk-cli/src/cli/render_error.rs | 60 ++-- crates/bsk-cli/src/daemon/file_transfer.rs | 94 ++++- crates/bsk-cli/src/daemon/ipc.rs | 8 +- .../schema/tool_download_params.json | 14 +- .../schema/tool_download_result.json | 14 +- .../bsk-protocol/src/tools/file_transfer.rs | 12 +- docs/architecture.md | 4 +- skill/SKILL.md | 15 +- 20 files changed, 1033 insertions(+), 630 deletions(-) create mode 100644 apps/extension/src/tools/download-capture.ts delete mode 100644 apps/extension/src/tools/file-chooser.ts create mode 100644 apps/extension/src/tools/file-input-transaction.ts diff --git a/apps/extension/src/tools/__tests__/dispatcher.test.ts b/apps/extension/src/tools/__tests__/dispatcher.test.ts index c05cdb2..113fc20 100644 --- a/apps/extension/src/tools/__tests__/dispatcher.test.ts +++ b/apps/extension/src/tools/__tests__/dispatcher.test.ts @@ -232,6 +232,94 @@ describe("ToolDispatcher", () => { }); }); + it("bypasses and restores the control overlay for an upload trigger click", async () => { + const sendMessage = vi.fn(async () => undefined); + vi.stubGlobal("chrome", { + tabs: { + get: vi.fn(async () => ({ id: 7, windowId: 4242, active: true })), + query: vi.fn(async () => [{ id: 7, windowId: 4242, active: true }]), + sendMessage, + }, + }); + const { transport, sent, deliver } = fakeTransport(); + const sessions = new SessionManager({ + agentWindow: { + create: vi.fn(async () => 4242), + remove: vi.fn(async () => {}), + ensureActiveTab: vi.fn(async () => {}), + }, + }); + const ctx = await sessions.start("aa11"); + ctx.refStore.set("e1", 123, { tabId: 7 }); + const send = vi.fn(async (_tabId: number, method: string, params?: object) => { + if (method === "Page.getLayoutMetrics") { + return { cssLayoutViewport: { clientWidth: 1280, clientHeight: 720 } } as T; + } + if (method === "DOM.getContentQuads") { + return { quads: [[0, 0, 20, 0, 20, 20, 0, 20]] } as T; + } + if (method === "DOM.describeNode") return { node: { backendNodeId: 456 } } as T; + if (method === "Runtime.evaluate") { + const expression = (params as { expression?: string }).expression ?? ""; + if (expression.includes("overlayDetails")) { + return { result: { value: { hitIndex: 0 } } } as T; + } + if (expression.includes("overlayHostPresent")) { + return { + result: { + value: { overlayHostPresent: true, overlayHostConnected: true }, + }, + } as T; + } + if (expression.includes("count:")) { + return { result: { value: { count: 1, multiple: false } } } as T; + } + if (expression.includes("?.inputs[0]")) { + return { result: { objectId: "input-object" } } as T; + } + return { result: { value: true } } as T; + } + return {} as T; + }); + const cdp = { + send, + detachSession: vi.fn(async () => {}), + ensureNetworkCapture: vi.fn(async () => {}), + networkEntriesSince: vi.fn(() => ({ + tab_id: 7, + entries: [], + next_since: 0, + truncated: false, + })), + setDeviceMetricsOverride: vi.fn(async () => {}), + clearDeviceMetricsOverride: vi.fn(async () => {}), + setUserAgentOverride: vi.fn(async () => {}), + setTouchEmulationEnabled: vi.fn(async () => {}), + }; + const dispatcher = new ToolDispatcher({ transport, sessions, cdp: cdp as TestDispatcherCdp }); + dispatcher.start(); + + deliver( + makeRequest("tool.upload", { + session_id: "aa11", + ref: "@e1", + files: [{ transfer_id: "tr_1", name: "test.png", staged_path: "/stage/test.png" }], + }), + ); + await flushMicrotasks(); + await vi.waitFor(() => expect(sent).toHaveLength(1)); + + expect(sent[0]).toMatchObject({ result: { tab_id: 7, file_names: ["test.png"] } }); + expect(sendMessage).toHaveBeenNthCalledWith(1, 7, { + type: "bh-automation-bypass", + enabled: true, + }); + expect(sendMessage).toHaveBeenNthCalledWith(2, 7, { + type: "bh-automation-bypass", + enabled: false, + }); + }); + it("detaches CDP state before stopping a session", async () => { const { transport, sent, deliver } = fakeTransport(); const sessions = new SessionManager({ diff --git a/apps/extension/src/tools/__tests__/file-transfer.test.ts b/apps/extension/src/tools/__tests__/file-transfer.test.ts index 182744b..75d0b7e 100644 --- a/apps/extension/src/tools/__tests__/file-transfer.test.ts +++ b/apps/extension/src/tools/__tests__/file-transfer.test.ts @@ -1,7 +1,8 @@ import { describe, expect, it, vi } from "vitest"; import { SessionManager } from "@/session-manager/manager"; import { type DownloadsApi, handleDownload } from "../download"; -import { uploadThroughFileChooser } from "../file-chooser"; +import { captureBrowserDownload } from "../download-capture"; +import { uploadThroughActivatedFileInput } from "../file-input-transaction"; import type { CdpRunner } from "../shared"; import { handleUpload } from "../upload"; @@ -24,49 +25,57 @@ function tabsApi() { }; } -function fakeEvent void>() { +function fakeEvent unknown>() { const listeners = new Set(); return { addListener: (listener: T) => listeners.add(listener), removeListener: (listener: T) => listeners.delete(listener), - hasListener: (listener: T) => listeners.has(listener), - hasListeners: () => listeners.size > 0, emit: (...args: Parameters) => { for (const listener of listeners) listener(...args); }, }; } +function uploadCdp(options: { inputCount?: number; multiple?: boolean; pickerCall?: string } = {}) { + const calls: Array<{ method: string; params?: object }> = []; + const send = vi.fn(async (_tabId: number, method: string, params?: object) => { + calls.push({ method, params }); + if (method === "Page.getLayoutMetrics") + return { cssLayoutViewport: { clientWidth: 1280, clientHeight: 720 } }; + if (method === "DOM.getContentQuads") return { quads: [[0, 0, 20, 0, 20, 20, 0, 20]] }; + if (method === "Runtime.evaluate") { + const expression = (params as { expression?: string }).expression ?? ""; + if (expression.includes("count:")) { + return { + result: { + value: { + count: options.inputCount ?? 1, + multiple: options.multiple ?? true, + pickerCall: options.pickerCall, + }, + }, + }; + } + if (expression.includes("?.inputs[0]")) { + return { result: { objectId: "input-object" } }; + } + return { result: { value: true } }; + } + if (method === "DOM.describeNode") return { node: { backendNodeId: 456 } }; + return {}; + }); + return { + calls, + cdp: { send: send as unknown as CdpRunner["send"] } satisfies CdpRunner, + }; +} + describe("file transfer tools", () => { - it("enables chooser observation before clicking and injects only staged paths", async () => { + it("captures the file input activated by the requested click and injects only staged paths", async () => { const manager = sessions(); const ctx = await manager.start("s1"); ctx.refStore.set("e3", 123, { tabId: 4 }); - let eventHandler: Parameters>[0] | undefined; - const calls: Array<{ method: string; params?: object }> = []; - const send = vi.fn(async (_tabId: number, method: string, params?: object) => { - calls.push({ method, params }); - if (method === "Page.getLayoutMetrics") - return { cssLayoutViewport: { clientWidth: 1280, clientHeight: 720 } }; - if (method === "DOM.getContentQuads") return { quads: [[0, 0, 20, 0, 20, 20, 0, 20]] }; - if ( - method === "Input.dispatchMouseEvent" && - (params as { type?: string }).type === "mousePressed" - ) { - eventHandler?.({ tabId: 4 }, "Page.fileChooserOpened", { - backendNodeId: 456, - mode: "selectMultiple", - }); - } - return {}; - }); - const cdp: CdpRunner = { - onEvent: (handler) => { - eventHandler = handler; - return { dispose: vi.fn() }; - }, - send: send as unknown as CdpRunner["send"], - }; + const { cdp, calls } = uploadCdp(); const result = await handleUpload( manager, @@ -82,61 +91,60 @@ describe("file transfer tools", () => { ); expect(result).toMatchObject({ tab_id: 4, file_names: ["one.png", "two.png"] }); - expect(calls[0]).toMatchObject({ - method: "Page.enable", - params: { enableFileChooserOpenedEvent: true }, - }); + const armCall = calls.find( + (call) => + call.method === "Runtime.evaluate" && + (call.params as { expression?: string }).expression?.includes("showOpenFilePicker"), + ); + expect(armCall).toBeDefined(); + expect((armCall?.params as { expression?: string }).expression).toContain( + "event.preventDefault()", + ); expect(calls).toContainEqual({ method: "DOM.setFileInputFiles", params: { files: ["/private/stage/one", "/private/stage/two"], backendNodeId: 456 }, }); - expect(calls.at(-1)).toEqual({ - method: "Page.enable", - params: { enableFileChooserOpenedEvent: false }, - }); }); - it("reports File System Access pickers as an explicit manual fallback", async () => { - let eventHandler: Parameters>[0] | undefined; - const cdp: CdpRunner = { - onEvent: (handler) => { - eventHandler = handler; - return { dispose: vi.fn() }; - }, - send: vi.fn(async () => ({})) as unknown as CdpRunner["send"], - }; - - const result = await uploadThroughFileChooser({ + it("fails immediately when the trigger does not activate a file input", async () => { + const { cdp } = uploadCdp({ inputCount: 0 }); + const result = await uploadThroughActivatedFileInput({ cdp, target: { tabId: 4 }, files: ["/private/stage/one"], timeoutMs: 100, - trigger: async () => { - eventHandler?.({ tabId: 4 }, "Page.fileChooserOpened", { mode: "selectSingle" }); - return { tab_id: 4, x: 10, y: 10 }; - }, + trigger: async () => ({ tab_id: 4, x: 10, y: 10 }), }); - expect(result).toEqual({ + expect(result).toMatchObject({ code: "unsupported", - message: "this file chooser is not backed by an input element", - data: { - reason: "unsupported_file_chooser", - chooser_kind: "non_input", - manual_fallback_available: true, - }, + data: { reason: "file_input_not_activated", phase: "resolve_input" }, }); }); - it("bounds a stuck chooser-control command and resets uncertain CDP state", async () => { - const detach = vi.fn(async () => {}); + it("reports a File System Access picker without waiting for a timeout", async () => { + const { cdp } = uploadCdp({ inputCount: 0, pickerCall: "showOpenFilePicker" }); + const result = await uploadThroughActivatedFileInput({ + cdp, + target: { tabId: 4 }, + files: ["/private/stage/one"], + timeoutMs: 100, + trigger: async () => ({ tab_id: 4, x: 10, y: 10 }), + }); + + expect(result).toMatchObject({ + code: "unsupported", + message: "upload trigger invoked showOpenFilePicker instead of an input[type=file]", + data: { reason: "file_input_not_activated", phase: "resolve_input" }, + }); + }); + + it("bounds a stuck file-input probe", async () => { const cdp: CdpRunner = { - onEvent: () => ({ dispose: vi.fn() }), send: vi.fn(() => new Promise(() => {})) as unknown as CdpRunner["send"], - detach, }; - const result = await uploadThroughFileChooser({ + const result = await uploadThroughActivatedFileInput({ cdp, target: { tabId: 4 }, files: ["/private/stage/one"], @@ -146,95 +154,49 @@ describe("file transfer tools", () => { expect(result).toMatchObject({ code: "timeout", - data: { reason: "file_chooser_control_failed", phase: "enable_events" }, - }); - expect(detach).toHaveBeenCalledWith(4); - }); - - it("distinguishes a completed click that did not open a chooser", async () => { - const send = vi.fn(async () => ({})) as unknown as CdpRunner["send"]; - const cdp: CdpRunner = { - onEvent: () => ({ dispose: vi.fn() }), - send, - }; - - const result = await uploadThroughFileChooser({ - cdp, - target: { tabId: 4 }, - files: ["/private/stage/one"], - timeoutMs: 5, - trigger: async () => ({ tab_id: 4, x: 10, y: 10 }), - }); - - expect(result).toMatchObject({ - code: "timeout", - data: { reason: "file_chooser_not_opened", phase: "await_event" }, - }); - expect(send).toHaveBeenLastCalledWith(4, "Page.enable", { - enableFileChooserOpenedEvent: false, + data: { reason: "file_input_probe_failed", phase: "arm_input_probe" }, }); }); - it("accepts chooser events only from the exact CDP target", async () => { - let eventHandler: Parameters>[0] | undefined; - const setFiles = vi.fn(); - const cdp: CdpRunner = { - onEvent: (handler) => { - eventHandler = handler; - return { dispose: vi.fn() }; - }, - send: vi.fn(async () => ({})) as unknown as CdpRunner["send"], - sendToTarget: vi.fn(async (_target, method, params) => { - if (method === "DOM.setFileInputFiles") setFiles(params); - return {}; - }) as unknown as NonNullable, - }; - - const result = await uploadThroughFileChooser({ - cdp, - target: { tabId: 4, sessionId: "child-a" }, - files: ["/private/stage/one"], - timeoutMs: 100, - trigger: async () => { - eventHandler?.({ tabId: 4, sessionId: "child-b" }, "Page.fileChooserOpened", { - backendNodeId: 111, - }); - eventHandler?.({ tabId: 4, sessionId: "child-a" }, "Page.fileChooserOpened", { - backendNodeId: 222, - }); - return { tab_id: 4, x: 10, y: 10 }; - }, - }); - - expect(result).toMatchObject({ click: { tab_id: 4 } }); - expect(setFiles).toHaveBeenCalledOnce(); - expect(setFiles).toHaveBeenCalledWith({ - files: ["/private/stage/one"], - backendNodeId: 222, - }); - }); - - it("captures the one download created under daemon staging", async () => { + it("routes one exact-target download through a browser-relative capability", async () => { const manager = sessions(); const ctx = await manager.start("s1"); ctx.refStore.set("e3", 123, { tabId: 4 }); const onCreated = fakeEvent<(item: chrome.downloads.DownloadItem) => void>(); const onChanged = fakeEvent<(delta: chrome.downloads.DownloadDelta) => void>(); - const item = { + const onDeterminingFilename = + fakeEvent< + ( + item: chrome.downloads.DownloadItem, + suggest: (suggestion?: chrome.downloads.DownloadFilenameSuggestion) => void, + ) => void | true + >(); + const initial = { id: 9, - filename: "/private/stage/tr_1/result.zip", - state: "complete", - fileSize: 12, + url: "https://example.test/result.zip", + finalUrl: "https://example.test/result.zip", + filename: "result.zip", + state: "in_progress", + fileSize: -1, totalBytes: 12, mime: "application/zip", danger: "safe", } as chrome.downloads.DownloadItem; + const completed = { + ...initial, + filename: "/profile/Downloads/BrowserSkill/tr_1/result.zip", + state: "complete", + fileSize: 12, + } as chrome.downloads.DownloadItem; const downloads: DownloadsApi = { - onCreated: onCreated as unknown as DownloadsApi["onCreated"], - onChanged: onChanged as unknown as DownloadsApi["onChanged"], - search: vi.fn(async () => [item]), + onCreated, + onChanged, + onDeterminingFilename, + search: vi.fn(async () => [completed]), cancel: vi.fn(async () => {}), }; + let cdpEvent: Parameters>[0] | undefined; + let suggested: chrome.downloads.DownloadFilenameSuggestion | undefined; const send = vi.fn(async (_tabId: number, method: string, params?: object) => { if (method === "Page.getLayoutMetrics") return { cssLayoutViewport: { clientWidth: 1280, clientHeight: 720 } }; @@ -243,25 +205,103 @@ describe("file transfer tools", () => { method === "Input.dispatchMouseEvent" && (params as { type?: string }).type === "mousePressed" ) { - onCreated.emit(item); + cdpEvent?.({ tabId: 4 }, "Page.downloadWillBegin", { + url: initial.url, + suggestedFilename: "result.zip", + }); + await new Promise((resolve) => { + onDeterminingFilename.emit(initial, (value) => { + suggested = value; + resolve(); + }); + }); + onCreated.emit(completed); } return {}; }); const cdp: CdpRunner = { send: send as unknown as CdpRunner["send"], + onEvent: (handler) => { + cdpEvent = handler; + return { dispose: vi.fn() }; + }, }; const result = await handleDownload( manager, - { session_id: "s1", ref: "@e3", staging_path: "/private/stage/tr_1" }, + { session_id: "s1", ref: "@e3", browser_relative_dir: "BrowserSkill/tr_1" }, { cdp, tabsApi: tabsApi(), downloads }, ); + expect(suggested).toEqual({ + filename: "BrowserSkill/tr_1/result.zip", + conflictAction: "overwrite", + }); expect(result).toMatchObject({ tab_id: 4, suggested_filename: "result.zip", byte_size: 12, - staged_path: item.filename, + browser_path: completed.filename, + }); + }); + + it("does not claim a download without an intent from the exact target", async () => { + const onCreated = fakeEvent<(item: chrome.downloads.DownloadItem) => void>(); + const onChanged = fakeEvent<(delta: chrome.downloads.DownloadDelta) => void>(); + const onDeterminingFilename = + fakeEvent< + ( + item: chrome.downloads.DownloadItem, + suggest: (suggestion?: chrome.downloads.DownloadFilenameSuggestion) => void, + ) => void | true + >(); + const downloads: DownloadsApi = { + onCreated, + onChanged, + onDeterminingFilename, + search: vi.fn(async () => []), + cancel: vi.fn(async () => {}), + }; + let cdpEvent: Parameters>[0] | undefined; + const cdp: CdpRunner = { + send: vi.fn(async () => ({})) as unknown as CdpRunner["send"], + onEvent: (handler) => { + cdpEvent = handler; + return { dispose: vi.fn() }; + }, + }; + const unrelated = { + id: 17, + url: "https://example.test/unrelated.zip", + finalUrl: "https://example.test/unrelated.zip", + filename: "unrelated.zip", + state: "in_progress", + } as chrome.downloads.DownloadItem; + let defaultSuggestionCalled = false; + + const result = await captureBrowserDownload({ + cdp, + target: { tabId: 4, sessionId: "expected-child" }, + downloads, + browserRelativeDir: "BrowserSkill/tr_1", + timeoutMs: 5, + trigger: async () => { + cdpEvent?.({ tabId: 4, sessionId: "other-child" }, "Page.downloadWillBegin", { + url: unrelated.url, + suggestedFilename: unrelated.filename, + }); + onDeterminingFilename.emit(unrelated, (suggestion) => { + defaultSuggestionCalled = suggestion === undefined; + }); + return { tab_id: 4, x: 10, y: 10 }; + }, + }); + + expect(defaultSuggestionCalled).toBe(true); + expect(downloads.cancel).not.toHaveBeenCalled(); + expect(result).toMatchObject({ + code: "cdp_failed", + data: { reason: "download_capture_failed" }, }); }); }); diff --git a/apps/extension/src/tools/dispatcher.ts b/apps/extension/src/tools/dispatcher.ts index c7cca85..f6366f2 100644 --- a/apps/extension/src/tools/dispatcher.ts +++ b/apps/extension/src/tools/dispatcher.ts @@ -526,6 +526,7 @@ export class ToolDispatcher { cdp: this.cdp, tabsApi: chromeTabsApi, signal, + bypassOverlay, }) : Promise.resolve({ code: "unsupported", @@ -542,6 +543,7 @@ export class ToolDispatcher { cdp: this.cdp, tabsApi: chromeTabsApi, signal, + bypassOverlay, }) : Promise.resolve({ code: "unsupported", diff --git a/apps/extension/src/tools/download-capture.ts b/apps/extension/src/tools/download-capture.ts new file mode 100644 index 0000000..afab991 --- /dev/null +++ b/apps/extension/src/tools/download-capture.ts @@ -0,0 +1,211 @@ +// Transaction-scoped capture for one web download. Chrome owns the actual +// download and writes only beneath its Downloads root; BrowserSkill supplies a +// daemon-minted relative directory and reports the completed absolute path +// back to the daemon for validated import. + +import type { CdpTarget } from "@/browser-driver/frame-graph"; +import type { ClickResult, RpcError } from "@/transport/types"; +import { type CdpRunner, isRpcError } from "./shared"; + +type DeterminingFilenameListener = ( + item: chrome.downloads.DownloadItem, + suggest: (suggestion?: chrome.downloads.DownloadFilenameSuggestion) => void, +) => void | true; + +interface ListenerEvent { + addListener(listener: T): void; + removeListener(listener: T): void; +} + +export interface DownloadsApi { + onCreated: ListenerEvent<(item: chrome.downloads.DownloadItem) => void>; + onChanged: ListenerEvent<(delta: chrome.downloads.DownloadDelta) => void>; + onDeterminingFilename: ListenerEvent; + search(query: chrome.downloads.DownloadQuery): Promise; + cancel(downloadId: number): Promise; +} + +export const chromeDownloadsApi: DownloadsApi = { + get onCreated() { + return chrome.downloads.onCreated; + }, + get onChanged() { + return chrome.downloads.onChanged; + }, + get onDeterminingFilename() { + return chrome.downloads.onDeterminingFilename; + }, + search: (query) => chrome.downloads.search(query), + cancel: (id) => chrome.downloads.cancel(id), +}; + +export interface DownloadCaptureOptions { + cdp: CdpRunner; + target: CdpTarget; + downloads: DownloadsApi; + browserRelativeDir: string; + timeoutMs: number; + signal?: AbortSignal; + trigger(): Promise; +} + +export interface DownloadCaptureResult { + click: ClickResult; + item: chrome.downloads.DownloadItem; +} + +function safeBasename(filename: string): string { + const basename = filename.split(/[\\/]/).pop()?.trim(); + return basename && basename !== "." && basename !== ".." ? basename : "download"; +} + +function captureError(message: string): RpcError { + return { + code: "cdp_failed", + message, + data: { reason: "download_capture_failed" }, + }; +} + +function sameTarget(source: { tabId?: number; sessionId?: string }, target: CdpTarget): boolean { + return source.tabId === target.tabId && source.sessionId === target.sessionId; +} + +function matchesIntent( + item: chrome.downloads.DownloadItem, + intent: { url: string; suggestedFilename: string }, +): boolean { + const urlMatches = item.url === intent.url || item.finalUrl === intent.url; + return urlMatches && safeBasename(item.filename) === safeBasename(intent.suggestedFilename); +} + +export async function captureBrowserDownload( + options: DownloadCaptureOptions, +): Promise { + let capturedId: number | undefined; + let intent: { url: string; suggestedFilename: string } | undefined; + const createdItems = new Map(); + let settled = false; + let succeeded = false; + let timer: ReturnType | undefined; + let rejectCompletion!: (error: Error) => void; + let resolveCompletion!: (item: chrome.downloads.DownloadItem) => void; + + const completion = new Promise((resolve, reject) => { + resolveCompletion = resolve; + rejectCompletion = reject; + }); + const fail = (error: Error) => { + if (settled) return; + settled = true; + rejectCompletion(error); + }; + const complete = (item: chrome.downloads.DownloadItem) => { + if (settled) return; + settled = true; + resolveCompletion(item); + }; + const claim = (item: chrome.downloads.DownloadItem): boolean => { + if (capturedId === undefined) { + capturedId = item.id; + return true; + } + if (capturedId === item.id) return true; + void options.downloads.cancel(item.id).catch(() => undefined); + fail(new Error("download trigger produced more than one file")); + return false; + }; + + const determiningListener: DeterminingFilenameListener = (item, suggest) => { + try { + if (!intent || !matchesIntent(item, intent) || !claim(item)) { + suggest(); + return; + } + suggest({ + filename: `${options.browserRelativeDir}/${safeBasename(intent.suggestedFilename)}`, + conflictAction: "overwrite", + }); + const created = createdItems.get(item.id); + if (created?.state === "complete") complete(created); + } catch (err) { + suggest(); + fail(err instanceof Error ? err : new Error(String(err))); + } + }; + const createdListener = (item: chrome.downloads.DownloadItem) => { + createdItems.set(item.id, item); + if (capturedId !== item.id) return; + if (item.state === "interrupted") { + fail(new Error(item.error ?? "download interrupted")); + } else if (item.state === "complete") { + complete(item); + } + }; + const changedListener = async (delta: chrome.downloads.DownloadDelta) => { + if (capturedId === undefined || delta.id !== capturedId || settled) return; + if (delta.state?.current === "interrupted" || delta.error?.current) { + fail(new Error(delta.error?.current ?? "download interrupted")); + return; + } + if (delta.state?.current === "complete") { + try { + const [item] = await options.downloads.search({ id: delta.id }); + if (item) complete(item); + else fail(new Error("completed download disappeared")); + } catch (err) { + fail(err instanceof Error ? err : new Error(String(err))); + } + } + }; + const onAbort = () => { + fail(new DOMException("aborted", "AbortError")); + }; + const cdpSubscription = options.cdp.onEvent?.((source, method, raw) => { + if (method !== "Page.downloadWillBegin" || !sameTarget(source, options.target)) return; + const event = raw as { url?: unknown; suggestedFilename?: unknown }; + if (typeof event.url !== "string" || typeof event.suggestedFilename !== "string") return; + if (intent) { + fail(new Error("download trigger produced more than one browser download intent")); + return; + } + intent = { url: event.url, suggestedFilename: event.suggestedFilename }; + }); + if (!cdpSubscription) { + return captureError("CDP download intent subscription unavailable"); + } + + options.downloads.onDeterminingFilename.addListener(determiningListener); + options.downloads.onCreated.addListener(createdListener); + options.downloads.onChanged.addListener(changedListener); + options.signal?.addEventListener("abort", onAbort, { once: true }); + timer = setTimeout( + () => fail(new Error("download did not complete before timeout")), + options.timeoutMs, + ); + + try { + const triggered = await options.trigger(); + if (isRpcError(triggered)) { + void completion.catch(() => undefined); + return triggered; + } + const item = await completion; + succeeded = true; + return { click: triggered, item }; + } catch (err) { + if (err instanceof DOMException && err.name === "AbortError") throw err; + return captureError(err instanceof Error ? err.message : String(err)); + } finally { + settled = true; + if (timer) clearTimeout(timer); + if (!succeeded && capturedId !== undefined) { + await options.downloads.cancel(capturedId).catch(() => undefined); + } + options.signal?.removeEventListener("abort", onAbort); + options.downloads.onDeterminingFilename.removeListener(determiningListener); + options.downloads.onCreated.removeListener(createdListener); + options.downloads.onChanged.removeListener(changedListener); + cdpSubscription.dispose(); + } +} diff --git a/apps/extension/src/tools/download.ts b/apps/extension/src/tools/download.ts index e1b942e..d337315 100644 --- a/apps/extension/src/tools/download.ts +++ b/apps/extension/src/tools/download.ts @@ -1,31 +1,16 @@ -// One-click/one-file browser download capture. chrome.downloads events are -// browser-global, so a coordinator prevents two bsk sessions from racing. +// Download orchestration: validate session/tab ownership, then delegate one +// browser-global chrome.downloads transaction to download-capture.ts. import type { SessionManager } from "@/session-manager/manager"; import type { ClickParams, DownloadParams, DownloadResult, RpcError } from "@/transport/types"; -import { handleClick, type InteractionDeps } from "./interaction"; +import { captureBrowserDownload, chromeDownloadsApi, type DownloadsApi } from "./download-capture"; +import { handleClick, type InteractionDeps, resolveBackendNode } from "./interaction"; import { enforceAgentWindow, isRpcError, lookupSession, resolveTargetTab } from "./shared"; -export interface DownloadsApi { - onCreated: chrome.events.Event<(item: chrome.downloads.DownloadItem) => void>; - onChanged: chrome.events.Event<(delta: chrome.downloads.DownloadDelta) => void>; - search(query: chrome.downloads.DownloadQuery): Promise; - cancel(downloadId: number): Promise; -} - -const chromeDownloadsApi: DownloadsApi = { - get onCreated() { - return chrome.downloads.onCreated; - }, - get onChanged() { - return chrome.downloads.onChanged; - }, - search: (query) => chrome.downloads.search(query), - cancel: (id) => chrome.downloads.cancel(id), -}; - let downloadActive = false; +export type { DownloadsApi } from "./download-capture"; + export interface DownloadDeps extends InteractionDeps { downloads?: DownloadsApi; } @@ -37,11 +22,6 @@ export async function handleDownload( ): Promise { if (downloadActive) return { code: "invalid_params", message: "another bsk download is active" }; downloadActive = true; - const downloads = deps.downloads ?? chromeDownloadsApi; - let capturedId: number | undefined; - let createdListener: ((item: chrome.downloads.DownloadItem) => void) | undefined; - let changedListener: ((delta: chrome.downloads.DownloadDelta) => void) | undefined; - let timer: ReturnType | undefined; try { const ctx = lookupSession(manager, params, "download"); if (isRpcError(ctx)) return ctx; @@ -49,90 +29,43 @@ export async function handleDownload( if (isRpcError(target)) return target; const denied = enforceAgentWindow(ctx, target, "download"); if (denied) return denied; - if (!params.staging_path) - return { code: "invalid_params", message: "download requires daemon staging" }; - const normalisePath = (path: string) => path.replaceAll("\\", "/"); - const normalisedStaging = normalisePath(params.staging_path); - const stagingPrefix = normalisedStaging.endsWith("/") - ? normalisedStaging - : `${normalisedStaging}/`; - let rejectCompleted!: (error: Error) => void; - const completed = new Promise((resolve, reject) => { - rejectCompleted = reject; - createdListener = (item) => { - if (!normalisePath(item.filename).startsWith(stagingPrefix)) return; - if (capturedId !== undefined && capturedId !== item.id) { - reject(new Error("download trigger produced more than one file")); - return; - } - capturedId = item.id; - if (item.state === "complete") resolve(item); - }; - changedListener = async (delta) => { - if (capturedId === undefined || delta.id !== capturedId) return; - if (delta.state?.current === "interrupted" || delta.error?.current) { - reject(new Error(delta.error?.current ?? "download interrupted")); - return; - } - if (delta.state?.current === "complete") { - const [item] = await downloads.search({ id: delta.id }); - if (item) resolve(item); - else reject(new Error("completed download disappeared")); - } - }; - downloads.onCreated.addListener(createdListener); - downloads.onChanged.addListener(changedListener); - timer = setTimeout( - () => reject(new Error("download did not complete before timeout")), - params.timeout_ms ?? 120_000, - ); - }); - const onAbort = () => { - if (capturedId !== undefined) void downloads.cancel(capturedId).catch(() => undefined); - rejectCompleted(new DOMException("aborted", "AbortError")); - }; - deps.signal?.addEventListener("abort", onAbort, { once: true }); - try { - await deps.cdp.send(target.tabId, "Page.setDownloadBehavior", { - behavior: "allow", - downloadPath: params.staging_path, - }); - const clickParams: ClickParams = { - session_id: params.session_id, - ref: params.ref, - selector: params.selector, - tab_id: params.tab_id, - timeout_ms: params.timeout_ms, - }; - const clicked = await handleClick(manager, clickParams, deps); - if (isRpcError(clicked)) { - void completed.catch(() => undefined); - return clicked; - } - const item = await completed; - return { - tab_id: target.tabId, - used_ref: clicked.used_ref, - used_selector: clicked.used_selector, - suggested_filename: item.filename.split(/[\\/]/).pop() ?? "download", - byte_size: item.fileSize >= 0 ? item.fileSize : item.totalBytes, - mime: item.mime || undefined, - danger: item.danger, - staged_path: item.filename, - }; - } finally { - deps.signal?.removeEventListener("abort", onAbort); - try { - await deps.cdp.send(target.tabId, "Page.setDownloadBehavior", { behavior: "default" }); - } catch {} + if (!params.browser_relative_dir) { + return { code: "invalid_params", message: "download requires a daemon capability directory" }; } - } catch (err) { - if (err instanceof DOMException && err.name === "AbortError") throw err; - return { code: "cdp_failed", message: err instanceof Error ? err.message : String(err) }; + const address = await resolveBackendNode(deps.cdp, ctx, target, params, "download"); + if (isRpcError(address)) return address; + + const capture = await captureBrowserDownload({ + cdp: deps.cdp, + target: address.cdpTarget, + downloads: deps.downloads ?? chromeDownloadsApi, + browserRelativeDir: params.browser_relative_dir, + timeoutMs: params.timeout_ms ?? 120_000, + signal: deps.signal, + trigger: () => { + const clickParams: ClickParams = { + session_id: params.session_id, + ref: params.ref, + selector: params.selector, + tab_id: params.tab_id, + timeout_ms: params.timeout_ms, + }; + return handleClick(manager, clickParams, deps); + }, + }); + if (isRpcError(capture)) return capture; + const { click, item } = capture; + return { + tab_id: target.tabId, + used_ref: click.used_ref, + used_selector: click.used_selector, + suggested_filename: item.filename.split(/[\\/]/).pop() ?? "download", + byte_size: item.fileSize >= 0 ? item.fileSize : item.totalBytes, + mime: item.mime || undefined, + danger: item.danger, + browser_path: item.filename, + }; } finally { - if (timer) clearTimeout(timer); - if (createdListener) downloads.onCreated.removeListener(createdListener); - if (changedListener) downloads.onChanged.removeListener(changedListener); downloadActive = false; } } diff --git a/apps/extension/src/tools/file-chooser.ts b/apps/extension/src/tools/file-chooser.ts deleted file mode 100644 index ab72241..0000000 --- a/apps/extension/src/tools/file-chooser.ts +++ /dev/null @@ -1,265 +0,0 @@ -// One file-chooser transaction for an upload. Chrome's extension debugger -// bridge may leave a command pending indefinitely, so every state-changing -// command is bounded and an uncertain target is detached before returning. - -import type { CdpTarget } from "@/browser-driver/frame-graph"; -import type { ClickResult, RpcError } from "@/transport/types"; -import { type CdpRunner, isRpcError, sendToCdpTarget } from "./shared"; - -const CLEANUP_TIMEOUT_MS = 1_000; - -type ChooserPhase = "enable_events" | "trigger" | "await_event" | "set_files"; - -interface FileChooserOpened { - source: { tabId?: number; sessionId?: string }; - backendNodeId?: number; - mode?: string; -} - -export interface FileChooserUploadOptions { - cdp: CdpRunner; - target: CdpTarget; - files: string[]; - timeoutMs: number; - signal?: AbortSignal; - trigger(timeoutMs: number): Promise; -} - -export interface FileChooserUploadResult { - click: ClickResult; - mode?: string; -} - -class BoundedWaitError extends Error { - constructor( - readonly kind: "timeout" | "aborted", - message: string, - ) { - super(message); - } -} - -function remainingMs(deadline: number): number { - return Math.max(0, deadline - Date.now()); -} - -async function waitBounded( - promise: Promise, - deadline: number, - signal: AbortSignal | undefined, - timeoutMessage: string, -): Promise { - const remaining = remainingMs(deadline); - if (signal?.aborted) throw new BoundedWaitError("aborted", "file chooser operation aborted"); - if (remaining === 0) throw new BoundedWaitError("timeout", timeoutMessage); - - let timer: ReturnType | undefined; - let onAbort: (() => void) | undefined; - const boundary = new Promise((_resolve, reject) => { - timer = setTimeout(() => reject(new BoundedWaitError("timeout", timeoutMessage)), remaining); - if (signal) { - onAbort = () => reject(new BoundedWaitError("aborted", "file chooser operation aborted")); - signal.addEventListener("abort", onAbort, { once: true }); - } - }); - try { - return await Promise.race([promise, boundary]); - } finally { - if (timer) clearTimeout(timer); - if (signal && onAbort) signal.removeEventListener("abort", onAbort); - } -} - -function chooserError( - code: RpcError["code"], - message: string, - reason: "file_chooser_control_failed" | "file_chooser_not_opened", - phase: ChooserPhase, -): RpcError { - return { code, message, data: { reason, phase } }; -} - -function sameTarget(source: FileChooserOpened["source"], target: CdpTarget): boolean { - return source.tabId === target.tabId && source.sessionId === target.sessionId; -} - -async function resetTarget(cdp: CdpRunner, tabId: number): Promise { - if (!cdp.detach) return; - try { - await waitBounded( - cdp.detach(tabId), - Date.now() + CLEANUP_TIMEOUT_MS, - undefined, - "CDP detach timed out", - ); - } catch { - // The target may already have detached or closed. - } -} - -export async function uploadThroughFileChooser( - options: FileChooserUploadOptions, -): Promise { - const deadline = Date.now() + options.timeoutMs; - let resolveOpened!: (value: FileChooserOpened) => void; - const opened = new Promise((resolve) => { - resolveOpened = resolve; - }); - const subscription = options.cdp.onEvent?.((source, method, raw) => { - if (method !== "Page.fileChooserOpened" || !sameTarget(source, options.target)) return; - const event = raw as { backendNodeId?: unknown; mode?: unknown }; - resolveOpened({ - source, - ...(typeof event.backendNodeId === "number" ? { backendNodeId: event.backendNodeId } : {}), - ...(typeof event.mode === "string" ? { mode: event.mode } : {}), - }); - }); - if (!subscription) { - return { code: "unsupported", message: "CDP event subscription unavailable" }; - } - - let eventsEnabled = false; - let targetStateUnknown = false; - let outcome: FileChooserUploadResult | RpcError; - let aborted: BoundedWaitError | undefined; - try { - try { - await waitBounded( - sendToCdpTarget(options.cdp, options.target, "Page.enable", { - enableFileChooserOpenedEvent: true, - }), - deadline, - options.signal, - "enabling file chooser events timed out", - ); - eventsEnabled = true; - } catch (err) { - if (err instanceof BoundedWaitError) { - targetStateUnknown = true; - if (err.kind === "aborted") throw err; - outcome = chooserError( - "timeout", - err.message, - "file_chooser_control_failed", - "enable_events", - ); - } else { - outcome = chooserError( - "cdp_failed", - err instanceof Error ? err.message : String(err), - "file_chooser_control_failed", - "enable_events", - ); - } - return outcome; - } - - let click: ClickResult | RpcError; - try { - click = await waitBounded( - options.trigger(remainingMs(deadline)), - deadline, - options.signal, - "upload trigger timed out", - ); - } catch (err) { - if (err instanceof BoundedWaitError) { - targetStateUnknown = true; - if (err.kind === "aborted") throw err; - return chooserError("timeout", err.message, "file_chooser_control_failed", "trigger"); - } - return chooserError( - "cdp_failed", - err instanceof Error ? err.message : String(err), - "file_chooser_control_failed", - "trigger", - ); - } - if (isRpcError(click)) return click; - - let chooser: FileChooserOpened; - try { - chooser = await waitBounded( - opened, - deadline, - options.signal, - "upload trigger did not open a file chooser", - ); - } catch (err) { - if (err instanceof BoundedWaitError && err.kind === "aborted") throw err; - return chooserError( - "timeout", - err instanceof Error ? err.message : String(err), - "file_chooser_not_opened", - "await_event", - ); - } - - if (chooser.backendNodeId === undefined) { - return { - code: "unsupported", - message: "this file chooser is not backed by an input element", - data: { - reason: "unsupported_file_chooser", - chooser_kind: "non_input", - manual_fallback_available: true, - }, - }; - } - if (chooser.mode === "selectSingle" && options.files.length !== 1) { - return { code: "invalid_params", message: "file chooser accepts exactly one file" }; - } - - try { - await waitBounded( - sendToCdpTarget(options.cdp, options.target, "DOM.setFileInputFiles", { - files: options.files, - backendNodeId: chooser.backendNodeId, - }), - deadline, - options.signal, - "setting file input files timed out", - ); - } catch (err) { - if (err instanceof BoundedWaitError) { - targetStateUnknown = true; - if (err.kind === "aborted") throw err; - return chooserError("timeout", err.message, "file_chooser_control_failed", "set_files"); - } - return chooserError( - "cdp_failed", - err instanceof Error ? err.message : String(err), - "file_chooser_control_failed", - "set_files", - ); - } - outcome = { click, ...(chooser.mode ? { mode: chooser.mode } : {}) }; - } catch (err) { - if (err instanceof BoundedWaitError && err.kind === "aborted") aborted = err; - else throw err; - outcome = { code: "cancelled", message: "file chooser operation aborted" }; - } finally { - subscription.dispose(); - if (targetStateUnknown) { - await resetTarget(options.cdp, options.target.tabId); - } else if (eventsEnabled) { - try { - await waitBounded( - sendToCdpTarget(options.cdp, options.target, "Page.enable", { - enableFileChooserOpenedEvent: false, - }), - Date.now() + CLEANUP_TIMEOUT_MS, - undefined, - "restoring file chooser events timed out", - ); - } catch { - await resetTarget(options.cdp, options.target.tabId); - } - } - } - if (aborted) { - const error = new DOMException(aborted.message, "AbortError"); - throw error; - } - return outcome; -} diff --git a/apps/extension/src/tools/file-input-transaction.ts b/apps/extension/src/tools/file-input-transaction.ts new file mode 100644 index 0000000..50297ad --- /dev/null +++ b/apps/extension/src/tools/file-input-transaction.ts @@ -0,0 +1,339 @@ +// One upload transaction, independent of Page.fileChooserOpened delivery. +// A transaction-scoped DOM listener records the file input actually activated +// by the requested click and cancels its native default action. File System +// Access picker entry points are replaced only for the same transaction, so a +// non-input picker fails promptly without opening an OS dialog. + +import type { CdpTarget } from "@/browser-driver/frame-graph"; +import type { ClickResult, RpcError } from "@/transport/types"; +import { type CdpRunner, isRpcError, sendToCdpTarget } from "./shared"; + +const CLEANUP_TIMEOUT_MS = 1_000; + +type UploadPhase = "arm_input_probe" | "trigger" | "resolve_input" | "set_files"; + +interface RuntimeReply { + result?: { + value?: unknown; + objectId?: string; + }; + exceptionDetails?: { + text?: string; + exception?: { description?: string }; + }; +} + +export interface FileInputTransactionOptions { + cdp: CdpRunner; + target: CdpTarget; + files: string[]; + timeoutMs: number; + signal?: AbortSignal; + trigger(timeoutMs: number): Promise; +} + +export interface FileInputTransactionResult { + click: ClickResult; + multiple: boolean; +} + +class BoundedWaitError extends Error { + constructor( + readonly kind: "timeout" | "aborted", + message: string, + ) { + super(message); + } +} + +function remainingMs(deadline: number): number { + return Math.max(0, deadline - Date.now()); +} + +async function waitBounded( + promise: Promise, + deadline: number, + signal: AbortSignal | undefined, + timeoutMessage: string, +): Promise { + const remaining = remainingMs(deadline); + if (signal?.aborted) throw new BoundedWaitError("aborted", "upload transaction aborted"); + if (remaining === 0) throw new BoundedWaitError("timeout", timeoutMessage); + + let timer: ReturnType | undefined; + let onAbort: (() => void) | undefined; + const boundary = new Promise((_resolve, reject) => { + timer = setTimeout(() => reject(new BoundedWaitError("timeout", timeoutMessage)), remaining); + if (signal) { + onAbort = () => reject(new BoundedWaitError("aborted", "upload transaction aborted")); + signal.addEventListener("abort", onAbort, { once: true }); + } + }); + try { + return await Promise.race([promise, boundary]); + } finally { + if (timer) clearTimeout(timer); + if (signal && onAbort) signal.removeEventListener("abort", onAbort); + } +} + +function uploadError( + code: RpcError["code"], + message: string, + reason: "file_input_probe_failed" | "file_input_not_activated" | "set_file_input_failed", + phase: UploadPhase, +): RpcError { + return { code, message, data: { reason, phase } }; +} + +function runtimeError(reply: RuntimeReply, fallback: string): Error | null { + if (!reply.exceptionDetails) return null; + return new Error( + reply.exceptionDetails.exception?.description ?? reply.exceptionDetails.text ?? fallback, + ); +} + +export async function uploadThroughActivatedFileInput( + options: FileInputTransactionOptions, +): Promise { + const deadline = Date.now() + options.timeoutMs; + const objectGroup = `bsk-upload-${crypto.randomUUID()}`; + const stateKey = `__bskUpload_${crypto.randomUUID().replaceAll("-", "")}`; + const stateKeyLiteral = JSON.stringify(stateKey); + let probeAttempted = false; + + try { + try { + probeAttempted = true; + const armed = await waitBounded( + sendToCdpTarget(options.cdp, options.target, "Runtime.evaluate", { + expression: `(() => { + const key = ${stateKeyLiteral}; + const owner = globalThis; + const doc = document; + const state = { inputs: [], listener: null, pickerCalls: [], pickers: [] }; + Object.defineProperty(owner, key, { value: state, configurable: true }); + state.listener = event => { + const path = typeof event.composedPath === "function" ? event.composedPath() : []; + const candidate = path[0] || event.target; + if (candidate && candidate.nodeType === 1 && + candidate.localName === "input" && candidate.type === "file") { + if (!state.inputs.includes(candidate)) state.inputs.push(candidate); + event.preventDefault(); + } + }; + doc.addEventListener("click", state.listener, true); + const win = doc.defaultView; + for (const name of ["showOpenFilePicker", "showSaveFilePicker", "showDirectoryPicker"]) { + if (!win || typeof win[name] !== "function") continue; + const hadOwn = Object.prototype.hasOwnProperty.call(win, name); + const descriptor = Object.getOwnPropertyDescriptor(win, name); + state.pickers.push({ name, hadOwn, descriptor }); + Object.defineProperty(win, name, { + configurable: true, + enumerable: descriptor?.enumerable ?? true, + writable: true, + value: () => { + state.pickerCalls.push(name); + return Promise.reject( + new DOMException("Picker intercepted by BrowserSkill", "AbortError") + ); + }, + }); + } + return true; + })()`, + objectGroup, + returnByValue: true, + }), + deadline, + options.signal, + "arming file input probe timed out", + ); + const armError = runtimeError(armed, "failed to arm file input probe"); + if (armError) throw armError; + } catch (err) { + if (err instanceof BoundedWaitError && err.kind === "aborted") throw err; + return uploadError( + err instanceof BoundedWaitError ? "timeout" : "cdp_failed", + err instanceof Error ? err.message : String(err), + "file_input_probe_failed", + "arm_input_probe", + ); + } + + let click: ClickResult | RpcError; + try { + click = await waitBounded( + options.trigger(remainingMs(deadline)), + deadline, + options.signal, + "upload trigger timed out", + ); + } catch (err) { + if (err instanceof BoundedWaitError && err.kind === "aborted") throw err; + return uploadError( + err instanceof BoundedWaitError ? "timeout" : "cdp_failed", + err instanceof Error ? err.message : String(err), + "file_input_probe_failed", + "trigger", + ); + } + if (isRpcError(click)) return click; + + try { + const summary = await waitBounded( + sendToCdpTarget(options.cdp, options.target, "Runtime.evaluate", { + expression: `(() => { + const s = globalThis[${stateKeyLiteral}]; + return s + ? { count: s.inputs.length, multiple: s.inputs[0]?.multiple === true, + pickerCall: s.pickerCalls[0] } + : { count: 0, multiple: false }; + })()`, + returnByValue: true, + }), + deadline, + options.signal, + "resolving activated file input timed out", + ); + const summaryError = runtimeError(summary, "failed to inspect activated file input"); + if (summaryError) throw summaryError; + const value = summary.result?.value as + | { count?: unknown; multiple?: unknown; pickerCall?: unknown } + | undefined; + const count = typeof value?.count === "number" ? value.count : 0; + const multiple = value?.multiple === true; + if (typeof value?.pickerCall === "string") { + return uploadError( + "unsupported", + `upload trigger invoked ${value.pickerCall} instead of an input[type=file]`, + "file_input_not_activated", + "resolve_input", + ); + } + if (count !== 1) { + return uploadError( + "unsupported", + count === 0 + ? "upload trigger did not activate an input[type=file]" + : "upload trigger activated more than one input[type=file]", + "file_input_not_activated", + "resolve_input", + ); + } + if (!multiple && options.files.length !== 1) { + return { code: "invalid_params", message: "file input accepts exactly one file" }; + } + + const input = await waitBounded( + sendToCdpTarget(options.cdp, options.target, "Runtime.evaluate", { + expression: `globalThis[${stateKeyLiteral}]?.inputs[0]`, + objectGroup, + returnByValue: false, + }), + deadline, + options.signal, + "resolving activated file input object timed out", + ); + const inputError = runtimeError(input, "failed to resolve activated file input object"); + if (inputError) throw inputError; + const inputObjectId = input.result?.objectId; + if (!inputObjectId) throw new Error("activated file input returned no objectId"); + + const described = await waitBounded( + sendToCdpTarget<{ node?: { backendNodeId?: number } }>( + options.cdp, + options.target, + "DOM.describeNode", + { objectId: inputObjectId }, + ), + deadline, + options.signal, + "describing activated file input timed out", + ); + const backendNodeId = described.node?.backendNodeId; + if (typeof backendNodeId !== "number") { + throw new Error("DOM.describeNode returned no file input backendNodeId"); + } + + try { + await waitBounded( + sendToCdpTarget(options.cdp, options.target, "DOM.setFileInputFiles", { + files: options.files, + backendNodeId, + }), + deadline, + options.signal, + "setting file input files timed out", + ); + } catch (err) { + if (err instanceof BoundedWaitError && err.kind === "aborted") throw err; + return uploadError( + err instanceof BoundedWaitError ? "timeout" : "cdp_failed", + err instanceof Error ? err.message : String(err), + "set_file_input_failed", + "set_files", + ); + } + return { click, multiple }; + } catch (err) { + if (err instanceof BoundedWaitError && err.kind === "aborted") throw err; + return uploadError( + err instanceof BoundedWaitError ? "timeout" : "cdp_failed", + err instanceof Error ? err.message : String(err), + "file_input_probe_failed", + "resolve_input", + ); + } + } catch (err) { + if (err instanceof BoundedWaitError && err.kind === "aborted") { + return { code: "cancelled", message: "upload transaction aborted" }; + } + throw err; + } finally { + if (probeAttempted) { + try { + await waitBounded( + sendToCdpTarget(options.cdp, options.target, "Runtime.evaluate", { + expression: `(() => { + const key = ${stateKeyLiteral}; + const state = globalThis[key]; + if (state?.listener) { + document.removeEventListener("click", state.listener, true); + } + const win = document.defaultView; + if (win) { + for (const picker of state?.pickers || []) { + try { + if (picker.hadOwn && picker.descriptor) { + Object.defineProperty(win, picker.name, picker.descriptor); + } else { + delete win[picker.name]; + } + } catch {} + } + } + delete globalThis[key]; + })()`, + }), + Date.now() + CLEANUP_TIMEOUT_MS, + undefined, + "cleaning file input probe timed out", + ); + } catch { + // Navigation may have invalidated the object; its document is gone too. + } + } + try { + await waitBounded( + sendToCdpTarget(options.cdp, options.target, "Runtime.releaseObjectGroup", { + objectGroup, + }), + Date.now() + CLEANUP_TIMEOUT_MS, + undefined, + "releasing upload object group timed out", + ); + } catch {} + } +} diff --git a/apps/extension/src/tools/interaction.ts b/apps/extension/src/tools/interaction.ts index 182cabd..3ea1373 100644 --- a/apps/extension/src/tools/interaction.ts +++ b/apps/extension/src/tools/interaction.ts @@ -128,7 +128,7 @@ async function wait(ms: number, signal?: AbortSignal): Promise { * `RpcError` if the caller supplied neither (or both), or if neither * lookup matched. */ -async function resolveBackendNode( +export async function resolveBackendNode( cdp: CdpRunner, ctx: SessionContext, target: { tabId: number }, diff --git a/apps/extension/src/tools/upload.ts b/apps/extension/src/tools/upload.ts index e131392..6622bc4 100644 --- a/apps/extension/src/tools/upload.ts +++ b/apps/extension/src/tools/upload.ts @@ -1,11 +1,11 @@ // Upload orchestration: validate the session-scoped request, resolve its click -// target, then delegate the browser protocol transaction to file-chooser.ts. +// target, then delegate the browser protocol transaction to the file-input +// transaction module. -import type { CdpTarget } from "@/browser-driver/frame-graph"; import type { SessionManager } from "@/session-manager/manager"; import type { ClickParams, RpcError, UploadParams, UploadResult } from "@/transport/types"; -import { uploadThroughFileChooser } from "./file-chooser"; -import { handleClick, type InteractionDeps } from "./interaction"; +import { uploadThroughActivatedFileInput } from "./file-input-transaction"; +import { handleClick, type InteractionDeps, resolveBackendNode } from "./interaction"; import { type CdpRunner, enforceAgentWindow, @@ -13,7 +13,6 @@ import { lookupSession, resolveTargetTab, } from "./shared"; -import { resolveSnapshotRef } from "./snapshot-ref"; const DEFAULT_TIMEOUT_MS = 120_000; @@ -21,21 +20,6 @@ export interface UploadDeps extends InteractionDeps { cdp: CdpRunner; } -function chooserTarget( - params: UploadParams, - tabId: number, - manager: SessionManager, -): CdpTarget | RpcError { - const ctx = manager.get(params.session_id); - if (!ctx) return { code: "not_found", message: `session ${params.session_id} unknown` }; - if (params.ref) { - const ref = resolveSnapshotRef(ctx, params.ref, tabId); - if (isRpcError(ref)) return ref; - return { tabId, ...(ref.cdpSessionId ? { sessionId: ref.cdpSessionId } : {}) }; - } - return { tabId }; -} - export async function handleUpload( manager: SessionManager, params: UploadParams, @@ -47,8 +31,6 @@ export async function handleUpload( if (isRpcError(target)) return target; const denied = enforceAgentWindow(ctx, target, "upload"); if (denied) return denied; - if (!deps.cdp?.onEvent) - return { code: "unsupported", message: "CDP event subscription unavailable" }; if ( params.files.length === 0 || params.files.length > 20 || @@ -56,12 +38,12 @@ export async function handleUpload( ) { return { code: "invalid_params", message: "upload requires daemon-staged files" }; } - const cdpTarget = chooserTarget(params, target.tabId, manager); - if (isRpcError(cdpTarget)) return cdpTarget; + const address = await resolveBackendNode(deps.cdp, ctx, target, params, "upload"); + if (isRpcError(address)) return address; const timeoutMs = params.timeout_ms ?? DEFAULT_TIMEOUT_MS; - const transaction = await uploadThroughFileChooser({ + const transaction = await uploadThroughActivatedFileInput({ cdp: deps.cdp, - target: cdpTarget, + target: address.cdpTarget, files: params.files.map((file) => file.staged_path as string), timeoutMs, signal: deps.signal, diff --git a/apps/extension/src/transport/types.ts b/apps/extension/src/transport/types.ts index c161169..4504ff1 100644 --- a/apps/extension/src/transport/types.ts +++ b/apps/extension/src/transport/types.ts @@ -33,9 +33,10 @@ export type RpcErrorReason = | "restricted_tab_url" | "borrow_conflict" | "screenshot_capture_failed" - | "file_chooser_control_failed" - | "file_chooser_not_opened" - | "unsupported_file_chooser" + | "file_input_probe_failed" + | "file_input_not_activated" + | "set_file_input_failed" + | "download_capture_failed" | "cleanup_failed"; export interface RpcErrorData { @@ -531,7 +532,7 @@ export interface DownloadParams { selector?: string; tab_id?: number; timeout_ms?: number; - staging_path?: string; + browser_relative_dir?: string; } export interface DownloadResult { @@ -542,7 +543,7 @@ export interface DownloadResult { byte_size: number; mime?: string; danger?: string; - staged_path?: string; + browser_path?: string; transfer_id?: string; } diff --git a/crates/bsk-cli/skill/SKILL.md b/crates/bsk-cli/skill/SKILL.md index 3d10831..4233d1f 100644 --- a/crates/bsk-cli/skill/SKILL.md +++ b/crates/bsk-cli/skill/SKILL.md @@ -210,20 +210,21 @@ Both capture from the moment the tab is attached and read a bounded per-tab buff | Command | Summary | |---------|---------| -| `bsk upload --file ` | Click one file chooser and attach an agent-readable local file (`--file` is repeatable) | +| `bsk upload --file ` | Click one upload trigger and attach an agent-readable local file (`--file` is repeatable) | | `bsk download --out ` | Click one download trigger and copy the single completed file to an exact local path (`--overwrite` is opt-in) | The agent/harness decides whether a file transfer is appropriate and which local path belongs to the task. Treat upload as disclosure of that file to the current website, and download as accepting website-controlled bytes onto the local filesystem. Use only paths that are necessary for the user's bounded goal. -BrowserSkill enforces the mechanical boundary: files are staged under a session-scoped opaque transfer, only daemon-minted paths reach the extension, upload/download still obey Agent Window tab checks, and transfers are chunk/size bounded. Download staging is removed after the CLI copies it; upload staging remains available for a later form submission and is removed when the session ends. Downloads cannot overwrite an existing destination unless `--overwrite` is explicit. BrowserSkill does not inspect file content or decide whether its meaning is sensitive. +BrowserSkill enforces the mechanical boundary: files are staged under a session-scoped opaque transfer, only daemon-minted capabilities reach the extension, upload/download still obey Agent Window tab checks, and transfers are chunk/size bounded. Upload captures the file input actually activated by the requested click and assigns only the staged file paths. Download requires an exact-target browser intent before it claims one Chrome download, routes it through a daemon-minted relative directory, and lets the daemon validate and import it. Upload staging remains available for a later form submission and is removed when the session ends. Downloads cannot overwrite an existing destination unless `--overwrite` is explicit. BrowserSkill does not inspect file content or decide whether its meaning is sensitive. -Do not use `request-help` merely because a native file chooser or browser download is involved; try these commands first. For upload failures, use the structured error instead of retrying blindly: +Do not use `request-help` merely because a native file chooser or browser download is involved; try these commands first. For transfer failures, use the structured error instead of retrying blindly: -- `reason=unsupported_file_chooser` means the page opened a non-input picker such as `window.showOpenFilePicker()`. Do not retry `upload`; if human help is available, call `request-help` and tell the user the exact original file path to choose. The staged daemon path is an internal implementation detail and must not be shown to the user. -- `reason=file_chooser_control_failed` means BrowserSkill could not establish or complete the browser-side transaction. Do not retry the same action blindly; use `request-help` when available. -- `reason=file_chooser_not_opened` means the click completed without opening a chooser. Observe once to verify that the target is the actual upload action (for example, after expanding a menu), then retry with the corrected target or ask for help. +- `reason=file_input_not_activated` means the requested click did not activate exactly one ``; the page may use a non-input picker such as `window.showOpenFilePicker()`. Do not retry blindly. If human help is available, call `request-help` and tell the user the exact original local path to choose. The staged daemon path is internal and must not be shown to the user. +- `reason=file_input_probe_failed` means BrowserSkill could not safely establish the browser-side upload transaction. Do not repeat the same action; use `request-help` when available. +- `reason=set_file_input_failed` means BrowserSkill found the activated file input but Chrome rejected the staged path or assignment. Check the extension's file-URL access permission; otherwise use `request-help`. +- `reason=download_capture_failed` means BrowserSkill could not attribute exactly one completed download to the requested target. Do not retry blindly or accept an unrelated browser download; use `request-help` when available. -If `request-help` returns `outcome="disabled"`, do not retry it. Stop gracefully and report that the workflow requires a non-input file picker or unavailable local file. +If `request-help` returns `outcome="disabled"`, do not retry it. Stop gracefully and report the transfer mechanism that requires human intervention. ### Scripting & timing diff --git a/crates/bsk-cli/src/cli/download.rs b/crates/bsk-cli/src/cli/download.rs index ededf99..863206d 100644 --- a/crates/bsk-cli/src/cli/download.rs +++ b/crates/bsk-cli/src/cli/download.rs @@ -55,7 +55,7 @@ pub fn dispatch(args: DownloadArgs, format: Format) -> Result<(), CliError> { selector, tab_id: args.tab_id, timeout_ms: Some(args.timeout), - staging_path: None, + browser_relative_dir: None, }; let reply: DownloadResult = crate::cli::business_rpc::call( info.sock_path.clone(), @@ -82,7 +82,7 @@ pub fn dispatch(args: DownloadArgs, format: Format) -> Result<(), CliError> { value["path"] = serde_json::json!(args.out.to_string_lossy()); if let Some(obj) = value.as_object_mut() { obj.remove("transfer_id"); - obj.remove("staged_path"); + obj.remove("browser_path"); } println!("{}", serde_json::to_string_pretty(&value).unwrap()); } diff --git a/crates/bsk-cli/src/cli/render_error.rs b/crates/bsk-cli/src/cli/render_error.rs index 2be4682..bb2a0a6 100644 --- a/crates/bsk-cli/src/cli/render_error.rs +++ b/crates/bsk-cli/src/cli/render_error.rs @@ -49,9 +49,10 @@ pub mod reason { pub const TAB_NOT_ACTIVE: &str = "tab_not_active"; pub const BORROW_CONFLICT: &str = "borrow_conflict"; pub const SCREENSHOT_CAPTURE_FAILED: &str = "screenshot_capture_failed"; - pub const FILE_CHOOSER_CONTROL_FAILED: &str = "file_chooser_control_failed"; - pub const FILE_CHOOSER_NOT_OPENED: &str = "file_chooser_not_opened"; - pub const UNSUPPORTED_FILE_CHOOSER: &str = "unsupported_file_chooser"; + pub const FILE_INPUT_PROBE_FAILED: &str = "file_input_probe_failed"; + pub const FILE_INPUT_NOT_ACTIVATED: &str = "file_input_not_activated"; + pub const SET_FILE_INPUT_FAILED: &str = "set_file_input_failed"; + pub const DOWNLOAD_CAPTURE_FAILED: &str = "download_capture_failed"; pub const SESSION_BUSY: &str = crate::rpc_reason::SESSION_BUSY; pub const RECORD_START_PAGE_UNREACHABLE: &str = "record_start_page_unreachable"; } @@ -283,26 +284,33 @@ pub fn info_for_error(code: ErrorCode, data: Option<&serde_json::Value>) -> Rend ), exit_code: base.exit_code, }, - (ErrorCode::Unsupported, reason::UNSUPPORTED_FILE_CHOOSER) => RenderInfo { - summary: "the page opened a non-input file picker", + (ErrorCode::Unsupported, reason::FILE_INPUT_NOT_ACTIVATED) => RenderInfo { + summary: "the upload trigger did not activate a file input", hint: Some( - "do not retry upload; use `bsk request-help` and tell the user the exact original local path to select, or stop if human help is disabled", + "the page may use a non-input picker such as showOpenFilePicker(); do not retry blindly — use `bsk request-help` with the exact original local path, or stop if human help is disabled", ), exit_code: base.exit_code, }, - (ErrorCode::Timeout | ErrorCode::CdpFailed, reason::FILE_CHOOSER_CONTROL_FAILED) => { + (ErrorCode::Timeout | ErrorCode::CdpFailed, reason::FILE_INPUT_PROBE_FAILED) => { RenderInfo { - summary: "the browser file-chooser transaction could not complete", + summary: "the browser upload transaction could not be established", hint: Some( "do not retry the same upload blindly; use `bsk request-help` when human help is available", ), exit_code: base.exit_code, } } - (ErrorCode::Timeout, reason::FILE_CHOOSER_NOT_OPENED) => RenderInfo { - summary: "the upload action did not open a file chooser", + (ErrorCode::Timeout | ErrorCode::CdpFailed, reason::SET_FILE_INPUT_FAILED) => RenderInfo { + summary: "the browser could not attach the staged file to the input", hint: Some( - "observe once after expanding any upload menu, then retry with the actual upload action or use `bsk request-help`", + "check that BrowserSkill has Chrome's 'Allow access to file URLs' permission; otherwise use `bsk request-help`", + ), + exit_code: base.exit_code, + }, + (ErrorCode::CdpFailed, reason::DOWNLOAD_CAPTURE_FAILED) => RenderInfo { + summary: "the browser download could not be attributed or completed", + hint: Some( + "do not retry blindly; the target browser may not expose a reliable download intent signal", ), exit_code: base.exit_code, }, @@ -446,24 +454,26 @@ mod tests { } #[test] - fn file_chooser_reasons_render_actionable_fallbacks() { - let unsupported = serde_json::json!({ "reason": reason::UNSUPPORTED_FILE_CHOOSER }); + fn file_transfer_reasons_render_actionable_fallbacks() { + let unsupported = serde_json::json!({ "reason": reason::FILE_INPUT_NOT_ACTIVATED }); let info = info_for_error(ErrorCode::Unsupported, Some(&unsupported)); - assert_eq!(info.summary, "the page opened a non-input file picker"); - assert!(info.hint.unwrap().contains("exact original local path")); - - let control = serde_json::json!({ "reason": reason::FILE_CHOOSER_CONTROL_FAILED }); - let info = info_for_error(ErrorCode::Timeout, Some(&control)); - assert!(info.summary.contains("transaction could not complete")); - assert!(info.hint.unwrap().contains("do not retry")); - - let not_opened = serde_json::json!({ "reason": reason::FILE_CHOOSER_NOT_OPENED }); - let info = info_for_error(ErrorCode::Timeout, Some(¬_opened)); assert_eq!( info.summary, - "the upload action did not open a file chooser" + "the upload trigger did not activate a file input" ); - assert!(info.hint.unwrap().contains("expanding any upload menu")); + assert!(info.hint.unwrap().contains("exact original local path")); + + let control = serde_json::json!({ "reason": reason::FILE_INPUT_PROBE_FAILED }); + let info = info_for_error(ErrorCode::Timeout, Some(&control)); + assert!( + info.summary + .contains("transaction could not be established") + ); + assert!(info.hint.unwrap().contains("do not retry")); + + let download = serde_json::json!({ "reason": reason::DOWNLOAD_CAPTURE_FAILED }); + let info = info_for_error(ErrorCode::CdpFailed, Some(&download)); + assert!(info.summary.contains("download could not be attributed")); } #[test] diff --git a/crates/bsk-cli/src/daemon/file_transfer.rs b/crates/bsk-cli/src/daemon/file_transfer.rs index a3084eb..71123e1 100644 --- a/crates/bsk-cli/src/daemon/file_transfer.rs +++ b/crates/bsk-cli/src/daemon/file_transfer.rs @@ -43,7 +43,7 @@ struct Entry { #[derive(Debug)] pub struct DownloadStaging { pub transfer_id: String, - pub directory: PathBuf, + pub browser_relative_dir: String, } #[derive(Debug)] @@ -247,32 +247,70 @@ impl TransferRegistry { }, ); Ok(DownloadStaging { + browser_relative_dir: format!("BrowserSkill/{id}"), transfer_id: id, - directory: dir, }) } - pub fn finish_download(&self, id: &str, reported_path: &Path) -> Result { - let canonical = reported_path.canonicalize().map_err(io_error)?; + pub fn import_download(&self, id: &str, reported_path: &Path) -> Result { let mut entries = self.entries.lock().unwrap(); let entry = entries .get_mut(id) .ok_or_else(|| not_found("download transfer not found"))?; - let dir = entry.path.canonicalize().map_err(io_error)?; - if entry.direction != Direction::Download || !canonical.starts_with(&dir) { - return Err(permission("download escaped its staging directory")); + if entry.direction != Direction::Download || !reported_path.is_absolute() { + return Err(permission( + "download path is outside its browser capability", + )); } + let expected_parent = Path::new("BrowserSkill").join(id); + let reported_parent = reported_path + .parent() + .ok_or_else(|| permission("download path has no parent directory"))?; + if !reported_parent.ends_with(&expected_parent) { + return Err(permission( + "download escaped its browser capability directory", + )); + } + for path in [reported_path, reported_parent] { + if fs::symlink_metadata(path) + .map_err(io_error)? + .file_type() + .is_symlink() + { + return Err(permission("download capability path contains a symlink")); + } + } + let canonical = reported_path.canonicalize().map_err(io_error)?; let meta = fs::metadata(&canonical).map_err(io_error)?; if !meta.is_file() || meta.len() > MAX_TRANSFER_BYTES { return Err(invalid( "download is not a regular file or exceeds the transfer limit", )); } - entry.path = canonical; - entry.written = meta.len(); - entry.expected_size = Some(meta.len()); + + let destination = entry.path.join("payload"); + let source = File::open(&canonical).map_err(io_error)?; + let mut destination_file = OpenOptions::new() + .write(true) + .create_new(true) + .open(&destination) + .map_err(io_error)?; + let copied = std::io::copy( + &mut source.take(MAX_TRANSFER_BYTES.saturating_add(1)), + &mut destination_file, + ) + .map_err(io_error)?; + if copied > MAX_TRANSFER_BYTES { + let _ = fs::remove_file(&destination); + return Err(invalid("download exceeds the transfer limit")); + } + entry.path = destination; + entry.written = copied; + entry.expected_size = Some(copied); entry.ready = true; - Ok(meta.len()) + let _ = fs::remove_file(&canonical); + let _ = fs::remove_dir(reported_parent); + Ok(copied) } pub fn read_chunk(&self, p: TransferChunkParams) -> Result { @@ -430,25 +468,45 @@ mod tests { } #[test] - fn download_must_finish_inside_its_minted_directory() { + fn download_must_import_from_its_browser_capability_directory() { let (temp, registry) = registry(); let staging = registry.begin_download("s1").unwrap(); let outside = temp.path().join("outside.bin"); fs::write(&outside, b"secret").unwrap(); assert!( registry - .finish_download(&staging.transfer_id, &outside) + .import_download(&staging.transfer_id, &outside) .is_err() ); - let inside = staging.directory.join("result.bin"); + let browser_dir = temp + .path() + .join("Downloads") + .join("BrowserSkill") + .join(&staging.transfer_id); + fs::create_dir_all(&browser_dir).unwrap(); + let inside = browser_dir.join("result.bin"); fs::write(&inside, b"result").unwrap(); assert_eq!( registry - .finish_download(&staging.transfer_id, &inside) + .import_download(&staging.transfer_id, &inside) .unwrap(), 6 ); + assert!(!inside.exists()); + let chunk = registry + .read_chunk(TransferChunkParams { + transfer_id: staging.transfer_id, + offset: 0, + data_base64: String::new(), + }) + .unwrap(); + assert_eq!( + base64::engine::general_purpose::STANDARD + .decode(chunk.data_base64.unwrap()) + .unwrap(), + b"result" + ); } #[test] @@ -456,9 +514,11 @@ mod tests { let (_temp, registry) = registry(); let first = registry.begin_download("s1").unwrap(); let second = registry.begin_download("s2").unwrap(); + let first_directory = registry.root.join(&first.transfer_id); + let second_directory = registry.root.join(&second.transfer_id); registry.release_session("s1"); - assert!(!first.directory.exists()); - assert!(second.directory.exists()); + assert!(!first_directory.exists()); + assert!(second_directory.exists()); } #[test] diff --git a/crates/bsk-cli/src/daemon/ipc.rs b/crates/bsk-cli/src/daemon/ipc.rs index 64a03d3..9a6d0bd 100644 --- a/crates/bsk-cli/src/daemon/ipc.rs +++ b/crates/bsk-cli/src/daemon/ipc.rs @@ -381,7 +381,7 @@ async fn handle_tool_dispatch( Ok(v) => v, Err(err) => return ResponseBody::Err(err), }; - download.staging_path = Some(staging.directory.to_string_lossy().into_owned()); + download.browser_relative_dir = Some(staging.browser_relative_dir); download_transfer_id = Some(staging.transfer_id); params = serde_json::to_value(download).unwrap_or(Value::Null); } @@ -416,19 +416,19 @@ async fn handle_tool_dispatch( }); } }; - let Some(path) = result.staged_path.take() else { + let Some(path) = result.browser_path.take() else { state .transfers .release(TransferIdParams { transfer_id: id }); return ResponseBody::Err(RpcError { code: ErrorCode::ProtocolError, - message: "tool.download returned no staged_path".into(), + message: "tool.download returned no browser_path".into(), data: None, }); }; match state .transfers - .finish_download(&id, std::path::Path::new(&path)) + .import_download(&id, std::path::Path::new(&path)) { Ok(size) => { result.byte_size = size; diff --git a/crates/bsk-protocol/schema/tool_download_params.json b/crates/bsk-protocol/schema/tool_download_params.json index ec3c844..b6487cc 100644 --- a/crates/bsk-protocol/schema/tool_download_params.json +++ b/crates/bsk-protocol/schema/tool_download_params.json @@ -6,6 +6,13 @@ "session_id" ], "properties": { + "browser_relative_dir": { + "description": "Daemon-injected relative directory beneath Chrome's Downloads root.", + "type": [ + "string", + "null" + ] + }, "ref": { "type": [ "string", @@ -21,13 +28,6 @@ "session_id": { "type": "string" }, - "staging_path": { - "description": "Daemon-injected destination directory for the browser process.", - "type": [ - "string", - "null" - ] - }, "tab_id": { "type": [ "integer", diff --git a/crates/bsk-protocol/schema/tool_download_result.json b/crates/bsk-protocol/schema/tool_download_result.json index ea2ac7c..726ca50 100644 --- a/crates/bsk-protocol/schema/tool_download_result.json +++ b/crates/bsk-protocol/schema/tool_download_result.json @@ -8,6 +8,13 @@ "tab_id" ], "properties": { + "browser_path": { + "description": "Extension-internal completed Chrome download path, stripped by the daemon.", + "type": [ + "string", + "null" + ] + }, "byte_size": { "type": "integer", "format": "uint64", @@ -25,13 +32,6 @@ "null" ] }, - "staged_path": { - "description": "Extension-internal completed path, stripped by the daemon.", - "type": [ - "string", - "null" - ] - }, "suggested_filename": { "type": "string" }, diff --git a/crates/bsk-protocol/src/tools/file_transfer.rs b/crates/bsk-protocol/src/tools/file_transfer.rs index 2c2f5b8..f2517ce 100644 --- a/crates/bsk-protocol/src/tools/file_transfer.rs +++ b/crates/bsk-protocol/src/tools/file_transfer.rs @@ -60,9 +60,9 @@ pub struct DownloadParams { pub tab_id: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub timeout_ms: Option, - /// Daemon-injected destination directory for the browser process. + /// Daemon-injected relative directory beneath Chrome's Downloads root. #[serde(default, skip_serializing_if = "Option::is_none")] - pub staging_path: Option, + pub browser_relative_dir: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] @@ -78,9 +78,9 @@ pub struct DownloadResult { pub mime: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub danger: Option, - /// Extension-internal completed path, stripped by the daemon. + /// Extension-internal completed Chrome download path, stripped by the daemon. #[serde(default, skip_serializing_if = "Option::is_none")] - pub staged_path: Option, + pub browser_path: Option, /// Opaque id returned by the daemon to the CLI. #[serde(default, skip_serializing_if = "Option::is_none")] pub transfer_id: Option, @@ -162,9 +162,9 @@ mod tests { selector: Some("#export".into()), tab_id: None, timeout_ms: None, - staging_path: None, + browser_relative_dir: None, }) .unwrap(); - assert!(value.get("staging_path").is_none()); + assert!(value.get("browser_relative_dir").is_none()); } } diff --git a/docs/architecture.md b/docs/architecture.md index 2560016..d86748d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -147,8 +147,8 @@ flowchart LR - The invoking agent/harness decides whether a transfer is authorized and supplies the task-local source or destination path. - The CLI is the only component that reads an upload source or writes the final download destination. The extension never receives either agent-facing path. -- The daemon issues opaque, session-scoped transfer IDs and stages bounded chunks in a private runtime directory. It injects only its own staging paths into extension RPCs and validates that a completed download remains inside its minted directory. -- The extension only performs the browser-side mechanism: handle one file-chooser transaction or capture one download, scoped to the requested session and exact Agent Window target. Automatic upload is limited to choosers backed by ``; non-input pickers such as `window.showOpenFilePicker()` return a structured unsupported result for the agent to handle. The extension does not classify content or make policy decisions. +- The daemon issues opaque, session-scoped transfer IDs and stages bounded chunks in a private runtime directory. For upload it injects only its private staged paths. For download it mints a relative Chrome directory capability, validates the reported completed path against that capability, rejects symlinks and oversized/non-regular files, then imports the bytes into private staging. +- The extension only performs the browser-side mechanism, scoped to the requested session and exact Agent Window target. Upload installs a transaction-scoped DOM guard that records the one `` actually activated by the requested click while cancelling its native default action, then uses `DOM.setFileInputFiles`. File System Access picker entry points are intercepted only for that transaction and return a structured fallback. Download requires an exact-target `Page.downloadWillBegin` intent before a matching `chrome.downloads` item is routed into the daemon-minted relative directory. The extension does not classify content or make policy decisions. - Download staging is released after the CLI commits the file. Upload staging remains until session teardown because the page may read an attached file only on a later form submission. All staging is released on session stop/browser disconnect and on daemon startup after a crash. Existing download destinations are not overwritten unless the CLI caller explicitly opts in. ## Repository layout diff --git a/skill/SKILL.md b/skill/SKILL.md index 3d10831..4233d1f 100644 --- a/skill/SKILL.md +++ b/skill/SKILL.md @@ -210,20 +210,21 @@ Both capture from the moment the tab is attached and read a bounded per-tab buff | Command | Summary | |---------|---------| -| `bsk upload --file ` | Click one file chooser and attach an agent-readable local file (`--file` is repeatable) | +| `bsk upload --file ` | Click one upload trigger and attach an agent-readable local file (`--file` is repeatable) | | `bsk download --out ` | Click one download trigger and copy the single completed file to an exact local path (`--overwrite` is opt-in) | The agent/harness decides whether a file transfer is appropriate and which local path belongs to the task. Treat upload as disclosure of that file to the current website, and download as accepting website-controlled bytes onto the local filesystem. Use only paths that are necessary for the user's bounded goal. -BrowserSkill enforces the mechanical boundary: files are staged under a session-scoped opaque transfer, only daemon-minted paths reach the extension, upload/download still obey Agent Window tab checks, and transfers are chunk/size bounded. Download staging is removed after the CLI copies it; upload staging remains available for a later form submission and is removed when the session ends. Downloads cannot overwrite an existing destination unless `--overwrite` is explicit. BrowserSkill does not inspect file content or decide whether its meaning is sensitive. +BrowserSkill enforces the mechanical boundary: files are staged under a session-scoped opaque transfer, only daemon-minted capabilities reach the extension, upload/download still obey Agent Window tab checks, and transfers are chunk/size bounded. Upload captures the file input actually activated by the requested click and assigns only the staged file paths. Download requires an exact-target browser intent before it claims one Chrome download, routes it through a daemon-minted relative directory, and lets the daemon validate and import it. Upload staging remains available for a later form submission and is removed when the session ends. Downloads cannot overwrite an existing destination unless `--overwrite` is explicit. BrowserSkill does not inspect file content or decide whether its meaning is sensitive. -Do not use `request-help` merely because a native file chooser or browser download is involved; try these commands first. For upload failures, use the structured error instead of retrying blindly: +Do not use `request-help` merely because a native file chooser or browser download is involved; try these commands first. For transfer failures, use the structured error instead of retrying blindly: -- `reason=unsupported_file_chooser` means the page opened a non-input picker such as `window.showOpenFilePicker()`. Do not retry `upload`; if human help is available, call `request-help` and tell the user the exact original file path to choose. The staged daemon path is an internal implementation detail and must not be shown to the user. -- `reason=file_chooser_control_failed` means BrowserSkill could not establish or complete the browser-side transaction. Do not retry the same action blindly; use `request-help` when available. -- `reason=file_chooser_not_opened` means the click completed without opening a chooser. Observe once to verify that the target is the actual upload action (for example, after expanding a menu), then retry with the corrected target or ask for help. +- `reason=file_input_not_activated` means the requested click did not activate exactly one ``; the page may use a non-input picker such as `window.showOpenFilePicker()`. Do not retry blindly. If human help is available, call `request-help` and tell the user the exact original local path to choose. The staged daemon path is internal and must not be shown to the user. +- `reason=file_input_probe_failed` means BrowserSkill could not safely establish the browser-side upload transaction. Do not repeat the same action; use `request-help` when available. +- `reason=set_file_input_failed` means BrowserSkill found the activated file input but Chrome rejected the staged path or assignment. Check the extension's file-URL access permission; otherwise use `request-help`. +- `reason=download_capture_failed` means BrowserSkill could not attribute exactly one completed download to the requested target. Do not retry blindly or accept an unrelated browser download; use `request-help` when available. -If `request-help` returns `outcome="disabled"`, do not retry it. Stop gracefully and report that the workflow requires a non-input file picker or unavailable local file. +If `request-help` returns `outcome="disabled"`, do not retry it. Stop gracefully and report the transfer mechanism that requires human intervention. ### Scripting & timing