fix: secure routed browser command boundaries

Author: chubes4

.github/workflows/agent-task-contracts.yml

@@ -14,6 +14,9 @@ on:
       - "fixtures/agent-task-runtime-paths-run-29305012941.json"
       - "package-lock.json"
       - "package.json"
+      - "packages/runtime-playground/src/**"
+      - "tests/browser-*.test.ts"
+      - "tests/editor-*.test.ts"
       - "tests/agent-task-*.test.ts"
       - "tests/runtime-sources-materialization.test.ts"
       - "tests/runtime-sources-playground-integration.test.ts"
@@ -47,6 +50,9 @@ on:
       - "fixtures/agent-task-runtime-paths-run-29305012941.json"
       - "package-lock.json"
       - "package.json"
+      - "packages/runtime-playground/src/**"
+      - "tests/browser-*.test.ts"
+      - "tests/editor-*.test.ts"
       - "tests/agent-task-*.test.ts"
       - "tests/runtime-sources-materialization.test.ts"
       - "tests/runtime-sources-playground-integration.test.ts"
@@ -99,6 +105,7 @@ jobs:
       - run: npm run test:runtime-command-artifact-bounds
       - run: npm run test:redaction
       - run: npm run test:browser-preview-routing
+      - run: npm run test:browser-routed-command-security
       - run: npm run test:production-boundary-enforcement
       - run: npm run test:runtime-tool-policy
 

package.json

@@ -109,6 +109,7 @@
     "smoke": "tsx scripts/run-smoke.ts",
     "test:redaction": "tsx tests/redaction.test.ts",
     "test:browser-preview-routing": "tsx --test tests/browser-preview-routing.test.ts",
+    "test:browser-routed-command-security": "tsx --test tests/browser-routed-command-security.test.ts",
     "test:cloudflare-runtime": "node --test tests/cloudflare-d1-provisioner.test.mjs && tsx tests/cloudflare-site-context.test.ts && tsx tests/cloudflare-coordinator-site-partitioning.test.ts && tsx tests/cloudflare-d1-operation-repository.test.ts && tsx tests/cloudflare-provisioning-api.test.ts && tsx tests/cloudflare-runtime.test.ts && node ./node_modules/typescript/bin/tsc -p packages/runtime-cloudflare --noEmit",
     "test:cloudflare-administrator-claim": "tsx tests/cloudflare-provisioning-api.test.ts",
     "test:cloudflare-wordpress-auth": "tsx tests/cloudflare-wordpress-auth.test.ts",

packages/runtime-playground/src/browser-actions-runner.ts

