From 254278bac970ece63151a45d23e9b2da6c7fa1e0 Mon Sep 17 00:00:00 2001
From: Rachel Petry <287457313+rachel-petry@users.noreply.github.com>
Date: Wed, 21 Oct 2026 17:33:25 -0500
Subject: [PATCH] fix(deploy): copy error text verbatim, never re-parse it as
 HTML

The Copy error button set a detached div's innerHTML to data-copy and
copied its textContent. The browser has already decoded the escaped
attribute, so this parsed the raw error text as HTML a second time: it
stripped <...> from copied stack traces and let markup in an error run
(e.g. <img onerror>). Copy this.dataset.copy directly in both
src/deploy/index.js and the inlined copy in deploy/index.html.

Replace the test that asserted the innerHTML handler, guard the inlined
copy against regressing, and add a Playwright test that clicks Copy on a
stack trace containing <script>, <T> and <img onerror> and checks the
clipboard gets the exact string and nothing executes.

Closes #595

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 deploy/index.html              |  4 ++-
 deploy/tests/ui.spec.js        | 45 ++++++++++++++++++++++++++++++++++
 src/deploy/index.js            |  4 ++-
 src/deploy/tests/index.test.js | 26 ++++++++++++++++----
 4 files changed, 72 insertions(+), 7 deletions(-)

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 @@ <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) {
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('<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 }) => {
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) {
     `<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>`
 
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('<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', () => {