diff --git a/deploy/index.html b/deploy/index.html index 2576e467..9dc1ad1d 100644 --- a/deploy/index.html +++ b/deploy/index.html @@ -2002,8 +2002,10 @@

3Configure your scripts

`
` + `Error detail` + `
${safeErrorDetail}
` + + // The browser decodes the escaped attribute, so dataset.copy is already + // the original text. Never re-parse it as HTML (#595). `` + `
` if (isApiDisabled) { diff --git a/deploy/tests/ui.spec.js b/deploy/tests/ui.spec.js index ac56b68a..431f991f 100644 --- a/deploy/tests/ui.spec.js +++ b/deploy/tests/ui.spec.js @@ -1284,6 +1284,51 @@ test.describe('deploy index.html', () => { expect(html).not.toContain('Quota') }) + test('Copy error copies the stack trace verbatim and never parses it as HTML (#595)', async ({ + page, + }) => { + const stack = + 'Error: bad at foo ()\n' + + ' at List.get ()\n' + + '' + + '' + + // Stub the clipboard so the test can read what was written. + await page.evaluate(() => { + window.__copied = [] + Object.defineProperty(navigator, 'clipboard', { + configurable: true, + value: { + writeText: (text) => { + window.__copied.push(text) + return Promise.resolve() + }, + }, + }) + }) + + await page.evaluate((errStack) => { + const err = new Error('bad ') + err.stack = errStack + showStatus('error', buildDeployErrorHtml(err)) + }, stack) + + await page.locator('.status-error details summary').click() + await page.locator('.status-error .copy-btn').click() + await expect.poll(() => page.evaluate(() => window.__copied.length)).toBe(1) + + expect(await page.evaluate(() => window.__copied[0])).toBe(stack) + + // Control: a detached with this payload does fire onerror here, so a + // re-parse would have set __pwned before __control is set. + await page.evaluate(() => { + const d = document.createElement('div') + d.innerHTML = '' + }) + await page.waitForFunction(() => window.__control === 1) + expect(await page.evaluate(() => window.__pwned)).toBeUndefined() + }) + // ── showStatus type classes ───────────────────────────────────────────────── test('error sign-in response shows status-error class', async ({ page }) => { diff --git a/src/deploy/index.js b/src/deploy/index.js index 4d3217c3..f4951926 100644 --- a/src/deploy/index.js +++ b/src/deploy/index.js @@ -315,8 +315,10 @@ function buildDeployErrorHtml(err) { `
` + `Error detail` + `
${safeErrorDetail}
` + + // The browser decodes the escaped attribute, so dataset.copy is already the + // original text. Never re-parse it as HTML (#595). `` + `
` diff --git a/src/deploy/tests/index.test.js b/src/deploy/tests/index.test.js index 7f54558f..0e2ff5fc 100644 --- a/src/deploy/tests/index.test.js +++ b/src/deploy/tests/index.test.js @@ -1,5 +1,7 @@ 'use strict' +const fs = require('fs') +const path = require('path') const { APPS_SCRIPT_API_BASE, getScriptCatalog, @@ -714,16 +716,30 @@ describe('buildDeployErrorHtml', () => { expect(html).toContain('copy-btn') }) - test('copy button decodes HTML entities via innerHTML before writing to clipboard', () => { + test('copy button writes data-copy verbatim and never re-parses it as HTML', () => { const err = new Error('') const html = buildDeployErrorHtml(err) - // data-copy must hold the HTML-escaped value so quotes in the attribute are safe + // data-copy holds the HTML-escaped value so the attribute is safe to insert expect(html).toContain('data-copy="') expect(html).toContain('<script>') - // onclick must use div.innerHTML decode trick so the clipboard receives unescaped text - expect(html).toContain('d.innerHTML=this.dataset.copy') - expect(html).toContain('d.textContent') + expect(html).not.toContain('