Security & stability fixes (local proxy, crashes, import)#44
Merged
Conversation
📝 WalkthroughWalkthroughThis PR adds two new configurable preferences ( ChangesMixed Inbound Gating Feature
AppManagerActivity Coroutine Filter Refactor
Bug Fixes and Infrastructure
Sequence Diagram(s)sequenceDiagram
participant User as User (Settings)
participant GlobalPrefs as global_preferences.xml
participant DataStore as DataStore
participant ConfigBuilder as ConfigBuilder
participant SingBox as Sing-box Inbounds
User->>GlobalPrefs: toggle requireProxyInVPN / proxyModeInboundAuth
GlobalPrefs->>DataStore: persist preference
Note over DataStore: mixedInboundNeedsAuth reads proxyModeInboundAuth<br/>when serviceMode == PROXY
DataStore->>ConfigBuilder: requireProxyInVPN, appendHttpProxy, isVPN
ConfigBuilder->>ConfigBuilder: compute keepMixedInbound
alt keepMixedInbound == true
ConfigBuilder->>SingBox: add Inbound_MixedOptions("mixed-in")
ConfigBuilder->>SingBox: add routing rule TAG_MIXED → mainProxyTag
else keepMixedInbound == false
ConfigBuilder->>SingBox: skip mixed inbound and routing rule
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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
🧹 Nitpick comments (1)
app/src/main/java/io/nekohasekai/sagernet/ui/AppManagerActivity.kt (1)
163-165: ⚡ Quick winConsider
@Volatilefor thread-safe visibility of shared state.
appsis read onDispatchers.DefaultincomputeFilteredbut written from multiple threads (Default in invert/clear actions, IO in reload, Main in selectProxyApp). Similarly,sysAppsis read on Default but written on Main. Without@Volatile, the JVM memory model doesn't guarantee that writes are visible to other threads.In practice this likely works due to dispatch latency, but the worst case is filtering against a stale list. Adding
@Volatileprovides proper visibility guarantees.♻️ Suggested fix
private var filterJob: Job? = null -private var apps = emptyList<ProxiedApp>() +@Volatile private var apps = emptyList<ProxiedApp>() private val appsAdapter = AppsAdapter()And for
sysAppsat line 270:-private var sysApps = true +@Volatile private var sysApps = true🤖 Prompt for 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. In `@app/src/main/java/io/nekohasekai/sagernet/ui/AppManagerActivity.kt` around lines 163 - 165, The apps and sysApps properties are being accessed and modified from multiple threads (Dispatchers.Default, Dispatchers.IO, and Dispatchers.Main) without proper synchronization guarantees. Add the `@Volatile` annotation to both the apps property (currently at line 163) and the sysApps property (at line 270) to ensure that writes to these shared state variables are properly visible to other threads. This prevents potential issues where filtering operations in computeFiltered could work with stale cached values.
🤖 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 `@app/src/main/java/io/nekohasekai/sagernet/ktx/Formats.kt`:
- Line 269: Fix the typo in the comment on line 269 where it says "unparyable" -
change it to the correct spelling "unparsable" to fix the misspelled word in the
comment describing the behavior when no profile links are parsed but an
unparsable URL is encountered.
---
Nitpick comments:
In `@app/src/main/java/io/nekohasekai/sagernet/ui/AppManagerActivity.kt`:
- Around line 163-165: The apps and sysApps properties are being accessed and
modified from multiple threads (Dispatchers.Default, Dispatchers.IO, and
Dispatchers.Main) without proper synchronization guarantees. Add the `@Volatile`
annotation to both the apps property (currently at line 163) and the sysApps
property (at line 270) to ensure that writes to these shared state variables are
properly visible to other threads. This prevents potential issues where
filtering operations in computeFiltered could work with stale cached values.
🪄 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
Run ID: fdd727f1-0478-48fb-8258-2492014d6a20
📒 Files selected for processing (13)
.gitignoreapp/src/main/AndroidManifest.xmlapp/src/main/java/io/nekohasekai/sagernet/Constants.ktapp/src/main/java/io/nekohasekai/sagernet/database/DataStore.ktapp/src/main/java/io/nekohasekai/sagernet/fmt/ConfigBuilder.ktapp/src/main/java/io/nekohasekai/sagernet/group/RawUpdater.ktapp/src/main/java/io/nekohasekai/sagernet/ktx/Formats.ktapp/src/main/java/io/nekohasekai/sagernet/ui/AboutFragment.ktapp/src/main/java/io/nekohasekai/sagernet/ui/AppManagerActivity.ktapp/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.ktapp/src/main/res/values/strings.xmlapp/src/main/res/xml/global_preferences.xmlrelease.keystore
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
This was referenced Jun 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backports and original security/stability fixes for upstream issues that are not
addressed in our base. All changes verified to build/review clean locally on Android.
Changes
maxAliasesForCollectionsto 1000 so large Clash/Mihomosubscriptions with many anchors import (kept
codePointLimit+SafeConstructor).IllegalStateException: Fragment not attachedby using the passed
activityContextandisAddedchecks in async callbacks.android.widget.Filter(a thread per call →pthread_create failedOOM) with coroutine-based, debounced, cancellable filtering on the per-apps screen.
orientation|screenSize| keyboardHiddenconfig changes and guard the test dialog dismiss.http(s)promo link; only treat input as a subscription when no profiles parsed.closed in VPN mode by default, and an option to authenticate the loopback inbound in
Proxy mode. LAN-exposed binds remain mandatorily authenticated.
release.keystore; ignore signing keys going forward.Testing
Notes
behavior except VPN-mode inbound, which is now closed unless opted in.
Greptile Summary
This PR delivers six targeted security and stability fixes across the local proxy inbound, YAML subscription parsing, per-app filter threading, screen-rotation crash, and the About screen detachment crash, plus untracking a committed signing key.
ConfigBuilder.kt,DataStore.kt,Constants.kt,global_preferences.xml,strings.xml): adds akeepMixedInboundgate that closes the SOCKS/HTTP inbound by default in VPN/TUN mode (new opt-inrequireProxyInVPN), and an opt-inproxyModeInboundAuthflag that extends credential enforcement to Proxy-mode loopback. Both inbound and route rule are consistently guarded; themixedInboundNeedsAuthproperty is updated to match.AppManagerActivity.kt,ConfigurationFragment.kt,AboutFragment.kt,AndroidManifest.xml): replacesandroid.widget.Filter(onepthreadper call) with a coroutine-based, debounced, cancellable filter; guards bothdialog.dismiss()sites with a targetedcatch (IllegalStateException); addsisAddedchecks before async UI callbacks inAboutFragment; and registersorientation|screenSize|keyboardHiddenconfig-changes to prevent Activity restart during URL tests.Formats.kt,RawUpdater.kt): defersSubscriptionFoundExceptionso stray promo URLs in a multi-profile file no longer abort the entire import; raisesmaxAliasesForCollectionsfrom 50 → 200 to cover large Clash/Mihomo configs while keeping alias-expansion memory bounded.Confidence Score: 5/5
Safe to merge; all changes are well-scoped fixes with consistent logic across DataStore, ConfigBuilder, and the preference UI.
Each fix addresses a discrete, verified issue (OOM thread exhaustion, fragment detachment crash, rotation crash, subscription parse abort, local proxy exposure). The new keepMixedInbound flag and mixedInboundNeedsAuth extension are correctly threaded through every consumer. The coroutine filter replacement is sound with appropriate @volatile annotations. No regressions were identified in the changed paths.
No files require special attention. The most behaviorally significant change — closing the VPN-mode local proxy by default — is intentional and documented.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[buildConfig called] --> B{forTest?} B -- yes --> C[keepMixedInbound = false\nNo local inbound added] B -- no --> D{isVPN?} D -- no\nProxy mode --> E[keepMixedInbound = true] D -- yes\nVPN/TUN mode --> F{requireProxyInVPN?} F -- yes --> E F -- no --> G{appendHttpProxy\n+ Android Q+?} G -- yes --> E G -- no --> C E --> H[Add mixed inbound to config] H --> I{mixedInboundNeedsAuth?} I -- LAN exposed\nallowAccess=true --> J[Auth mandatory] I -- VPN mode\nno appendHttpProxy --> J I -- Proxy mode\nproxyModeInboundAuth=true --> J I -- Proxy mode\nproxyModeInboundAuth=false --> K[Auth disabled\nopen loopback proxy] J --> L[Inbound with username/password] K --> M[Inbound with no credentials] C --> N[No local proxy port opened\nReduced attack surface]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[buildConfig called] --> B{forTest?} B -- yes --> C[keepMixedInbound = false\nNo local inbound added] B -- no --> D{isVPN?} D -- no\nProxy mode --> E[keepMixedInbound = true] D -- yes\nVPN/TUN mode --> F{requireProxyInVPN?} F -- yes --> E F -- no --> G{appendHttpProxy\n+ Android Q+?} G -- yes --> E G -- no --> C E --> H[Add mixed inbound to config] H --> I{mixedInboundNeedsAuth?} I -- LAN exposed\nallowAccess=true --> J[Auth mandatory] I -- VPN mode\nno appendHttpProxy --> J I -- Proxy mode\nproxyModeInboundAuth=true --> J I -- Proxy mode\nproxyModeInboundAuth=false --> K[Auth disabled\nopen loopback proxy] J --> L[Inbound with username/password] K --> M[Inbound with no credentials] C --> N[No local proxy port opened\nReduced attack surface]Reviews (3): Last reviewed commit: "review: address CR/Greptile feedback (al..." | Re-trigger Greptile