Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions apps/daemon/src/routes/static-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1486,9 +1486,12 @@ function normalizeDesignSystemCraftApplies(value: unknown): string[] | undefined
}

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

export function rewriteSkillAssetUrls(
Expand Down
19 changes: 19 additions & 0 deletions apps/daemon/tests/assemble-example-title-patterns.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Regression (#6795): `assembleExample` interpolates skill-derived slide HTML
// and titles verbatim. A string replacement would expand `$$`, `$&`, `$`` and
// `$'` through String.prototype.replace's GetSubstitution and corrupt them.
import assert from 'node:assert/strict';

import { test } from 'vitest';

import { assembleExample } from '../src/routes/static-resource.js';

test('assembleExample keeps replacement-pattern sequences verbatim', () => {
const template = '<html><head><title>Old</title></head><body><!-- SLIDES_HERE --></body></html>';
const slides = '<section>Save $$$ deck</section>';
for (const title of ['Save $$$ This Quarter', "Rock $'n Roll Tour", 'Before $& After', 'Backtick $` Pattern']) {
assert.equal(
assembleExample(template, slides, title),
`<html><head><title>${title} | Open Design Example</title></head><body>${slides}</body></html>`,
);
}
});
5 changes: 4 additions & 1 deletion apps/desktop/src/main/artifact-export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ function injectBaseHref(doc: string, baseHref: string | undefined): string {

function injectTitle(doc: string, title: string): string {
const tag = `<title>${escapeText(title)}</title>`;
if (/<title[^>]*>.*?<\/title>/is.test(doc)) return doc.replace(/<title[^>]*>.*?<\/title>/is, tag);
// Function replacement: a string replacement would expand `$$`, `$&`, `$``,
// and `$'` inside the (user-derived) title via String.prototype.replace's
// GetSubstitution, corrupting titles that contain them (#6795).
if (/<title[^>]*>.*?<\/title>/is.test(doc)) return doc.replace(/<title[^>]*>.*?<\/title>/is, () => tag);
if (/<head[^>]*>/i.test(doc)) return doc.replace(/<head[^>]*>/i, (m) => `${m}${tag}`);
return doc;
}
Expand Down
5 changes: 4 additions & 1 deletion apps/desktop/src/main/pdf-export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,10 @@ function injectBaseHref(doc: string, baseHref: string | undefined): string {

function injectTitle(doc: string, title: string): string {
const tag = `<title>${escapeHtmlText(title)}</title>`;
if (/<title[^>]*>.*?<\/title>/is.test(doc)) return doc.replace(/<title[^>]*>.*?<\/title>/is, tag);
// Function replacement: a string replacement would expand `$$`, `$&`, `$``,
// and `$'` inside the (user-derived) title via String.prototype.replace's
// GetSubstitution, corrupting titles that contain them (#6795).
if (/<title[^>]*>.*?<\/title>/is.test(doc)) return doc.replace(/<title[^>]*>.*?<\/title>/is, () => tag);
if (/<head[^>]*>/i.test(doc)) return doc.replace(/<head[^>]*>/i, (match) => `${match}${tag}`);
if (/<html[^>]*>/i.test(doc)) return doc.replace(/<html[^>]*>/i, (match) => `${match}<head>${tag}</head>`);
return `<!doctype html><html><head>${tag}</head><body>${doc}</body></html>`;
Expand Down
125 changes: 125 additions & 0 deletions apps/desktop/tests/main/export-title-replacement-patterns.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// Regression (#6795): artifact titles containing String.prototype.replace
// replacement patterns (`$$`, `$&`, `$``, `$'`) must land in exported PDF /
// image documents verbatim (HTML-escaped only). A string replacement in
// `injectTitle` would expand them and corrupt the rendered document.
import { mkdtemp, rm } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { dirname, join } from 'node:path';

import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';

const rendererState = vi.hoisted(() => ({ loadedUrls: [] as string[], saveDir: '' }));

vi.mock('electron', () => {
const image = {
getSize: () => ({ height: 1, width: 1 }),
toBitmap: () => Buffer.alloc(4),
toJPEG: () => Buffer.from('jpeg'),
toPNG: () => Buffer.from('png'),
};

class BrowserWindow {
readonly webContents = {
capturePage: async () => image,
debugger: {
attach: () => {
throw new Error('debugger unavailable in title-pattern test');
},
detach: () => undefined,
sendCommand: async () => undefined,
},
executeJavaScript: async (source: string): Promise<unknown> => {
if (source.includes("document.querySelectorAll('.slide")) return 0;
if (source.includes('document.documentElement.scrollHeight')) return 1;
if (source === 'window.devicePixelRatio || 1') return 1;
return true;
},
on: () => undefined,
printToPDF: async () => Buffer.from('pdf'),
setWindowOpenHandler: () => undefined,
};

async loadURL(url: string): Promise<void> {
rendererState.loadedUrls.push(url);
}

destroy(): void {}
getContentSize(): [number, number] { return [1440, 900]; }
isDestroyed(): boolean { return false; }
setContentSize(): void {}
setOpacity(): void {}
showInactive(): void {}
}

return {
BrowserWindow,
dialog: { showSaveDialog: async () => ({ canceled: false, filePath: join(rendererState.saveDir, 'out.pdf') }) },
nativeImage: { createFromBitmap: () => image },
};
});

import { exportArtifact } from '../../src/main/artifact-export.js';
import { exportPdfFromHtml } from '../../src/main/pdf-export.js';

const SOURCE_DOC = '<html><head><title>Old</title></head><body><p>BODY MARKER</p></body></html>';
const TITLES = ['Save $$$ This Quarter', "Rock $'n Roll Tour", 'Before $& After', 'Backtick $` Pattern'] as const;

beforeAll(async () => {
rendererState.saveDir = await mkdtemp(join(tmpdir(), 'od-title-export-'));
});

afterAll(async () => {
await rm(rendererState.saveDir, { force: true, recursive: true });
});

function lastLoadedDocument(): string {
const url = rendererState.loadedUrls[rendererState.loadedUrls.length - 1];
if (!url?.startsWith('data:text/html;charset=utf-8,')) throw new Error(`unexpected loaded URL: ${url}`);
return decodeURIComponent(url.slice('data:text/html;charset=utf-8,'.length));
}

/** The exact HTML-escaping the exporters apply before interpolation. */
function escapedTitle(title: string): string {
return title.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;');
}

describe('export title replacement-pattern safety (#6795)', () => {
it('keeps replacement-pattern sequences verbatim in PDF export titles', async () => {
for (const title of TITLES) {
rendererState.loadedUrls.length = 0;
const result = await exportPdfFromHtml({
baseHref: undefined,
deck: true,
defaultFilename: 'out.pdf',
html: SOURCE_DOC,
title,
});
expect(result.ok).toBe(true);
const doc = lastLoadedDocument();
expect(doc).toContain(`<title>${escapedTitle(title)}</title>`);
expect(doc).toContain('<p>BODY MARKER</p>');
}
}, 30_000);

it('keeps replacement-pattern sequences verbatim in image export titles', async () => {
for (const title of TITLES) {
rendererState.loadedUrls.length = 0;
const result = await exportArtifact({
baseHref: undefined,
deck: false,
format: 'image',
html: SOURCE_DOC,
imageFormat: 'png',
title,
} as const);
try {
expect(result.ok).toBe(true);
const doc = lastLoadedDocument();
expect(doc).toContain(`<title>${escapedTitle(title)}</title>`);
expect(doc).toContain('<p>BODY MARKER</p>');
} finally {
if (result.path) await rm(dirname(result.path), { force: true, recursive: true });
}
}
}, 30_000);
});
Loading