Open web links in the system browser, not in the webview - #132
Conversation
The `</>` chip was the only way to reach the escape hatch — a mouse-only control, which K1 says is a bug rather than a missing nicety. ⇧⌘E is ⌘E's shift-sibling on purpose: ⌘E changes whether you are editing, ⇧⌘E changes what you are looking at, and both work in either mode because the flip is one sticky flag serving the reading view and the editor alike. ⇧ as "the same idea, one step over" is the pattern ⌘N/⇧⌘N and ⌘F/⇧⌘F already set. Declared once in the registry, so the dispatcher, the `?` sheet and the recorder all derive from it and the three checkers had their say — CodeMirror leaves ⇧⌘E alone, so it stays live while editing. Both copies of the chip now project the chord into their tooltip out of the *live* registry (graphToggleHtml's rule) rather than spelling it, since a rebound chord would make a hard-coded one a lie. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012jyACvvK1Bkgur1MRdQqBL
Clicking an `https://…` in a note navigated the webview — and the webview *is* the application, so B2 was replaced by a web page in a window with no address bar, no back button and no way home. The app was gone until it was relaunched. So a link is an OS handoff, exactly as *Open in system default* already is for a resource: `links.ts` decides which hrefs are the system's (http, https, mailto — GFM autolinks a bare email into the third), the click delegation cancels the click, and `open_external` hands the URL to the user's browser. ⏎ on a focused anchor dispatches a click, so the keyboard takes the same path (K1). Inside live preview's rendered table a link now falls through to it rather than dropping the caret, the way a wikilink already did. The host re-checks the scheme against its own copy of that list. The frontend's copy is *routing*; the host's is the refusal, and the refusal is the point — a note is untrusted input (E5), and `open` launches whatever app has registered a scheme, so an unfiltered handoff would let a `.md` name a program to run. The schemes are spelled on both sides of the seam and pinned by a test on each, like WRITE_CONFLICT_MESSAGE and VAULT_CHANGED_EVENT: change them together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012jyACvvK1Bkgur1MRdQqBL
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe desktop app now validates and opens supported Markdown links through the host OS. It also adds the ChangesExternal link handoff
Markdown source toggle
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant RenderedMarkdown
participant api.openExternal
participant open_external
participant OSOpener
RenderedMarkdown->>api.openExternal: pass approved URL
api.openExternal->>open_external: invoke URL
open_external->>open_external: validate http, https, or mailto scheme
open_external->>OSOpener: open validated URL
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/src/links.ts`:
- Around line 22-47: Align URI sanitization with externalUrl by updating
ui/src/sanitize.ts to allow only http://, https://, and mailto: schemes, while
rejecting unsupported schemes and preserving control-character protection;
ui/src/links.ts requires no direct change. At ui/src/main.ts lines 3501-3524,
add the requested fallback in the a[href] click handler to prevent default
navigation when externalUrl(el.getAttribute("href")) returns null.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f6e51af9-947d-46ba-8fd4-f2ac1ed5dccc
📒 Files selected for processing (15)
CLAUDE.mdcrates/b2-desktop/CLAUDE.mdcrates/b2-desktop/Cargo.tomlcrates/b2-desktop/src/commands.rscrates/b2-desktop/src/error.rscrates/b2-desktop/src/main.rsdocs/design/invariants.mdui/src/api.tsui/src/bindings.tsui/src/links.test.tsui/src/links.tsui/src/livepreview.tsui/src/main.tsui/src/render.tsui/src/shortcuts.ts
The sanitizer's allow-list is wider than `externalUrl`'s, and the two were not talking to each other. DOMPurify drops `javascript:`, `data:` and `file:`, but it passes `ftp:`, `tel:`, `sms:`, `callto:`, `xmpp:`, `matrix:` and `cid:` — and every relative path. All of those reach the document as live anchors, none of them were routed, so a click on one still navigated the webview: the exact failure the handoff exists to prevent, reached by the door nobody was watching. A plain `[x](other.md)` was enough. So a link B2 won't follow is now cancelled and says so, rather than silently doing nothing — a dead click reads as a broken app. The branch falls through instead of returning, because one href must keep its click: a wikilink is `href="#"`, and the follow handler is further down. Leaving fragments alone also keeps a note's own `[top](#heading)` scrolling, the single navigation that doesn't unload the app. Not tightening the sanitizer's `ALLOWED_URI_REGEXP` to match, which was the other way to close this: it would strip the href off a `tel:` a human wrote, deleting authored content from the reading view to solve a problem that isn't in the document — it's in what the click does. The renderer stays a document renderer (sanitize.ts's stated posture); the click handler is where B2 decides what it will follow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012jyACvvK1Bkgur1MRdQqBL
2 participants
Summary
Prevent web links in note content from navigating the webview (which would replace the entire B2 application with a web page). Instead, route
http://,https://, andmailto:links to the system default browser or mail app via a newopen_externalcommand, mirroring the existing Open in system default behavior for resource files.Key Changes
New
open_externalcommand (crates/b2-desktop/src/commands.rs): Host-side validation and OS handoff for web links, with an allow-list of three schemes (http://,https://,mailto:). Includesis_openable_link()function that validates URLs byte-wise to prevent UTF-8 slicing panics and rejects control characters (which can smuggle multi-line attacks).Frontend routing (
ui/src/links.ts): NewexternalUrl()function that decides which hrefs are the system's to open. Pure string logic, no DOM, so it's testable in Node. Rejects control characters, relative links, fragments, and any scheme outside the allow-list.Click delegation (
ui/src/main.ts): Wire external links in the click handler — high in the delegation chain so it applies everywhere (reading view, live preview, backlinks). Cancels the click and hands the URL toapi.openExternal(). Keyboard-complete for free: ⏎ on a focused anchor dispatches a click.New keybinding (
ui/src/bindings.ts,ui/src/shortcuts.ts): ⇧⌘E (source.toggle) to flip between rendered and raw Markdown in the editor. Complements ⌘E (enter/leave edit mode) — both work in either mode because the source flag is sticky.Editor source toggle (
ui/src/main.ts,ui/src/render.ts): Implement thesource.togglekeybinding and update the</>chip's tooltip to show the live keybinding (so it's correct if the user rebinds ⇧⌘E).Error handling (
crates/b2-desktop/src/error.rs): NewCmdError::UnsupportedLinkvariant. The error message is generic ("B2 only opens web links…") and never echoes the URL, following the pattern that note content stays server-side.Test coverage:
only_web_links_are_openable()in Rust tests the host-side allow-list with the same cases as the frontend.links.test.tsin Node tests the frontend routing rule — pure string logic, no DOM.Documentation: Updated
CLAUDE.md,docs/design/invariants.md, andCLAUDE.mdto explain the OS handoff posture and the two-layer validation (frontend routing + host refusal).Implementation Details
Two independent layers: The frontend's
externalUrl()is routing (decides what to send to the host); the host'sis_openable_link()is the refusal (the authority). Both check the same three schemes, so they must be changed together — the same discipline asWRITE_CONFLICT_MESSAGEandVAULT_CHANGED_EVENT.Byte-wise validation:
is_openable_link()works on bytes, not characters, so a URL beginning mid-UTF-8 can't panic the slice. Control characters are rejected outright — they can't appear unencoded in a real URL and are how a newline-spliced second line would smuggle a second command to the OS.Least-privilege: The webview is granted no opener permission (Tauri capabilities). The host-side command validates before calling
tauri_plugin_opener::open_url().Keyboard accessibility (K1): ⏎ on a focused anchor dispatches a click, so the keyboard and mouse share one activation path. The sheet's "Follow the focused link" row already covers it.
https://claude.ai/code/session_012jyACvvK1Bkgur1MRdQqBL
Summary by CodeRabbit
New Features
Mod-Shift-eshortcut for toggling Markdown source view.Bug Fixes
Documentation