[don-petry]

Closes #595

What

The deploy page's 📋 Copy error button set a detached div's innerHTML to this.dataset.copy and copied its textContent. The browser has already decoded the entity-escaped data-copy attribute, so this parsed the raw error text as HTML a second time:

• copied stack traces lost anything tag-like (<anonymous>, List<T>, …)
• markup in an error ran on click (a detached <img src=x onerror=…> still loads and fires)

The handler is now navigator.clipboard.writeText(this.dataset.copy). data-copy stays HTML-escaped, which is correct for an attribute.

deploy/index.html is a hand-kept mirror of src/deploy/index.js, not generated from it, so both copies change.

Tests

Acceptance criterion	Covered by
1. Both files use the direct handler; neither contains d.innerHTML=this.dataset.copy	Diff; plus a new Jest test that reads deploy/index.html and fails if the inlined copy regresses
2. Replace the test asserting the buggy handler	src/deploy/tests/index.test.js: asserts the direct handler, no innerHTML/textContent, and that data-copy still holds <script>
3. DOM-level test: exact string reaches the clipboard, window.__pwned stays undefined	deploy/tests/ui.spec.js (Playwright, real Chromium, against the shipped deploy/index.html). Renders a stack with <tag>, <anonymous>, List<T>, <script> and <img onerror>, clicks Copy with a stubbed clipboard, and checks the exact string. A control <img onerror> in a detached div proves the payload would fire in this browser, then asserts __pwned is still undefined. Fails against main.
4. Existing deploy UI tests pass	Local: Jest 504/504, Playwright 98/98

Playwright stands in for the jsdom test the issue suggests: the repo has no jest-environment-jsdom, and Playwright exercises the inlined page that actually ships.

Local checks: npm run check (prettier + eslint: 0 errors, 22 pre-existing warnings outside these files), npm run typecheck, npm test -- --coverage, npx playwright test.

