fix(app): stop external links from navigating the main webview away - #6000
fix(app): stop external links from navigating the main webview away#6000Guykaganovsky1 wants to merge 1 commit into
Conversation
The desktop shell is a single webview with no back button and no address bar, so a top-level navigation to a remote page is one-way: the chat is gone until the app restarts. Clicking a link to a site the agent built did exactly that. Chat bubbles already route their links through `openUrl` (`AgentMessageBubble`'s `MarkdownAnchor`), but that is one component's discipline — every other anchor the app renders inherits the webview's default navigation instead, and there is no shell-level guard: the main window is declared in `tauri.conf.json`, and `on_navigation` exists only on `WebviewWindowBuilder`, so a config window has nowhere to attach one. Install a document-level click guard above the router. It listens in the BUBBLE phase deliberately: in the capture phase it would run before the owning component's handler and a chat link would open twice, once here and once in `MarkdownAnchor`. Bubbling lets the component go first, and the default navigation has still not happened, so preventing it there is not too late.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
How this change flows0 changed behaviours across 2 relationships. 2 surrounding behaviours are shown (60 graph nodes walked). 17 further behaviours left out to keep the diagram readable. flowchart LR
n0["startInternetStatusListener"]:::impacted
n1["snapshot"]:::impacted
n0 -->|calls| n1
n0 -->|uses| n1
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
tinysweeper
Bot
added
the
priority: p3
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe app now intercepts external HTTP(S) anchor navigation and forwards it to ChangesExternal link guard
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to External HTTP(S) links now open in the system browser instead of replacing the app webview, while in-app routes and modified or handled clicks retain their existing behavior. The change is ready to merge. Sequence Diagram(s)sequenceDiagram
participant Anchor
participant ExternalLinkGuard
participant openUrl
Anchor->>ExternalLinkGuard: dispatch click
ExternalLinkGuard->>ExternalLinkGuard: check href and click modifiers
ExternalLinkGuard->>openUrl: open external HTTP(S) URL
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21b19c7726
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (url.protocol !== 'http:' && url.protocol !== 'https:') return false; | ||
| return url.origin !== appOrigin; |
There was a problem hiding this comment.
When assistant text contains a relative Markdown link such as [docs](/docs), MarkdownText renders it as a plain anchor, but this predicate ignores it because it resolves to the app origin (or to Tauri's non-HTTP asset scheme). The application only routes #/… URLs, so following /docs performs a top-level asset navigation rather than an in-app transition and can still replace the main webview with an unavailable page. Allow only same-document hash routes, rather than treating every same-origin or non-HTTP URL as safe.
AGENTS.md reference: AGENTS.md:L175-L175
Useful? React with 👍 / 👎.
1 participant
Summary
Clicking a link in the app — for example to a site the agent just built — navigated the main webview to that page. The desktop shell is a single webview with no back button and no address bar, so this is one-way: the chat is gone until the app is restarted.
Chat bubbles already route their links through
openUrl(AgentMessageBubble'sMarkdownAnchor), but that is one component's discipline. Every other anchor the app renders inherits the webview's default navigation, and there is no shell-level guard to catch them: the main window is declared intauri.conf.json, andon_navigationexists only onWebviewWindowBuilder, so a config-declared window has nowhere to attach one.Related issue
None.
Changes
app/src/utils/externalLinkGuard.ts—isExternalNavigation(pure predicate) plusinstallExternalLinkGuard, a document-level click guard that hands remotehttp(s)links toopenUrland prevents the navigation.app/src/App.tsx— install it once, above the router.The guard listens in the bubble phase deliberately. In the capture phase this document-level listener would run before the owning component's handler, so a chat link would be opened twice — once here and once by
MarkdownAnchor— i.e. two browser tabs for one click. Bubbling lets the component go first; anything it alreadypreventDefault-ed is skipped, and the default navigation has still not happened, so preventing it there is not too late. This was caught by the test suite, not by reasoning after the fact.In-app hash routes, non-http schemes (
mailto:,openhuman://,data:), modifier-clicks and non-primary buttons are all left alone.API or behavior changes
Behavior only: an anchor that previously replaced the app now opens in the user's browser. No public API.
Validation
npx vitest run --config test/vitest.config.ts src/utils/externalLinkGuard.test.ts— 7 passed.pnpm typecheck— clean.pnpm lint— 0 errors (82 pre-existing warnings, none in the new files).Tests
app/src/utils/externalLinkGuard.test.ts— external link is intercepted and opened outside; in-app hash routes pass through; non-http schemes ignored; an already-handled click is deferred to (the double-open regression); teardown removes the listener. jsdom's "Not implemented: navigation to another Document" in the teardown case is the un-guarded behaviour this fix exists to stop.Checklist
#[allow(...)],#[ignore], or relaxed lints.envcontents in the diff or the descriptionSummary by CodeRabbit
New Features
Bug Fixes