Skip to content

Commit c144c80

Browse files
authored
fix(desktop,daemon): keep replacement-pattern sequences verbatim in export titles (#6795) (#6947)
String.replace expands $$, $&, $` and $' in string replacements, corrupting user-derived artifact titles in PDF/image export and skill-derived titles in example assembly. Switch the affected interpolations to function replacements and add regression tests for both exporters and assembleExample.
1 parent 4034380 commit c144c80

5 files changed

Lines changed: 157 additions & 4 deletions

File tree

apps/daemon/src/routes/static-resource.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1486,9 +1486,12 @@ function normalizeDesignSystemCraftApplies(value: unknown): string[] | undefined
14861486
}
14871487

14881488
export function assembleExample(templateHtml: string, slidesHtml: string, title: string) {
1489+
// Function replacements: string replacements would expand `$$`, `$&`, `$``,
1490+
// and `$'` inside the skill-derived inputs via String.prototype.replace's
1491+
// GetSubstitution (#6795).
14891492
return templateHtml
1490-
.replace('<!-- SLIDES_HERE -->', slidesHtml)
1491-
.replace(/<title>.*?<\/title>/, `<title>${title} | Open Design Example</title>`);
1493+
.replace('<!-- SLIDES_HERE -->', () => slidesHtml)
1494+
.replace(/<title>.*?<\/title>/, () => `<title>${title} | Open Design Example</title>`);
14921495
}
14931496

