fix: validate identity.url scheme before rendering as external link#644
fix: validate identity.url scheme before rendering as external link#644
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Addresses a security issue where an ENS-supplied identity.url could be rendered directly into an external link without validation.
Changes:
- Added
sanitizeExternalUrlutility to validate/normalize user-supplied URLs to safehttp(s)absolute URLs. - Updated the profile page to use the sanitized URL (or skip rendering the link when invalid).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
lib/utils.tsx |
Adds sanitizeExternalUrl for scheme normalization and protocol validation. |
components/Profile/index.tsx |
Uses sanitizeExternalUrl(identity?.url) before rendering an external website link. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| title={safeIdentityUrl} | ||
| target="__blank" | ||
| rel="noopener noreferrer" |
There was a problem hiding this comment.
target should be _blank (single underscore) to open a new tab/window. Using __blank doesn’t match the standard browsing context name and may not behave as intended.
| const withScheme = /^[a-zA-Z][a-zA-Z0-9+.-]*:/.test(trimmed) | ||
| ? trimmed | ||
| : `https://${trimmed}`; | ||
| try { | ||
| const parsed = new URL(withScheme); | ||
| if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { |
There was a problem hiding this comment.
sanitizeExternalUrl currently prepends https:// for any input without a detected scheme, but this produces incorrect results for protocol-relative URLs like //example.com/path (it becomes https:////example.com/path, which parses with an empty host and doesn’t reliably become an external link). Handle the //... case explicitly (e.g., prefix with https:) and consider rejecting other relative-leading inputs (like /..., #..., ?...) or requiring parsed.hostname to be non-empty to ensure the result is truly an absolute external URL.
| const withScheme = /^[a-zA-Z][a-zA-Z0-9+.-]*:/.test(trimmed) | |
| ? trimmed | |
| : `https://${trimmed}`; | |
| try { | |
| const parsed = new URL(withScheme); | |
| if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { | |
| if (/^[/?#]/.test(trimmed) && !trimmed.startsWith("//")) return null; | |
| const withScheme = trimmed.startsWith("//") | |
| ? `https:${trimmed}` | |
| : /^[a-zA-Z][a-zA-Z0-9+.-]*:/.test(trimmed) | |
| ? trimmed | |
| : `https://${trimmed}`; | |
| try { | |
| const parsed = new URL(withScheme); | |
| if ( | |
| (parsed.protocol !== "http:" && parsed.protocol !== "https:") || | |
| !parsed.hostname | |
| ) { |
| export const sanitizeExternalUrl = ( | ||
| url: string | null | undefined | ||
| ): string | null => { | ||
| if (!url) return null; | ||
| const trimmed = url.trim(); | ||
| if (!trimmed) return null; | ||
| const withScheme = /^[a-zA-Z][a-zA-Z0-9+.-]*:/.test(trimmed) | ||
| ? trimmed | ||
| : `https://${trimmed}`; | ||
| try { | ||
| const parsed = new URL(withScheme); | ||
| if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { | ||
| return null; | ||
| } | ||
| return parsed.toString(); | ||
| } catch { | ||
| return null; | ||
| } | ||
| }; |
There was a problem hiding this comment.
There are existing Jest unit tests for lib/utils (see lib/utils.test.ts), but the new sanitizeExternalUrl behavior isn’t covered. Add focused tests for allowed inputs (http/https + schemeless hostnames that get https:// prefixed) and rejected inputs (javascript:, data:, malformed URLs, etc.) to prevent regressions.
Orchestrator profile URLs come from ENS text records — a user-controlled field. As rendered, values like `javascript:alert(...)`, `data:...`, or schemeless `evil.com/path` (treated as a relative URL) would all become clickable from the profile page. React 16+ warns on `javascript:` hrefs but doesn't reliably block them. Add `sanitizeExternalUrl` in `lib/utils.tsx` that auto-prefixes `https://` for schemeless input, parses via `new URL(...)`, and rejects anything outside `http:` / `https:`. Use the sanitized value for both `href` and the displayed text; suppress the entire link block when sanitization fails. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rickstaa
force-pushed
the
fix/profile-url-href-validation
branch
from
3b84d27 to
249bc08
Compare
Closes #643
Summary
Sanitizes the ENS-supplied
identity.urlvalue before rendering it as an external link in the orchestrator profile header. AddssanitizeExternalUrltolib/utils.tsxand uses it for bothhrefand the displayed text.Why
identity.urlis set by the orchestrator via their ENSurltext record — fully user-controlled. Without validation, values likejavascript:alert(...),data:text/html,..., or schemelessevil.com/path(which resolves as a relative link to explorer.livepeer.org) are rendered verbatim into the DOM. React 16+ warns onjavascript:hrefs but does not reliably block them.What changed
sanitizeExternalUrl(url)helper tolib/utils.tsx. It auto-prefixeshttps://for schemeless input, parses vianew URL(...), returns the canonical form on success, ornullfor anything that fails to parse or uses a non-http(s) protocol.components/Profile/index.tsx, computesafeIdentityUrlonce at the top of the component and use it for the link'shref,title, and the visible text. Suppress the entire link block when sanitization fails.https://twitter.com//https://github.com/URLs and are not the same risk.Test plan
pnpm typecheckpasses (via pre-commit hook).pnpm lintandprettier --checkpass.http(s)URL — link still works as before.javascript:URL doesn't render the link block at all.🤖 Generated with Claude Code