Skip to content

Commit efd6ba7

Browse files
fix(images): deterministic host image paths to stop screenshot churn (#43)
Agent output references the same artifact through volatile `src` forms — a relative `artifacts/…` path, a container absolute path (`/opt/cursor/artifacts/…`), a repo/directory-named path (`/home/user/<repo>/artifacts/…`), or a per-session download URL (`…/v1/agents/<id>/artifacts/download?path=artifacts/…`). The host-injectable image refactor dropped the normalization the old `artifact-images.ts` did, so the host renderer now echoes `src` verbatim into `data-host-image-path`. Those volatile segments (container dir, repo name, directory layout, session id) then leak into the rendered DOM and churn the downstream e2e screenshots every time the environment differs. Restore determinism as a generic, app-agnostic primitive: - Add `normalizeHostImagePath(src, { rootMarker })` — strips any leading absolute/dir prefix down to the marker segment (default `artifacts`), prefers a URL's `?path=` param, returns volatile query params separately, and rejects path traversal. No host path is hardcoded, keeping the core decoupled. - Route the host-image test policy through it and render only the stable path. - Regression test proving five volatile forms of one artifact collapse to a single identical placeholder and no session id leaks. Hosts should call `normalizeHostImagePath(src).path` and keep volatile query params out of any snapshot-visible attribute. Claude-Session: https://claude.ai/code/session_01U5XyfYm8fojvk9USphcx4E Co-authored-by: Claude <noreply@anthropic.com>
1 parent 23fcf53 commit efd6ba7

5 files changed

Lines changed: 222 additions & 6 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,18 @@ When extending the renderer or its CSS, preserve these rules:
6060
`<img>` becomes (e.g. an app's artifact placeholder). The core escapes every
6161
`<img>` by default; the renderer's output bypasses escaping via a placeholder and
6262
is restored afterward.
63+
- `normalizeHostImagePath` (`raw-images.ts`) — determinism primitive for that renderer.
64+
Agent output references the same artifact through volatile `src` forms — a relative
65+
`artifacts/…` path, a container absolute path (`/opt/cursor/artifacts/…`), a
66+
repo/directory-named path (`/home/user/<repo>/artifacts/…`), or a per-session download
67+
URL (`…/v1/agents/<id>/artifacts/download?path=artifacts/…`). Left verbatim in the
68+
rendered attribute, those volatile segments (container dir, repo name, directory layout,
69+
session id) change per run and churn the host's e2e **screenshots**. This collapses each
70+
to the same stable `artifacts/…` path (marker segment configurable; no host path is
71+
hardcoded) and keeps URL query params out of the path, so a host that renders
72+
`normalizeHostImagePath(src).path` gets identical output across machines. Hosts should
73+
route their artifact `<img>` through it and never fold volatile query params into a
74+
snapshot-visible attribute.
6375
- `setSanitizeExtension` (`sanitize.ts`) — widens the sanitizer allowlist and adds a
6476
per-element gate so a host's injected markup (e.g. its artifact `<img>`) survives
6577
sanitization. The core allowlist stays the security gate; keep additions narrow.

‎src/host-image-path.test.ts‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import { describe, it } from 'node:test'
2+
import assert from 'node:assert/strict'
3+
import { normalizeHostImagePath } from './raw-images.ts'
4+
import { withHostImagePolicy } from '../tests/host-image-test-policy.ts'
5+
import { renderMarkdown } from './renderer.ts'
6+
7+
describe('normalizeHostImagePath — screenshot determinism (#churn)', () => {
8+
it('leaves an already-relative artifacts path unchanged', () => {
9+
assert.deepEqual(normalizeHostImagePath('artifacts/screenshots/x.png'), {
10+
path: 'artifacts/screenshots/x.png',
11+
})
12+
})
13+
14+
it('strips a container absolute prefix down to the root marker', () => {
15+
assert.deepEqual(normalizeHostImagePath('/opt/cursor/artifacts/screenshots/x.png'), {
16+
path: 'artifacts/screenshots/x.png',
17+
})
18+
})
19+
20+
it('strips a repo/directory-named prefix so repo names never leak', () => {
21+
assert.deepEqual(normalizeHostImagePath('/home/user/some-repo/artifacts/screenshots/x.png'), {
22+
path: 'artifacts/screenshots/x.png',
23+
})
24+
})
25+
26+
it('collapses every volatile form of the same artifact to one identical path', () => {
27+
const forms = [
28+
'artifacts/screenshots/x.png',
29+
'/opt/cursor/artifacts/screenshots/x.png',
30+
'/home/user/some-repo/artifacts/screenshots/x.png',
31+
'/tmp/build-9f3a/checkout/artifacts/screenshots/x.png',
32+
'https://host.example/v1/agents/session-abc123/artifacts/download?path=artifacts/screenshots/x.png',
33+
]
34+
const paths = new Set(forms.map((src) => normalizeHostImagePath(src)?.path))
35+
assert.deepEqual([...paths], ['artifacts/screenshots/x.png'])
36+
})
37+
38+
it('keeps a URL session id out of the stable path (in params, not the rendered attribute)', () => {
39+
const normalized = normalizeHostImagePath(
40+
'https://host.example/v1/agents/session-abc123/artifacts/download?path=artifacts/screenshots/x.png&token=xyz',
41+
)
42+
assert.equal(normalized?.path, 'artifacts/screenshots/x.png')
43+
// Volatile bits are surfaced separately, never folded into `path`.
44+
assert.deepEqual(normalized?.params, { token: 'xyz' })
45+
assert.doesNotMatch(normalized?.path ?? '', /session-abc123/)
46+
})
47+
48+
it('accepts a caller-supplied root marker (no host path is hardcoded in core)', () => {
49+
assert.deepEqual(normalizeHostImagePath('/srv/data/uploads/a/b.png', { rootMarker: 'uploads' }), {
50+
path: 'uploads/a/b.png',
51+
})
52+
})
53+
54+
it('returns null when the marker is absent (host falls through to escaping)', () => {
55+
assert.equal(normalizeHostImagePath('/etc/passwd'), null)
56+
assert.equal(normalizeHostImagePath('https://host.example/logo.png'), null)
57+
assert.equal(normalizeHostImagePath(''), null)
58+
})
59+
60+
it('rejects path traversal that would escape the root', () => {
61+
assert.equal(normalizeHostImagePath('artifacts/../../etc/passwd'), null)
62+
})
63+
})
64+
65+
describe('host image policy renders volatile srcs to one stable placeholder', () => {
66+
it('renders the container-absolute and relative forms identically', () => {
67+
withHostImagePolicy(() => {
68+
const abs = renderMarkdown('<img alt="shot" src="/opt/cursor/artifacts/screenshots/x.png" />')
69+
const rel = renderMarkdown('<img alt="shot" src="artifacts/screenshots/x.png" />')
70+
assert.equal(abs, rel)
71+
assert.match(abs, /data-host-image-path="artifacts\/screenshots\/x\.png"/)
72+
})
73+
})
74+
75+
it('renders the download-URL form identically and never leaks the session id', () => {
76+
withHostImagePolicy(() => {
77+
const url = renderMarkdown(
78+
'<img alt="shot" src="https://host.example/v1/agents/session-abc123/artifacts/download?path=artifacts/screenshots/x.png" />',
79+
)
80+
const rel = renderMarkdown('<img alt="shot" src="artifacts/screenshots/x.png" />')
81+
assert.equal(url, rel)
82+
assert.doesNotMatch(url, /session-abc123/)
83+
})
84+
})
85+
})

‎src/index.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,14 @@ export {
2424
// `@copse/streaming-markdown/sanitizers/dompurify` entry so it is only bundled
2525
// when a host explicitly opts in.
2626
export { browserSanitizerBackend, isBrowserSanitizerSupported } from './sanitize-browser.ts'
27-
export { setRawImageRenderer, type RawImageRenderer, type RawImageTag } from './raw-images.ts'
27+
export {
28+
setRawImageRenderer,
29+
normalizeHostImagePath,
30+
type RawImageRenderer,
31+
type RawImageTag,
32+
type NormalizedImagePath,
33+
type NormalizeImagePathOptions,
34+
} from './raw-images.ts'
2835
export { escapeHtml, escapeHtmlTextNodes, decodeSafeMarkdownEntities } from './escape.ts'
2936
// Syntax highlighting is a pluggable backend. The core (`highlight.ts`) carries
3037
// no highlight.js code and renders escaped plain text until a backend is

‎src/raw-images.ts‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,104 @@ export function restoreRawImages(text: string, images: readonly string[]): strin
8080
if (images.length === 0) return text
8181
return text.replace(PLACEHOLDER_RE, (_match, index: string) => images[Number(index)] ?? '')
8282
}
83+
84+
/** A host image `src` reduced to a stable, machine-independent form. */
85+
export interface NormalizedImagePath {
86+
/**
87+
* The image path relative to (and including) the root marker segment, e.g.
88+
* `artifacts/screenshots/x.png`. Deterministic across machines: any leading
89+
* absolute/container/repo directory prefix is stripped.
90+
*/
91+
path: string
92+
/**
93+
* Query params carried by a URL `src` (e.g. a per-session agent id). These are
94+
* volatile and MUST be kept out of any attribute that ends up in a rendered
95+
* snapshot/screenshot — surface them separately (or use them only at fetch
96+
* time) so they can't churn the committed image. Omitted when the `src` was a
97+
* plain path with no query string.
98+
*/
99+
params?: Record<string, string>
100+
}
101+
102+
export interface NormalizeImagePathOptions {
103+
/**
104+
* The path segment that anchors the stable relative path. Everything before the
105+
* first occurrence of this segment is discarded; the segment and everything
106+
* after it are kept. Defaults to `'artifacts'`.
107+
*/
108+
rootMarker?: string
109+
}
110+
111+
/**
112+
* Reduce a raw image `src` to a stable, machine-independent {@link NormalizedImagePath},
113+
* or `null` when it does not contain the root marker (host should then fall through
114+
* to escaping). This is the determinism primitive behind screenshot churn: agent
115+
* output references the same artifact through volatile forms —
116+
*
117+
* - `artifacts/screenshots/x.png` (already relative)
118+
* - `/opt/cursor/artifacts/screenshots/x.png` (container abs path)
119+
* - `/home/user/some-repo/artifacts/screenshots/x.png` (repo/dir names leak)
120+
* - `https://host/v1/agents/<session>/artifacts/download?path=artifacts/screenshots/x.png`
121+
*
122+
* — all of which must render identically. This collapses each to the same
123+
* `artifacts/screenshots/x.png` so the rendered DOM (and any screenshot of it) stops
124+
* changing when the container dir, repo name, directory layout, or session id changes.
125+
*
126+
* The core stays app-agnostic: no host path is hardcoded — the anchor segment is the
127+
* caller-supplied {@link NormalizeImagePathOptions.rootMarker}. Path traversal
128+
* (`..`) is rejected as a safety measure.
129+
*/
130+
export function normalizeHostImagePath(
131+
rawSrc: string,
132+
options: NormalizeImagePathOptions = {},
133+
): NormalizedImagePath | null {
134+
const rootMarker = options.rootMarker ?? 'artifacts'
135+
const src = rawSrc.trim()
136+
if (!src) return null
137+
138+
let candidate = src
139+
let params: Record<string, string> | undefined
140+
141+
// A URL src carries the stable path in its `?path=` query param (the URL's own
142+
// pathname holds a volatile per-session id); prefer it. Other query params are
143+
// returned separately so the host keeps them out of snapshot-visible attributes.
144+
const url = tryParseUrl(src)
145+
if (url) {
146+
const pathParam = url.searchParams.get('path')
147+
candidate = pathParam ?? url.pathname
148+
const rest: Record<string, string> = {}
149+
for (const [key, value] of url.searchParams) {
150+
if (key === 'path') continue
151+
rest[key] = value
152+
}
153+
if (Object.keys(rest).length > 0) params = rest
154+
}
155+
156+
const path = stableRelativePath(candidate, rootMarker)
157+
if (path == null) return null
158+
return params ? { path, params } : { path }
159+
}
160+
161+
function tryParseUrl(src: string): URL | null {
162+
// Only absolute URLs (with a scheme) are URLs here; bare paths must not be
163+
// coerced (no base is supplied), so `new URL(src)` naturally rejects them.
164+
try {
165+
return new URL(src)
166+
} catch {
167+
return null
168+
}
169+
}
170+
171+
/**
172+
* Keep the `rootMarker` segment and everything after it, discarding any leading
173+
* directory prefix. Returns `null` if the marker is absent or a `..` segment would
174+
* escape the root.
175+
*/
176+
function stableRelativePath(rawPath: string, rootMarker: string): string | null {
177+
const segments = rawPath.split('/').filter((segment) => segment !== '' && segment !== '.')
178+
const start = segments.indexOf(rootMarker)
179+
if (start === -1) return null
180+
const kept = segments.slice(start)
181+
if (kept.includes('..')) return null
182+
return kept.join('/')
183+
}

‎tests/host-image-test-policy.ts‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,30 @@
22
// core renderer escapes raw `<img>` and allows none through the sanitizer. This
33
// fixture simulates a host (the real policy lives in the consuming app) so the
44
// injection surface — `setRawImageRenderer` + `setSanitizeExtension` — can be
5-
// exercised. Only an `<img>` whose `src` starts with `artifacts/` becomes a
6-
// locked-down, src-less placeholder; everything else falls through to escaping.
7-
import { setRawImageRenderer, type RawImageTag } from '../src/raw-images.ts'
5+
// exercised. An `<img>` whose `src` resolves (via `normalizeHostImagePath`) to a
6+
// stable `artifacts/…` path becomes a locked-down, src-less placeholder; every
7+
// other tag falls through to escaping.
8+
//
9+
// The renderer normalizes the path so volatile prefixes — a container abs path
10+
// (`/opt/cursor/artifacts/…`), a repo/dir name (`/home/user/<repo>/artifacts/…`),
11+
// or a per-session download URL — all collapse to the same
12+
// `data-host-image-path`. That determinism is what stops the rendered DOM (and
13+
// any screenshot of it) from churning when those environment details change.
14+
import { setRawImageRenderer, normalizeHostImagePath, type RawImageTag } from '../src/raw-images.ts'
815
import { setSanitizeExtension } from '../src/sanitize.ts'
916
import { escapeHtml } from '../src/escape.ts'
1017

1118
const HOST_IMAGE_CLASS = 'host-image'
1219

1320
function hostImageRenderer({ attrs }: RawImageTag): string | null {
1421
const src = attrs['src']
15-
if (!src || !src.startsWith('artifacts/')) return null
22+
if (!src) return null
23+
const normalized = normalizeHostImagePath(src)
24+
if (!normalized) return null
1625
const alt = escapeHtml(attrs['alt'] ?? '')
17-
return `<img class="${HOST_IMAGE_CLASS}" data-host-image-path="${escapeHtml(src)}" alt="${alt}" loading="lazy">`
26+
// Only the stable relative path reaches the rendered attribute; the volatile
27+
// query params (e.g. a session id) stay out of anything a screenshot captures.
28+
return `<img class="${HOST_IMAGE_CLASS}" data-host-image-path="${escapeHtml(normalized.path)}" alt="${alt}" loading="lazy">`
1829
}
1930

2031
const hostImageSanitize = {

0 commit comments

Comments
 (0)