diff --git a/package.json b/package.json index 651895e9f..0d0dd831b 100644 --- a/package.json +++ b/package.json @@ -108,6 +108,7 @@ "generate:cloudflare-canonical-mdi-seed": "npm run generate:cloudflare-mdi-runtime-bundle && php scripts/build-cloudflare-canonical-mdi-seed.php", "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: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", diff --git a/packages/runtime-core/src/redaction.ts b/packages/runtime-core/src/redaction.ts index 70e00e212..4902cf783 100644 --- a/packages/runtime-core/src/redaction.ts +++ b/packages/runtime-core/src/redaction.ts @@ -96,12 +96,25 @@ export function redactJsonValue(value: unknown, options: RedactJsonOptions = {}, export function redactString(value: string, options: RedactStringOptions = {}): string { return value + .replace(/(^|\r?\n)([ \t]*([A-Za-z0-9_-]+)[ \t]*:[ \t]*)[^\r\n]*/g, (line, prefix: string, assignment: string, key: string) => ( + isSensitiveKey(key, options) ? `${prefix}${assignment}${REDACTED_VALUE}` : line + )) .replace(/https?:\/\/[^\s"'<>]+/gi, (match) => redactUrl(match, options)) .replace(SECRET_LIKE_VALUE_GLOBAL_PATTERN, REDACTED_VALUE) .replace(/([?&][^=&#\s"'<>]+)=([^&#\s"'<>]+)/g, options.redactQueryAssignments ? `$1=${REDACTED_VALUE}` : "$&") .replace(/((?:[A-Za-z0-9_-]*)(?:access[_-]?token|auth|bearer|code|cookie|credential|key|login|nonce|pass|password|secret|session|state|token)(?:[A-Za-z0-9_-]*)(?:["'\s:=]+))[^&#\s"'<>]+/gi, `$1${REDACTED_VALUE}`) } +export function redactError(error: unknown, options: RedactStringOptions = {}): Error { + const source = error instanceof Error ? error : new Error(String(error)) + const redacted = new Error(redactString(source.message, options)) + redacted.name = redactString(source.name, options) + if (source.stack) { + redacted.stack = redactString(source.stack, options) + } + return redacted +} + export function redactUrl(value: string, options: RedactStringOptions = {}): string { try { const url = new URL(value) diff --git a/packages/runtime-playground/src/browser-metrics.ts b/packages/runtime-playground/src/browser-metrics.ts index 37ce68cd5..fe3b7a0d1 100644 --- a/packages/runtime-playground/src/browser-metrics.ts +++ b/packages/runtime-playground/src/browser-metrics.ts @@ -1,7 +1,7 @@ import { createHash } from "node:crypto" import { access, readFile } from "node:fs/promises" import { join } from "node:path" -import { redactString, type ArtifactManifest } from "@automattic/wp-codebox-core" +import { redactError, redactString, type ArtifactManifest } from "@automattic/wp-codebox-core" import { isPlainObject as isRecord, now } from "@automattic/wp-codebox-core/internals" import type { ConsoleMessage, Request, Response } from "playwright" import type { @@ -636,11 +636,8 @@ export function serializeBrowserConsoleMessage(message: ConsoleMessage): Record< } export function serializeBrowserError(type: BrowserProbeErrorRecord["type"], error: unknown): BrowserProbeErrorRecord { - if (error instanceof Error) { - return { type, name: error.name, message: error.message, stack: error.stack, timestamp: now() } - } - - return { type, name: "Error", message: String(error), timestamp: now() } + const sanitized = redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true }) + return { type, name: sanitized.name, message: sanitized.message, stack: sanitized.stack, timestamp: now() } } export function jsonLines(records: unknown[]): string { diff --git a/packages/runtime-playground/src/browser-preview-routing.ts b/packages/runtime-playground/src/browser-preview-routing.ts index 82eb6edaa..7b19d775f 100644 --- a/packages/runtime-playground/src/browser-preview-routing.ts +++ b/packages/runtime-playground/src/browser-preview-routing.ts @@ -1,9 +1,12 @@ -import type { RuntimeCreateSpec } from "@automattic/wp-codebox-core" +import { redactError, redactString, type RuntimeCreateSpec } from "@automattic/wp-codebox-core" import type { BrowserProbeNetworkPolicySummary, BrowserProbePreviewMode, BrowserProbePreviewRouting } from "./browser-artifacts.js" import { argValue, commaListArg, strictBooleanArg } from "./commands.js" import type { Page, Route } from "playwright" const BROWSER_PREVIEW_ROUTE_DRAIN_TIMEOUT_MS = 5_000 +const BROWSER_PREVIEW_ROUTE_DOCUMENT_FETCH_ATTEMPTS = 3 +const BROWSER_PREVIEW_ROUTE_SUBRESOURCE_FETCH_ATTEMPTS = 2 +const BROWSER_PREVIEW_ROUTE_RETRY_DELAY_MS = 25 export interface BrowserPreviewNetworkPolicy { mode: "allow" | "block" | "record" @@ -258,8 +261,7 @@ export async function drainBrowserPreviewRouteTracker(tracker: BrowserPreviewRou } if (tracker.errors.length > 0) { - const error = tracker.errors[0] - throw error instanceof Error ? error : new Error(String(error)) + throw sanitizeBrowserPreviewRouteError(tracker.errors[0]) } } @@ -310,8 +312,8 @@ async function routeBrowserPreviewNetwork(routePattern: (url: string, handler: ( try { await task } catch (error) { - tracker?.errors.push(error) - throw error + tracker?.errors.push(sanitizeBrowserPreviewRouteError(error)) + await route.abort("failed").catch(() => undefined) } finally { tracker?.pending.delete(task) } @@ -400,25 +402,42 @@ async function fetchBrowserPreviewRoutedHost(route: Route, requestUrl: URL, poli routedUrl.hostname = origin.hostname routedUrl.port = origin.port - let response: Awaited> - try { - response = await route.fetch({ - url: routedUrl.toString(), - headers: { - ...route.request().headers(), - host: currentUrl.host, - "x-forwarded-host": currentUrl.host, - "x-forwarded-port": currentUrl.port || (currentUrl.protocol === "https:" ? "443" : "80"), - "x-forwarded-proto": currentUrl.protocol.replace(":", ""), - }, - maxRedirects: 0, - }) - } catch (error) { - if (!isBrowserPreviewRouteFetchRecoverableError(error)) { - throw error + let response: Awaited> | undefined + const resourceType = route.request().resourceType() + const maxAttempts = resourceType === "document" ? BROWSER_PREVIEW_ROUTE_DOCUMENT_FETCH_ATTEMPTS : BROWSER_PREVIEW_ROUTE_SUBRESOURCE_FETCH_ATTEMPTS + for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { + try { + response = await route.fetch({ + url: routedUrl.toString(), + headers: { + ...route.request().headers(), + host: currentUrl.host, + "x-forwarded-host": currentUrl.host, + "x-forwarded-port": currentUrl.port || (currentUrl.protocol === "https:" ? "443" : "80"), + "x-forwarded-proto": currentUrl.protocol.replace(":", ""), + }, + maxRedirects: 0, + }) + break + } catch (error) { + if (!isBrowserPreviewRouteFetchRecoverableError(error)) { + throw sanitizeBrowserPreviewRouteError(error) + } + + const retryable = isBrowserPreviewRouteFetchTransientTransportError(error) + if (retryable && attempt < maxAttempts) { + await wait(BROWSER_PREVIEW_ROUTE_RETRY_DELAY_MS * attempt) + continue + } + + if (resourceType !== "document" || !retryable) { + await route.abort("failed").catch(() => undefined) + return undefined + } + throw browserPreviewRouteFetchExhaustedError(route, currentUrl, attempt, error) } - - await route.abort("failed").catch(() => undefined) + } + if (!response) { return undefined } @@ -458,13 +477,31 @@ export function isBrowserPreviewRouteFetchRequestContextDisposedError(error: unk } export function isBrowserPreviewRouteFetchRecoverableError(error: unknown): boolean { - return isBrowserPreviewRouteFetchRequestContextDisposedError(error) || isBrowserPreviewRouteFetchContentDecodingError(error) + return isBrowserPreviewRouteFetchRequestContextDisposedError(error) || isBrowserPreviewRouteFetchContentDecodingError(error) || isBrowserPreviewRouteFetchTransientTransportError(error) } export function isBrowserPreviewRouteFetchContentDecodingError(error: unknown): boolean { return error instanceof Error && /\broute\.fetch:\s*failed to decompress\b/i.test(error.message) } +export function isBrowserPreviewRouteFetchTransientTransportError(error: unknown): boolean { + return error instanceof Error && /\b(?:ECONNRESET|ECONNREFUSED|EPIPE|ETIMEDOUT|UND_ERR_SOCKET|socket (?:hang up|closed|ended)|connection (?:reset|refused|closed)|other side closed)\b/i.test(error.message) +} + +function browserPreviewRouteFetchExhaustedError(route: Route, requestUrl: URL, attempts: number, error: unknown): Error { + const method = route.request().method() + const resourceType = route.request().resourceType() + const classification = isBrowserPreviewRouteFetchTransientTransportError(error) ? "upstream-transport" : "route-fetch" + const safeUrl = redactString(requestUrl.toString(), { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true }) + const exhausted = new Error(`wordpress.browser-probe route-host fetch failed after ${attempts} attempt(s): classification=${classification} method=${method} resourceType=${resourceType} url=${safeUrl}`) + exhausted.name = "BrowserPreviewRouteFetchError" + return exhausted +} + +function sanitizeBrowserPreviewRouteError(error: unknown): Error { + return redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true }) +} + function urlProtocol(url: string): string | undefined { try { return new URL(url).protocol diff --git a/packages/runtime-playground/src/browser-probe-runner.ts b/packages/runtime-playground/src/browser-probe-runner.ts index 1125c69e3..4528a97de 100644 --- a/packages/runtime-playground/src/browser-probe-runner.ts +++ b/packages/runtime-playground/src/browser-probe-runner.ts @@ -1,4 +1,4 @@ -import { BROWSER_PROBE_BROWSER_VALUES, BROWSER_PROBE_CAPTURE_VALUES, BROWSER_PROBE_CHROMIUM_PROFILE_IDS, BROWSER_PROBE_PROFILES, BROWSER_PROBE_THROTTLE_PROFILE_IDS, browserGeolocation, type BrowserGeolocationPermissionState, type BrowserProbeProfileDefinition, type ExecutionSpec, type RuntimeCreateSpec } from "@automattic/wp-codebox-core" +import { BROWSER_PROBE_BROWSER_VALUES, BROWSER_PROBE_CAPTURE_VALUES, BROWSER_PROBE_CHROMIUM_PROFILE_IDS, BROWSER_PROBE_PROFILES, BROWSER_PROBE_THROTTLE_PROFILE_IDS, browserGeolocation, redactError, type BrowserGeolocationPermissionState, type BrowserProbeProfileDefinition, type ExecutionSpec, type RuntimeCreateSpec } from "@automattic/wp-codebox-core" import { BrowserArtifactSession } from "./browser-artifact-session.js" import { BrowserCommandArtifactError } from "./browser-command-artifact-error.js" import type { BrowserArtifactFiles, BrowserProbeArtifact, BrowserProbeAuthSummary, BrowserProbeCapabilityDiagnostics, BrowserProbeCheckpointRecord, BrowserProbeContextDetails, BrowserProbeErrorRecord, BrowserProbeLifecycleArtifact, BrowserProbeMemoryArtifact, BrowserProbeNetworkRecord, BrowserProbePerformanceArtifact, BrowserProbeScriptMetadata, BrowserProbeViewport, BrowserProbeWebSocketRecord, BrowserWordPressDiagnosticsSummary } from "./browser-artifacts.js" @@ -440,8 +440,9 @@ export async function runSingleBrowserProbeCommand({ } finalUrl = page.url() } catch (error) { - pendingError = error instanceof Error ? error : new Error(String(error)) - if (isBrowserCommandLivenessError(pendingError)) { + const livenessError = isBrowserCommandLivenessError(error) + pendingError = redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true }) + if (livenessError) { await page?.close().catch(() => undefined) page = null } @@ -456,7 +457,7 @@ export async function runSingleBrowserProbeCommand({ try { await drainBrowserPreviewRouteTracker(routeTracker) } catch (error) { - const routeError = error instanceof Error ? error : new Error(String(error)) + const routeError = redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true }) if (!pendingError && runPlan.routeHostDrain === "required") { pendingError = routeError progress.fail("probe-error", routeError) @@ -674,7 +675,7 @@ export async function runBoundedBrowserDiagnostic({ }) return { ok: true, value } } catch (error) { - const normalized = error instanceof Error ? error : new Error(String(error)) + const normalized = redactError(error, { redactAllUrlQueryValues: true, redactUrlHash: true, redactQueryAssignments: true }) onError(normalized) return { ok: false, error: normalized } } diff --git a/tests/browser-preview-routing.test.ts b/tests/browser-preview-routing.test.ts new file mode 100644 index 000000000..28c1eff38 --- /dev/null +++ b/tests/browser-preview-routing.test.ts @@ -0,0 +1,151 @@ +import assert from "node:assert/strict" +import test from "node:test" + +import type { BrowserContext, Route } from "playwright" + +import { browserPreviewNetworkPolicy, browserPreviewRouting, createBrowserPreviewRouteTracker, drainBrowserPreviewRouteTracker, isBrowserPreviewRouteFetchContentDecodingError, isBrowserPreviewRouteFetchRecoverableError, isBrowserPreviewRouteFetchRequestContextDisposedError, isBrowserPreviewRouteFetchTransientTransportError, routeBrowserPreviewContextNetwork } from "../packages/runtime-playground/src/browser-preview-routing.js" +import { jsonLines, serializeBrowserError } from "../packages/runtime-playground/src/browser-metrics.js" + +const SENTINELS = ["SENTINEL_COOKIE_2094", "SENTINEL_AUTH_2094", "SENTINEL_NONCE_2094", "SENTINEL_TOKEN_2094"] +const ROUTED_URL = "http://routed.test/wp-includes/app.js?token=SENTINEL_TOKEN_2094" + +test("routed fetch classifiers include transport resets and preserve disposal/decompression recovery", () => { + assert.equal(isBrowserPreviewRouteFetchTransientTransportError(routeFetchError("read ECONNRESET")), true) + assert.equal(isBrowserPreviewRouteFetchTransientTransportError(routeFetchError("connect ECONNREFUSED")), true) + assert.equal(isBrowserPreviewRouteFetchTransientTransportError(routeFetchError("socket hang up")), true) + assert.equal(isBrowserPreviewRouteFetchRequestContextDisposedError(routeFetchError("Request context disposed.")), true) + assert.equal(isBrowserPreviewRouteFetchContentDecodingError(routeFetchError("failed to decompress 'br' encoding")), true) + assert.equal(isBrowserPreviewRouteFetchRecoverableError(routeFetchError("read ECONNRESET")), true) +}) + +test("subresource resets retry once, abort safely, and never reject the route callback", async () => { + const fixture = await routedFixture("script", [routeFetchError("read ECONNRESET"), routeFetchError("socket closed")]) + await assert.doesNotReject(fixture.run()) + assert.equal(fixture.fetchCalls(), 2) + assert.equal(fixture.abortCalls(), 1) + assert.equal(fixture.tracker.errors.length, 0) + assert.equal(fixture.tracker.pending.size, 0) +}) + +test("document resets retry deterministically and fulfill after recovery", async () => { + const response = routedResponse() + const fixture = await routedFixture("document", [routeFetchError("connect ECONNREFUSED"), routeFetchError("read ECONNRESET"), response]) + await assert.doesNotReject(fixture.run()) + await assert.doesNotReject(drainBrowserPreviewRouteTracker(fixture.tracker)) + assert.equal(fixture.fetchCalls(), 3) + assert.equal(fixture.fulfilledResponse(), response) + assert.equal(fixture.abortCalls(), 0) +}) + +test("exhausted document resets fail through a sanitized tracker error without leaking to persisted surfaces", async () => { + const fixture = await routedFixture("document", [routeFetchError("read ECONNRESET"), routeFetchError("read ECONNRESET"), routeFetchError("read ECONNRESET")]) + await assert.doesNotReject(fixture.run()) + assert.equal(fixture.fetchCalls(), 3) + assert.equal(fixture.abortCalls(), 1) + assert.equal(fixture.tracker.pending.size, 0) + assert.equal(fixture.tracker.errors.length, 1) + + const tracked = fixture.tracker.errors[0] + await assert.rejects(drainBrowserPreviewRouteTracker(fixture.tracker), /classification=upstream-transport.*resourceType=document.*token=\[redacted\]/) + const serialized = serializeBrowserError("probe-error", tracked) + const persistedSurfaces = { + stdout: JSON.stringify(serialized), + stderr: tracked instanceof Error ? `${tracked.message}\n${tracked.stack}` : String(tracked), + diagnostics: JSON.stringify({ errors: [serialized] }), + manifest: JSON.stringify({ files: [{ diagnostics: serialized }] }), + artifact: jsonLines([serialized]), + tracker: JSON.stringify(fixture.tracker.errors.map((error) => serializeBrowserError("probe-error", error))), + snapshot: JSON.stringify(serialized), + } + for (const [surface, contents] of Object.entries(persistedSurfaces)) { + for (const sentinel of SENTINELS) assert.doesNotMatch(contents, new RegExp(sentinel), `${surface} must not contain ${sentinel}`) + } +}) + +test("concurrent routed requests drain after independent retry and cleanup", async () => { + const first = await routedFixture("script", [routeFetchError("read ECONNRESET"), routedResponse()]) + const second = await routedFixture("image", [routeFetchError("socket ended"), routeFetchError("socket ended")], first.tracker) + await Promise.all([first.run(), second.run()]) + await assert.doesNotReject(drainBrowserPreviewRouteTracker(first.tracker)) + assert.equal(first.tracker.pending.size, 0) + assert.equal(first.fetchCalls(), 2) + assert.equal(second.fetchCalls(), 2) + assert.equal(second.abortCalls(), 1) +}) + +test("disposed contexts and decompression failures abort without retrying or tracking errors", async () => { + for (const message of ["Request context disposed.", "failed to decompress 'gzip' encoding"]) { + const fixture = await routedFixture("document", [routeFetchError(message)]) + await assert.doesNotReject(fixture.run()) + await assert.doesNotReject(drainBrowserPreviewRouteTracker(fixture.tracker)) + assert.equal(fixture.fetchCalls(), 1) + assert.equal(fixture.abortCalls(), 1) + } +}) + +async function routedFixture(resourceType: string, outcomes: unknown[], tracker = createBrowserPreviewRouteTracker()) { + let handler: ((route: Route) => Promise) | undefined + let fetchCalls = 0 + let abortCalls = 0 + let fulfilled: unknown + const context = { + route: async (_pattern: string, nextHandler: (route: Route) => Promise) => { + handler = nextHandler + }, + } as BrowserContext + const preview = browserPreviewRouting([], undefined, "http://127.0.0.1:9400") + const policy = browserPreviewNetworkPolicy([], ["routed.test"], preview) + await routeBrowserPreviewContextNetwork(context, policy, preview.effectiveOrigin, tracker) + + const route = { + request: () => ({ + url: () => ROUTED_URL, + method: () => "GET", + resourceType: () => resourceType, + headers: () => ({ + cookie: `wordpress_logged_in=${SENTINELS[0]}`, + authorization: `Bearer ${SENTINELS[1]}`, + "x-wp-nonce": SENTINELS[2], + "x-session-token": SENTINELS[3], + }), + }), + fetch: async () => { + const outcome = outcomes[Math.min(fetchCalls, outcomes.length - 1)] + fetchCalls += 1 + if (outcome instanceof Error) throw outcome + return outcome + }, + abort: async () => { + abortCalls += 1 + }, + fulfill: async ({ response }: { response: unknown }) => { + fulfilled = response + }, + continue: async () => {}, + } as unknown as Route + + return { + tracker, + run: async () => { + assert(handler) + await handler(route) + }, + fetchCalls: () => fetchCalls, + abortCalls: () => abortCalls, + fulfilledResponse: () => fulfilled, + } +} + +function routeFetchError(reason: string): Error { + const message = `route.fetch: ${reason}\nCall log:\n - → GET ${ROUTED_URL}\n cookie: wordpress_logged_in=${SENTINELS[0]}\n authorization: Bearer ${SENTINELS[1]}\n x-wp-nonce: ${SENTINELS[2]}\n x-session-token: ${SENTINELS[3]}` + const error = new Error(message) + error.stack = `Error: ${message}` + return error +} + +function routedResponse() { + return { + status: () => 200, + headers: () => ({}), + } +} diff --git a/tests/redaction.test.ts b/tests/redaction.test.ts index 64b6ae24e..1e0c669c6 100644 --- a/tests/redaction.test.ts +++ b/tests/redaction.test.ts @@ -1,6 +1,6 @@ import assert from "node:assert/strict" import { readFile } from "node:fs/promises" -import { containsSecretLikeValue, isRedactedValue, isSensitiveKey, redactJsonValue, redactString, redactUrl, type RedactionPolicyProfileName } from "../packages/runtime-core/src/redaction.js" +import { containsSecretLikeValue, isRedactedValue, isSensitiveKey, redactError, redactJsonValue, redactString, redactUrl, type RedactionPolicyProfileName } from "../packages/runtime-core/src/redaction.js" const sensitiveKeyCases: Array<[string, boolean]> = [ ["token", true], @@ -49,6 +49,19 @@ assert.equal( "https://example.com/path?plain=ok&token=[redacted]#frag", ) +const headerSentinels = ["SENTINEL_COOKIE_2094", "SENTINEL_AUTH_2094", "SENTINEL_NONCE_2094", "SENTINEL_TOKEN_2094"] +const headerLog = `route.fetch: read ECONNRESET\n cookie: wordpress_logged_in=${headerSentinels[0]}; preference=visible\nAuthorization: Bearer ${headerSentinels[1]}\nx-wp-nonce: ${headerSentinels[2]}\nx-session-token: ${headerSentinels[3]}\naccept: text/html` +const redactedHeaderLog = redactString(headerLog) +assert.match(redactedHeaderLog, /cookie: \[redacted\]/i) +assert.match(redactedHeaderLog, /authorization: \[redacted\]/i) +assert.match(redactedHeaderLog, /x-wp-nonce: \[redacted\]/i) +assert.match(redactedHeaderLog, /x-session-token: \[redacted\]/i) +assert.match(redactedHeaderLog, /accept: text\/html/i) +for (const sentinel of headerSentinels) assert.doesNotMatch(redactedHeaderLog, new RegExp(sentinel)) + +const redactedError = redactError(Object.assign(new Error(headerLog), { stack: `Error: ${headerLog}` })) +for (const sentinel of headerSentinels) assert.doesNotMatch(`${redactedError.message}\n${redactedError.stack}`, new RegExp(sentinel)) + const fixture = JSON.parse(await readFile(new URL("./fixtures/redaction-policy-profiles.json", import.meta.url), "utf8")) as { profiles: Record }