Skip to content

fix(desktop): allow Obsidian deep links in openExternal - #84593

Open
thefreenickmurray wants to merge 1 commit into
NousResearch:mainfrom
thefreenickmurray:fix/desktop-allow-obsidian-external-links
Open

fix(desktop): allow Obsidian deep links in openExternal#84593
thefreenickmurray wants to merge 1 commit into
NousResearch:mainfrom
thefreenickmurray:fix/desktop-allow-obsidian-external-links

Conversation

@thefreenickmurray

Copy link
Copy Markdown

Summary

  • Desktop chat rejected obsidian://… markdown links with Invalid external URL because openExternalUrl only allowed http: / https: / mailto:.
  • Extract an explicit shell-open scheme allowlist (apps/desktop/electron/external-url.ts) and add obsidian: so vault deep links (e.g. obsidian://open?vault=…&file=…) launch the local Obsidian app when the OS has the protocol handler registered.
  • Dangerous / unlisted schemes (javascript:, data:, file: via this path, IDE schemes not requested, etc.) remain denied. file: still goes through the existing shell.openPath path-hardening branch.

Test plan

  • cd apps/desktop && npx vitest run electron/external-url.test.ts --project electron (5 tests pass)
  • In Desktop chat, click a markdown link like [note](obsidian://open?vault=<vault-id>&file=Path%2FNote.md) with Obsidian installed → opens the note
  • Click https://example.com → still opens in the browser
  • Click javascript:alert(1) / bare unlisted custom scheme → still rejected (no OS handoff)

Chat markdown links like obsidian://open?vault=…&file=… were rejected
with "Invalid external URL" because openExternalUrl only allowed
http/https/mailto. Extract an explicit shell-open scheme allowlist and
add obsidian: so vault deep links launch the local Obsidian app when
registered with the OS. Dangerous schemes stay denied.
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) needs-decision Awaiting maintainer decision before any implementation labels Aug 12, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #75457 proposes a broader local-file and Obsidian response-link policy. This PR is the narrow shell.openExternal allowlist path only; maintainers should choose or consolidate the policy before merging.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(desktop): allow Obsidian deep links in openExternal

  1. apps/desktop/electron/main.ts:1354 — the URL is parsed once for earlier checks, then parsed.toString() is re-parsed inside isAllowedShellOpenExternalUrl, and the helper only consults parsed.protocol. Passing the already-parsed parsed.protocol (or accepting a URL object) would avoid the serialize→re-parse round-trip and keep the two parse sites from drifting.
  2. apps/desktop/electron/external-url.ts:75-80 — the allowlist gates on scheme only, so any obsidian:// URL (e.g. obsidian://open?vault=...&file=... pointing at arbitrary local files, or obsidian://new) is handed to the OS handler. Since Obsidian can navigate to local vault content, consider validating the host/action for obsidian: (only open with a vault param) or at least documenting in the file that the scheme alone is the security boundary.
  3. No blocking issues — the test coverage (empty/null input, dangerous schemes, case-insensitivity, round-trip of http://127.0.0.1:5174/) is thorough and the regression case is well documented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants