Skip to content

Normalize non-breaking spaces when copying to clipboard#87

Merged
jonrohan merged 2 commits into
mainfrom
copilot/fix-web-interface-copy-issue
Jun 15, 2026
Merged

Normalize non-breaking spaces when copying to clipboard#87
jonrohan merged 2 commits into
mainfrom
copilot/fix-web-interface-copy-issue

Conversation

Copilot AI commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Closes https://github.com/github/primer/issues/1962

Non-printable characters like the non-breaking space (U+00A0) render identically to a regular space but are copied verbatim to the clipboard. This lets an attacker hide command fragments inside otherwise innocuous-looking text (e.g. a markdown code block or blob), so that pasting a copied command into a terminal executes something the victim never saw (HackerOne #1805414).

// Rendered as: wget -O - https://example.com/hello.sh | bash
// Previously copied as: "...hello.sh\u00A0| bash"  → resolves a different, attacker-controlled path
// Now copied as:        "...hello.sh | bash"

Changes

  • src/clipboard.ts
    • Added normalizeText() to replace U+00A0 with a regular ASCII space so copied text matches what is rendered.
    • Routed all copy paths through normalization: copyText() normalizes before writing, and copyNode() now delegates to copyText().
    • Extracted the legacy execCommand selection logic into copySelection(), operating on a normalized node so the non-navigator.clipboard fallback is covered too.
  • test/test.js — added cases asserting normalization for both element-content (for) and value-attribute copy paths.

GitHub Advanced Security started work on behalf of liuliu-dev June 15, 2026 16:52 View session
Copilot AI changed the title [WIP] Fix web interface copy button vulnerability Normalize non-breaking spaces when copying to clipboard Jun 15, 2026
GitHub Advanced Security finished work on behalf of liuliu-dev June 15, 2026 16:53
Copilot AI requested a review from liuliu-dev June 15, 2026 16:53
@jonrohan jonrohan marked this pull request as ready for review June 15, 2026 16:53
@jonrohan jonrohan requested a review from a team as a code owner June 15, 2026 16:53
Copilot AI review requested due to automatic review settings June 15, 2026 16:53
@maclarel

Copy link
Copy Markdown

@liuliu-dev this probably shouldn't be in a public repo. Looks like the entire bounty report got included in this.

@jonrohan jonrohan self-assigned this Jun 15, 2026
@jonrohan jonrohan merged commit af7f06f into main Jun 15, 2026
6 checks passed
@jonrohan jonrohan deleted the copilot/fix-web-interface-copy-issue branch June 15, 2026 16:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR mitigates a clipboard spoofing risk by normalizing non‑breaking spaces (U+00A0) to regular ASCII spaces so copied text matches what users see, covering both modern navigator.clipboard and the legacy execCommand fallback paths.

Changes:

  • Added normalizeText() and applied it to all copy paths.
  • Refactored legacy selection/execCommand logic into copySelection() and routed copyNode() through copyText().
  • Added tests asserting U+00A0 normalization for both for-target content and the value attribute path.
Show a summary per file
File Description
src/clipboard.ts Adds normalization and refactors copy flow to ensure normalization is applied across clipboard and fallback paths.
test/test.js Adds normalization assertions for element-content and value-attribute copy paths.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 3

Comment thread src/clipboard.ts
Comment on lines 33 to 35
document.execCommand('copy')
selection.removeAllRanges()
return Promise.resolve()
Comment thread src/clipboard.ts
Comment on lines +54 to 58
const node = createNode(normalized)
body.appendChild(node)
copyNode(node)
copySelection(node)
body.removeChild(node)
return Promise.resolve()
Comment thread test/test.js
Comment on lines +157 to +180
it('normalizes non-breaking spaces to regular spaces', async function () {
const target = document.createElement('div')
target.textContent = 'wget -O - https://example.com/hello.sh\u00A0| bash'
target.id = 'copy-target'
document.body.append(target)

const button = document.querySelector('clipboard-copy')
button.click()

const text = await whenCopied
assert.equal(text, 'wget -O - https://example.com/hello.sh | bash')
assert.notInclude(text, '\u00A0')
})

it('normalizes non-breaking spaces from the value attribute', async function () {
const button = document.querySelector('clipboard-copy')
button.setAttribute('value', 'hello.sh\u00A0| bash')

button.click()

const text = await whenCopied
assert.equal(text, 'hello.sh | bash')
assert.notInclude(text, '\u00A0')
})
@jonrohan

Copy link
Copy Markdown
Member

@maclarel Thanks for the heads up, sorry about the ping 🙇🏻 How have you been?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants