Skip to content
This repository was archived by the owner on Aug 6, 2026. It is now read-only.

fix(canvas): securely open external links - #3645

Merged
trunk-io[bot] merged 4 commits into
mainfrom
posthog-code/secure-canvas-external-links
Jul 22, 2026
Merged

fix(canvas): securely open external links#3645
trunk-io[bot] merged 4 commits into
mainfrom
posthog-code/secure-canvas-external-links

Conversation

@k11kirky

@k11kirky k11kirky commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Problem

Links rendered inside freeform canvases cannot open because the iframe intentionally has no popup permission. Granting popup access would unnecessarily broaden the sandbox capability.

Changes

  • Broker external-link requests through the existing host boundary.
  • Allow only absolute https://posthog.com (or *.posthog.com) URLs, enforced in the Zod message schema and re-checked in the handler; opens are rate-limited host-side since canvas code can post without a user gesture.
  • Preserve existing target="_blank" canvas links without enabling iframe popups: the sandbox click broker reads the href attribute (works for SVG anchors, skips relative hrefs), matches _blank case-insensitively, is immune to stopPropagation, and honors a canvas preventDefault().

How did you test this?

  • Unit tests for the URL allowlist, the message schema, the anchor resolver, and the FreeformCanvas message path (allow / reject / throttle)
  • UI, core, and shared typechecks
  • Biome checks

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Created with PostHog Code

Generated-By: PostHog Code
Task-Id: 2734dc0c-3cac-44d9-bc73-c86e58685df1
@trunk-io

trunk-io Bot commented Jul 21, 2026

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit 1e5a48b.

@k11kirky
k11kirky marked this pull request as ready for review July 22, 2026 08:38
},
// External navigation is brokered by the host. The iframe has no popup
// permission; the host validates the scheme before opening anything.
openExternal: (url) => post({ type: "open-external", url }),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium: External launch does not require user activation

A malicious saved or generated canvas can call ph.openExternal() during module evaluation or an effect, causing arbitrary webpages or localhost URLs to open as soon as a viewer loads the canvas. This also runs in dashboard thumbnails, which render canvas code automatically without any click. Require transient user activation before posting the request, or expose this capability only through the trusted anchor-click broker.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 39d785a. The host can't observe gestures inside the null-origin iframe, but a real link click moves focus into it — the open-external handler now drops requests while the canvas iframe is not document.activeElement, so module-eval/effect auto-opens (including offscreen dashboard thumbnails) are ignored. Combined with the PostHog-only https allowlist enforced in the message schema (c1e8a3c) and a per-second throttle. Covered by unit tests on the message path.

@veria-ai

veria-ai Bot commented Jul 22, 2026

Copy link
Copy Markdown

PR overview

This PR updates the canvas sandbox runtime behavior for opening external links from canvas content. It focuses on the ph.openExternal() path used by freeform canvas code.

There is still an open issue where canvas code can trigger external link launches automatically when a canvas or dashboard thumbnail renders, without a user click. That leaves a malicious saved or generated canvas able to open arbitrary webpages or localhost URLs in a viewer’s environment. No issues have been fixed yet, so the PR still needs a user-activation or trusted-click gating change before the security posture is acceptable.

Open issues (1)

Fixed/addressed: 0 · PR risk: 6/10

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Security Review

The scheme validation is applied at both the canvas host handler and service boundary. The web launcher still exposes window.opener to sandbox-selected HTTP(S) pages, which allows reverse tabnabbing.

Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
packages/ui/src/features/canvas/freeform/FreeformCanvas.tsx:188
**Opened Page Retains Host Access**

On the web host, an HTTP(S) URL from the sandbox reaches the existing `window.open` launcher without opener isolation. The opened page can use `window.opener` to navigate the PostHog tab, so this new capability enables reverse tabnabbing unless the browser launcher uses `noopener` or clears the opener.

### Issue 2 of 2
packages/ui/src/features/canvas/freeform/sandboxRuntime.ts:251
**SVG Links Send Non-String URLs**

For an inline SVG `<a target="_blank">`, `closest("a[href]")` can return an `SVGAElement`, whose `href` is an `SVGAnimatedString` rather than a URL string. The message then fails the host's `z.string()` schema after the click has been prevented, leaving a valid external SVG link unresponsive; distinguish HTML and SVG anchors and send the SVG href value explicitly.

