Skip to content

fix(desktop): open dashboard links in the default browser - #6303

Merged
lidge-jun merged 2 commits into
devfrom
codex/desktop-external-links
Sep 30, 2026
Merged

lidge-jun merged 2 commits into
devfrom
codex/desktop-external-links

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 30, 2026 •

Copy link
Copy Markdown
Owner

Summary

In the desktop app, clicking a login link that should open the system browser often did nothing. That covered the OAuth "Didn't open? Open the login page" fallback and the verification link that device-code logins (Copilot, Kimi, Nous, Meta Muse, Kiro, Codex device) depend on. Those links now open in the default browser on macOS, Windows and Linux.

There were two causes in the Tauri shell:

  • tauri_plugin_opener::init() injects a click listener that cancels every target="_blank" click and then calls plugin:opener|open_url over IPC. The dashboard is loaded from the remote origin http://127.0.0.1:*, and no capability grants it an opener permission. The IPC call was denied after the click had already been cancelled, so nothing opened.
  • Neither the main window nor the tray popup installed on_new_window. Without that handler, wry 0.55.1 drops window.open on WebView2 and WebKitGTK. WKWebView reaches the browser only because its navigation policy sees the URL first.

The fix turns off the JS interceptor with open_js_links_on_click(false) and sends new-window requests from both webviews through one Rust handler. That handler opens http/https URLs with the OS default browser and returns NewWindowResponse::Deny. The tray popup's navigation policy now also opens external URLs before refusing them, because WKWebView checks that policy before it creates a window for a _blank link. No IPC permission is added for the remote dashboard origin, and non-web schemes (file:, javascript:, custom schemes) never reach the launcher.

Out of scope: the server still discards the result of its own openUrl launch, and device-code logins still do not open a browser automatically.

