Merge pull request #527 from webdriverio/fix/replay-script-isolation

Run no script from a replayed trace, in any player mode

Author: vishnuv688

.changeset/replay-runs-no-script.md

@@ -0,0 +1,6 @@
+---
+'@wdio/devtools-app': patch
+'@wdio/devtools-backend': patch
+---
+
+A replayed trace runs no script. The page a trace recorded is rendered in a sandboxed frame, scripts, inline event handlers, `srcdoc` and `javascript:` URLs are dropped from it on every update, not only the first render, and the dashboard and `show-trace` now send the same Content-Security-Policy as the hosted player. A trace someone else recorded can therefore be opened safely in any of them.

CLAUDE.md

@@ -105,7 +105,7 @@ They import from `shared` and `core`, never from each other. They aren't importe
 
 ### Backend and app are framework-agnostic
 
-`backend` and `app` import from `shared` (for contracts) and from each other via the WS/HTTP boundary. Neither imports an adapter package. `backend` also imports `trace`, to write and read zips; `app` imports only `trace`'s reader, which the hosted player (`trace.webdriver.io`, built with `pnpm player:hosted`, deployed by Vercel from `vercel.json`) runs in the browser to open a dropped zip with no backend at all. It runs the reader in a module Web Worker loaded from the player's own origin (`app/src/controller/trace-parse.worker.ts`, which `script-src 'self'` already admits), so the tab stays responsive and the landing screen names the phase the open is on; the live build never references the worker. That build is `HOSTED_PLAYER` in `app/src/controller/constants.ts`; it never writes the trace to browser storage, and `vercel.json` is also what `vite preview` serves, so its CSP is checked locally before it ships. Several zips or a folder open as a run (`controller/trace-run.ts`), in the hosted player and in `show-trace`, which lists them at `TRACE_API.run` for the app to read: each zip is parsed once, one at a time, and its payload kept for when its row is picked. A zip records no test state, so a row's outcome is read from its rows (`utils/trace-summary.ts`: a failed action or step means failed, actions with none failed mean passed, no actions mean unknown); a jest `expect` that records no row can therefore fail a test whose trace reads as passed. Keeping every payload costs memory in proportion to the run; a reader that reads entries on demand would shrink each payload, which is why the run holds the reader's own payload rather than a cache of its own.
+`backend` and `app` import from `shared` (for contracts) and from each other via the WS/HTTP boundary. Neither imports an adapter package. `backend` also imports `trace`, to write and read zips; `app` imports only `trace`'s reader, which the hosted player (`trace.webdriver.io`, built with `pnpm player:hosted`, deployed by Vercel from `vercel.json`) runs in the browser to open a dropped zip with no backend at all. It runs the reader in a module Web Worker loaded from the player's own origin (`app/src/controller/trace-parse.worker.ts`, which `script-src 'self'` already admits), so the tab stays responsive and the landing screen names the phase the open is on; the live build never references the worker. A trace is untrusted input, and its replay must run no script in any mode: the replay iframe is sandboxed (`allow-same-origin` only), `components/browser/replay-safety.ts` drops `<script>`, `on*`, `srcdoc` and `javascript:` URLs on the first render and on every replayed mutation alike, and the CSP (`shared/player-csp.ts`, sent by `vercel.json` and by the backend on its HTML, a test holding the two equal) blocks what the first two miss; the hosted e2e check opens a hostile trace to prove it. That build is `HOSTED_PLAYER` in `app/src/controller/constants.ts`; it never writes the trace to browser storage, and `vercel.json` is also what `vite preview` serves, so its CSP is checked locally before it ships. Several zips or a folder open as a run (`controller/trace-run.ts`), in the hosted player and in `show-trace`, which lists them at `TRACE_API.run` for the app to read: each zip is parsed once, one at a time, and its payload kept for when its row is picked. A zip records no test state, so a row's outcome is read from its rows (`utils/trace-summary.ts`: a failed action or step means failed, actions with none failed mean passed, no actions mean unknown); a jest `expect` that records no row can therefore fail a test whose trace reads as passed. Keeping every payload costs memory in proportion to the run; a reader that reads entries on demand would shrink each payload, which is why the run holds the reader's own payload rather than a cache of its own.
 
 Framework-specific behavior in the backend is contained in two files: `runner.ts` and `framework-filters.ts`. Both branch on a typed `TestRunnerId` from shared, never on a magic string. The `framework-filters` dispatch is a `switch` over `TestRunnerId` (not a table lookup) so CodeQL's `unvalidated-dynamic-method-call` query trusts the call site.
 