@@ -12,7 +12,8 @@ import { browserAssertionsSummary, browserStepRecord, executeBrowserInteractionS
 import { browserCommandLivenessPolicy, isBrowserCommandLivenessError, withBrowserCommandLiveness } from "./browser-liveness.js"
 import { serializeBrowserError } from "./browser-metrics.js"
 import { executeBrowserObservationAssertion } from "./browser-observation-assertions.js"
-import { browserPreviewNetworkPolicyIsActive, browserPreviewNetworkPolicySummary, browserPreviewNeedsContextRouting, browserPreviewTopology, resolveBrowserPreviewUrl, routeBrowserPreviewContextNetwork } from "./browser-preview-routing.js"
+import { browserPreviewNetworkPolicyIsActive, browserPreviewNetworkPolicySummary, browserPreviewNeedsContextRouting, browserPreviewTopology, closeBrowserAndDrainPreviewRoutes, createBrowserPreviewRouteTracker, resolveBrowserPreviewUrl, routeBrowserPreviewContextNetwork } from "./browser-preview-routing.js"
+import { browserCommandResult } from "./browser-result-sanitization.js"
 import { BROWSER_PROBE_STATE_INIT_SCRIPT, browserProbeReplayability, browserProbeViewport } from "./browser-probe.js"
 import { runBrowserProbeCommand, type BrowserProbeRunPlan } from "./browser-probe-runner.js"
 import { browserActionTargetUrls, browserAuthRequest, browserProbeWaterfallArtifact, browserProbeWebSocketArtifact, browserProbeWebSocketSummary, browserRedirectDiagnosticsArtifact, browserRequestCoverageArtifact, browserStorageStateAuthSummary, browserStorageStateImportFromArgs, browserWordPressDiagnosticsArtifact, createBrowserProbeProgressTracker, fileSha256, installBrowserWordPressDiagnostics, installWordPressAdminAuthCookies, livenessRemainingWallTimeMs, normalizeBrowserProbeScriptCheckpoint, type BrowserCommandProgressEvent, type BrowserStorageStateImport } from "./browser-probe-support.js"
@@ -127,6 +128,7 @@ export async function runBrowserActionsCommand({
   const browser = await launchChromiumBrowser()
   const topology = browserPreviewTopology(args, runtimeSpec, server.serverUrl)
   const { preview, networkPolicy } = topology
+  const routeTracker = createBrowserPreviewRouteTracker()
   let requestedUrl = initialUrl ? topology.resolveUrl(initialUrl) : preview.effectiveOrigin
   let finalUrl = requestedUrl
   let htmlSha256: string | undefined
@@ -148,7 +150,7 @@ export async function runBrowserActionsCommand({
       ...(storageStateImport ? { storageState: storageStateImport.storageState } : {}),
     }) : null
     if (context) {
-      await routeBrowserPreviewContextNetwork(context, networkPolicy, preview.effectiveOrigin)
+      await routeBrowserPreviewContextNetwork(context, networkPolicy, preview.effectiveOrigin, routeTracker)
     }
     const page = context ? await context.newPage() : await browser.newPage()
     if (onProgress) {
@@ -410,8 +412,11 @@ export async function runBrowserActionsCommand({
       }
     }
   } finally {
-    await settleBrowserNetworkTasks(networkTasks, livenessPolicy.networkSettleTimeoutMs)
-    await browser.close()
+    await settleBrowserNetworkTasks(networkTasks, livenessPolicy.networkSettleTimeoutMs).catch((error) => errors.push(serializeBrowserError("probe-error", error)))
+    for (const routeError of await closeBrowserAndDrainPreviewRoutes(browser, routeTracker)) {
+      errors.push(serializeBrowserError("probe-error", routeError))
+      pendingError ??= routeError
+    }
     if (capture.has("steps")) {
       await artifactSession.writeJsonLines("steps", "steps.jsonl", stepRecords)
     }
@@ -547,9 +552,7 @@ export async function runBrowserActionsCommand({
     throw new Error("wordpress.browser-actions did not produce a browser artifact")
   }
 
-  return {
-    artifact,
-    output: `${JSON.stringify({
+  return browserCommandResult(artifact, {
       command: "wordpress.browser-actions",
       requestedUrl,
       preview,
@@ -558,8 +561,7 @@ export async function runBrowserActionsCommand({
       files: artifact.files,
       summary: artifact.summary,
       steps: stepRecords,
-    }, null, 2)}\n`,
-  }
+  })
 }
 
 export function browserToolVerifierUnsupportedResult(step: BrowserInteractionStep, index: number, startedAt: string): BrowserToolVerifierResult {
@@ -944,17 +946,14 @@ export async function runBrowserScenarioCommand({
     throw new BrowserCommandArtifactError(`wordpress.browser-scenario failed: ${pendingError.message}`, artifact)
   }
 
-  return {
-    artifact,
-    output: `${JSON.stringify({
+  return browserCommandResult(artifact, {
       command: "wordpress.browser-scenario",
       requestedUrl: artifact.requestedUrl,
       finalUrl,
       files: artifact.files,
       summary: artifact.summary,
       scenario: scenarioSummary,
-    }, null, 2)}\n`,
-  }
+  })
 }
 
 async function browserScenarioFromArgs(args: string[]): Promise<BrowserScenarioInput> {

packages/runtime-playground/src/browser-artifact-session.ts

@@ -3,6 +3,7 @@ import { basename, join } from "node:path"
 import type { ArtifactProvenanceMetadata } from "@automattic/wp-codebox-core"
 import { ArtifactBundleWriter } from "./artifact-bundle-writer.js"
 import { browserArtifactFileManifest, type BrowserArtifactFiles } from "./browser-artifacts.js"
+import { sanitizeBrowserResultValue } from "./browser-result-sanitization.js"
 
 export class BrowserArtifactSession {
   readonly writer: ArtifactBundleWriter
@@ -32,11 +33,11 @@ export class BrowserArtifactSession {
   }
 
   async writeJson(key: keyof BrowserArtifactFiles, fileName: string, value: unknown): Promise<void> {
-    await this.writer.writeJson(this.path(fileName), value, this.manifestWithoutContentType(key))
+    await this.writer.writeJson(this.path(fileName), sanitizeBrowserResultValue(value), this.manifestWithoutContentType(key))
   }
 
   async writeJsonLines(key: keyof BrowserArtifactFiles, fileName: string, records: unknown[]): Promise<void> {
-    await this.writer.writeJsonLines(this.path(fileName), records, this.manifestWithoutContentType(key))
+    await this.writer.writeJsonLines(this.path(fileName), sanitizeBrowserResultValue(records), this.manifestWithoutContentType(key))
   }
 
   async writeGenerated(key: keyof BrowserArtifactFiles, fileName: string, write: (absolutePath: string) => Promise<void>): Promise<void> {

packages/runtime-playground/src/browser-command-artifact-error.ts

@@ -1,9 +1,14 @@
 import type { BrowserArtifact } from "./browser-artifacts.js"
+import { sanitizeBrowserArtifact, sanitizeBrowserResultValue } from "./browser-result-sanitization.js"
 
 export class BrowserCommandArtifactError extends Error {
-  constructor(message: string, readonly artifact: BrowserArtifact, readonly artifactRoot?: string) {
-    super(message)
+  readonly artifact: BrowserArtifact
+
+  constructor(message: string, artifact: BrowserArtifact, readonly artifactRoot?: string) {
+    super(sanitizeBrowserResultValue(message, "message"))
     this.name = "BrowserCommandArtifactError"
+    Object.assign(artifact, sanitizeBrowserArtifact(artifact))
+    this.artifact = artifact
   }
 }
 

packages/runtime-playground/src/browser-multi-actor-scenario-runner.ts

@@ -6,7 +6,8 @@ import { BrowserCommandArtifactError } from "./browser-command-artifact-error.js
 import { attachBrowserCaptureListeners, launchChromiumBrowser, settleBrowserNetworkTasks } from "./browser-capture-session.js"
 import { executeBrowserInteractionStep } from "./browser-interactions.js"
 import { browserProbeReplayability } from "./browser-probe.js"
-import { browserPreviewTopology, routeBrowserPreviewContextNetwork } from "./browser-preview-routing.js"
+import { browserPreviewTopology, closeBrowserAndDrainPreviewRoutes, createBrowserPreviewRouteTracker, routeBrowserPreviewContextNetwork } from "./browser-preview-routing.js"
+import { browserCommandResult } from "./browser-result-sanitization.js"
 import { installWordPressAdminAuthCookies } from "./browser-probe-support.js"
 import { bootstrapPhpCode } from "./php-bootstrap.js"
 import { assertPlaygroundResponseOk, type PlaygroundRunResponse } from "./playground-command-errors.js"
@@ -30,6 +31,7 @@ export async function runBrowserMultiActorScenarioCommand(input: {
   const artifacts = new BrowserArtifactSession(artifactRoot, "files/browser", { source: "wordpress.browser-scenario", operation: "browser-multi-actor-scenario" })
   const topology = browserPreviewTopology([], runtimeSpec, server.serverUrl)
   const browser = await launchChromiumBrowser()
+  const routeTracker = createBrowserPreviewRouteTracker()
   const evidence: Record<string, ActorEvidence> = {}
   let result: BrowserMultiActorScenarioResult | undefined
   let failure: Error | undefined
@@ -44,7 +46,7 @@ export async function runBrowserMultiActorScenarioCommand(input: {
       if (!session) throw new Error(`Actor ${actor.name} requires user session ${actor.userSession}`)
       const userId = await actorUserId(actor.name, session.user.userId, session.user, runtimeSpec, runPlaygroundCommand, server)
       const context = await browser.newContext()
-      await routeBrowserPreviewContextNetwork(context, topology.networkPolicy, topology.preview.effectiveOrigin)
+      await routeBrowserPreviewContextNetwork(context, topology.networkPolicy, topology.preview.effectiveOrigin, routeTracker)
       const page = await context.newPage()
       await context.tracing.start({ screenshots: true, snapshots: true })
       await installWordPressAdminAuthCookies({ command: "wordpress.browser-scenario", cookieUrls: topology.authCookieUrls([topology.resolveUrl(scenario.url)]), page, runPlaygroundCommand, runtimeSpec, server, userId })
@@ -61,7 +63,8 @@ export async function runBrowserMultiActorScenarioCommand(input: {
     failure = error instanceof Error ? error : new Error(String(error))
     if (error instanceof BrowserMultiActorScenarioError) result = error.result
   } finally {
-    await browser.close()
+    const routeErrors = await closeBrowserAndDrainPreviewRoutes(browser, routeTracker)
+    failure ??= routeErrors[0]
   }
 
   const replay = result?.replay ?? { schema: "wp-codebox/browser-multi-actor-replay/v1", seed: scenario.seed, scenario, schedule: [] }
@@ -79,7 +82,7 @@ export async function runBrowserMultiActorScenarioCommand(input: {
   const traces = Object.values(evidence).map((actor) => actor.files.trace).filter((path): path is string => Boolean(path))
   const artifact = { artifactType: "scenario" as const, requestedUrl: target, url: target, preview: topology.preview, ...topology.origins, files: { summary: "files/browser/multi-actor-scenario-summary.json", steps: "files/browser/multi-actor-events.json", network: "files/browser/multi-actor-network.json", requestCoverage: "files/browser/multi-actor-request-coverage.json", waterfall: "files/browser/multi-actor-waterfall.json", ...(traces.length > 0 ? { traces } : {}) }, summary: { actions: scenario.actions.length, steps: scenario.actions.length, consoleMessages: Object.values(evidence).reduce((total, actor) => total + actor.console.length, 0), errors: Object.values(evidence).reduce((total, actor) => total + actor.errors.length, 0), finalUrl: target, htmlSnapshot: false, networkEvents: network.length, replayability: browserProbeReplayability(captures), screenshot: captures.has("screenshot"), viewport: null, multiActor: { seed: scenario.seed, finalState: result?.finalState ?? "failed", actors: Object.keys(evidence), replay: "files/browser/multi-actor-replay.json" } } } satisfies BrowserArtifact
   if (failure) throw new BrowserCommandArtifactError(`wordpress.browser-scenario failed: ${failure.message}`, artifact)
-  return { artifact, output: `${JSON.stringify({ command: "wordpress.browser-scenario", files: artifact.files, summary: artifact.summary, scenario: summary }, null, 2)}\n` }
+  return browserCommandResult(artifact, { command: "wordpress.browser-scenario", files: artifact.files, summary: artifact.summary, scenario: summary })
 }
 
 export async function navigateBrowserMultiActorPages(

packages/runtime-playground/src/browser-preview-routing.ts

@@ -296,6 +296,22 @@ export async function drainBrowserPreviewRouteTracker(tracker: BrowserPreviewRou
   }
 }
 
+export async function closeBrowserAndDrainPreviewRoutes(browser: Pick<import("playwright").Browser, "close">, tracker: BrowserPreviewRouteTracker): Promise<Error[]> {
+  const errors: Error[] = []
+  try {
+    await browser.close()
+  } catch (error) {
+    errors.push(browserPreviewLifecycleError("browser-close", error))
+  } finally {
+    try {
+      await drainBrowserPreviewRouteTracker(tracker)
+    } catch (error) {
+      errors.push(browserPreviewLifecycleError("route-drain", error))
+    }
+  }
+  return errors
+}
+
 function browserPreviewMode(args: string[], publicOrigin: string | undefined): BrowserProbePreviewMode {
   const raw = argValue(args, "preview-mode")?.trim() || (publicOrigin ? "public" : "local")
   if (raw === "local" || raw === "public" || raw === "secure") {
@@ -575,6 +591,13 @@ function browserPreviewRouteRequestSummary(route: Route): { method: string; reso
   }
 }
 
+function browserPreviewLifecycleError(operation: string, error: unknown): Error {
+  const cause = sanitizeBrowserPreviewRouteError(error).message.replace(/[\r\n]+/g, " ")
+  const diagnostic = new Error(`wordpress.browser-probe route lifecycle failed: operation=${operation} cause=${cause}`)
+  diagnostic.name = "BrowserPreviewRouteLifecycleError"
+  return diagnostic
+}
+
 function browserPreviewRouteFetchExhaustedError(route: Route, requestUrl: URL, attempts: number, error: unknown): Error {
   const method = route.request().method()
   const resourceType = route.request().resourceType()

packages/runtime-playground/src/browser-probe-runner.ts

@@ -6,7 +6,7 @@ import { attachBrowserCaptureListeners, chromiumBrowserMetadata, launchChromiumB
 import { browserCommandLivenessPolicy, isBrowserCommandLivenessError, withBrowserCommandLiveness } from "./browser-liveness.js"
 import { browserProbeLifecycleArtifact, browserProbeLifecycleInitScript, collectBrowserProbeLifecycle } from "./browser-lifecycle.js"
 import { browserProbeBenchMetrics, serializeBrowserError } from "./browser-metrics.js"
-import { browserPreviewNetworkPolicyIsActive, browserPreviewNetworkPolicySummary, browserPreviewNeedsContextRouting, browserPreviewReadinessError, browserPreviewSecureContextError, browserPreviewTopology, createBrowserPreviewRouteTracker, drainBrowserPreviewRouteTracker, routeBrowserPreviewContextNetwork, routeBrowserPreviewPageNetwork } from "./browser-preview-routing.js"
+import { browserPreviewNetworkPolicyIsActive, browserPreviewNetworkPolicySummary, browserPreviewNeedsContextRouting, browserPreviewReadinessError, browserPreviewSecureContextError, browserPreviewTopology, closeBrowserAndDrainPreviewRoutes, createBrowserPreviewRouteTracker, drainBrowserPreviewRouteTracker, routeBrowserPreviewContextNetwork, routeBrowserPreviewPageNetwork } from "./browser-preview-routing.js"
 import { BROWSER_PROBE_PERFORMANCE_INIT_SCRIPT, BROWSER_PROBE_STATE_INIT_SCRIPT, browserProbeAssertionsFromArgs, browserProbeCheckpoint, browserProbeMemoryArtifact, browserProbePendingCheckpoints, browserProbePerformanceArtifact, browserProbeViewport, executeBrowserProbeAssertions, navigateBrowserProbe } from "./browser-probe.js"
 import { argValue, commaListArg, durationArg, strictBooleanArg, viewportArg } from "./commands.js"
 import type { PlaygroundRunResponse } from "./playground-command-errors.js"
@@ -492,18 +492,28 @@ export async function runSingleBrowserProbeCommand({
         }
       }
     }
-    await settleBrowserNetworkTasks(networkTasks, livenessPolicy.networkSettleTimeoutMs)
-    await geolocationPermissionCleanup?.()
-    await browser.close()
     try {
-      await drainBrowserPreviewRouteTracker(routeTracker)
+      await settleBrowserNetworkTasks(networkTasks, livenessPolicy.networkSettleTimeoutMs)
+    } catch (error) {
+      errors.push(serializeBrowserError("probe-error", error))
+    }
+    try {
+      await geolocationPermissionCleanup?.()
     } catch (error) {
-      const routeError = redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true })
-      if (!pendingError && runPlan.routeHostDrain === "required") {
+      const cleanupError = redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true })
+      if (!pendingError) {
+        pendingError = cleanupError
+        progress.fail("probe-error", cleanupError)
+      }
+      errors.push(serializeBrowserError("probe-error", error))
+    }
+    for (const routeError of await closeBrowserAndDrainPreviewRoutes(browser, routeTracker)) {
+      const browserCloseFailed = routeError.message.includes("operation=browser-close")
+      if (!pendingError && (browserCloseFailed || runPlan.routeHostDrain === "required")) {
         pendingError = routeError
         progress.fail("probe-error", routeError)
       }
-      errors.push(serializeBrowserError("probe-error", error))
+      errors.push(serializeBrowserError("probe-error", routeError))
     }
     if (captureSelection.console) {
       await artifactSession.writeJsonLines("console", "console.jsonl", consoleMessages)