Skip to content

fix(viewer): validate the openLink scheme host-side#120

Merged
benvinegar merged 2 commits into
mainfrom
fix/openlink-scheme-validation
Jun 24, 2026
Merged

fix(viewer): validate the openLink scheme host-side#120
benvinegar merged 2 commits into
mainfrom
fix/openlink-scheme-validation

Conversation

@benvinegar

Copy link
Copy Markdown
Member

What & why

openLink reaches the host's window.open. The in-frame click handler only forwards http(s) hrefs:

if (a && /^https?:/.test(a.href)) { e.preventDefault(); window.openLink(a.href); }

…but window.openLink(url) is a global a surface can call directly with any string, and it can post the open-link bridge message raw — both bypassing that filter. The host then opened it after a confirm without re-checking the scheme:

} else if (d.type === "open-link" && isOwnFrame(ev.source)) {
  if (confirm(`Open external link?\n\n${d.url}`)) window.open(d.url, "_blank", "noopener");
}

So javascript:, data:, file: URLs were reachable.

Severity: low. "noopener" opens these in an isolated browsing context that can't reach back into the board (no opener, opaque origin), so they can't read surfaces or act as the user — the genuine residual was confirm-gated phishing. This is contract-consistency / defense-in-depth: there's no reason to open a non-link at all.

Fix

Re-validate the scheme host-side, where it can't be bypassed (one line, mirrors the in-frame filter):

const url = String(d.url);
if (!/^https?:\/\//i.test(url)) return;
if (confirm(`Open external link?\n\n${url}`)) window.open(url, "_blank", "noopener");

Tests

e2e/isolation.spec.ts adds two tests against a surface that auto-fires the raw bridge message (the actual bypass an attacker would use):

  • a javascript: URL raises no confirm dialog (blocked before the prompt);

  • an https:// URL still prompts (the dialog text contains the URL).

  • npm run typecheck, npm run lint, npm run format:check, npm test (205/205) ✅

  • npx playwright test isolation --project=chromium — 5/5 ✅ (WebKit can't launch in the dev env; CI runs it)

Not included (deliberately)

The sibling copy bridge message (navigator.clipboard.writeText) is not changed here. It backs the code part's Copy button (viewer/src/CodePart.tsx), so it can't simply be removed, and it's partly browser-mitigated (clipboard writes need focus/activation). Hardening it cleanly hits the same can't-distinguish-a-cross-frame-gesture wall as sendPrompt, so it's left as a known low-severity residual rather than degraded here.

🤖 Generated with Claude Code

A surface can call openLink() directly, or post the open-link bridge message
raw, with any scheme — the in-frame click handler's http(s) filter only covers
plain <a href> clicks. The host then opened it after a confirm without
re-checking, so javascript:/data:/file: URLs were reachable. noopener already
kept those from touching the board, but the host now refuses anything that isn't
http(s):// outright, matching the documented "external link" contract. Adds e2e
coverage for both edges (a non-http(s) scheme raises no prompt; an http(s) url
still prompts) against a surface that auto-fires the raw message.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread viewer/src/App.tsx Fixed
…s scheme

Address review feedback: rather than regex-checking the raw string and then
handing that same raw string to window.open (validate one representation, act
on another — a parser-differential smell), parse it once with the URL
constructor, validate `protocol`, and open the normalized `href` from the same
parse. What we check and what opens are now byte-identical, and a malformed
string is rejected outright. The negative test now also covers data: and an
unparseable string.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@benvinegar benvinegar merged commit 58c515f into main Jun 24, 2026
9 checks passed
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.

2 participants