README.md

@@ -271,7 +271,7 @@ pnpm show-trace a/trace.zip b/trace.zip              # several zips
 
 Each zip is read once, as the list fills in, and kept for when it is picked. Anything in the folder that is not a `.zip` (screenshots, videos, the artifacts manifest) is skipped; a zip that cannot be opened stays in the list with the reason, and the rest open normally. A zip records no test state, so the outcome is read from the trace itself: **failed** when an action or step in it failed, **passed** when it holds actions and none failed, **unknown** when it holds none. An assertion library that records no row (a jest `expect`, chai) can fail a test whose trace still reads as passed. One zip opens exactly as before, with no list.
 
-**Hosted player: [trace.webdriver.io](https://trace.webdriver.io).** The same player as a web page, for opening a trace without installing anything: pick one or several `.zip` files, or drop them or a folder of them on the page. The archive is parsed in the browser tab and never uploaded, which matters because a trace records everything the run showed or typed. Run controls are absent, as in `show-trace`. A replayed page still loads its own stylesheets and images from the site under test, as it does in `show-trace`; from an `https` page the browser blocks the `http` ones, so a trace of an `http` site can render unstyled there and fully in `show-trace`. To try the hosted build locally:
+**Hosted player: [trace.webdriver.io](https://trace.webdriver.io).** The same player as a web page, for opening a trace without installing anything: pick one or several `.zip` files, or drop them or a folder of them on the page. The archive is parsed in the browser tab and never uploaded, which matters because a trace records everything the run showed or typed. Run controls are absent, as in `show-trace`. A trace from someone else is treated as untrusted: its replayed page runs no script, in the hosted player, `show-trace` and the live dashboard alike. A replayed page still loads its own stylesheets and images from the site under test, as it does in `show-trace`; from an `https` page the browser blocks the `http` ones, so a trace of an `http` site can render unstyled there and fully in `show-trace`. To try the hosted build locally:
 
 ```sh
 pnpm player:hosted   # builds it and serves it at http://localhost:4173 under the production headers

packages/app/src/components/browser/replay-safety.ts

@@ -0,0 +1,30 @@
+// A trace is untrusted input: whoever recorded it chose the DOM it replays.
+const URL_ATTRIBUTES = new Set([
+  'href',
+  'src',
+  'action',
+  'formaction',
+  'xlink:href'
+])
+const SCRIPT_URL = /^\s*javascript:/i
+
+/** An attribute that could run script or navigate the replay frame, whatever the page's policy. */
+export function isUnsafeReplayAttribute(name: string, value: unknown): boolean {
+  const attribute = name.toLowerCase()
+  if (
+    /^on[a-z]/.test(attribute) ||
+    attribute === 'srcdoc' ||
+    attribute === 'http-equiv'
+  ) {
+    return true
+  }
+  return (
+    URL_ATTRIBUTES.has(attribute) &&
+    typeof value === 'string' &&
+    SCRIPT_URL.test(value)
+  )
+}
+
+export function isUnsafeReplayElement(type: unknown): boolean {
+  return typeof type === 'string' && type.toLowerCase() === 'script'
+}

packages/app/src/components/browser/snapshot.ts

@@ -19,9 +19,10 @@ import { commandPageUrl } from './url-at-timestamp.js'
 import { mutationForCommand } from './mutation-at-command.js'
 import { booleanAttributeOn, isBooleanAttribute } from './boolean-attribute.js'
 
-import { type ComponentChildren, h, render, type VNode } from 'preact'
+import { render } from 'preact'
 import { customElement, query } from 'lit/decorators.js'
-import { transform } from './vnode-transform.js'
+import { transform, withBaseUrl } from './vnode-transform.js'
+import { isUnsafeReplayAttribute } from './replay-safety.js'
 import type { SimplifiedVNode } from '@wdio/devtools-script/types'
 // Type-only, like the `script/types` import above: the collector owns the
 // characterData wire shape (parent ref + child index), so the replay reads it
@@ -515,22 +516,7 @@ export class DevtoolsBrowser extends Element {
   }
 
   async #renderNewDocument(doc: SimplifiedVNode, baseUrl: string) {
-    const root = transform(doc)
-    const baseTag = h('base', { href: baseUrl })
-    const head: VNode<{}> | undefined = (root.props.children as VNode[])
-      .filter(Boolean)
-      .find((node) => node!.type === 'head')
-    if (head) {
-      head.props.children = [
-        baseTag,
-        ...(head.props.children as ComponentChildren[])
-      ]
-    } else {
-      const head = h('head', {}, baseTag)
-      const docChildren = (root.props.children as ComponentChildren[]) || []
-      docChildren.unshift(head)
-    }
-    render(root, this.#vdom)
+    render(withBaseUrl(transform(doc), baseUrl), this.#vdom)
   }
 
   #renderVdom() {
@@ -641,6 +627,9 @@ export class DevtoolsBrowser extends Element {
     }
 
     const value = mutation.attributeValue
+    if (isUnsafeReplayAttribute(name, value)) {
+      return
+    }
     el.setAttribute(name, value)
     // Form-field state lives on the PROPERTY, not just the attribute — mirror it
     // so a replayed input shows the captured value, including a field cleared
@@ -1041,7 +1030,7 @@ export class DevtoolsBrowser extends Element {
     if (hasMutations) {
       return html`<div class="iframe-wrapper iframe-wrapper--replay">
         <div class="iframe-sizer">
-          <iframe class="origin-top-left"></iframe>
+          <iframe class="origin-top-left" sandbox="allow-same-origin"></iframe>
         </div>
       </div>`
     }

packages/app/src/components/browser/vnode-transform.ts

@@ -1,9 +1,13 @@
 // Converts a serialized VNode (captured by the injected script) into a Preact
 // VNode the snapshot iframe can render. Pure — no DOM or component state.
 
-import { type VNode, h } from 'preact'
+import { type ComponentChildren, type VNode, h, toChildArray } from 'preact'
 
 import { isBooleanAttribute } from './boolean-attribute.js'
+import {
+  isUnsafeReplayAttribute,
+  isUnsafeReplayElement
+} from './replay-safety.js'
 
 interface SerializedVNode {
   type?: string
@@ -19,20 +23,18 @@ type TransformInput = SerializedVNode | string | number | null
  * Preact reads any `on*` prop as a listener: it stamps its own bookkeeping
  * property onto the value, which throws `Cannot create property … on string`
  * and aborts the whole render. One such attribute anywhere in the document was
- * enough to leave the replay iframe blank with nothing logged — the render
- * never reached an `<html>` element, so the caller's own guard returned early.
+ * enough to leave the replay iframe blank with nothing logged.
  *
- * Dropping them costs nothing: the replay is a static reconstruction and
- * `#renderVdom` strips `<script>` tags for the same reason, so a handler has
- * nothing left to fire. A function value (never produced by capture) is left
- * alone so a real listener still binds.
+ * They, and every other attribute that could run script, are dropped: the
+ * replay is a static reconstruction of an untrusted trace. A function value
+ * (never produced by capture) is left alone so a real listener still binds.
  */
-function withoutInlineHandlers(
+function withoutUnsafeAttributes(
   props: Record<string, unknown>
 ): Record<string, unknown> {
   const kept: Record<string, unknown> = {}
   for (const [key, value] of Object.entries(props)) {
-    if (!(/^on[A-Za-z]/.test(key) && typeof value === 'string')) {
+    if (typeof value === 'function' || !isUnsafeReplayAttribute(key, value)) {
       kept[key] = value
     }
   }
@@ -71,8 +73,12 @@ export function transform(node: TransformInput): VNode<{}> {
     return node as unknown as VNode<{}>
   }
 
+  // Every replayed node passes here, an added one included; Preact renders a null child as nothing.
+  if (isUnsafeReplayElement(node.type)) {
+    return null as unknown as VNode<{}>
+  }
   const { children, ...rawProps } = node.props ?? {}
-  const props = withBooleanAttributeState(withoutInlineHandlers(rawProps))
+  const props = withBooleanAttributeState(withoutUnsafeAttributes(rawProps))
   /**
    * ToDo(Christian): fix way we collect data on added nodes in script
    */
@@ -92,3 +98,23 @@ export function transform(node: TransformInput): VNode<{}> {
     : [childrenRequired]
   return h(node.type as string, props, ...c.map(transform)) as VNode<{}>
 }
+
+type WithChildren = { children?: ComponentChildren }
+
+/** Puts `<base href>` first in a replayed document's head, adding a head when it has none. */
+export function withBaseUrl(root: VNode<{}>, href: string): VNode<{}> {
+  // Preact keeps one child, or none, as a bare value rather than an array.
+  const props = root.props as WithChildren
+  const children = toChildArray(props.children)
+  const base = h('base', { href })
+  const head = children.find(
+    (child): child is VNode<WithChildren> =>
+      typeof child === 'object' && child.type === 'head'
+  )
+  if (head) {
+    head.props.children = [base, ...toChildArray(head.props.children)]
+  } else {
+    props.children = [h('head', {}, base), ...children]
+  }
+  return root
+}

packages/app/tests/hosted-csp.test.ts

@@ -1,5 +1,7 @@
 import { describe, expect, it } from 'vitest'
 
+import { playerCsp } from '@wdio/devtools-shared'
+
 import { hostingHeaders } from '../hosting-headers.js'
 
 // The replay iframe is same-origin, so it inherits this policy.
@@ -37,4 +39,8 @@ describe('the hosted player CSP', () => {
   it('keeps the year-long asset cache off index.html and every other path', () => {
     expect(hostingHeaders()['Cache-Control']).toBeUndefined()
   })
+
+  it('is the policy the local backend sends, so the two cannot drift', () => {
+    expect(policy).toBe(playerCsp())
+  })
 })

packages/app/tests/replay-safety.test.ts

@@ -0,0 +1,46 @@
+import { describe, expect, it } from 'vitest'
+
+import {
+  isUnsafeReplayAttribute,
+  isUnsafeReplayElement
+} from '../src/components/browser/replay-safety.js'
+
+describe('isUnsafeReplayAttribute', () => {
+  it.each([
+    ['onload', 'top.x = 1'],
+    ['onError', 'top.x = 1'],
+    ['ONCLICK', 'top.x = 1'],
+    ['srcdoc', '<script>top.x = 1</script>'],
+    ['http-equiv', 'refresh'],
+    ['href', 'javascript:top.x = 1'],
+    ['href', '  JavaScript:top.x = 1'],
+    ['src', 'javascript:top.x = 1'],
+    ['action', 'javascript:top.x = 1'],
+    ['formaction', 'javascript:top.x = 1'],
+    ['xlink:href', 'javascript:top.x = 1']
+  ])('refuses %s="%s"', (name, value) => {
+    expect(isUnsafeReplayAttribute(name, value)).toBe(true)
+  })
+
+  it.each([
+    ['href', 'https://example.com/javascript:x'],
+    ['src', 'data:image/png;base64,AAAA'],
+    ['class', 'onload'],
+    ['data-on', 'x'],
+    ['title', 'javascript:x']
+  ])('keeps %s="%s"', (name, value) => {
+    expect(isUnsafeReplayAttribute(name, value)).toBe(false)
+  })
+})
+
+describe('isUnsafeReplayElement', () => {
+  it('refuses a script in any case', () => {
+    expect(isUnsafeReplayElement('script')).toBe(true)
+    expect(isUnsafeReplayElement('SCRIPT')).toBe(true)
+  })
+
+  it('keeps everything else', () => {
+    expect(isUnsafeReplayElement('img')).toBe(false)
+    expect(isUnsafeReplayElement(undefined)).toBe(false)
+  })
+})

packages/app/tests/vnode-transform.test.ts

@@ -4,7 +4,10 @@
 import { describe, expect, it } from 'vitest'
 import { render, type VNode } from 'preact'
 
-import { transform } from '../src/components/browser/vnode-transform.js'
+import {
+  transform,
+  withBaseUrl
+} from '../src/components/browser/vnode-transform.js'
 // The producers on the other side of the wire. `transform` only ever sees what
 // these two emit, so the shapes below are taken from them rather than guessed:
 // `parseDocument` serializes the full-DOM anchor (collector.captureCurrentDom),
@@ -427,3 +430,95 @@ describe('transform', () => {
     })
   })
 })
+
+describe('a replayed node from an untrusted trace', () => {
+  it('renders no script, wherever it sits', () => {
+    const holder = document.createElement('div')
+    render(
+      transform({
+        type: 'div',
+        props: {
+          children: [
+            { type: 'script', props: { children: 'top.ran = 1' } },
+            { type: 'span', props: { children: 'kept' } }
+          ]
+        }
+      }),
+      holder
+    )
+
+    expect(holder.querySelector('script')).toBeNull()
+    expect(holder.querySelector('span')?.textContent).toBe('kept')
+  })
+
+  it('drops script URLs and srcdoc but keeps ordinary ones', () => {
+    const link = transform({
+      type: 'a',
+      props: { href: 'javascript:top.ran = 1', title: 'x' }
+    })
+    const frame = transform({
+      type: 'iframe',
+      props: {
+        srcdoc: '<script>top.ran = 1</script>',
+        src: 'https://example.com/'
+      }
+    })
+
+    expect(propsOf(link).href).toBeUndefined()
+    expect(propsOf(link).title).toBe('x')
+    expect(propsOf(frame).srcdoc).toBeUndefined()
+    expect(propsOf(frame).src).toBe('https://example.com/')
+  })
+})
+
+describe('withBaseUrl', () => {
+  const page = (head: Serialized[] | Serialized | undefined) =>
+    transform({
+      type: 'html',
+      props: {
+        children: [
+          { type: 'head', props: head === undefined ? {} : { children: head } },
+          { type: 'body', props: { children: 'page' } }
+        ]
+      }
+    })
+
+  const baseOf = (root: VNode<{}>) => {
+    const holder = document.createElement('div')
+    render(withBaseUrl(root, 'https://example.com/login'), holder)
+    return holder.querySelector('head')
+  }
+
+  // Preact keeps zero or one child as a bare value, which once crashed the replay of such a page.
+  it.each([
+    ['an empty head', undefined],
+    ['a head with one child', { type: 'title', props: { children: 'One' } }],
+    [
+      'a head with several children',
+      [
+        { type: 'title', props: { children: 'Many' } },
+        { type: 'meta', props: { charset: 'utf-8' } }
+      ]
+    ]
+  ])('puts the base first in %s', (_name, head) => {
+    const rendered = baseOf(page(head as Serialized[] | Serialized | undefined))
+
+    expect(rendered?.firstElementChild?.getAttribute('href')).toBe(
+      'https://example.com/login'
+    )
+  })
+
+  it('adds a head to a document without one', () => {
+    const root = transform({
+      type: 'html',
+      props: { children: { type: 'body', props: { children: 'page' } } }
+    })
+    const holder = document.createElement('div')
+    render(withBaseUrl(root, 'https://example.com/'), holder)
+
+    expect(holder.querySelector('head > base')?.getAttribute('href')).toBe(
+      'https://example.com/'
+    )
+    expect(holder.querySelector('body')?.textContent).toBe('page')
+  })
+})

packages/backend/src/index.ts

@@ -41,7 +41,9 @@ import {
   type BaselineSavedWsPayload,
   type BaselineClearedWsPayload,
   type RunnerRequestBody,
-  type SocketMessage
+  type SocketMessage,
+  dashboardSocketSource,
+  playerCsp
 } from '@wdio/devtools-shared'
 
 let server: FastifyInstance | undefined
@@ -471,6 +473,16 @@ export async function start(
   await server.register(rateLimit, { max: 100, timeWindow: '1 minute' })
   await server.register(websocket)
   await server.register(staticServer, { root: appPath })
+  // The dashboard and show-trace replay recorded pages too, so they get the hosted player's policy.
+  server.addHook('onSend', async (request, reply, payload) => {
+    if (String(reply.getHeader('content-type') ?? '').startsWith('text/html')) {
+      reply.header(
+        'Content-Security-Policy',
+        playerCsp(dashboardSocketSource(request.headers.host))
+      )
+    }
+    return payload
+  })
 
   registerTraceRoutes(server, opts)
   registerCollectorRoute(server, collectorSource)