14941497
export function rewriteSkillAssetUrls(
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Regression (#6795): `assembleExample` interpolates skill-derived slide HTML
2+
// and titles verbatim. A string replacement would expand `$$`, `$&`, `$`` and
3+
// `$'` through String.prototype.replace's GetSubstitution and corrupt them.
4+
import assert from 'node:assert/strict';
5+
6+
import { test } from 'vitest';
7+
8+
import { assembleExample } from '../src/routes/static-resource.js';
9+
10+
test('assembleExample keeps replacement-pattern sequences verbatim', () => {
11+
const template = '<html><head><title>Old</title></head><body><!-- SLIDES_HERE --></body></html>';
12+
const slides = '<section>Save $$$ deck</section>';
13+
for (const title of ['Save $$$ This Quarter', "Rock $'n Roll Tour", 'Before $& After', 'Backtick $` Pattern']) {
14+
assert.equal(
15+
assembleExample(template, slides, title),
16+
`<html><head><title>${title} | Open Design Example</title></head><body>${slides}</body></html>`,
17+
);
18+
}
19+
});

apps/desktop/src/main/artifact-export.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,10 @@ function injectBaseHref(doc: string, baseHref: string | undefined): string {
114114

115115
function injectTitle(doc: string, title: string): string {
116116
const tag = `<title>${escapeText(title)}</title>`;
117-
if (/<title[^>]*>.*?<\/title>/is.test(doc)) return doc.replace(/<title[^>]*>.*?<\/title>/is, tag);
117+
// Function replacement: a string replacement would expand `$$`, `$&`, `$``,
118+
// and `$'` inside the (user-derived) title via String.prototype.replace's
119+
// GetSubstitution, corrupting titles that contain them (#6795).
120+
if (/<title[^>]*>.*?<\/title>/is.test(doc)) return doc.replace(/<title[^>]*>.*?<\/title>/is, () => tag);
118121
if (/<head[^>]*>/i.test(doc)) return doc.replace(/<head[^>]*>/i, (m) => `${m}${tag}`);
119122
return doc;
120123
}

apps/desktop/src/main/pdf-export.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,10 @@ function injectBaseHref(doc: string, baseHref: string | undefined): string {
266266

267267
function injectTitle(doc: string, title: string): string {
268268
const tag = `<title>${escapeHtmlText(title)}</title>`;
269-
if (/<title[^>]*>.*?<\/title>/is.test(doc)) return doc.replace(/<title[^>]*>.*?<\/title>/is, tag);
269+
// Function replacement: a string replacement would expand `$$`, `$&`, `$``,
270+
// and `$'` inside the (user-derived) title via String.prototype.replace's
271+
// GetSubstitution, corrupting titles that contain them (#6795).
272+
if (/<title[^>]*>.*?<\/title>/is.test(doc)) return doc.replace(/<title[^>]*>.*?<\/title>/is, () => tag);
270273
if (/<head[^>]*>/i.test(doc)) return doc.replace(/<head[^>]*>/i, (match) => `${match}${tag}`);
271274
if (/<html[^>]*>/i.test(doc)) return doc.replace(/<html[^>]*>/i, (match) => `${match}<head>${tag}</head>`);
272275
return `<!doctype html><html><head>${tag}</head><body>${doc}</body></html>`;
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
// Regression (#6795): artifact titles containing String.prototype.replace
2+
// replacement patterns (`$$`, `$&`, `$``, `$'`) must land in exported PDF /
3+
// image documents verbatim (HTML-escaped only). A string replacement in
4+
// `injectTitle` would expand them and corrupt the rendered document.
5+
import { mkdtemp, rm } from 'node:fs/promises';
6+
import { tmpdir } from 'node:os';
7+
import { dirname, join } from 'node:path';
8+
9+
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
10+
11+
const rendererState = vi.hoisted(() => ({ loadedUrls: [] as string[], saveDir: '' }));
12+
13+
vi.mock('electron', () => {
14+
const image = {
15+
getSize: () => ({ height: 1, width: 1 }),
16+
toBitmap: () => Buffer.alloc(4),
17+
toJPEG: () => Buffer.from('jpeg'),
18+
toPNG: () => Buffer.from('png'),
19+
};
20+
21+
class BrowserWindow {
22+
readonly webContents = {
23+
capturePage: async () => image,
24+
debugger: {
25+
attach: () => {
26+
throw new Error('debugger unavailable in title-pattern test');
27+
},
28+
detach: () => undefined,
29+
sendCommand: async () => undefined,
30+
},
31+
executeJavaScript: async (source: string): Promise<unknown> => {
32+
if (source.includes("document.querySelectorAll('.slide")) return 0;
33+
if (source.includes('document.documentElement.scrollHeight')) return 1;
34+
if (source === 'window.devicePixelRatio || 1') return 1;
35+
return true;
36+
},
37+
on: () => undefined,
38+
printToPDF: async () => Buffer.from('pdf'),
39+
setWindowOpenHandler: () => undefined,
40+
};
41+
42+
async loadURL(url: string): Promise<void> {
43+
rendererState.loadedUrls.push(url);
44+
}
45+
46+
destroy(): void {}
47+
getContentSize(): [number, number] { return [1440, 900]; }
48+
isDestroyed(): boolean { return false; }
49+
setContentSize(): void {}
50+
setOpacity(): void {}
51+
showInactive(): void {}
52+
}
53+
54+
return {
55+
BrowserWindow,
56+
dialog: { showSaveDialog: async () => ({ canceled: false, filePath: join(rendererState.saveDir, 'out.pdf') }) },
57+
nativeImage: { createFromBitmap: () => image },
58+
};
59+
});
60+
61+
import { exportArtifact } from '../../src/main/artifact-export.js';
62+
import { exportPdfFromHtml } from '../../src/main/pdf-export.js';
63+
64+
const SOURCE_DOC = '<html><head><title>Old</title></head><body><p>BODY MARKER</p></body></html>';
65+
const TITLES = ['Save $$$ This Quarter', "Rock $'n Roll Tour", 'Before $& After', 'Backtick $` Pattern'] as const;
66+
67+
beforeAll(async () => {
68+
rendererState.saveDir = await mkdtemp(join(tmpdir(), 'od-title-export-'));
69+
});
70+
71+
afterAll(async () => {
72+
await rm(rendererState.saveDir, { force: true, recursive: true });
73+
});
74+
75+
function lastLoadedDocument(): string {
76+
const url = rendererState.loadedUrls[rendererState.loadedUrls.length - 1];
77+
if (!url?.startsWith('data:text/html;charset=utf-8,')) throw new Error(`unexpected loaded URL: ${url}`);
78+
return decodeURIComponent(url.slice('data:text/html;charset=utf-8,'.length));
79+
}
80+
81+
/** The exact HTML-escaping the exporters apply before interpolation. */
82+
function escapedTitle(title: string): string {
83+
return title.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;');
84+
}
85+
86+
describe('export title replacement-pattern safety (#6795)', () => {
87+
it('keeps replacement-pattern sequences verbatim in PDF export titles', async () => {
88+
for (const title of TITLES) {
89+
rendererState.loadedUrls.length = 0;
90+
const result = await exportPdfFromHtml({
91+
baseHref: undefined,
92+
deck: true,
93+
defaultFilename: 'out.pdf',
94+
html: SOURCE_DOC,
95+
title,
96+
});
97+
expect(result.ok).toBe(true);
98+
const doc = lastLoadedDocument();
99+
expect(doc).toContain(`<title>${escapedTitle(title)}</title>`);
100+
expect(doc).toContain('<p>BODY MARKER</p>');
101+
}
102+
}, 30_000);
103+
104+
it('keeps replacement-pattern sequences verbatim in image export titles', async () => {
105+
for (const title of TITLES) {
106+
rendererState.loadedUrls.length = 0;
107+
const result = await exportArtifact({
108+
baseHref: undefined,
109+
deck: false,
110+
format: 'image',
111+
html: SOURCE_DOC,
112+
imageFormat: 'png',
113+
title,
114+
} as const);
115+
try {
116+
expect(result.ok).toBe(true);
117+
const doc = lastLoadedDocument();
118+
expect(doc).toContain(`<title>${escapedTitle(title)}</title>`);
119+
expect(doc).toContain('<p>BODY MARKER</p>');
120+
} finally {
121+
if (result.path) await rm(dirname(result.path), { force: true, recursive: true });
122+
}
123+
}
124+
}, 30_000);
125+
});

0 commit comments

Comments
 (0)