diff --git a/CHANGELOG.md b/CHANGELOG.md index 0107dd9..6df9be3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased - Added `bun run dev:deploy` for validated extension builds, backups, atomic receipt replacement, rollback, and bounded readiness checks. Reject symlinked writable paths and unsupported default reload platforms before deployment. Extension health checks now require a ready host and an actual extension round-trip. - Added a DOM editable fallback to accessibility snapshots. When Chrome's debugger-backed accessibility tree omits an editable element, for example a framework's pre-hydration composer textarea, full-tree snapshots lead with up to 40 `dom_fallback` textbox nodes minted from DOM query results, scanning past accessibility-covered and unsupported matches until the limit of accepted editables or a bounded inspection budget is reached. Each carries a normal revisioned ref that the existing fill, type, and click paths already accept; password inputs are skipped, partial (`root_ref`) snapshots are unchanged, the `max_nodes` budget and `truncated` flag now cover the combined result, and a fallback capture failure never fails the snapshot itself. +- Added unique CSS selector targets for Standard `click`, `type`, and `fill`, retaining sensitive-field and Commit checks and executing against the checked backend node. Normalize and truncate selector text in operation displays. - Replaced blocking human handoffs with task-owned attention notices. Agents can request, inspect, resolve, or dismiss a notice without popup acknowledgement; open notices do not block ordinary commands or task cleanup. Legacy handoffs migrate without restoring a lock, and stale notice IDs cannot clear newer requests. Explicit Pause, ownership, sensitive-field restrictions, and configured Commit review remain independent. - Enabled YOLO mode for new and legacy default state so recognizable consequential controls execute in the original `browser_act` call. Turning YOLO mode off restores staged Commit review. Task ownership, origin policy, revision checks, credential isolation, and all other Standard boundaries remain enforced. - Preserved existing permissive-mode settings and managed 1Password defaults during the notice cutover, including GUI-host executable discovery and legacy created-tab provenance sanitization. diff --git a/docs/mcp.md b/docs/mcp.md index c4d063c..b67628c 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -72,6 +72,15 @@ For MCP, the capability store namespace is `mcp`; OMP uses `omp`; Pi uses `pi`. | `browser_credentials` | `prepare` requires a task tab and expected page revision, then returns an opaque short-lived token when the default-enabled 1Password broker is available and one through three Login items match the host-derived current origin. Owner-only policy can disable it. `fill` consumes that token and selected username, password, or one-time-code field refs without returning any value. `next` advances to another bounded candidate. It never submits the form. | | `browser_finish` | Accepts `disposition: "auto" | "close" | "keep"` and optional task-owned `keep_tab_ids`. Automatic mode follows the popup cleanup policy: close task-created tabs while retaining adopted tabs, ask for confirmation, or retain all tabs. Successful finalization ungroups retained tabs, releases ownership, closes the Core connection, and returns closed and retained tab IDs. An open handoff notice never defers finalization; staged Commit review and other in-flight work defer it without destroying resumability. | +For `click`, `type`, and `fill`, supply exactly one of `ref` or `selector`. +A CSS selector is limited to 1–2048 characters and must match exactly one element; +missing and ambiguous targets fail before mutation. The same task ownership, +revision, sensitive-field, and Commit checks apply to either target form. +Before Commit, the selector is resolved again and checked against the staged +element identity. Execute against that checked element, never a later selector +match. Existing `upload_file` selector first-match behavior is unchanged. + + Every existing-page mutation carries its expected page revision. If navigation or document replacement makes that revision stale, AgentTab rejects the operation rather than selecting a new target. `automation_route` is `full` for ordinary HTTP, HTTPS, and `about:blank` tabs. It is `tab_only` with `route_reason: "browser_restricted_origin"` for Chrome system pages, extension pages, DevTools, the Chrome Web Store, malformed URLs, and unknown schemes. Page inspection or interaction requested on a `tab_only` tab returns `browser_restricted_origin` with `outcome: "not_started"` and recovery that explicitly says not to retry the same AgentTab route. This is a browser platform boundary, not a policy denial and not permission that can be granted through AgentTab. diff --git a/host-rs/crates/agenttab-protocol/src/lib.rs b/host-rs/crates/agenttab-protocol/src/lib.rs index fd5aa56..1a76359 100644 --- a/host-rs/crates/agenttab-protocol/src/lib.rs +++ b/host-rs/crates/agenttab-protocol/src/lib.rs @@ -449,14 +449,23 @@ pub struct BrowserActParams { #[serde(tag = "kind", rename_all = "snake_case", deny_unknown_fields)] pub enum BrowserAction { Click { - r#ref: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + r#ref: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + selector: Option, }, Type { - r#ref: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + r#ref: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + selector: Option, text: String, }, Fill { - r#ref: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + r#ref: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + selector: Option, text: String, }, Select { @@ -893,9 +902,53 @@ impl MethodParams { fn validate_action(method: RpcMethod, action: &BrowserAction) -> Result<(), ProtocolError> { match action { - BrowserAction::Click { r#ref } => require_ref(method, r#ref), - BrowserAction::Type { r#ref, text } | BrowserAction::Fill { r#ref, text } => { - require_ref(method, r#ref)?; + BrowserAction::Click { r#ref, selector } => match (r#ref, selector) { + (Some(value), None) => require_ref(method, value), + (None, Some(value)) => require_len(method, value, 1, MAX_SELECTOR_CHARS, "selector"), + _ => require( + method, + false, + "click requires exactly one of ref or selector", + ), + }, + BrowserAction::Type { + r#ref, + selector, + text, + } => { + match (r#ref, selector) { + (Some(value), None) => require_ref(method, value)?, + (None, Some(value)) => { + require_len(method, value, 1, MAX_SELECTOR_CHARS, "selector")?; + } + _ => { + return require( + method, + false, + "type requires exactly one of ref or selector", + ) + } + } + require_len(method, text, 0, MAX_ACTION_TEXT_CHARS, "text") + } + BrowserAction::Fill { + r#ref, + selector, + text, + } => { + match (r#ref, selector) { + (Some(value), None) => require_ref(method, value)?, + (None, Some(value)) => { + require_len(method, value, 1, MAX_SELECTOR_CHARS, "selector")?; + } + _ => { + return require( + method, + false, + "fill requires exactly one of ref or selector", + ) + } + } require_len(method, text, 0, MAX_ACTION_TEXT_CHARS, "text") } BrowserAction::Select { r#ref, value } => { @@ -2266,6 +2319,53 @@ mod tests { .is_err()); } + let (_, act_selectors) = RpcRequest::parse(request( + "browser_act", + json!({ + "tab_id": 7, + "expected_page_revision": 1, + "actions": [ + {"kind": "click", "selector": "button.submit"}, + {"kind": "type", "selector": "input.search", "text": "agenttab"}, + {"kind": "fill", "selector": "input.email", "text": "test@example.com"} + ] + }), + true, + )) + .unwrap(); + assert!(matches!( + act_selectors, + MethodParams::Act(BrowserActParams { actions, .. }) + if matches!( + actions.as_slice(), + [ + BrowserAction::Click { selector: Some(sel1), r#ref: None }, + BrowserAction::Type { selector: Some(sel2), r#ref: None, text: t1 }, + BrowserAction::Fill { selector: Some(sel3), r#ref: None, text: t2 }, + ] if sel1 == "button.submit" && sel2 == "input.search" && t1 == "agenttab" && sel3 == "input.email" && t2 == "test@example.com" + ) + )); + + for action in [ + json!({"kind": "click"}), + json!({"kind": "click", "ref": "e1", "selector": "button"}), + json!({"kind": "type", "text": "hello"}), + json!({"kind": "type", "ref": "e1", "selector": "input", "text": "hello"}), + json!({"kind": "fill", "text": "hello"}), + json!({"kind": "fill", "ref": "e1", "selector": "input", "text": "hello"}), + ] { + assert!(RpcRequest::parse(request( + "browser_act", + json!({ + "tab_id": 7, + "expected_page_revision": 1, + "actions": [action] + }), + true, + )) + .is_err()); + } + let (_, close) = RpcRequest::parse(request("agenttab.close", json!({}), false)).unwrap(); assert!(matches!(close, MethodParams::Close(_))); diff --git a/packages/extension/src/browser.ts b/packages/extension/src/browser.ts index c9f4484..0970aca 100644 --- a/packages/extension/src/browser.ts +++ b/packages/extension/src/browser.ts @@ -567,7 +567,26 @@ export class StandardBrowserRuntime { }); } await this.revisions.assertExpected(tabId, pageRevision); - const stagedConsequence = await this.consequence(tabId, pageRevision, action); + const hasRef = typeof action.ref === "string"; + const hasSelector = typeof action.selector === "string"; + if ( + action.kind === "click" || + action.kind === "type" || + action.kind === "fill" || + action.kind === "upload_file" + ) { + if (hasRef === hasSelector) { + throw Object.assign(new Error(`${action.kind} requires exactly one of ref or selector`), { + code: "invalid_request", + }); + } + } + const resolvedBackendNodeId = hasSelector + ? await this.backendNodeIdFromSelector(tabId, String(action.selector), String(action.kind)) + : hasRef + ? this.backendNodeId(pageRevision, action.ref) + : undefined; + const stagedConsequence = await this.consequence(tabId, pageRevision, action, resolvedBackendNodeId); if (stagedConsequence && !(await readState()).skipCommitReview) { const staged: StagedCommit = { native_token: randomToken(), @@ -589,6 +608,7 @@ export class StandardBrowserRuntime { kind: action.kind, target: stagedConsequence.target, ...(typeof action.ref === "string" ? { ref: action.ref } : {}), + ...(typeof action.selector === "string" ? { selector: action.selector } : {}), }, ...(stagedConsequence.dialog !== undefined ? { dialog: stagedConsequence.dialog.binding } : {}), }; @@ -613,7 +633,7 @@ export class StandardBrowserRuntime { }, }; } - completedActions.push(await this.performAction(tabId, pageRevision, action)); + completedActions.push(await this.performAction(tabId, pageRevision, action, resolvedBackendNodeId)); } return { result: { @@ -838,11 +858,27 @@ export class StandardBrowserRuntime { code: "staged_commit_mismatch", }); } - const currentTarget = typeof action.ref === "string" - ? await this.targetDescriptor(staged.tab_id, staged.page_revision, action.ref, action) - : action.kind === "upload_file" && typeof action.selector === "string" - ? await this.targetDescriptorBySelector(staged.tab_id, action.selector) - : { kind: action.kind }; + let checkedBackendNodeId: number | undefined; + let currentTarget: Record; + try { + if (typeof action.ref === "string") { + checkedBackendNodeId = this.backendNodeId(staged.page_revision, action.ref); + currentTarget = await this.targetDescriptorForBackendNode(staged.tab_id, checkedBackendNodeId, action); + } else if (typeof action.selector === "string") { + checkedBackendNodeId = await this.backendNodeIdFromSelector(staged.tab_id, action.selector, String(action.kind)); + currentTarget = await this.targetDescriptorForBackendNode(staged.tab_id, checkedBackendNodeId, action); + } else { + currentTarget = { kind: action.kind }; + } + } catch (error) { + await mutateState((state) => { + delete state.stagedCommits[String(nativeToken)]; + }); + throw Object.assign(new Error("Staged target changed before Commit"), { + code: "staged_commit_mismatch", + cause: error, + }); + } if ( await this.stageFingerprint(taskId, staged.tab_id, staged.page_revision, action, currentTarget) !== staged.fingerprint @@ -859,7 +895,7 @@ export class StandardBrowserRuntime { }); const result = action.kind === "dialog" && action.decision === "accept" ? await this.acceptStagedDialog(staged.tab_id, action, staged.dialog) - : await this.performAction(staged.tab_id, staged.page_revision, action); + : await this.performAction(staged.tab_id, staged.page_revision, action, checkedBackendNodeId); return { tab_id: staged.tab_id, page_revision: await this.revisions.current(staged.tab_id), @@ -1144,6 +1180,7 @@ export class StandardBrowserRuntime { tabId: number, pageRevision: number, action: Record, + resolvedBackendNodeId?: number, ): Promise { if (action.kind === "close") { return { @@ -1153,11 +1190,14 @@ export class StandardBrowserRuntime { } if (action.kind === "upload_file") { const count = Array.isArray(action.files) ? action.files.length : 0; + const target = typeof resolvedBackendNodeId === "number" + ? await this.targetDescriptorForBackendNode(tabId, resolvedBackendNodeId, action) + : typeof action.ref === "string" + ? await this.targetDescriptor(tabId, pageRevision, action.ref) + : await this.targetDescriptorBySelector(tabId, String(action.selector ?? ""), action); return { effect: `Upload ${count} ${count === 1 ? "file" : "files"} to the page`, - target: typeof action.ref === "string" - ? await this.targetDescriptor(tabId, pageRevision, action.ref) - : await this.targetDescriptorBySelector(tabId, String(action.selector ?? "")), + target, }; } if (action.kind === "dialog" && action.decision === "accept") { @@ -1175,7 +1215,18 @@ export class StandardBrowserRuntime { ) { return null; } - const target = await this.targetDescriptor(tabId, pageRevision, action.ref, action); + const hasRef = typeof action.ref === "string"; + const hasSelector = typeof action.selector === "string"; + if (hasRef === hasSelector) { + throw Object.assign(new Error(`${action.kind} requires exactly one of ref or selector`), { + code: "invalid_request", + }); + } + const target = typeof resolvedBackendNodeId === "number" + ? await this.targetDescriptorForBackendNode(tabId, resolvedBackendNodeId, action) + : hasRef + ? await this.targetDescriptor(tabId, pageRevision, action.ref, action) + : await this.targetDescriptorBySelector(tabId, String(action.selector ?? ""), action); const label = [ target.role, target.text, @@ -1290,10 +1341,12 @@ export class StandardBrowserRuntime { private async targetDescriptorBySelector( tabId: number, selector: string, + action?: Record, ): Promise> { return this.targetDescriptorForBackendNode( tabId, await this.backendNodeIdFromSelector(tabId, selector), + action, ); } @@ -1327,13 +1380,17 @@ export class StandardBrowserRuntime { recovery: "Start browser_handoff for this tab and let the human enter the sensitive value.", }); } - return described.result.value; + return { + ...described.result.value, + backend_node_id: backendNodeId, + }; } private async performAction( tabId: number, pageRevision: number, action: Record, + resolvedBackendNodeId?: number, ): Promise> { await this.authorizeDebuggerUse(tabId); const kind = action.kind; @@ -1380,9 +1437,20 @@ export class StandardBrowserRuntime { }); return { kind, completed: true }; } - const backendNodeId = kind === "upload_file" && typeof action.selector === "string" - ? await this.backendNodeIdFromSelector(tabId, action.selector) - : this.backendNodeId(pageRevision, action.ref); + const hasRef = typeof action.ref === "string"; + const hasSelector = typeof action.selector === "string"; + if (kind === "click" || kind === "type" || kind === "fill" || kind === "upload_file") { + if (hasRef === hasSelector) { + throw Object.assign(new Error(`${kind} requires exactly one of ref or selector`), { + code: "invalid_request", + }); + } + } + const backendNodeId = typeof resolvedBackendNodeId === "number" + ? resolvedBackendNodeId + : hasSelector + ? await this.backendNodeIdFromSelector(tabId, String(action.selector), String(kind)) + : this.backendNodeId(pageRevision, action.ref); if (kind === "click") { const [activeTab, existingTabs] = await Promise.all([ chrome.tabs.query({ active: true, lastFocusedWindow: true }).then(([tab]) => tab), @@ -1545,28 +1613,63 @@ export class StandardBrowserRuntime { return Number(match[2]); } - private async backendNodeIdFromSelector(tabId: number, selector: string): Promise { + private async backendNodeIdFromSelector( + tabId: number, + selector: string, + actionKind?: string, + ): Promise { const document = await this.send(tabId, "DOM.getDocument", { depth: 0 }); if (!isRecord(document.root) || typeof document.root.nodeId !== "number") { - throw Object.assign(new Error("Could not inspect the upload document"), { + throw Object.assign(new Error("Could not inspect the document"), { code: "action_failed", }); } - const selected = await this.send(tabId, "DOM.querySelector", { + if (actionKind === "upload_file") { + const selected = await this.send(tabId, "DOM.querySelector", { + nodeId: document.root.nodeId, + selector, + }); + if (typeof selected.nodeId !== "number" || selected.nodeId === 0) { + throw Object.assign(new Error(`Selector did not match: ${selector}`), { + code: "selector_not_found", + }); + } + const described = await this.send(tabId, "DOM.describeNode", { + nodeId: selected.nodeId, + depth: 0, + }); + if (!isRecord(described.node) || typeof described.node.backendNodeId !== "number") { + throw Object.assign(new Error("Selected upload target could not be resolved"), { + code: "action_failed", + }); + } + return described.node.backendNodeId; + } + const selected = await this.send(tabId, "DOM.querySelectorAll", { nodeId: document.root.nodeId, selector, }); - if (typeof selected.nodeId !== "number" || selected.nodeId === 0) { + const nodeIds = Array.isArray(selected.nodeIds) + ? selected.nodeIds + : typeof selected.nodeId === "number" && selected.nodeId !== 0 + ? [selected.nodeId] + : []; + if (nodeIds.length === 0) { throw Object.assign(new Error(`Selector did not match: ${selector}`), { code: "selector_not_found", }); } + if (nodeIds.length > 1) { + throw Object.assign(new Error(`Selector matched ${nodeIds.length} elements: ${selector}`), { + code: "ambiguous_selector", + }); + } const described = await this.send(tabId, "DOM.describeNode", { - nodeId: selected.nodeId, + nodeId: nodeIds[0], depth: 0, }); if (!isRecord(described.node) || typeof described.node.backendNodeId !== "number") { - throw Object.assign(new Error("Selected upload target could not be resolved"), { + throw Object.assign(new Error("Selected target could not be resolved"), { code: "action_failed", }); } diff --git a/packages/extension/src/protocol.ts b/packages/extension/src/protocol.ts index d20a852..3e4f504 100644 --- a/packages/extension/src/protocol.ts +++ b/packages/extension/src/protocol.ts @@ -254,13 +254,23 @@ function assertAction(value: unknown): Record { const action = value; switch (action.kind) { case "click": - assertExactObject(action, ["kind", "ref"], [], "click action"); - assertBoundedString(action.ref, "click.ref", 1, 256); + assertExactObject(action, ["kind"], ["ref", "selector"], "click action"); + if ((action.ref === undefined) === (action.selector === undefined)) { + commandError("click requires exactly one of ref or selector"); + } + if (action.ref !== undefined) assertBoundedString(action.ref, "click.ref", 1, 256); + if (action.selector !== undefined) assertBoundedString(action.selector, "click.selector", 1, 2_048); return action; case "type": case "fill": - assertExactObject(action, ["kind", "ref", "text"], [], `${action.kind} action`); - assertBoundedString(action.ref, `${action.kind}.ref`, 1, 256); + assertExactObject(action, ["kind", "text"], ["ref", "selector"], `${action.kind} action`); + if ((action.ref === undefined) === (action.selector === undefined)) { + commandError(`${action.kind} requires exactly one of ref or selector`); + } + if (action.ref !== undefined) assertBoundedString(action.ref, `${action.kind}.ref`, 1, 256); + if (action.selector !== undefined) { + assertBoundedString(action.selector, `${action.kind}.selector`, 1, 2_048); + } assertBoundedString(action.text, `${action.kind}.text`, 0, 1_048_576); return action; case "select": diff --git a/packages/extension/test/extension.test.ts b/packages/extension/test/extension.test.ts index a8a01ea..6a36359 100644 --- a/packages/extension/test/extension.test.ts +++ b/packages/extension/test/extension.test.ts @@ -1102,6 +1102,56 @@ describe("native protocol", () => { })).toThrow("exactly one of ref or selector"); }); + test("accepts selector-addressed click, type, and fill actions", () => { + const envelope = { + protocol: "agenttab.native", + version: 1, + kind: "command", + request_id: "018f47b8-2f80-7c20-9c77-f8a38c9e621d", + task_id: "018f47b8-2f80-7c20-9c77-f8a38c9e621e", + connection_id: "018f47b8-2f80-7c20-9c77-f8a38c9e621f", + method: "browser_act", + params: { + tab_id: 42, + expected_page_revision: 1, + actions: [ + { kind: "click", selector: "button.submit" }, + { kind: "type", selector: "input.search", text: "query" }, + { kind: "fill", selector: "input.email", text: "user@example.com" }, + ], + }, + }; + expect(parseCommand(envelope).params).toEqual(envelope.params); + + // Both ref and selector + expect(() => parseCommand({ + ...envelope, + params: { ...envelope.params, actions: [{ kind: "click", ref: "e1", selector: "button" }] }, + })).toThrow("click requires exactly one of ref or selector"); + expect(() => parseCommand({ + ...envelope, + params: { ...envelope.params, actions: [{ kind: "type", ref: "e1", selector: "input", text: "hi" }] }, + })).toThrow("type requires exactly one of ref or selector"); + expect(() => parseCommand({ + ...envelope, + params: { ...envelope.params, actions: [{ kind: "fill", ref: "e1", selector: "input", text: "hi" }] }, + })).toThrow("fill requires exactly one of ref or selector"); + + // Missing both ref and selector + expect(() => parseCommand({ + ...envelope, + params: { ...envelope.params, actions: [{ kind: "click" }] }, + })).toThrow("click requires exactly one of ref or selector"); + expect(() => parseCommand({ + ...envelope, + params: { ...envelope.params, actions: [{ kind: "type", text: "hi" }] }, + })).toThrow("type requires exactly one of ref or selector"); + expect(() => parseCommand({ + ...envelope, + params: { ...envelope.params, actions: [{ kind: "fill", text: "hi" }] }, + })).toThrow("fill requires exactly one of ref or selector"); + }); + test("rejects removed focus and prompt-input action capabilities", () => { const command = { protocol: "agenttab.native", @@ -1598,6 +1648,198 @@ describe("page revision monotonicity", () => { await runtime.detach(65); }); + test("selector actions reject ambiguous and missing element targets", async () => { + tabStore.set(70, { + id: 70, + windowId: 1, + groupId: -1, + url: "https://example.test/", + status: "complete", + }); + const runtime = new StandardBrowserRuntime( + new RevisionTracker(), + async () => undefined, + () => undefined, + async () => undefined, + ); + + // Ambiguous selector (matches >1 elements) + debuggerCommandOverride = (method) => { + if (method === "DOM.querySelectorAll") return { nodeIds: [101, 102] }; + return undefined; + }; + await expect(runtime.act(TASK_A, 70, 1, [{ + kind: "click", + selector: "button.duplicate", + }])).rejects.toMatchObject({ + code: "ambiguous_selector", + }); + + // Missing selector (matches 0 elements) + debuggerCommandOverride = (method) => { + if (method === "DOM.querySelectorAll") return { nodeIds: [] }; + return undefined; + }; + await expect(runtime.act(TASK_A, 70, 1, [{ + kind: "click", + selector: "#not-found", + }])).rejects.toMatchObject({ + code: "selector_not_found", + }); + + await runtime.detach(70); + }); + + test("selector actions reject sensitive fields and stage consequential controls with replacement check", async () => { + await mutateState((state) => { state.skipCommitReview = false; }); + tabStore.set(71, { + id: 71, + windowId: 1, + groupId: -1, + url: "https://example.test/", + status: "complete", + }); + const runtime = new StandardBrowserRuntime( + new RevisionTracker(), + async () => undefined, + () => undefined, + async () => undefined, + ); + + // 1. Sensitive field via selector is rejected + debuggerCommandOverride = (method) => { + if (method === "DOM.querySelectorAll") return { nodeIds: [80] }; + if (method === "DOM.describeNode") return { node: { backendNodeId: 880 } }; + if (method === "Runtime.callFunctionOn") { + return { result: { value: { agenttab_sensitive_field: true } } }; + } + return undefined; + }; + await expect(runtime.act(TASK_A, 71, 1, [{ + kind: "fill", + selector: 'input[name="password"]', + text: "secret123", + }])).rejects.toMatchObject({ + code: "sensitive_field_requires_handoff", + }); + + // 2. Consequential control via selector stages for commit review + let currentBackendNodeId = 881; + debuggerCommandOverride = (method) => { + if (method === "DOM.querySelectorAll") return { nodeIds: [81] }; + if (method === "DOM.describeNode") return { node: { backendNodeId: currentBackendNodeId } }; + if (method === "Runtime.callFunctionOn") { + return { + result: { + value: { + tag: "BUTTON", + role: "button", + text: "Place order now", + }, + }, + }; + } + return undefined; + }; + + const execution = await runtime.act(TASK_A, 71, 1, [{ + kind: "click", + selector: "button.checkout", + }]); + expect(execution.staged).toBeDefined(); + expect(execution.staged?.effect).toContain("Place order"); + expect(execution.staged?.preview).toMatchObject({ + kind: "click", + selector: "button.checkout", + }); + + // 3. If target is replaced before commit, commit rejects with staged_commit_mismatch + currentBackendNodeId = 999; // node was replaced in DOM + await expect(runtime.commit(TASK_A, { + native_token: execution.staged?.native_token, + })).rejects.toMatchObject({ + code: "staged_commit_mismatch", + }); + // Staged commit was cleaned up + expect((await readState()).stagedCommits[String(execution.staged?.native_token)]).toBeUndefined(); + + await runtime.detach(71); + }); + + test("pins resolved backend node id between describe and execution preventing replacement races", async () => { + tabStore.set(72, { + id: 72, + windowId: 1, + groupId: -1, + url: "https://example.test/", + status: "complete", + }); + const runtime = new StandardBrowserRuntime( + new RevisionTracker(), + async () => undefined, + () => undefined, + async () => undefined, + ); + + let queryCount = 0; + const executedObjectIds: string[] = []; + debuggerCommandOverride = (method, params) => { + if (method === "DOM.querySelectorAll") { + queryCount++; + // First query returns node 50; a replacement in DOM would be node 99 + return { nodeIds: [queryCount === 1 ? 50 : 99] }; + } + if (method === "DOM.describeNode") { + return { node: { backendNodeId: params.nodeId === 50 ? 500 : 990 } }; + } + if (method === "DOM.resolveNode") { + return { object: { objectId: `obj-${params.backendNodeId}` } }; + } + if (method === "Runtime.callFunctionOn") { + executedObjectIds.push(String(params.objectId)); + return { result: { value: { role: "button", text: "Submit" } } }; + } + return undefined; + }; + + // 1. Selector is resolved once; same backendNodeId is classified and executed + await runtime.act(TASK_A, 72, 1, [{ + kind: "click", + selector: "button.submit", + }]); + expect(queryCount).toBe(1); + expect(executedObjectIds).toEqual(["obj-500", "obj-500"]); + + // 2. If node is removed between describe and execution, DOM.resolveNode fails with stale_ref + let describedOnce = false; + debuggerCommandOverride = (method, params) => { + if (method === "DOM.querySelectorAll") return { nodeIds: [51] }; + if (method === "DOM.describeNode") return { node: { backendNodeId: 510 } }; + if (method === "DOM.resolveNode") { + // First resolveNode during consequence classification succeeds + if (!describedOnce) { + describedOnce = true; + return { object: { objectId: "obj-510" } }; + } + // Node removed from DOM before execution + return {}; + } + if (method === "Runtime.callFunctionOn") { + return { result: { value: { role: "button", text: "Submit" } } }; + } + return undefined; + }; + + await expect(runtime.act(TASK_A, 72, 1, [{ + kind: "click", + selector: "button.replaced", + }])).rejects.toMatchObject({ + code: "stale_ref", + }); + + await runtime.detach(72); + }); + test("appends DOM fallback refs for editables missing from the accessibility tree", async () => { tabStore.set(66, { id: 66, diff --git a/packages/omp/src/index.ts b/packages/omp/src/index.ts index 98c8916..7d11a20 100644 --- a/packages/omp/src/index.ts +++ b/packages/omp/src/index.ts @@ -177,10 +177,14 @@ const DEFINITIONS: ReadonlyArray<{ approval: "write", schema: (z) => { const ref = z.string().min(1).max(256); + const selector = z.string().min(1).max(2048); const action = z.union([ z.object({ kind: z.literal("click"), ref }).strict(), + z.object({ kind: z.literal("click"), selector }).strict(), z.object({ kind: z.literal("type"), ref, text: z.string().max(STANDARD_ACTION_VALUE_MAX_CHARS) }).strict(), + z.object({ kind: z.literal("type"), selector, text: z.string().max(STANDARD_ACTION_VALUE_MAX_CHARS) }).strict(), z.object({ kind: z.literal("fill"), ref, text: z.string().max(STANDARD_ACTION_VALUE_MAX_CHARS) }).strict(), + z.object({ kind: z.literal("fill"), selector, text: z.string().max(STANDARD_ACTION_VALUE_MAX_CHARS) }).strict(), z.object({ kind: z.literal("select"), ref, value: z.string().max(STANDARD_ACTION_VALUE_MAX_CHARS) }).strict(), z.object({ kind: z.literal("scroll"), @@ -203,6 +207,11 @@ const DEFINITIONS: ReadonlyArray<{ ref, files: z.array(z.string().min(1).max(512)).min(1).max(4), }).strict(), + z.object({ + kind: z.literal("upload_file"), + selector, + files: z.array(z.string().min(1).max(512)).min(1).max(4), + }).strict(), ]); return z.object({ tab_id: z.number().int().min(0), diff --git a/packages/omp/src/pi-schema.ts b/packages/omp/src/pi-schema.ts index b95fcba..072c283 100644 --- a/packages/omp/src/pi-schema.ts +++ b/packages/omp/src/pi-schema.ts @@ -14,6 +14,7 @@ const object = (properties: Record) => { return Type.Object(typedProperties, { additionalProperties: false }); }; const ref = () => Type.String({ minLength: 1, maxLength: 256 }); +const selector = () => Type.String({ minLength: 1, maxLength: 2048 }); const schemas: Record = { browser_open: Type.Union([ @@ -62,8 +63,11 @@ const schemas: Record = { expected_page_revision: Type.Integer({ minimum: 0 }), actions: Type.Array(Type.Union([ object({ kind: Type.Literal("click"), ref: ref() }), + object({ kind: Type.Literal("click"), selector: selector() }), object({ kind: Type.Literal("type"), ref: ref(), text: Type.String({ maxLength: STANDARD_ACTION_VALUE_MAX_CHARS }) }), + object({ kind: Type.Literal("type"), selector: selector(), text: Type.String({ maxLength: STANDARD_ACTION_VALUE_MAX_CHARS }) }), object({ kind: Type.Literal("fill"), ref: ref(), text: Type.String({ maxLength: STANDARD_ACTION_VALUE_MAX_CHARS }) }), + object({ kind: Type.Literal("fill"), selector: selector(), text: Type.String({ maxLength: STANDARD_ACTION_VALUE_MAX_CHARS }) }), object({ kind: Type.Literal("select"), ref: ref(), value: Type.String({ maxLength: STANDARD_ACTION_VALUE_MAX_CHARS }) }), object({ kind: Type.Literal("scroll"), @@ -83,6 +87,11 @@ const schemas: Record = { ref: ref(), files: Type.Array(Type.String({ minLength: 1, maxLength: 512 }), { minItems: 1, maxItems: 4 }), }), + object({ + kind: Type.Literal("upload_file"), + selector: selector(), + files: Type.Array(Type.String({ minLength: 1, maxLength: 512 }), { minItems: 1, maxItems: 4 }), + }), ]), { minItems: 1, maxItems: 64 }), }), browser_wait: object({ diff --git a/packages/omp/src/render.ts b/packages/omp/src/render.ts index 185a28b..1f5f9e4 100644 --- a/packages/omp/src/render.ts +++ b/packages/omp/src/render.ts @@ -57,6 +57,7 @@ export interface OperationCard { const SENSITIVE_KEY = /(authorization|cookie|credential|password|secret|token)/i; const MAX_EXPANDED_LINES = 160; +export const MAX_SELECTOR_LENGTH = 72; const STATUS_LABEL: Readonly> = { planned: "🧭 Plan", running: "🔄 Working", @@ -291,25 +292,33 @@ function actionDescriptions(args: Record): string[] { upload_file: "Upload file", }; let target: string | undefined; + const targetRefOrSelector = fieldString(action, "ref") ?? fieldString(action, "selector"); if (kind === "type" || kind === "fill") { - target = joinMeta(selectorMeta(fieldString(action, "ref")), charCount(fieldString(action, "text"))); + target = joinMeta(selectorMeta(targetRefOrSelector), charCount(fieldString(action, "text"))); } else if (kind === "select") { - target = joinMeta(selectorMeta(fieldString(action, "ref")), "value hidden"); + target = joinMeta(selectorMeta(targetRefOrSelector), "value hidden"); } else if (kind === "drag") { - target = joinMeta(selectorMeta(fieldString(action, "ref")), "→", selectorMeta(fieldString(action, "target_ref"))); + target = joinMeta( + selectorMeta(targetRefOrSelector), + "→", + selectorMeta(fieldString(action, "target_ref") ?? fieldString(action, "target_selector")), + ); } else if (kind === "navigate") { target = safeUrl(fieldString(action, "url")); } else if (kind === "scroll") { target = joinMeta( - selectorMeta(fieldString(action, "ref")), + selectorMeta(targetRefOrSelector), `${fieldNumber(action, "delta_x") ?? 0},${fieldNumber(action, "delta_y") ?? 0}`, ); } else if (kind === "dialog") { target = fieldString(action, "decision"); } else if (kind === "upload_file") { - target = countSummary(Array.isArray(action.files) ? action.files : [], "file"); + target = joinMeta( + selectorMeta(targetRefOrSelector), + countSummary(Array.isArray(action.files) ? action.files : [], "file"), + ); } else { - target = selectorMeta(fieldString(action, "ref")); + target = selectorMeta(targetRefOrSelector); } return joinMeta(titles[kind] ?? humanize(kind), target) ?? humanize(kind); }); @@ -705,9 +714,13 @@ function tabMeta(tabId: number | undefined): string | undefined { return tabId === undefined ? undefined : `tab ${tabId}`; } -function selectorMeta(selector: string | undefined): string | undefined { +export function selectorMeta(selector: string | undefined): string | undefined { if (!selector) return undefined; - return selector.startsWith("ref=") ? selector.slice(4) : selector; + const cleaned = selector.replace(/[\s\x00-\x1f\x7f-\x9f\u2028\u2029]+/g, " ").trim(); + if (!cleaned) return undefined; + const target = cleaned.startsWith("ref=") ? cleaned.slice(4).trim() : cleaned; + if (!target) return undefined; + return truncate(target, MAX_SELECTOR_LENGTH); } function safeUrl(value: string | undefined): string | undefined { @@ -758,7 +771,7 @@ function renderHeader( } function truncate(value: string, width: number): string { - const text = value.replace(/[\r\n\t]+/g, " "); + const text = value.replace(/[\r\n\t\x00-\x1f\x7f-\x9f\u2028\u2029]+/g, " "); if (text.length <= width) return text; return width <= 1 ? "…" : `${text.slice(0, width - 1)}…`; } diff --git a/packages/omp/test/render.test.ts b/packages/omp/test/render.test.ts index 99c621b..fe162ff 100644 --- a/packages/omp/test/render.test.ts +++ b/packages/omp/test/render.test.ts @@ -1,7 +1,10 @@ import { describe, expect, test } from "bun:test"; import { + createCallCard, createCallComponent, createResultComponent, + MAX_SELECTOR_LENGTH, + selectorMeta, type RenderTheme, } from "../src/render"; @@ -259,4 +262,139 @@ describe("AgentTab operation card rendering", () => { ); expect(component.render(120)).toEqual(["🔄 Working · Working… · task-owned"]); }); + + test("selectorMeta sanitizes tabs, newlines, and control chars without multiline injection", () => { + const maliciousSelector = "button\r\n\t.submit\x00\x1b[31m[data-test='evil']\x7f"; + const card = createCallCard("browser_act", { + tab_id: 1, + expected_page_revision: 2, + actions: [{ kind: "click", selector: maliciousSelector }], + }); + + expect(card.title).toBe("Click"); + expect(card.meta[0]).toBe("button .submit [31m[data-test='evil']"); + expect(card.meta[0]).not.toMatch(/[\r\n\t\x00-\x1f\x7f-\x9f]/); + + const component = createCallComponent("browser_act", { + tab_id: 1, + expected_page_revision: 2, + actions: [{ kind: "click", selector: maliciousSelector }], + }, theme); + const lines = component.render(120); + + expect(lines.length).toBe(2); + expect(lines[0]).toBe( + "🧭 Plan · Click · button .submit [31m[data-test='evil'] · tab 1 · rev 2 · task-owned", + ); + for (const line of lines) { + expect(line).not.toMatch(/[\r\n\t\x00-\x1f\x7f-\x9f]/); + } + }); + + test("selectorMeta truncates long selectors to MAX_SELECTOR_LENGTH before joinMeta", () => { + const longSelector = "div.outer-wrapper > section.main-content > form#login-form > div.input-group > input.form-control[type='email'][name='username']"; + expect(longSelector.length).toBeGreaterThan(MAX_SELECTOR_LENGTH); + + const card = createCallCard("browser_act", { + tab_id: 5, + expected_page_revision: 1, + actions: [{ kind: "click", selector: longSelector }], + }); + + expect(card.meta[0].length).toBe(MAX_SELECTOR_LENGTH); + expect(card.meta[0].endsWith("…")).toBe(true); + expect(card.meta[0]).toBe( + longSelector.slice(0, MAX_SELECTOR_LENGTH - 1) + "…", + ); + expect(card.meta[0]).not.toContain(longSelector); + + const component = createCallComponent("browser_act", { + tab_id: 5, + expected_page_revision: 1, + actions: [{ kind: "click", selector: longSelector }], + }, theme); + const rendered = component.render(120).join("\n"); + + expect(rendered).not.toContain(longSelector); + expect(rendered).toContain(longSelector.slice(0, MAX_SELECTOR_LENGTH - 1) + "…"); + }); + + test("expanded multi-action cards prevent multiline injection and bound long selectors in action lists", () => { + const dirtySelector = "div\n\t.sidebar\r\n> a.link"; + const longSelector = "header.site-header > nav.navigation-menu > ul.nav-list > li.nav-item > a.nav-link.active[aria-current='page']"; + expect(longSelector.length).toBeGreaterThan(MAX_SELECTOR_LENGTH); + + const card = createCallCard("browser_act", { + tab_id: 8, + expected_page_revision: 3, + actions: [ + { kind: "click", selector: dirtySelector }, + { kind: "fill", selector: longSelector, text: "hello" }, + ], + }); + + expect(card.actions.length).toBe(2); + expect(card.actions[0]).toBe("Click · div .sidebar > a.link"); + expect(card.actions[1]).toBe(`Fill · ${longSelector.slice(0, MAX_SELECTOR_LENGTH - 1)}… · 5 characters`); + + const component = createCallComponent("browser_act", { + tab_id: 8, + expected_page_revision: 3, + actions: [ + { kind: "click", selector: dirtySelector }, + { kind: "fill", selector: longSelector, text: "hello" }, + ], + }, theme); + const lines = component.render(120); + + for (const line of lines) { + expect(line).not.toMatch(/[\r\n\t\x00-\x1f\x7f-\x9f]/); + expect(line).not.toContain(longSelector); + } + }); + + test("browser_snapshot and browser_wait sanitize and truncate selectors", () => { + const longDirtySelector = "main\n\t#content\r\n > div.container > section.post > article.body > div.paragraph > span.highlight"; + expect(longDirtySelector.length).toBeGreaterThan(MAX_SELECTOR_LENGTH); + + const snapshot = createCallComponent("browser_snapshot", { + tab_id: 3, + mode: "text", + selector: longDirtySelector, + }, theme); + const snapshotLines = snapshot.render(120); + expect(snapshotLines[0]).toContain("Snapshot text"); + expect(snapshotLines[0]).toContain("main #content > div.container > section.post > article.body > div.parag…"); + expect(snapshotLines[0]).not.toMatch(/[\r\n\t]/); + expect(snapshotLines[0]).not.toContain(longDirtySelector); + + const wait = createCallComponent("browser_wait", { + tab_id: 4, + condition: { kind: "selector", value: longDirtySelector }, + }, theme); + const waitLines = wait.render(120); + expect(waitLines[0]).toContain("Wait for Selector"); + expect(waitLines[0]).toContain("main #content > div.container > section.post > article.body > div.parag…"); + expect(waitLines[0]).not.toMatch(/[\r\n\t]/); + expect(waitLines[0]).not.toContain(longDirtySelector); + }); + + test("selectorMeta handles ref prefixes, whitespace padding, and empty edge cases", () => { + expect(selectorMeta(undefined)).toBeUndefined(); + expect(selectorMeta("")).toBeUndefined(); + expect(selectorMeta(" \r\n\t ")).toBeUndefined(); + expect(selectorMeta("\x00\x1f\x7f")).toBeUndefined(); + expect(selectorMeta("ref=")).toBeUndefined(); + expect(selectorMeta(" ref= ")).toBeUndefined(); + expect(selectorMeta("ref=e5")).toBe("e5"); + expect(selectorMeta(" ref=e5\n\t ")).toBe("e5"); + expect(selectorMeta("\tref=\te5\r\n")).toBe("e5"); + expect(selectorMeta("ref= btn#submit ")).toBe("btn#submit"); + + const emptyRefCard = createCallComponent("browser_act", { + tab_id: 10, + actions: [{ kind: "click", ref: " \t\n " }], + }, theme); + expect(emptyRefCard.render(120)[0]).toBe("🧭 Plan · Click · tab 10 · task-owned"); + }); }); diff --git a/packages/sdk-typescript/src/index.ts b/packages/sdk-typescript/src/index.ts index afc8ca3..b08a608 100644 --- a/packages/sdk-typescript/src/index.ts +++ b/packages/sdk-typescript/src/index.ts @@ -45,8 +45,8 @@ export type BrowserSnapshotParams = }; export type BrowserAction = - | { kind: "click"; ref: string } - | { kind: "type" | "fill"; ref: string; text: string } + | ({ kind: "click" } & ({ ref: string; selector?: never } | { ref?: never; selector: string })) + | ({ kind: "type" | "fill"; text: string } & ({ ref: string; selector?: never } | { ref?: never; selector: string })) | { kind: "select"; ref: string; value: string } | { kind: "scroll"; delta_x: number; delta_y: number; ref?: string } | { kind: "drag"; ref: string; target_ref: string } diff --git a/schemas/rpc/v1/browser-act.schema.json b/schemas/rpc/v1/browser-act.schema.json index fd7bf1a..78be11c 100644 --- a/schemas/rpc/v1/browser-act.schema.json +++ b/schemas/rpc/v1/browser-act.schema.json @@ -24,28 +24,46 @@ }, "click": { "type": "object", - "required": ["kind", "ref"], - "properties": { "kind": { "const": "click" }, "ref": { "$ref": "#/$defs/ref" } }, + "required": ["kind"], + "properties": { + "kind": { "const": "click" }, + "ref": { "$ref": "#/$defs/ref" }, + "selector": { "$ref": "#/$defs/selector" } + }, + "oneOf": [ + { "required": ["ref"] }, + { "required": ["selector"] } + ], "additionalProperties": false }, "type": { "type": "object", - "required": ["kind", "ref", "text"], + "required": ["kind", "text"], "properties": { "kind": { "const": "type" }, "ref": { "$ref": "#/$defs/ref" }, + "selector": { "$ref": "#/$defs/selector" }, "text": { "type": "string", "maxLength": 2048 } }, + "oneOf": [ + { "required": ["ref"] }, + { "required": ["selector"] } + ], "additionalProperties": false }, "fill": { "type": "object", - "required": ["kind", "ref", "text"], + "required": ["kind", "text"], "properties": { "kind": { "const": "fill" }, "ref": { "$ref": "#/$defs/ref" }, + "selector": { "$ref": "#/$defs/selector" }, "text": { "type": "string", "maxLength": 2048 } }, + "oneOf": [ + { "required": ["ref"] }, + { "required": ["selector"] } + ], "additionalProperties": false }, "select": { diff --git a/tests/architecture/verify_protocol_schemas.py b/tests/architecture/verify_protocol_schemas.py index 05e4c3a..e8c18d1 100755 --- a/tests/architecture/verify_protocol_schemas.py +++ b/tests/architecture/verify_protocol_schemas.py @@ -115,6 +115,19 @@ def verify_core_messages(schemas: dict[Path, dict], registry: Registry) -> int: }, mutation=True, ), + core_request( + "browser_act", + { + "tab_id": 1, + "expected_page_revision": 2, + "actions": [ + {"kind": "click", "selector": "button.submit"}, + {"kind": "type", "selector": "input.search", "text": "agenttab"}, + {"kind": "fill", "selector": "input.email", "text": "test@example.com"}, + ], + }, + mutation=True, + ), core_request( "browser_act", { @@ -222,6 +235,24 @@ def verify_core_messages(schemas: dict[Path, dict], registry: Registry) -> int: mutation=True, ) expect_invalid(request_validator, ambiguous_upload_target, "ambiguous upload target") + for invalid_action, label in [ + ({"kind": "click"}, "missing click target"), + ({"kind": "click", "ref": "e1", "selector": "button"}, "ambiguous click target"), + ({"kind": "type", "text": "hello"}, "missing type target"), + ({"kind": "type", "ref": "e1", "selector": "input", "text": "hello"}, "ambiguous type target"), + ({"kind": "fill", "text": "hello"}, "missing fill target"), + ({"kind": "fill", "ref": "e1", "selector": "input", "text": "hello"}, "ambiguous fill target"), + ]: + invalid_req = core_request( + "browser_act", + { + "tab_id": 1, + "expected_page_revision": 2, + "actions": [invalid_action], + }, + mutation=True, + ) + expect_invalid(request_validator, invalid_req, label) unknown = dict(requests[0], unexpected=True) expect_invalid(request_validator, unknown, "unknown request field")