fix: extract only origin

[jackkav]

In order to prevent an origin containing js being rendered, we parse it as a url and extract the origin which should prevent strings containing js being admitted.

[Copilot]

Pull request overview

This pull request implements a security fix to prevent malicious JavaScript from being rendered through the origin parameter in deep-link import functionality. The change sanitizes the origin field by parsing URLs and extracting only their origin component, replacing the previous approach of directly using the params.origin query parameter.

Changes:

• Added sanitizeUrlAndExtractOrigin function that parses URLs and extracts their origin, returning empty string on parse failure
• Modified URI import to derive origin from params.uri instead of params.origin
• Modified cURL import to derive origin from params.curl instead of params.origin

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Applying sanitizeUrlAndExtractOrigin to params.curl is incorrect. A cURL command is a command string (e.g., "curl --request GET --url http://insomnia.rest/"), not a URL. Parsing it as a URL will always fail and return an empty string, which defeats the purpose of displaying the origin to the user.

The origin should either be extracted from within the cURL command (by parsing the URL from the command), or obtained from params.origin if it was originally passed as a separate parameter. Consider using a cURL parser or regex to extract the URL from the command string, then extract its origin.

[Copilot]

The security fix changes from using params.origin (a separate query parameter that could contain malicious content) to extracting the origin from params.uri. However, this changes the semantics of the feature. If the original design was to allow specifying a separate origin parameter (e.g., to display where the import came from even if it differs from the URI itself), this change breaks that functionality.

If the intent is purely security-focused and the origin should always match the URI, then this approach is correct for the URI case. Consider documenting this design decision or verifying with the original implementers whether a separate origin parameter was intentionally supported.

[Copilot]

The sanitizeUrlAndExtractOrigin function lacks test coverage. Given that this function is introduced as a security fix to prevent malicious JavaScript from being rendered, it's critical to have tests that verify:

1. Valid URLs return their origins correctly
2. Invalid URLs return empty strings
3. Malicious inputs like javascript:alert(1) are handled safely
4. Edge cases like data URLs, file URLs, and URLs with unusual schemes are handled appropriately

Consider adding unit tests for this function.

[Copilot]

The sanitizeUrlAndExtractOrigin function is defined inside the useEffect callback, which makes it harder to test and reuse. Consider moving this function outside the component or to a utility module, especially since it's a security-critical function that would benefit from being easily testable and potentially reusable in other parts of the codebase.

[Copilot]

The function returns an empty string on error, but for certain URL schemes (like javascript:, data:, blob:, file:), the URL constructor succeeds but returns "null" as the origin. This means malicious URLs like javascript:alert(1) will result in the string "null" being set as the origin, which is then displayed to the user and could be passed to window.main.openInBrowser.

While "null" is safer than the original malicious URL, consider explicitly checking for and rejecting dangerous URL schemes (javascript:, data:, file:) or only allowing specific safe schemes (http:, https:). This would make the security intent clearer and prevent confusing user-facing messages.