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
4 changes: 3 additions & 1 deletion deploy/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -2002,8 +2002,10 @@ <h2><span class="step-badge">3</span>Configure your scripts</h2>
`<details style="margin-top:6px;font-size:12px;">` +
`<summary style="cursor:pointer;color:#888;">Error detail</summary>` +
`<div class="error-detail">${safeErrorDetail}</div>` +
// The browser decodes the escaped attribute, so dataset.copy is already
// the original text. Never re-parse it as HTML (#595).
`<button class="copy-btn" data-copy="${safeErrorDetail}"` +
` onclick="var d=document.createElement('div');d.innerHTML=this.dataset.copy;navigator.clipboard.writeText(d.textContent).catch(()=>{})">` +
` onclick="navigator.clipboard.writeText(this.dataset.copy).catch(()=>{})">` +
`📋 Copy error</button>` +
`</details>`
if (isApiDisabled) {
Expand Down
45 changes: 45 additions & 0 deletions deploy/tests/ui.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1284,6 +1284,51 @@ test.describe('deploy index.html', () => {
expect(html).not.toContain('<b>Quota</b>')
})

test('Copy error copies the stack trace verbatim and never parses it as HTML (#595)', async ({
page,
}) => {
const stack =
'Error: bad <tag> at foo (<anonymous>)\n' +
' at List<T>.get (<anonymous>)\n' +
'<script>window.__pwned=1</script>' +
'<img src=x onerror="window.__pwned=1">'

// 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 <tag>')
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 <img> 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 = '<img src=x onerror="window.__control=1">'
})
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 }) => {
Expand Down
4 changes: 3 additions & 1 deletion src/deploy/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,10 @@ function buildDeployErrorHtml(err) {
`<details style="margin-top:6px;font-size:12px;">` +
`<summary style="cursor:pointer;color:#888;">Error detail</summary>` +
`<div class="error-detail">${safeErrorDetail}</div>` +
// The browser decodes the escaped attribute, so dataset.copy is already the
// original text. Never re-parse it as HTML (#595).
`<button class="copy-btn" data-copy="${safeErrorDetail}"` +
` onclick="var d=document.createElement('div');d.innerHTML=this.dataset.copy;navigator.clipboard.writeText(d.textContent).catch(()=>{})">` +
` onclick="navigator.clipboard.writeText(this.dataset.copy).catch(()=>{})">` +
`📋 Copy error</button>` +
`</details>`

Expand Down
26 changes: 21 additions & 5 deletions src/deploy/tests/index.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict'

const fs = require('fs')
const path = require('path')
const {
APPS_SCRIPT_API_BASE,
getScriptCatalog,
Expand Down Expand Up @@ -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('<script>alert("xss")</script>')
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('&lt;script&gt;')
// 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('<script>')
// The parser already decodes the attribute, so the handler copies it as-is (#595)
expect(html).toContain(
'onclick="navigator.clipboard.writeText(this.dataset.copy).catch(()=>{})"'
)
expect(html).not.toContain('innerHTML')
expect(html).not.toContain('textContent')
})

test('deploy/index.html inlined copy handler never re-parses error text (#595)', () => {
const page = fs.readFileSync(
path.join(__dirname, '..', '..', '..', 'deploy', 'index.html'),
'utf8'
)

expect(page).toContain('navigator.clipboard.writeText(this.dataset.copy)')
expect(page).not.toMatch(/innerHTML\s*=\s*this\.dataset\.copy/)
})

test('handles errors with string representation', () => {
Expand Down
Loading