Reviews (1): Last reviewed commit: "fix(canvas): broker external links throu..." | Re-trigger Greptile

Comment thread packages/ui/src/features/canvas/freeform/FreeformCanvas.tsx Outdated
Comment thread packages/ui/src/features/canvas/freeform/sandboxRuntime.ts Outdated
k11kirky added 3 commits July 22, 2026 11:00
… broker

Address review findings on the open-external path:

- New isSafePostHogUrl in @posthog/shared: only absolute https posthog.com
  (or subdomain) URLs may leave the canvas sandbox. Enforced in the Zod
  message schema (safe for every consumer by construction) and re-checked
  in the FreeformCanvas handler with the blocked URL logged.
- Rate-limit successful opens host-side: canvas code can post open-external
  without a user gesture, so opens are throttled to one per second.
- Rewrite the sandbox click interceptor as an exported, unit-tested
  resolveExternalAnchorUrl inlined into the bootstrap: reads the href
  attribute (fixes SVG anchors and relative hrefs resolving against the
  host base URL), matches _blank case-insensitively, brokers absolute URLs
  only, listens in capture phase (immune to stopPropagation) and defers
  the open a tick so a canvas preventDefault() is honored.
- Replace source-string test assertions with behavioral tests, and cover
  the message path (allowlist, rejection, throttle) in FreeformCanvas.

Generated-By: PostHog Code
Task-Id: b3fd12d3-df9b-416d-a660-0dc12ad82803
Addresses the veria-ai review finding: canvas code could call
ph.openExternal during module evaluation or an effect, opening URLs as
soon as a viewer loads the canvas (including dashboard thumbnails, which
render without any click).

The host can't observe gestures inside the null-origin iframe, but a real
link click moves focus into it, so the open-external handler now ignores
requests while the canvas iframe is not the focused element. Combined
with the PostHog-only allowlist and the per-second throttle, load-time
auto-opens are dropped.

Generated-By: PostHog Code
Task-Id: b3fd12d3-df9b-416d-a660-0dc12ad82803
Generated-By: PostHog Code
Task-Id: b3fd12d3-df9b-416d-a660-0dc12ad82803
@k11kirky k11kirky added the Stamphog This will request an autostamp by stamphog on small changes label Jul 22, 2026
@stamphog

stamphog Bot commented Jul 22, 2026

Copy link
Copy Markdown

Note

🤖 stamphog reviewed 1e5a48b63bf41ed90970a755c6e65029faf4dc3a — verdict: ESCALATE

This is a security-sensitive sandbox change (granting canvases the ability to trigger external URL opens), and the reviewer bot's substantive "opens without user gesture" concern was only addressed by the author's own reply — no independent reviewer has re-confirmed the fix on the current head, unlike the other two issues which greptile explicitly signed off on.

  • Author wrote 0% of the modified lines and has 18 merged PRs in these paths (familiarity MODERATE).
  • 👍 on the PR from greptile-apps[bot].
  • Security-sensitive change (canvas sandbox is granted a new capability to open external URLs) — this is risky territory under the operating philosophy.
  • veria-ai flagged a Medium concern that canvas code can trigger ph.openExternal() during module eval/effects (including on dashboard thumbnails, which render automatically without any click), bypassing the intended user-gesture requirement.
  • The author's reply (commit 39d785a) mitigates this by gating opens on document.activeElement === iframeRef.current, but that review thread is still unresolved and neither veria-ai nor greptile has re-reviewed the fix commit — the only review activity on the current head is the author's own comment, which is not independent assurance.
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 100L, 6F substantive, 331L/10F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1d-complex (331L, 10F, single-area, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ f1563f9 · reviewed head 1e5a48b

@stamphog stamphog Bot removed the Stamphog This will request an autostamp by stamphog on small changes label Jul 22, 2026
@k11kirky

Copy link
Copy Markdown
Contributor Author

/truck merge

@k11kirky

Copy link
Copy Markdown
Contributor Author

/trunk merge

@trunk-io
trunk-io Bot merged commit d2afabe into main Jul 22, 2026
40 checks passed
@trunk-io
trunk-io Bot deleted the posthog-code/secure-canvas-external-links branch July 22, 2026 10:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants