Skip to content

[BUGFIX] AlertTable: only link runbook annotations that are http(s) URLs - #829

Merged
jgbernalp merged 3 commits into
perses:mainfrom
Yash121l:fix/alert-table-unsafe-runbook-url
Oct 1, 2026
Merged

jgbernalp merged 3 commits into
perses:mainfrom
Yash121l:fix/alert-table-unsafe-runbook-url

Conversation

@Yash121l

@Yash121l Yash121l commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes perses/perses#4473.

The Alert Table put the runbook_url annotation straight into the link's href, so any value got a "View runbook" button: javascript:, data:, vbscript: and file: included. The panel runs React 18, which only warns about javascript: URLs rather than blocking them.

Now the button only appears when the annotation resolves to an http: or https: URL. Relative URLs resolve against the current page, so /runbooks/alert still links, which matters when panels are embedded. Anything else gets no button.

AlertTablePanel.test.tsx renders the panel with each annotation from the issue. On main every one of them gets a link. With this change javascript:, data:, file:, vbscript: and mailto: get none, while absolute, relative and protocol-relative http(s) URLs still link.

Screenshots

No visual change for valid runbook URLs.

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.

Signed-off-by: Yash Lunawat <yash.lunawat@rtp.vc>
@Yash121l
Yash121l requested a review from a team as a code owner September 23, 2026 07:08
@Yash121l
Yash121l requested review from jgbernalp and removed request for a team September 23, 2026 07:08
'//evil.example.com/malicious',
'file:///etc/passwd',
' javascript:alert(1)',
'/runbooks/alert',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

relative urls should be allowed as they might redirect to safe internal URLS when perses panels are used in embedded mode.

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.

Done in d9f4228: the annotation now resolves against the current page and links if the result is http(s), so /runbooks/alert and runbooks/alert link again. One side effect: //host/path also links, since it resolves to https://host/path, the same as an absolute https URL. javascript:, data:, file:, vbscript: and mailto: are still refused.

@jgbernalp

Copy link
Copy Markdown
Contributor

@Yash121l mind checking the last comment?, it seems that is the only thing holding this PR

@Yash121l

Copy link
Copy Markdown
Contributor Author

Hey, I'll check this tonight!

Signed-off-by: Yash Lunawat <yash.lunawat@rtp.vc>
Signed-off-by: Yash Lunawat <yash.lunawat@rtp.vc>
@Yash121l

Yash121l commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

@jgbernalp can you have a look on this now

@jgbernalp
jgbernalp enabled auto-merge October 1, 2026 07:56
@jgbernalp
jgbernalp added this pull request to the merge queue Oct 1, 2026
Merged via the queue into perses:main with commit a866428 Oct 1, 2026
17 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.

[Alert Table] - Annotation values should be sanitized

2 participants