Skip to content

[BUGFIX] Canvas: sanitize node URLs before opening them - #925

Open
Nexucis wants to merge 1 commit into
mainfrom
nexucis/canvas-sanitize-url
Open

Nexucis wants to merge 1 commit into
mainfrom
nexucis/canvas-sanitize-url

Conversation

@Nexucis

@Nexucis Nexucis commented Nov 10, 2026

Copy link
Copy Markdown
Member

Description

Follow-up of #910 / #912, which restricted the trace panel links to relative URLs and the allowed schemes: the Canvas panel has the same kind of link sink, which wasn't covered.

When a node with a url is clicked, PanelNodeLayer interpolates the dashboard variables into the URL and passes the result to window.open() without checking its scheme. A javascript: or data: URL, written in the panel or provided by a variable, was therefore opened.

This isn't exploitable as such: the window is opened with noopener, so the URL doesn't run in the Perses origin. I checked it with Chrome 157, Firefox 142 and WebKit 26: Chrome and Firefox don't run it, WebKit runs it in an opaque (null) origin without access to the Perses cookies. But the panel shouldn't depend on that.

The URL is now verified with sanitizeUrl from @perses-dev/components once the variables are interpolated, like the other link sinks. A node whose URL uses a disallowed scheme doesn't open anything.

Tests: the new PanelNodeLayer.test.tsx covers an interpolated URL, a relative URL, javascript: (also obfuscated with mixed case, a leading space and a tab), data:, and a scheme provided by a variable. Without the fix, the 4 unsafe cases fail.

  • npm run lint -w canvas: no new warning (the only one is the existing src/setup-tests.ts warning)
  • npm run type-check -w canvas: passes
  • npm run test -w canvas: 17 files, 189 tests pass
  • npm run format:check: passes

This targets main; it can be cherry-picked to release/v0.56 if we want it in the v0.56 plugins.

Screenshots

N/A: a node with a safe URL behaves as before.

Checklist

  • Pull request has a descriptive title and context useful to a reviewer.
  • Pull request title follows the [<catalog_entry>] <commit message> naming convention using one of the
    following catalog_entry values: FEATURE, ENHANCEMENT, BUGFIX, BREAKINGCHANGE, DOC,IGNORE.
  • All commits have DCO signoffs.

UI Changes

  • Changes that impact the UI include screenshots and/or screencasts of the relevant changes.
  • Code follows the UI guidelines.

The URL of a Canvas node was interpolated and passed to window.open()
without the verification done for the other links of the plugins
(sanitizeUrl), so a javascript: or data: URL, written in the panel or
provided by a variable, was opened. As the window is opened with noopener,
such a URL doesn't run in the Perses origin, but the panel shouldn't rely
on it.

The URL is now verified with sanitizeUrl once the variables are interpolated,
and a node whose URL uses a disallowed scheme doesn't open anything.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Augustin Husson <augustin.husson@amadeus.com>
@Nexucis
Nexucis requested a review from a team as a code owner November 10, 2026 11:22
@Nexucis
Nexucis requested review from Gladorme and removed request for a team November 10, 2026 11:22
@github-actions

Copy link
Copy Markdown

React Doctor found no new issues. 🎉

Reviewed by React Doctor for commit c751f4d.

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.

3 participants