Split tunneling UI changes - #8970
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe change detects browser applications on Android, Windows, and macOS, propagates ChangesBrowser-aware app metadata
Bypass warning flows
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to The split tunneling UI and browser-detection changes introduce no actionable merge-blocking risk based on the available evidence; the PR is merge-ready after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant NativeAppDiscovery
participant LanternPlatformService
participant AppsSplitTunneling
participant AppDialog
participant LocalStorageService
NativeAppDiscovery->>LanternPlatformService: provide app data with isBrowser
LanternPlatformService->>AppsSplitTunneling: map installed apps
AppsSplitTunneling->>LocalStorageService: read bypass dialog state
AppsSplitTunneling->>AppDialog: show app or browser warning
AppDialog-->>AppsSplitTunneling: return cancel or add-anyway action
AppsSplitTunneling->>LocalStorageService: persist dialog state
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Pull request overview
This PR enhances split tunneling by detecting browser apps across platforms and surfacing clearer, safety-oriented UI warnings when users bypass browsers or bulk-bypass apps/websites. It also adds one-time explainer dialogs for bypass actions, tracked via persistent local storage so users aren’t repeatedly prompted.
Changes:
- Added
isBrowserpropagation across platform layers (Android/Kotlin, macOS/Swift, Windows/Go, Dart models + services) to flag browsers in the installed-apps feed. - Introduced new split-tunneling dialogs/warnings (browser bypass warning, “select all” warning) plus first-time bypass explainers for apps and websites.
- Added persistent LocalStorage flags to track “seen” state for the first-time bypass dialogs.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| macos/Runner/Handlers/AppStreamHandler.swift | Adds macOS browser detection via LaunchServices/NSWorkspace and annotates streamed app items with isBrowser. |
| lib/lantern/lantern_platform_service.dart | Maps isBrowser from platform raw app snapshots into AppData. |
| lib/lantern/lantern_ffi_service.dart | Maps isBrowser from FFI snapshots; adjusts split-tunnel FFI call result handling. |
| lib/features/split_tunneling/website_domain_input.dart | Adds first-time bypass-website explainer dialog and persistence gate. |
| lib/features/split_tunneling/split_tunneling.dart | Tweaks split tunneling subtitle rendering to use AutoSizeText. |
| lib/features/split_tunneling/apps_split_tunneling.dart | Adds browser bypass warning + first-time bypass-app explainer; adds “select all” browser warning dialog. |
| lib/core/services/local_storage_service.dart | Adds persistent keys and helpers for tracking one-time split-tunneling dialogs. |
| lib/core/models/app_data.dart | Adds isBrowser field to the shared Dart app model + (de)serialization/copyWith. |
| lib/core/common/app_dialog.dart | Adds centeredTitle support and a dedicated browser bypass warning dialog helper. |
| lantern-core/apps/browsers_windows.go | Implements Windows browser detection by scanning modern default-app registration in the registry; marks apps as browsers. |
| lantern-core/apps/browsers_other.go | Adds non-Windows stub for markBrowsers. |
| lantern-core/apps/apps.go | Ensures markBrowsers runs on cached and freshly discovered app lists before caching/return. |
| lantern-core/apps/apps_data.go | Adds isBrowser to the Go AppData JSON model. |
| assets/locales/en.po | Adds new localized strings for browser/all-app warnings and first-time bypass explainers. |
| android/app/src/main/kotlin/org/getlantern/lantern/apps/AppDataHandler.kt | Detects Android browsers via intent handlers and sets isBrowser on emitted app items. |
| android/app/src/main/kotlin/org/getlantern/lantern/apps/AppData.kt | Adds isBrowser to Android AppData model and JSON mapping. |
Suppressed comments (1)
lib/features/split_tunneling/apps_split_tunneling.dart:257
- This dialog is shown via showDialog in AppDialog.customDialog, so it should be dismissed via the dialog's Navigator (Navigator.of(context).pop()). Using appRouter.pop() can pop the wrong route if the dialog isn't on the same navigator as appRouter (and it diverges from AppDialog.show's dismissal behavior).
onPressed: () {
appRouter.pop();
onAddAllExceptBrowsers();
},
),
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@android/app/src/main/kotlin/org/getlantern/lantern/apps/AppDataHandler.kt`:
- Around line 240-245: Update browserPackages to query intent handlers for both
HTTP and HTTPS URLs, combining the resulting package names into one Set<String>.
Preserve the existing ACTION_VIEW, CATEGORY_BROWSABLE, MATCH_ALL, and null-safe
activityInfo handling for each query.
In `@lantern-core/apps/apps.go`:
- Around line 277-282: Update the discovery flow in loadInstalledAppsPlatform so
each newly discovered AppData has IsBrowser set before cb is invoked; move or
reuse the browser-marking logic within that platform callback path rather than
relying on the later markBrowsers(found) call. Preserve the existing callback
streaming behavior and final marking for any records not handled during
discovery.
In `@lantern-core/apps/browsers_windows.go`:
- Around line 171-188: The markBrowsers function should recompute IsBrowser for
each non-nil app by resetting app.IsBrowser to false before path normalization
and registry-index matching, then set it true only when the app matches
idx.paths or idx.basenames.
In `@lib/core/models/app_data.dart`:
- Around line 15-16: Update the documentation comment for isBrowser to state
that the value is populated by Android and desktop discovery, while retaining
that it defaults to false on unsupported platforms.
In `@lib/lantern/lantern_ffi_service.dart`:
- Around line 503-506: Update the addSplitTunnelItem and removeSplitTunnelItem
flows to convert the returned result and then call
_ffiService.freeCString(result) in a finally block, ensuring the pointer is
released on both success and conversion failure while preserving input-pointer
cleanup.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e5d870fa-ac70-4fa9-9cdb-39e3333b039d
📒 Files selected for processing (16)
android/app/src/main/kotlin/org/getlantern/lantern/apps/AppData.ktandroid/app/src/main/kotlin/org/getlantern/lantern/apps/AppDataHandler.ktassets/locales/en.polantern-core/apps/apps.golantern-core/apps/apps_data.golantern-core/apps/browsers_other.golantern-core/apps/browsers_windows.golib/core/common/app_dialog.dartlib/core/models/app_data.dartlib/core/services/local_storage_service.dartlib/features/split_tunneling/apps_split_tunneling.dartlib/features/split_tunneling/split_tunneling.dartlib/features/split_tunneling/website_domain_input.dartlib/lantern/lantern_ffi_service.dartlib/lantern/lantern_platform_service.dartmacos/Runner/Handlers/AppStreamHandler.swift
|
I have a couple fixes here: #8978 👍 LGTM Otherwise! |
This pull request introduces browser detection and new user warnings for bypassing browsers in split tunneling. The main changes add the ability to identify browsers across Android, Windows, and core data models, and surface appropriate warnings to users when they attempt to bypass browsers or all apps. Additionally, one-time explainer dialogs are implemented for bypassing apps or websites, with persistent tracking to avoid repeat prompts.
Browser detection and flag propagation:
isBrowserboolean property to theAppDatamodel across Android (AppData.kt), Dart (app_data.dart), and Go (apps_data.go), enabling the system to flag apps that register as HTTP(S) handlers (browsers). Android detects browsers via intent handlers, and Windows uses a registry scan for browser executables. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16]User-facing warnings and dialogs:
browserBypassWarningDialoginapp_dialog.dart, which centers the title and shows a warning icon when the user tries to bypass a browser. [1] [2] [3]One-time explainer dialogs and persistence:
LocalStorageService. [1] [2]These changes ensure that browsers are accurately detected and that users receive appropriate warnings and explanations when modifying split tunneling settings, improving both security and user experience.
Summary by CodeRabbit