Skip to content

Commit 4366692

Browse files
authored
fix: harden routed preview recovery boundaries (#2103)
* fix: harden routed preview recovery boundaries * fix: secure routed browser command boundaries * fix: complete browser cleanup security boundaries * fix: preserve routed security across browser sessions * fix: preserve canonical routing in shared scenarios
1 parent 3f9ca38 commit 4366692

18 files changed

Lines changed: 880 additions & 177 deletions

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ on:
1414
- "fixtures/agent-task-runtime-paths-run-29305012941.json"
1515
- "package-lock.json"
1616
- "package.json"
17+
- "packages/runtime-playground/src/**"
18+
- "tests/browser-*.test.ts"
19+
- "tests/editor-*.test.ts"
1720
- "tests/agent-task-*.test.ts"
1821
- "tests/runtime-sources-materialization.test.ts"
1922
- "tests/runtime-sources-playground-integration.test.ts"
@@ -58,6 +61,9 @@ on:
5861
- "fixtures/agent-task-runtime-paths-run-29305012941.json"
5962
- "package-lock.json"
6063
- "package.json"
64+
- "packages/runtime-playground/src/**"
65+
- "tests/browser-*.test.ts"
66+
- "tests/editor-*.test.ts"
6167
- "tests/agent-task-*.test.ts"
6268
- "tests/runtime-sources-materialization.test.ts"
6369
- "tests/runtime-sources-playground-integration.test.ts"
@@ -129,6 +135,8 @@ jobs:
129135
- run: npm run test:trusted-apply-artifact-channel
130136
- run: npm run test:runtime-command-artifact-bounds
131137
- run: npm run test:redaction
138+
- run: npm run test:browser-preview-routing
139+
- run: npm run test:browser-routed-command-security
132140
- run: npm run test:production-boundary-enforcement
133141
- run: npm run test:runtime-tool-policy
134142

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@
109109
"smoke": "tsx scripts/run-smoke.ts",
110110
"test:redaction": "tsx tests/redaction.test.ts",
111111
"test:browser-preview-routing": "tsx --test tests/browser-preview-routing.test.ts",
112+
"test:browser-routed-command-security": "tsx --test tests/browser-routed-command-security.test.ts",
112113
"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",
113114
"test:cloudflare-administrator-claim": "tsx tests/cloudflare-provisioning-api.test.ts",
114115
"test:cloudflare-wordpress-auth": "tsx tests/cloudflare-wordpress-auth.test.ts",

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

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ import { browserAssertionsSummary, browserStepRecord, executeBrowserInteractionS
1212
import { browserCommandLivenessPolicy, isBrowserCommandLivenessError, withBrowserCommandLiveness } from "./browser-liveness.js"
1313
import { serializeBrowserError } from "./browser-metrics.js"
1414
import { executeBrowserObservationAssertion } from "./browser-observation-assertions.js"
15-
import { browserPreviewNetworkPolicyIsActive, browserPreviewNetworkPolicySummary, browserPreviewNeedsContextRouting, browserPreviewReadinessError, browserPreviewTopology, resolveBrowserPreviewUrl, routeBrowserPreviewContextNetwork } from "./browser-preview-routing.js"
15+
import { browserPreviewNetworkPolicyIsActive, browserPreviewNetworkPolicySummary, browserPreviewNeedsContextRouting, browserPreviewReadinessError, browserPreviewTopology, closeBrowserAndDrainPreviewRoutes, createBrowserPreviewRouteTracker, resolveBrowserPreviewUrl, routeBrowserPreviewContextNetwork } from "./browser-preview-routing.js"
16+
import { browserCommandResult } from "./browser-result-sanitization.js"
1617
import { BROWSER_PROBE_STATE_INIT_SCRIPT, browserProbeReplayability, browserProbeViewport } from "./browser-probe.js"
1718
import { runBrowserProbeCommand, type BrowserProbeRunPlan } from "./browser-probe-runner.js"
1819
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"
@@ -133,6 +134,7 @@ export async function runBrowserActionsCommand({
133134
const browser = session?.browser ?? await launchChromiumBrowser()
134135
const topology = browserPreviewTopology(args, runtimeSpec, server.serverUrl, server.previewProxyDiagnostics?.targetOrigin)
135136
const { preview, networkPolicy } = topology
137+
const routeTracker = session?.routeTracker ?? createBrowserPreviewRouteTracker()
136138
let requestedUrl = initialUrl ? topology.resolveUrl(initialUrl) : preview.effectiveOrigin
137139
let finalUrl = requestedUrl
138140
let htmlSha256: string | undefined
@@ -173,7 +175,7 @@ export async function runBrowserActionsCommand({
173175
}) : undefined)
174176
const context = environmentRuntime?.context ?? null
175177
if (context && !session) {
176-
await routeBrowserPreviewContextNetwork(context, networkPolicy, topology.origins.localProxyOrigin)
178+
await routeBrowserPreviewContextNetwork(context, networkPolicy, topology.origins.localProxyOrigin, routeTracker)
177179
}
178180
const page = activePage = environmentRuntime?.page ?? await browser.newPage()
179181
if (onProgress) {
@@ -437,11 +439,12 @@ export async function runBrowserActionsCommand({
437439
pendingError = error instanceof Error ? error : new Error(String(error))
438440
errors.push(serializeBrowserError("probe-error", error))
439441
} finally {
440-
await settleBrowserNetworkTasks(networkTasks, livenessPolicy.networkSettleTimeoutMs)
442+
await settleBrowserNetworkTasks(networkTasks, livenessPolicy.networkSettleTimeoutMs).catch((error) => errors.push(serializeBrowserError("probe-error", error)))
441443
if (activePage && resolvedEnvironment) environmentEvidence = await observePlaywrightBrowserEnvironment(activePage, requestedEnvironment, resolvedEnvironment).catch(() => environmentEvidence)
442-
if (!session) {
443-
await environmentRuntime?.close().catch(() => undefined)
444-
await browser.close()
444+
const cleanupBrowser = session ? { close: async () => {} } : { close: async () => { await environmentRuntime?.close(); await browser.close() } }
445+
for (const routeError of await closeBrowserAndDrainPreviewRoutes(cleanupBrowser, routeTracker)) {
446+
errors.push(serializeBrowserError("probe-error", routeError))
447+
pendingError ??= routeError
445448
}
446449
if (capture.has("steps")) {
447450
await artifactSession.writeJsonLines("steps", "steps.jsonl", stepRecords)
@@ -580,9 +583,7 @@ export async function runBrowserActionsCommand({
580583
throw new Error("wordpress.browser-actions did not produce a browser artifact")
581584
}
582585

583-
return {
584-
artifact,
585-
output: `${JSON.stringify({
586+
return browserCommandResult(artifact, {
586587
command: "wordpress.browser-actions",
587588
requestedUrl,
588589
preview,
@@ -591,8 +592,7 @@ export async function runBrowserActionsCommand({
591592
files: artifact.files,
592593
summary: artifact.summary,
593594
steps: stepRecords,
594-
}, null, 2)}\n`,
595-
}
595+
})
596596
}
597597

598598
export function browserToolVerifierUnsupportedResult(step: BrowserInteractionStep, index: number, startedAt: string): BrowserToolVerifierResult {
@@ -967,6 +967,7 @@ export async function runBrowserScenarioCommand({
967967
let pendingError: Error | undefined
968968
let scenarioSession: PlaywrightBrowserEnvironmentSession | undefined
969969
let scenarioBrowser: Awaited<ReturnType<typeof launchChromiumBrowser>> | undefined
970+
let scenarioRouteTracker: ReturnType<typeof createBrowserPreviewRouteTracker> | undefined
970971

971972
try {
972973
if (runPlan.probe && runPlan.actions) {
@@ -976,12 +977,16 @@ export async function runBrowserScenarioCommand({
976977
if (unsupported.length > 0) {
977978
throw new Error(`wordpress.browser-scenario browser environment is unsupported: ${unsupported.join(", ")}`)
978979
}
980+
const topology = browserPreviewTopology(args, runtimeSpec, server.serverUrl, server.previewProxyDiagnostics?.targetOrigin)
979981
const runtime = await createPlaywrightBrowserEnvironmentContext(browser, resolved, {
980-
...(runPlan.actions.storageStateImport ? { contextOptions: { storageState: runPlan.actions.storageStateImport.storageState } } : {}),
982+
contextOptions: {
983+
...topology.contextOptions(),
984+
...(runPlan.actions.storageStateImport ? { storageState: runPlan.actions.storageStateImport.storageState } : {}),
985+
},
981986
})
982-
const topology = browserPreviewTopology(args, runtimeSpec, server.serverUrl)
983-
if (browserPreviewNeedsContextRouting(topology.networkPolicy)) await routeBrowserPreviewContextNetwork(runtime.context, topology.networkPolicy, topology.preview.effectiveOrigin)
984-
scenarioSession = { browser, requested: requestedEnvironment, resolved, runtime }
987+
const routeTracker = scenarioRouteTracker = createBrowserPreviewRouteTracker()
988+
if (browserPreviewNeedsContextRouting(topology.networkPolicy)) await routeBrowserPreviewContextNetwork(runtime.context, topology.networkPolicy, topology.origins.localProxyOrigin, routeTracker)
989+
scenarioSession = { browser, requested: requestedEnvironment, resolved, routeTracker, runtime }
985990
}
986991

987992
if (runPlan.probe) {
@@ -1006,8 +1011,15 @@ export async function runBrowserScenarioCommand({
10061011
}
10071012
}
10081013
} finally {
1009-
await scenarioSession?.runtime.close().catch(() => undefined)
1010-
await scenarioBrowser?.close().catch(() => undefined)
1014+
if (scenarioSession && scenarioBrowser && scenarioRouteTracker) {
1015+
const activeSession = scenarioSession
1016+
const activeBrowser = scenarioBrowser
1017+
const cleanupBrowser = { close: async () => { await activeSession.runtime.close(); await activeBrowser.close() } }
1018+
const routeErrors = await closeBrowserAndDrainPreviewRoutes(cleanupBrowser, scenarioRouteTracker)
1019+
pendingError ??= routeErrors[0]
1020+
} else {
1021+
await scenarioBrowser?.close().catch(() => undefined)
1022+
}
10111023
}
10121024

10131025
const primaryArtifact = actionsResult?.artifact ?? probeResult?.artifact
@@ -1067,17 +1079,14 @@ export async function runBrowserScenarioCommand({
10671079
throw new BrowserCommandArtifactError(`wordpress.browser-scenario failed: ${pendingError.message}`, artifact)
10681080
}
10691081

1070-
return {
1071-
artifact,
1072-
output: `${JSON.stringify({
1082+
return browserCommandResult(artifact, {
10731083
command: "wordpress.browser-scenario",
10741084
requestedUrl: artifact.requestedUrl,
10751085
finalUrl,
10761086
files: artifact.files,
10771087
summary: artifact.summary,
10781088
scenario: scenarioSummary,
1079-
}, null, 2)}\n`,
1080-
}
1089+
})
10811090
}
10821091

10831092
async function browserScenarioFromArgs(args: string[]): Promise<BrowserScenarioInput> {

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { basename, join } from "node:path"
33
import type { ArtifactProvenanceMetadata } from "@automattic/wp-codebox-core"
44
import { ArtifactBundleWriter } from "./artifact-bundle-writer.js"
55
import { browserArtifactFileManifest, type BrowserArtifactFiles } from "./browser-artifacts.js"
6+
import { sanitizeBrowserResultValue } from "./browser-result-sanitization.js"
67

78
export class BrowserArtifactSession {
89
readonly writer: ArtifactBundleWriter
@@ -32,11 +33,11 @@ export class BrowserArtifactSession {
3233
}
3334

3435
async writeJson(key: keyof BrowserArtifactFiles, fileName: string, value: unknown): Promise<void> {
35-
await this.writer.writeJson(this.path(fileName), value, this.manifestWithoutContentType(key))
36+
await this.writer.writeJson(this.path(fileName), sanitizeBrowserResultValue(value), this.manifestWithoutContentType(key))
3637
}
3738

3839
async writeJsonLines(key: keyof BrowserArtifactFiles, fileName: string, records: unknown[]): Promise<void> {
39-
await this.writer.writeJsonLines(this.path(fileName), records, this.manifestWithoutContentType(key))
40+
await this.writer.writeJsonLines(this.path(fileName), sanitizeBrowserResultValue(records), this.manifestWithoutContentType(key))
4041
}
4142

4243
async writeGenerated(key: keyof BrowserArtifactFiles, fileName: string, write: (absolutePath: string) => Promise<void>): Promise<void> {

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
import type { BrowserArtifact } from "./browser-artifacts.js"
2+
import { sanitizeBrowserArtifact, sanitizeBrowserResultValue } from "./browser-result-sanitization.js"
23

34
export class BrowserCommandArtifactError extends Error {
4-
constructor(message: string, readonly artifact: BrowserArtifact, readonly artifactRoot?: string) {
5-
super(message)
5+
readonly artifact: BrowserArtifact
6+
7+
constructor(message: string, artifact: BrowserArtifact, readonly artifactRoot?: string) {
8+
super(sanitizeBrowserResultValue(message, "message"))
69
this.name = "BrowserCommandArtifactError"
10+
Object.assign(artifact, sanitizeBrowserArtifact(artifact))
11+
this.artifact = artifact
712
}
813
}
914

packages/runtime-playground/src/browser-environment-matrix.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
} from "@automattic/wp-codebox-core"
1414
import type { Browser, BrowserContext, BrowserContextOptions, Page } from "playwright"
1515
import type { BrowserArtifactSummary } from "./browser-artifacts.js"
16+
import type { BrowserPreviewRouteTracker } from "./browser-preview-routing.js"
1617

1718
export const PLAYWRIGHT_BROWSER_ENVIRONMENT_CAPABILITIES = [
1819
"browser.environment.viewport",
@@ -75,6 +76,7 @@ export interface PlaywrightBrowserEnvironmentSession {
7576
browser: Browser
7677
requested: BrowserEnvironment
7778
resolved: ResolvedBrowserEnvironment
79+
routeTracker?: BrowserPreviewRouteTracker
7880
runtime: PlaywrightBrowserEnvironmentRuntime
7981
}
8082

packages/runtime-playground/src/browser-metrics.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,7 @@ export async function serializeBrowserFinishedRequest(request: Request, timestam
527527
if (!response) {
528528
return {
529529
type: "response",
530-
url: request.url(),
530+
url: redactBrowserNetworkUrl(request.url()),
531531
method: request.method(),
532532
resourceType: request.resourceType(),
533533
timestamp,
@@ -545,7 +545,7 @@ export async function serializeBrowserResponse(response: Response, timestamp = n
545545
const responseTextPreview = await browserDocument5xxResponsePreview(response)
546546
return {
547547
type: "response",
548-
url: response.url(),
548+
url: redactBrowserNetworkUrl(response.url()),
549549
method: request.method(),
550550
resourceType: request.resourceType(),
551551
status: response.status(),
@@ -596,17 +596,22 @@ function redactBrowserResponseText(body: string): string {
596596
}
597597

598598
export function serializeBrowserRequestFailure(request: Request, timestamp = now()): BrowserProbeNetworkRecord {
599+
const failure = request.failure()
599600
return {
600601
type: "requestfailed",
601-
url: request.url(),
602+
url: redactBrowserNetworkUrl(request.url()),
602603
method: request.method(),
603604
resourceType: request.resourceType(),
604605
timing: browserRequestTiming(request),
605-
failure: request.failure(),
606+
failure: failure ? { errorText: redactString(failure.errorText, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true }) } : null,
606607
timestamp,
607608
}
608609
}
609610

611+
function redactBrowserNetworkUrl(url: string): string {
612+
return redactString(url, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true })
613+
}
614+
610615
function browserRequestTiming(request: Request): Record<string, number> {
611616
return Object.fromEntries(
612617
Object.entries(request.timing()).filter((entry): entry is [string, number] => typeof entry[1] === "number" && Number.isFinite(entry[1])),

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ import { BrowserCommandArtifactError } from "./browser-command-artifact-error.js
66
import { attachBrowserCaptureListeners, launchChromiumBrowser, settleBrowserNetworkTasks } from "./browser-capture-session.js"
77
import { executeBrowserInteractionStep } from "./browser-interactions.js"
88
import { browserProbeReplayability } from "./browser-probe.js"
9-
import { browserPreviewReadinessError, browserPreviewTopology, routeBrowserPreviewContextNetwork } from "./browser-preview-routing.js"
9+
import { browserPreviewReadinessError, browserPreviewTopology, closeBrowserAndDrainPreviewRoutes, createBrowserPreviewRouteTracker, routeBrowserPreviewContextNetwork } from "./browser-preview-routing.js"
10+
import { browserCommandResult } from "./browser-result-sanitization.js"
1011
import { installWordPressAdminAuthCookies } from "./browser-probe-support.js"
1112
import { bootstrapPhpCode } from "./php-bootstrap.js"
1213
import { assertPlaygroundResponseOk, type PlaygroundRunResponse } from "./playground-command-errors.js"
@@ -33,6 +34,7 @@ export async function runBrowserMultiActorScenarioCommand(input: {
3334
const browserArgs = routeHost && !scenario.browserArgs.some((arg) => arg.startsWith("route-host=")) ? [...scenario.browserArgs, `route-host=${routeHost}`] : scenario.browserArgs
3435
const topology = browserPreviewTopology(browserArgs, runtimeSpec, server.serverUrl, server.previewProxyDiagnostics?.targetOrigin)
3536
const browser = await launchChromiumBrowser()
37+
const routeTracker = createBrowserPreviewRouteTracker()
3638
const evidence: Record<string, ActorEvidence> = {}
3739
let result: BrowserMultiActorScenarioResult | undefined
3840
let failure: Error | undefined
@@ -56,7 +58,7 @@ export async function runBrowserMultiActorScenarioCommand(input: {
5658
const userId = await actorUserId(actor.name, session.user.userId, session.user, runtimeSpec, runPlaygroundCommand, server)
5759
const environmentRuntime = await createPlaywrightBrowserEnvironmentContext(browser, resolvedEnvironment, { contextOptions: topology.contextOptions() })
5860
const context = environmentRuntime.context
59-
await routeBrowserPreviewContextNetwork(context, topology.networkPolicy, topology.origins.localProxyOrigin)
61+
await routeBrowserPreviewContextNetwork(context, topology.networkPolicy, topology.origins.localProxyOrigin, routeTracker)
6062
const page = environmentRuntime.page
6163
await context.tracing.start({ screenshots: true, snapshots: true })
6264
await installWordPressAdminAuthCookies({ command: "wordpress.browser-scenario", cookieUrls: topology.authCookieUrls([topology.resolveUrl(scenario.url)]), page, runPlaygroundCommand, runtimeSpec, server, userId })
@@ -73,7 +75,8 @@ export async function runBrowserMultiActorScenarioCommand(input: {
7375
failure = error instanceof Error ? error : new Error(String(error))
7476
if (error instanceof BrowserMultiActorScenarioError) result = error.result
7577
} finally {
76-
await browser.close()
78+
const routeErrors = await closeBrowserAndDrainPreviewRoutes(browser, routeTracker)
79+
failure ??= routeErrors[0]
7780
}
7881

7982
const replay = result?.replay ?? { schema: "wp-codebox/browser-multi-actor-replay/v1", seed: scenario.seed, scenario, schedule: [] }
@@ -91,7 +94,7 @@ export async function runBrowserMultiActorScenarioCommand(input: {
9194
const traces = Object.values(evidence).map((actor) => actor.files.trace).filter((path): path is string => Boolean(path))
9295
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, environment: Object.values(evidence)[0]?.environment, multiActor: { seed: scenario.seed, finalState: result?.finalState ?? "failed", actors: Object.keys(evidence), replay: "files/browser/multi-actor-replay.json" } } } satisfies BrowserArtifact
9396
if (failure) throw new BrowserCommandArtifactError(`wordpress.browser-scenario failed: ${failure.message}`, artifact)
94-
return { artifact, output: `${JSON.stringify({ command: "wordpress.browser-scenario", files: artifact.files, summary: artifact.summary, scenario: summary }, null, 2)}\n` }
97+
return browserCommandResult(artifact, { command: "wordpress.browser-scenario", files: artifact.files, summary: artifact.summary, scenario: summary })
9598
}
9699

97100
export async function navigateBrowserMultiActorPages(

0 commit comments

Comments
 (0)