Verification

  • cargo fmt --check, cargo clippy --all-targets -- -D warnings and cargo test for desktop/src-tauri (macOS arm64, using the CI job's placeholder sidecar and resource files) all exit 0. The test run reports 190 passed, including the new only_web_addresses_are_handed_to_the_default_browser.
  • bun test tests/clients/desktop-*.test.ts tests/server/local-management-capability.test.ts tests/ci-workflows/release-pipeline-contract.test.ts tests/ci-workflows/release-version-sources.test.ts tests/ci-workflows/repo-hygiene.test.ts reports 334 pass, 2 platform skips and 0 fail.
  • bun run privacy:scan and bun run structure:check both pass.
  • An independent review traced WKWebView, WebView2 and WebKitGTK behavior in the pinned wry 0.55.1 and tauri-runtime-wry 2.11.4 sources. The first round found the macOS tray-popup gap, which is fixed here, and the second round passed.
  • No packaged-app click test was run. Windows and Linux coverage comes from the source review and this PR's hosted desktop CI job.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. The devlog unit is in devlog/_fin/260930_desktop_external_links; there is no user-facing docs change.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. No capability is widened, and only http/https URLs reach the OS launcher.

Summary by CodeRabbit

  • Bug Fixes
    • External HTTP and HTTPS links opened from the desktop app now open in the default browser instead of attempting to create an in-app window. Other URL types are not opened externally.
    • External links in the popup now open in the default browser before in-app navigation is denied.

The opener plugin's click interceptor cancelled every target="_blank" click and
asked for plugin:opener|open_url over IPC, which the loopback dashboard origin is
not granted, so the OAuth "didn't open?" link and device-code verification links
did nothing in the app. No webview installed on_new_window either, so wry dropped
window.open on WebView2 and WebKitGTK.

Disable the interceptor and route new-window requests from the main window and
the tray popup through one Rust handler that opens http/https URLs with the OS
default browser and denies the in-app window. The tray popup's navigation policy
now opens external URLs before refusing them, which is the path WKWebView takes
for _blank links.
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 30, 2026 08:17
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T08:21:37.743762Z 3aa4196 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 30, 2026
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: f5c26265-9b29-4a04-beee-e6b58631718d

📥 Commits

Reviewing files that changed from the base of the PR and between 6538001 and 3aa4196.

📒 Files selected for processing (4)
  • desktop/src-tauri/src/lib.rs
  • desktop/src-tauri/src/popup.rs
  • desktop/src-tauri/src/window.rs
  • devlog/_fin/260930_desktop_external_links/010_plan.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The desktop app routes HTTP and HTTPS links from its main window and tray popup to the default browser. It denies new webview creation. For different-origin popup navigation, it opens the URL in the default browser before denying in-webview navigation.

Changes

Default-browser link routing

Layer / File(s) Summary
Browser-opening policy and handler
desktop/src-tauri/src/lib.rs, desktop/src-tauri/src/window.rs
The opener plugin’s JavaScript click handler is disabled. Rust handlers open only HTTP and HTTPS URLs and deny new webview creation. Tests cover accepted and rejected URL schemes.
Main window and popup integration
desktop/src-tauri/src/lib.rs, desktop/src-tauri/src/popup.rs, devlog/_fin/260930_desktop_external_links/010_plan.md
The main window and popup route new-window requests through the handler. The popup opens different-origin URLs in the default browser before denying navigation. The devlog records the implementation and verification.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Webview
  participant NewWindowHandler
  participant DefaultBrowser
  Webview->>NewWindowHandler: Request new window with URL
  NewWindowHandler->>DefaultBrowser: Open URL if scheme is HTTP or HTTPS
  NewWindowHandler-->>Webview: Deny new webview creation
Loading

Merge Risk: ⚪ Minimal · up to 3aa41

Emitted HTTP(S) new-window links are routed to the system browser while in-app popup creation is denied. The intended callback path is registered for both webviews, and no actionable introduced risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 3aa41

The browser-launch policy remains limited to HTTP and HTTPS, and additional in-app webviews are denied. No security regression was established, but cross-platform callback ordering and popup recovery behavior remain insufficiently verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — An actor able to influence URLs emitted by either shell webview can request arbitrary HTTP(S) browser destinations through delivered new-window events. The native sink is browser launch on that desktop installation, not file opening, custom-scheme dispatch, or creation of another privileged webview. Downstream browser redirects are outside this predicate.

Security Findings and Attack Paths

  • observed — Main-window navigation already exposed HTTP(S) browser launching in the base. The PR adds event paths to that class of native side effect, so the newly reachable launch path is not itself evidence of an arbitrary-scheme or broader IPC authorization vulnerability.

Trust Boundaries and Controls

  • observed — Rust event callbacks mediate the webview-to-OS boundary without requiring remote opener IPC authority. Scheme checking precedes launch, new-webview denial is unconditional, and existing main and popup policies continue restricting what may navigate inside the shell.

Resilience and Maintainability Implications

  • inferred — Popup reuse preserves the endpoint captured when its navigation policy was created. That capture predates the PR. If the endpoint changes while the popup survives, rejected navigation now attempts browser launch rather than only being denied; whether that transition occurs in production remains unresolved.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: desktop dashboard links now open in the default browser. It is specific, relevant, and consistent with the pull request objectives.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 70 / 80

이 PR은 데스크톱 앱에서 로그인 링크를 눌러도 기본 브라우저가 안 열리던 문제를 고칩니다. 베이스는 dev입니다. OAuth의 「열리지 않았나요?」 링크와, 기기 코드 로그인(Copilot·Kimi·Nous·Meta Muse·Kiro·Codex device)이 쓰는 확인 링크가 대표입니다.

원인은 두 가지였습니다. 첫째, opener 플러그인이 target="_blank" 클릭을 가로채 preventDefault한 뒤 IPC로 open_url을 부르는데, 대시보드는 http://127.0.0.1:*라서 opener 권한이 없어 IPC가 거절되고 클릭만 사라진 상태였습니다. 둘째, 메인 창과 트레이 팝업에 on_new_window가 없어 wry가 WebView2·WebKitGTK에서 window.open을 그냥 버렸습니다.

고친 방법은 권한이 넓은 IPC를 대시보드에 주지 않는 쪽입니다. open_js_links_on_click(false)로 JS 가로채기를 끄고, 메인·팝업 모두 Rust on_new_window 한곳에서 http/https만 OS 기본 브라우저로 연 뒤 앱 안 창은 거절합니다. macOS 트레이는 WKWebView가 _blank 전에 네비게이션 정책을 먼저 보기 때문에, 팝업도 외부 URL을 연 다음 거절하도록 맞춰 두었습니다. 스킴 필터 단위 테스트와 desktop CI·관련 Bun 스위트는 통과했다고 본문에 적혀 있고, 현재 헤드 체크도 desktop shell 포함 초록입니다. types/config 중복으로 닫을 PR은 없습니다.

라인 - desktop/src-tauri/src/window.rs open_in_default_browser · open_url 결과를 let _ =로 버립니다. 열기에 실패해도 로그·UI 힌트가 없어, 사용자 입장에서는 예전처럼 「아무 일도 안 일어남」으로 남을 수 있습니다.

라인 - 검증 · 패키지 앱에서 실제 클릭으로 연 테스트는 없습니다(본문도 인정). Windows·Linux는 wry 소스 리뷰와 호스트 desktop CI에 기대고, macOS 실기기 클릭도 이 PR 범위의 증거로는 없습니다.

라인 - 범위 밖(의도) · 서버 openUrl 결과 버리기와 기기 코드의 「자동으로 브라우저 열기」는 그대로입니다. 이번 수정은 GUI 쪽 「안 열렸을 때」 링크가 다시 먹게 만드는 쪽에 가깝습니다.

메인테이너의 판단이 필요한 지점

패키지 앱 클릭 확인 없이 소스·CI만으로 머지할지입니다. 보안 방향(원격 origin에 opener IPC 안 줌, http/https만 런처로)은 분명합니다. 기기 코드 자동 오픈·서버 openUrl 결과 처리는 후속으로 둘지, 이번 릴리스 노트에 「폴백 링크가 다시 열림」 정도로만 적을지도 정하면 됩니다.

너의 추천

머지해도 됩니다. 가능하면 macOS에서 OAuth 「안 열렸나요?」와 트레이 팝업 외부 링크만 한 번씩 손으로 눌러 보거나, open_url 실패 시 짧은 로그라도 남기면 나중에 같은 「안 열림」 제보를 가르기 쉽습니다. 서버 자동 오픈·결과 처리는 이 PR에 넣지 말고 후속으로 두세요. preview deploy 이야기는 생략합니다.

이 댓글은 grok-bot이 작성했습니다

@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration into dev under the MAINTAINERS.md dev-only exception. This is maintainer integration, not a self-approval or an independent review.

This decision uses exact-head verification for 3aa41964cf9b2144f3e3a58ad74b86e602d7d372. Every check that ran on this head passed, and the others were path-skipped:

  • In CI run 36688824947, desktop shell ran the Rust fmt, clippy and test steps and the Linux packaged-shell E2E. macos 1/2, macos 2/2, macos widget + bundle, test shards 1–4, gates, hygiene, enforce-target, docker smoke, the keyring jobs and the service-manager jobs also passed.
  • Codex review completed on 3aa4196 with no findings. CodeRabbit generated no actionable comments.
  • No maintainer change requests are outstanding. No capability or credential handling changed, and only http/https URLs reach the OS launcher.

@lidge-jun
lidge-jun merged commit 6944cc3 into dev Sep 30, 2026
39 checks passed
@lidge-jun
lidge-jun deleted the codex/desktop-external-links branch September 30, 2026 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant