Skip to content

Commit 39dda74

Browse files
committed
fix: preserve routed security across browser sessions
1 parent 6f72fbd commit 39dda74

4 files changed

Lines changed: 23 additions & 17 deletions

File tree

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ export async function runBrowserActionsCommand({
134134
const browser = session?.browser ?? await launchChromiumBrowser()
135135
const topology = browserPreviewTopology(args, runtimeSpec, server.serverUrl, server.previewProxyDiagnostics?.targetOrigin)
136136
const { preview, networkPolicy } = topology
137-
const routeTracker = createBrowserPreviewRouteTracker()
137+
const routeTracker = session?.routeTracker ?? createBrowserPreviewRouteTracker()
138138
let requestedUrl = initialUrl ? topology.resolveUrl(initialUrl) : preview.effectiveOrigin
139139
let finalUrl = requestedUrl
140140
let htmlSha256: string | undefined
@@ -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) {
@@ -980,8 +981,9 @@ export async function runBrowserScenarioCommand({
980981
...(runPlan.actions.storageStateImport ? { contextOptions: { storageState: runPlan.actions.storageStateImport.storageState } } : {}),
981982
})
982983
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 }
984+
const routeTracker = scenarioRouteTracker = createBrowserPreviewRouteTracker()
985+
if (browserPreviewNeedsContextRouting(topology.networkPolicy)) await routeBrowserPreviewContextNetwork(runtime.context, topology.networkPolicy, topology.origins.localProxyOrigin, routeTracker)
986+
scenarioSession = { browser, requested: requestedEnvironment, resolved, routeTracker, runtime }
985987
}
986988

987989
if (runPlan.probe) {
@@ -1006,8 +1008,15 @@ export async function runBrowserScenarioCommand({
10061008
}
10071009
}
10081010
} finally {
1009-
await scenarioSession?.runtime.close().catch(() => undefined)
1010-
await scenarioBrowser?.close().catch(() => undefined)
1011+
if (scenarioSession && scenarioBrowser && scenarioRouteTracker) {
1012+
const activeSession = scenarioSession
1013+
const activeBrowser = scenarioBrowser
1014+
const cleanupBrowser = { close: async () => { await activeSession.runtime.close(); await activeBrowser.close() } }
1015+
const routeErrors = await closeBrowserAndDrainPreviewRoutes(cleanupBrowser, scenarioRouteTracker)
1016+
pendingError ??= routeErrors[0]
1017+
} else {
1018+
await scenarioBrowser?.close().catch(() => undefined)
1019+
}
10111020
}
10121021

10131022
const primaryArtifact = actionsResult?.artifact ?? probeResult?.artifact

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-probe-runner.ts

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ export async function runSingleBrowserProbeCommand({
248248
const prePageScriptMetadata = prePageScript ? browserProbeScriptMetadata(prePageScript) : undefined
249249
const topology = browserPreviewTopology(args, runtimeSpec, server.serverUrl, server.previewProxyDiagnostics?.targetOrigin)
250250
const { preview, networkPolicy } = topology
251-
const routeTracker = createBrowserPreviewRouteTracker()
251+
const routeTracker = session?.routeTracker ?? createBrowserPreviewRouteTracker()
252252
const targetUrl = topology.resolveUrl(runPlan.url)
253253
const artifactSession = new BrowserArtifactSession(artifactRoot, browserFilesDirectory, { source: command, operation: "browser-probe" })
254254

@@ -513,17 +513,8 @@ export async function runSingleBrowserProbeCommand({
513513
} catch (error) {
514514
errors.push(serializeBrowserError("probe-error", error))
515515
}
516-
try {
517-
await geolocationPermissionCleanup?.()
518-
} catch (error) {
519-
const cleanupError = redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true })
520-
if (!pendingError) {
521-
pendingError = cleanupError
522-
progress.fail("probe-error", cleanupError)
523-
}
524-
errors.push(serializeBrowserError("probe-error", error))
525-
}
526-
for (const routeError of await closeBrowserAndDrainPreviewRoutes(browser, routeTracker)) {
516+
const cleanupBrowser = session ? { close: async () => {} } : { close: async () => { await environmentRuntime?.close(); await browser.close() } }
517+
for (const routeError of await closeBrowserAndDrainPreviewRoutes(cleanupBrowser, routeTracker)) {
527518
const browserCloseFailed = routeError.message.includes("operation=browser-close")
528519
if (!pendingError && (browserCloseFailed || runPlan.routeHostDrain === "required")) {
529520
pendingError = routeError

tests/browser-routed-command-security.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,9 @@ test("real browser commands sanitize console, artifacts, stdout, and failure std
9797
url: `${serverUrl}/editor?token=${TOKEN}`,
9898
actors: [{ name: "author", userSession: "author-session" }],
9999
actions: [],
100+
browserArgs: [],
100101
captures: ["steps", "network", "console"],
102+
environment: {},
101103
},
102104
server,
103105
})
@@ -144,7 +146,9 @@ test("real browser commands sanitize console, artifacts, stdout, and failure std
144146
url: `${serverUrl}/editor?token=${TOKEN}`,
145147
actors: [{ name: "author", userSession: "author-session" }],
146148
actions: [{ id: "missing-click", actor: "author", step: { kind: "click", selector: "#missing" } }],
149+
browserArgs: [],
147150
captures: ["errors"],
151+
environment: {},
148152
stepTimeoutMs: 250,
149153
},
150154
server,

0 commit comments

Comments
 (0)