diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 7e6845f..2e59ea1 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -165,15 +165,38 @@ export class WebSocketTransport implements BAPTransport { /** Called when reconnection succeeds */ onReconnected: (() => void) | null = null; + private baseUrl: string; + private token: string | undefined; + constructor( - private readonly url: string, + url: string, options: WebSocketTransportOptions = {} ) { + this.baseUrl = url; this.maxReconnectAttempts = options.maxReconnectAttempts ?? 5; this.reconnectDelay = options.reconnectDelay ?? 1000; this.autoReconnect = options.autoReconnect ?? false; } + /** + * Get the current connection URL, including token if set + */ + private getConnectionUrl(): string { + if (!this.token) { + return this.baseUrl; + } + const urlObj = new URL(this.baseUrl); + urlObj.searchParams.set("token", this.token); + return urlObj.toString(); + } + + /** + * Update the authentication token. Takes effect on the next connection/reconnection. + */ + updateToken(newToken: string): void { + this.token = newToken; + } + /** * Connect to the WebSocket server */ @@ -189,7 +212,7 @@ export class WebSocketTransport implements BAPTransport { this.ws = null; } - this.ws = new WebSocket(this.url); + this.ws = new WebSocket(this.getConnectionUrl()); this.ws.on("open", () => { this.reconnectAttempts = 0; @@ -382,13 +405,11 @@ export class BAPClient extends EventEmitter { }; if (typeof urlOrTransport === "string") { - let url = urlOrTransport; + const wsTransport = new WebSocketTransport(urlOrTransport); if (options.token) { - const urlObj = new URL(url); - urlObj.searchParams.set("token", options.token); - url = urlObj.toString(); + wsTransport.updateToken(options.token); } - this.transport = new WebSocketTransport(url); + this.transport = wsTransport; } else { this.transport = urlOrTransport; } @@ -398,6 +419,18 @@ export class BAPClient extends EventEmitter { this.transport.onError = (error) => this.emit("error", error); } + /** + * Update the authentication token. + * Takes effect on the next connection or reconnection attempt. + * Only works when the transport is a WebSocketTransport. + */ + updateToken(newToken: string): void { + this.options.token = newToken; + if (this.transport instanceof WebSocketTransport) { + this.transport.updateToken(newToken); + } + } + // =========================================================================== // Connection Management // =========================================================================== diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index 368e559..4263977 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -6,7 +6,7 @@ * Exposes Browser Agent Protocol as an MCP (Model Context Protocol) server. * Allows AI agents to control browsers through standardized MCP tools. * - * TODO (MEDIUM): Add input validation on tool arguments before passing to BAP client + * TODO (MEDIUM): Add input validation on tool arguments before passing to BAP client — DONE (Zod schemas for top 10 tools) * TODO (MEDIUM): Enforce session timeout (maxSessionDuration) - currently unused * TODO (MEDIUM): Add resource cleanup on partial failure in ensureClient() — DONE (v0.2.0) * TODO (LOW): parseSelector should validate empty/whitespace-only strings @@ -41,6 +41,99 @@ import { type AriaRole, type AgentObserveResult, } from "@browseragentprotocol/protocol"; +import { z } from "zod"; + +// ============================================================================= +// Input Validation Schemas +// ============================================================================= + +/** Validates a non-empty string argument */ +const nonEmptyString = z.string().min(1, "must be a non-empty string"); + +/** Validates a URL string (must include protocol) */ +const urlString = nonEmptyString.refine( + (val) => { + try { + new URL(val); + return true; + } catch { + return false; + } + }, + { message: "must be a valid URL (include protocol, e.g. https://)" } +); + +/** Validates a positive integer */ +const positiveInt = z.number().int().positive(); + +/** Validates a non-negative number */ +const nonNegativeNumber = z.number().nonnegative(); + +/** Validation schemas for tool arguments that take user-facing inputs */ +const ToolArgSchemas = { + navigate: z.object({ + url: urlString, + waitUntil: z.enum(["load", "domcontentloaded", "networkidle"]).optional(), + observe: z.boolean().optional(), + observeMaxElements: positiveInt.optional(), + }), + click: z.object({ + selector: nonEmptyString, + clickCount: positiveInt.optional(), + }), + type: z.object({ + selector: nonEmptyString, + text: z.string(), + delay: nonNegativeNumber.optional(), + }), + fill: z.object({ + selector: nonEmptyString, + value: z.string(), + }), + press: z.object({ + key: nonEmptyString, + selector: nonEmptyString.optional(), + }), + select: z.object({ + selector: nonEmptyString, + value: nonEmptyString, + }), + hover: z.object({ + selector: nonEmptyString, + }), + element: z.object({ + selector: nonEmptyString, + properties: z.array(z.string()).optional(), + }), + activate_page: z.object({ + pageId: nonEmptyString, + }), + extract: z.object({ + instruction: nonEmptyString, + schema: z.object({ type: z.string() }).passthrough(), + mode: z.enum(["single", "list", "table"]).optional(), + selector: nonEmptyString.optional(), + }), +} as const; + +/** + * Validate tool arguments against a Zod schema. + * Returns the parsed (and typed) arguments, or throws a descriptive error. + */ +function validateArgs( + toolName: string, + schema: T, + args: Record +): z.infer { + const result = schema.safeParse(args); + if (!result.success) { + const issues = result.error.issues + .map((i: z.ZodIssue) => `${i.path.join(".")}: ${i.message}`) + .join("; "); + throw new Error(`Invalid arguments for '${toolName}': ${issues}`); + } + return result.data; +} // ============================================================================= // Types @@ -939,7 +1032,8 @@ export class BAPMCPServer { switch (name) { // Navigation case "navigate": { - const url = args.url as string; + const validated = validateArgs("navigate", ToolArgSchemas.navigate, args); + const url = validated.url; // Security check if (!this.isAllowedDomain(url)) { @@ -963,17 +1057,17 @@ export class BAPMCPServer { }; } - const waitUntil = (args.waitUntil as WaitUntilState) ?? "load"; + const waitUntil = (validated.waitUntil as WaitUntilState) ?? "load"; // Fusion: navigate-observe kernel - const observeFlag = args.observe as boolean | undefined; + const observeFlag = validated.observe; const result = await client.navigate(url, { waitUntil, ...(observeFlag ? { observe: { includeMetadata: true, includeInteractiveElements: true, - maxElements: (args.observeMaxElements as number) ?? 50, + maxElements: validated.observeMaxElements ?? 50, }, } : {}), }); @@ -1005,48 +1099,48 @@ export class BAPMCPServer { // Element Interaction case "click": { - const selector = parseSelector(args.selector as string); - const options = args.clickCount ? { clickCount: args.clickCount as number } : undefined; + const validated = validateArgs("click", ToolArgSchemas.click, args); + const selector = parseSelector(validated.selector); + const options = validated.clickCount ? { clickCount: validated.clickCount } : undefined; await client.click(selector, options); return { - content: [{ type: "text", text: `Clicked: ${args.selector}` }], + content: [{ type: "text", text: `Clicked: ${validated.selector}` }], }; } case "type": { - const selector = parseSelector(args.selector as string); - const text = args.text as string; - const delay = args.delay as number | undefined; - await client.type(selector, text, { delay }); + const validated = validateArgs("type", ToolArgSchemas.type, args); + const selector = parseSelector(validated.selector); + await client.type(selector, validated.text, { delay: validated.delay }); return { - content: [{ type: "text", text: `Typed "${text}" into: ${args.selector}` }], + content: [{ type: "text", text: `Typed "${validated.text}" into: ${validated.selector}` }], }; } case "fill": { - const selector = parseSelector(args.selector as string); - const value = args.value as string; - await client.fill(selector, value); + const validated = validateArgs("fill", ToolArgSchemas.fill, args); + const selector = parseSelector(validated.selector); + await client.fill(selector, validated.value); return { - content: [{ type: "text", text: `Filled "${value}" into: ${args.selector}` }], + content: [{ type: "text", text: `Filled "${validated.value}" into: ${validated.selector}` }], }; } case "press": { - const key = args.key as string; - const selector = args.selector ? parseSelector(args.selector as string) : undefined; - await client.press(key, selector); + const validated = validateArgs("press", ToolArgSchemas.press, args); + const selector = validated.selector ? parseSelector(validated.selector) : undefined; + await client.press(validated.key, selector); return { - content: [{ type: "text", text: `Pressed: ${key}` }], + content: [{ type: "text", text: `Pressed: ${validated.key}` }], }; } case "select": { - const selector = parseSelector(args.selector as string); - const value = args.value as string; - await client.select(selector, value); + const validated = validateArgs("select", ToolArgSchemas.select, args); + const selector = parseSelector(validated.selector); + await client.select(selector, validated.value); return { - content: [{ type: "text", text: `Selected "${value}" in: ${args.selector}` }], + content: [{ type: "text", text: `Selected "${validated.value}" in: ${validated.selector}` }], }; } @@ -1061,10 +1155,11 @@ export class BAPMCPServer { } case "hover": { - const selector = parseSelector(args.selector as string); + const validated = validateArgs("hover", ToolArgSchemas.hover, args); + const selector = parseSelector(validated.selector); await client.hover(selector); return { - content: [{ type: "text", text: `Hovered over: ${args.selector}` }], + content: [{ type: "text", text: `Hovered over: ${validated.selector}` }], }; } @@ -1120,8 +1215,9 @@ export class BAPMCPServer { } case "element": { - const selector = parseSelector(args.selector as string); - const properties = (args.properties as ElementProperty[]) ?? ["visible", "enabled"]; + const validated = validateArgs("element", ToolArgSchemas.element, args); + const selector = parseSelector(validated.selector); + const properties = (validated.properties as ElementProperty[]) ?? ["visible", "enabled"]; const result = await client.element(selector, properties); return { content: [ @@ -1145,10 +1241,10 @@ export class BAPMCPServer { } case "activate_page": { - const pageId = args.pageId as string; - await client.activatePage(pageId); + const validated = validateArgs("activate_page", ToolArgSchemas.activate_page, args); + await client.activatePage(validated.pageId); return { - content: [{ type: "text", text: `Activated page: ${pageId}` }], + content: [{ type: "text", text: `Activated page: ${validated.pageId}` }], }; } @@ -1373,11 +1469,12 @@ export class BAPMCPServer { } case "extract": { + const validated = validateArgs("extract", ToolArgSchemas.extract, args); const result = await client.extract({ - instruction: args.instruction as string, - schema: args.schema as ExtractionSchema, - mode: args.mode as "single" | "list" | "table" | undefined, - selector: args.selector ? parseSelector(args.selector as string) : undefined, + instruction: validated.instruction, + schema: validated.schema as ExtractionSchema, + mode: validated.mode, + selector: validated.selector ? parseSelector(validated.selector) : undefined, }); if (result.success) { diff --git a/packages/server-playwright/src/server.ts b/packages/server-playwright/src/server.ts index e745f77..b529bf7 100644 --- a/packages/server-playwright/src/server.ts +++ b/packages/server-playwright/src/server.ts @@ -13,7 +13,8 @@ * See BAPScope type for available scopes. */ -import { randomUUID } from "crypto"; +import { randomUUID, timingSafeEqual } from "node:crypto"; +import fs from "node:fs"; import { EventEmitter } from "events"; import * as http from "http"; import * as path from "path"; @@ -116,6 +117,13 @@ import { createErrorResponse, createNotification, isRequest, + // Authorization (shared with protocol package) + type BAPScope, + ScopeProfiles, + MethodScopes, + hasScope, + parseScopes, + createAuthorizationError, } from "@browseragentprotocol/protocol"; import { generateStableRef, @@ -125,143 +133,6 @@ import { type PageElementRegistry, } from "@browseragentprotocol/protocol"; -// ============================================================================= -// Authorization Types (v0.2.0) - Inlined for build compatibility -// ============================================================================= - -/** - * BAP authorization scopes for fine-grained access control - */ -type BAPScope = - | '*' - | 'browser:*' | 'browser:launch' | 'browser:close' - | 'context:*' | 'context:create' | 'context:read' | 'context:destroy' - | 'page:*' | 'page:read' | 'page:create' | 'page:navigate' | 'page:close' - | 'action:*' | 'action:click' | 'action:type' | 'action:fill' | 'action:scroll' - | 'action:select' | 'action:upload' | 'action:drag' - | 'observe:*' | 'observe:screenshot' | 'observe:accessibility' | 'observe:dom' - | 'observe:element' | 'observe:content' | 'observe:pdf' - | 'storage:*' | 'storage:read' | 'storage:write' - | 'network:*' | 'network:intercept' - | 'emulate:*' | 'emulate:viewport' | 'emulate:geolocation' | 'emulate:offline' - | 'trace:*' | 'trace:start' | 'trace:stop'; - -/** Predefined scope profiles for common use cases */ -const ScopeProfiles = { - readonly: ['page:read', 'observe:*'] as BAPScope[], - standard: [ - 'browser:launch', 'browser:close', 'page:*', - 'action:*', - 'observe:*', 'emulate:viewport', - ] as BAPScope[], - full: ['browser:*', 'page:*', 'action:*', 'observe:*', 'emulate:*', 'trace:*'] as BAPScope[], - privileged: ['*'] as BAPScope[], -} as const; - -/** Method to required scopes mapping */ -const MethodScopes: Record = { - 'initialize': [], 'shutdown': [], 'notifications/initialized': [], - 'browser/launch': ['browser:launch', 'browser:*', '*'], - 'browser/close': ['browser:close', 'browser:*', '*'], - // Context methods (Multi-Context Support) - 'context/create': ['context:create', 'context:*', '*'], - 'context/list': ['context:read', 'context:*', '*'], - 'context/destroy': ['context:destroy', 'context:*', '*'], - // Page methods - 'page/create': ['page:create', 'page:*', '*'], - 'page/navigate': ['page:navigate', 'page:*', '*'], - 'page/reload': ['page:navigate', 'page:*', '*'], - 'page/goBack': ['page:navigate', 'page:*', '*'], - 'page/goForward': ['page:navigate', 'page:*', '*'], - 'page/close': ['page:close', 'page:*', '*'], - 'page/list': ['page:read', 'page:*', '*'], - 'page/activate': ['page:read', 'page:*', '*'], - // Frame methods (Frame & Shadow DOM Support) - 'frame/list': ['page:read', 'page:*', '*'], - 'frame/switch': ['page:navigate', 'page:*', '*'], - 'frame/main': ['page:navigate', 'page:*', '*'], - 'action/click': ['action:click', 'action:*', '*'], - 'action/dblclick': ['action:click', 'action:*', '*'], - 'action/type': ['action:type', 'action:*', '*'], - 'action/fill': ['action:fill', 'action:*', '*'], - 'action/clear': ['action:fill', 'action:*', '*'], - 'action/press': ['action:type', 'action:*', '*'], - 'action/hover': ['action:click', 'action:*', '*'], - 'action/scroll': ['action:scroll', 'action:*', '*'], - 'action/select': ['action:select', 'action:*', '*'], - 'action/check': ['action:click', 'action:*', '*'], - 'action/uncheck': ['action:click', 'action:*', '*'], - 'action/upload': ['action:upload', 'action:*', '*'], - 'action/drag': ['action:drag', 'action:*', '*'], - 'observe/screenshot': ['observe:screenshot', 'observe:*', '*'], - 'observe/accessibility': ['observe:accessibility', 'observe:*', '*'], - 'observe/dom': ['observe:dom', 'observe:*', '*'], - 'observe/element': ['observe:element', 'observe:*', '*'], - 'observe/pdf': ['observe:pdf', 'observe:*', '*'], - 'observe/content': ['observe:content', 'observe:*', '*'], - 'observe/ariaSnapshot': ['observe:accessibility', 'observe:*', '*'], - 'storage/getState': ['storage:read', 'storage:*', '*'], - 'storage/setState': ['storage:write', 'storage:*', '*'], - 'storage/getCookies': ['storage:read', 'storage:*', '*'], - 'storage/setCookies': ['storage:write', 'storage:*', '*'], - 'storage/clearCookies': ['storage:write', 'storage:*', '*'], - 'network/intercept': ['network:intercept', 'network:*', '*'], - 'network/fulfill': ['network:intercept', 'network:*', '*'], - 'network/abort': ['network:intercept', 'network:*', '*'], - 'network/continue': ['network:intercept', 'network:*', '*'], - 'emulate/setViewport': ['emulate:viewport', 'emulate:*', '*'], - 'emulate/setUserAgent': ['emulate:viewport', 'emulate:*', '*'], - 'emulate/setGeolocation': ['emulate:geolocation', 'emulate:*', '*'], - 'emulate/setOffline': ['emulate:offline', 'emulate:*', '*'], - 'dialog/handle': ['action:click', 'action:*', '*'], - 'trace/start': ['trace:start', 'trace:*', '*'], - 'trace/stop': ['trace:stop', 'trace:*', '*'], - 'events/subscribe': ['observe:*', '*'], - // Stream methods (Streaming Responses) - 'stream/cancel': ['observe:*', '*'], - // Approval methods (Human-in-the-Loop) - 'approval/respond': ['action:*', '*'], - // Agent methods (composite actions, observations, and data extraction) - 'agent/act': ['action:*', '*'], - 'agent/observe': ['observe:*', '*'], - 'agent/extract': ['observe:*', '*'], -}; - -/** Check if client has permission for a method */ -function hasScope(grantedScopes: BAPScope[], method: string): boolean { - if (grantedScopes.includes('*')) return true; - const requiredScopes = MethodScopes[method]; - if (!requiredScopes) return grantedScopes.includes('*'); - if (requiredScopes.length === 0) return true; - return requiredScopes.some(required => { - if (grantedScopes.includes(required)) return true; - const [category] = required.split(':'); - return grantedScopes.includes(`${category}:*` as BAPScope); - }); -} - -/** Parse scopes from string or array */ -function parseScopes(input: string | string[] | undefined): BAPScope[] { - if (!input) return []; - if (Array.isArray(input)) return input as BAPScope[]; - return input.split(',').map(s => s.trim()) as BAPScope[]; -} - -/** Authorization error code */ -const AuthorizationErrorCode = -32023; - -/** Create an authorization error */ -function createAuthorizationError(method: string, requiredScopes: BAPScope[]) { - return { - code: AuthorizationErrorCode, - message: `Insufficient permissions for '${method}'. Required scopes: ${requiredScopes.join(' or ')}`, - data: { - retryable: false, - details: { method, requiredScopes }, - }, - }; -} - /** Action confirmation event for agent feedback */ interface ActionConfirmationEvent { pageId: string; @@ -696,7 +567,6 @@ export class BAPPlaywrightServer extends EventEmitter { * SECURITY: Timing-safe token comparison to prevent timing attacks */ private secureTokenCompare(provided: string, expected: string): boolean { - const { timingSafeEqual } = require('crypto'); if (provided.length !== expected.length) { // Still do a comparison to maintain constant time timingSafeEqual(Buffer.from(provided), Buffer.from(provided)); @@ -1315,7 +1185,6 @@ export class BAPPlaywrightServer extends EventEmitter { // SECURITY FIX (CRIT-4): Validate downloads path to prevent path traversal attacks let validatedDownloadsPath: string | undefined = undefined; if (params.downloadsPath) { - const fs = require('fs'); const allowedDownloadDirs = process.env.BAP_ALLOWED_DOWNLOAD_DIRS?.split(',').filter(Boolean) || []; // Resolve the path first