No .github/workflows/ changes (#480).

🤖 Generated with Claude Code

Problem

See the linked issue.

Risk

Low — 4 file(s) changed; no CI- or runtime-critical paths touched.

Test plan

Tests added/updated: deploy/tests/ui.spec.js,src/deploy/tests/index.test.js. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.

Rollback

Revert this PR. No non-revertible side effects (no tags, migrations, or external state).

Monitoring

This PR's CI checks (Lint + bats) are the success/regression signal.

Summary by CodeRabbit

• Bug Fixes

  • Deployment error details now copy the exact original text, including HTML-like and script content, without interpreting it as HTML.
  • Copying deployment error details works consistently in both the deployed page and the application interface, preserving the displayed stack trace content.

• Tests

  • Added regression coverage verifying that stack traces are preserved when copied, including content that resembles HTML or scripts.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[don-petry]

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #643
No changes were committed, but the PR still can't be marked done: required checks SonarCloud, agent-shield / AgentShield, build-and-test, coverage are still pending. The retry cron will re-attempt automatically. Next attempt after: 2026-10-21T23:05:11Z

[don-petry]

Note

@don-petry I reviewed this PR and no code changes were needed, but I can't mark it done yet: required checks SonarCloud, agent-shield / AgentShield, build-and-test, coverage are still pending. I'll re-check automatically.
Next attempt after: 2026-10-21T23:05:11Z

[don-petry]

QA Lead — test-risk advisory

Risk tier: LOW. This PR shrinks risk: it removes an HTML re-parse that let markup in error text run. It is guarded at two levels: a Jest check on the HTML string, and a Playwright test in real Chromium that includes a control and is reported to fail on main. Both pass in CI.

What I'd shore up (highest leverage first):

• Mirror drift is the lasting risk. deploy/index.html is a hand-kept copy of src/deploy/index.js. The new Jest test only checks that the handler string is present. A parity test that pulls buildDeployErrorHtml out of both files and compares them (or renders both against the same error) would catch the next fix that lands in only one copy.
• Quote escaping is only tested indirectly. The new Playwright test fails if " isn't escaped, because the attribute would break. The unit test asserts <script> but not ". Add an explicit " assertion so the cheap test fails first.
• Timing in the XSS check. The test checks that the payload didn't run once the control <img> fires, and it assumes the payload's onerror would have fired first. That holds today, but a fixed order of async image errors isn't guaranteed. Add a small extra wait (a single task tick) after the control fires so the check can't pass by luck and slowly turn flaky.
• (Optional) There is no negative-path test for a rejected clipboard.writeText. Failures are silently swallowed, so add a one-liner only if user feedback is ever wanted.

Escalate? no. The defect is fixed and has regression coverage; these are hardening items.

Advisory only — I comment, I do not change code. Ground: BMAD Test Architecture
(bmad-tea v1.19.0). Opt out on this item with the qa-lead:hands-off label.

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-21T23:42:01Z.

[don-petry]

No description provided.

[don-petry]

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

[donpetry-bot]

No description provided.

[don-petry]

No actionable findings — the bot's review failed to complete due to a transient issue, and no specific code defects were reported. All CI checks (Node.js Tests, SonarCloud, CodeQL, Playwright, etc.) have passed. No code changes are needed.

[don-petry]

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning

Issues addressed: 0 (non-actionable notice)
- Code review failed to complete: operational notice, no specific findings reported [issue comment: disposition=informational]
Files changed: none
Skipped (informational): 1
```
The bot's comment reported a transient failure with no code findings. All CI checks pass. The notice has been dispositioned as `informational` since it requires no code changes.

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-22T01:23:16Z.

[don-petry]

No description provided.

[don-petry]

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning

**Disposition:** Posted `informational` disposition on the CodeRabbit summary comment (https://github.com/combsmith/google-app-scripts/pull/643#issuecomment-6333048346)
**Reason:** CodeRabbit explicitly reported "No actionable comments were generated in the recent review." The entire summary is a clean status report:
- ✓ All 5 pre-merge checks passed
- ✓ Merge risk: Minimal
- ✓ No Security Architecture Review findings
- ✓ No rate-limit or throttle notices
- ✓ No specific defects or issues tied to files/lines
This qualifies as an **informational** disposition since every finding-bearing section is empty or reports success.
**CI Status:** All checks passing (success or skipped)
**Result:** PR is ready for approval — no code changes needed, no Tier 1 blockers, bot comment now disposed.

[don-petry]

No description provided.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-22T06:15:58Z.

[don-petry]

Dev-Lead — fix-reviews (no-changes)

Agent reasoning

Addressed 0 threads. The PR had no open review threads, no `CHANGES_REQUESTED` reviews, and no failing CI checks. CodeRabbit was still in progress when the checks were listed.
- **Stale disposition:** I re-read CodeRabbit's edited summary comment `IC_kwDORFz9wc8AAAABeS43Sg`. It has no findings: no actionable comments, nitpicks, outside-diff items or architecture-review section. I posted a fresh `informational` disposition on the PR for it.
- **Tests:** I didn't run them, because I made no code changes.
- **Files changed:** none, and I made no commits.

[donpetry-bot]

No description provided.

[don-petry]

CodeRabbit's automated review and pre-merge checks all passed — no actionable findings or specific code issues identified. The comment summarizes the deployed error-copy fix across both the source handler and the shipped UI, with all validation passing.

[don-petry]

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning

**Bot:** coderabbitai[bot]  
**Issues addressed:** 0 (no actionable findings)  
**Comment disposition:** informational
**Actions taken:**
- Posted fresh disposition reply for stale comment `IC_kwDORFz9wc8AAAABeS43Sg`
  - The CodeRabbit summary reported no actionable code findings; all pre-merge checks passed
  - Disposition: **informational** (neutral overview with no specific code issues to address)
  - Reply posted at: 2026-10-22T09:54:37Z, after the comment's last edit
**Files changed:** None (no code changes required)
**Status:** The stale disposition has been cleared with a fresh reply. The harness will verify and minimize the bot's comment RESOLVED.

[donpetry-bot]

Automated review — APPROVED ✓

Risk: LOW
Reviewed commit: d5a91756ad42e6f11fedf9d554c5048c189f2743
Review mode: triage-approved (single reviewer)

Summary

Removes the HTML re-parse in the deploy page's Copy error button. The handler now writes this.dataset.copy straight to the clipboard, in both src/deploy/index.js and its hand-kept mirror deploy/index.html. Jest and Playwright regression tests cover it. Small, contained change that removes an injection sink. Triage's low-risk call is confirmed.

Linked issue analysis

Closes #595. All four acceptance criteria are met:

1. Both files now use navigator.clipboard.writeText(this.dataset.copy), and neither contains d.innerHTML=this.dataset.copy.
2. The old test that asserted the buggy handler is replaced. The new test asserts the direct handler, no innerHTML/textContent, and that data-copy still holds <script>. A second Jest test reads deploy/index.html so the mirror can't silently regress.
3. The DOM-level test is done in Playwright (real Chromium, against the shipped page) instead of jsdom; the PR explains why (no jsdom environment in the repo). It checks that the exact original string reaches the stubbed clipboard and that window.__pwned stays undefined, with a control <img onerror> showing the payload would fire.
4. Existing tests pass in CI (Node.js Tests, Playwright UI Tests, build-and-test, coverage).

Findings

No blocking findings.

• Correctness: escapeHtml escapes & < > " ', so data-copy is safe as an attribute. The parser decodes it once, so dataset.copy is the original text. Dropping the second parse fixes both the stripped stack-trace content and the onerror sink.
• The // comments sit between +-joined template literals, which is valid JS and doesn't change the output string.
• Non-blocking hardening (already raised in the QA-lead advisory): an explicit " assertion in the unit test, a parity test between the two mirrored copies of buildDeployErrorHtml, and an extra tick after the control fires in the XSS check to rule out ordering flakiness.
• No unresolved review threads and no human change requests. Codex's review failed for operational reasons with no findings, and CodeRabbit had no actionable comments.
• MCP secret scan: run_secret_scanning is not available in this run. The gitleaks check passed.

CI status

All required checks are green: build-and-test, coverage, Node.js Tests, Playwright UI Tests, CodeQL (actions, javascript-typescript, python), SonarCloud (quality gate passed), Secret scan (gitleaks), npm audit, and AgentShield. The rest were skipped because they don't apply.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.

[donpetry-bot]

pr-review approved on PARTIAL advisory evidence: 1/5 required advisory bots reported before the gate's head-age-timeout fallback proceeded. Recorded for the miss-rate metric (#1596).