fix(copilot): validate credential-link URL scheme before rendering#5416
Conversation
Only render the credential connect link as a clickable anchor when its value resolves to an http(s) URL, reusing the isSafeHttpUrl helper already used for chat file links.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview
Reviewed by Cursor Bugbot for commit fb774d8. Configure here. |
Greptile SummaryThis PR closes an XSS vector in the copilot credential-connect flow: the
Confidence Score: 5/5Safe to merge — the change is a targeted security guard with no impact on existing functionality beyond blocking malicious URL schemes in a single render path. The PR adds a well-scoped URL-scheme check before rendering a credential connect link from streamed model output. The isSafeHttpUrl helper is correctly placed in the shared utility module, its behaviour is unchanged from the previous location, both consumers import it from one canonical source, and the test suite covers the dangerous-scheme and happy-path cases across both features. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Model as Streamed Model Output
participant ST as SpecialTags (client)
participant CD as CredentialDisplay
participant Safe as isSafeHttpUrl (urls.ts)
participant Browser as Browser
Model->>ST: "segment { type: 'credential', data.value }"
ST->>CD: render CredentialDisplay
CD->>CD: "check canEdit & data.provider"
CD->>Safe: isSafeHttpUrl(data.value)
alt URL is http(s)
Safe-->>CD: true
CD->>Browser: "render <a href={data.value}>"
else URL is javascript: / data: / other
Safe-->>CD: false
CD-->>ST: return null (no anchor rendered)
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Model as Streamed Model Output
participant ST as SpecialTags (client)
participant CD as CredentialDisplay
participant Safe as isSafeHttpUrl (urls.ts)
participant Browser as Browser
Model->>ST: "segment { type: 'credential', data.value }"
ST->>CD: render CredentialDisplay
CD->>CD: "check canEdit & data.provider"
CD->>Safe: isSafeHttpUrl(data.value)
alt URL is http(s)
Safe-->>CD: true
CD->>Browser: "render <a href={data.value}>"
else URL is javascript: / data: / other
Safe-->>CD: false
CD-->>ST: return null (no anchor rendered)
end
Reviews (2): Last reviewed commit: "refactor(copilot): move isSafeHttpUrl to..." | Re-trigger Greptile |
Per Greptile's convention feedback: isSafeHttpUrl was consumed by both chat and workspace/home but defined inside a feature-specific 'use client' component. Move it alongside getBrowserOrigin (which it already depends on) in lib/core/utils/urls.ts, matching this repo's shared-utility rule.
|
@greptile review |
|
@cursor review |
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 fb774d8. Configure here.
Summary
isSafeHttpUrlhelper already used for chat file links.Type of Change
Testing
special-tags.test.tsxcovering non-http(s) scheme values (rendered as no anchor) and a normal https connect URL (renders correctly).bun run check:api-validationpasses.Checklist