perf + security: hot-path batching, action/source pins, local API token#115
Conversation
…containsKey for callback dedupe
…ellation in group dialogs; ignore .worklog
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
… API); drop unused imports
hawkff
force-pushed
the
feat/pr2-perf-security
branch
from
cdfdb76 to
b561ec5
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
hawkff
force-pushed
the
feat/pr2-perf-security
branch
from
b561ec5 to
846f0f8
Compare
hawkff
force-pushed
the
feat/pr2-perf-security
branch
from
846f0f8 to
7d97488
Compare
…ncel, address review)
…y before fragment (address review)
hawkff
force-pushed
the
feat/pr2-perf-security
branch
from
7d97488 to
d225538
Compare
|
Fixed the dashboard-URL findings: it's now built via Declining the keep-in-sync line-number note: the comment is a review aid pointing reviewers at the builder's derivation; the exact line numbers drifting is acceptable and cheaper than removing the pointer entirely. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
.github/workflows/release.yml (1)
23-24: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd
persist-credentials: falseto checkout steps.zizmor flags these
actions/checkoutsteps for credential persistence (artipacked): the default GitHub token is left in the local git config for the rest of the job, so any subsequent script (including third-party build tooling now being fetched/executed in this same PR) could exfiltrate it. Since this PR's stated purpose is security hardening, this is a good complementary fix.🔒 Proposed fix
- name: Checkout - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + with: + persist-credentials: falseThis applies to every checkout step in this workflow (and likely other
.github/workflows/*.ymlfiles sharing the same pattern).Also applies to: 55-56, 95-102, 154-156
🤖 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 @.github/workflows/release.yml around lines 23 - 24, Add persist-credentials: false to every actions/checkout step in this workflow so the GitHub token is not left in git config for later steps. Update each Checkout job block in release.yml that uses actions/checkout (the repeated checkout invocations in the workflow) to include this setting alongside the existing uses reference.Source: Linters/SAST tools
.github/workflows/ci.yml (1)
19-19: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider
persist-credentials: falseon checkout steps.Same as flagged in build.yml — these checkout steps persist the git token in local config for the rest of the job. Worth disabling given the security-hardening intent of this cohort.
🔒 Suggested addition
- name: Checkout uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + with: + persist-credentials: falseAlso applies to: 126-126
🤖 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 @.github/workflows/ci.yml at line 19, The checkout steps in the CI workflow still persist the git token in local config, which conflicts with the hardening intent. Update each actions/checkout usage in the workflow to disable credential persistence by setting persist-credentials to false, including the additional checkout instance referenced in the same job.Source: Linters/SAST tools
.github/workflows/build.yml (1)
21-22: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider
persist-credentials: falseon checkout steps.Static analysis flags these
actions/checkoutsteps for retaining the git credential in local config for the rest of the job. Since this PR is hardening CI supply-chain security (SHA pinning), also disabling credential persistence on checkout reduces the blast radius if a later step in the job is compromised.🔒 Suggested addition
- name: Checkout uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + with: + persist-credentials: falseAlso applies to: 90-91
🤖 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 @.github/workflows/build.yml around lines 21 - 22, The Checkout step in the workflow is still persisting git credentials, which weakens the CI hardening. Update the `actions/checkout` usages in the workflow to set `persist-credentials: false` so credentials are not left in local git config for later steps. Apply this to the relevant Checkout steps identified by the `actions/checkout` entries in `.github/workflows/build.yml`.Source: Linters/SAST tools
🤖 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/database/DataStore.kt`:
- Around line 187-195: The clashApiSecret getter in DataStore currently
generates and stores a UUID under `@Synchronized`, but that only protects one
process and can race with other processes reading the shared configurationStore.
Replace the read-then-generate flow with a single atomic insert-if-absent or
transactional claim that returns the already-stored value if present, and only
creates one new secret when missing. Use the existing clashApiSecret property
and configurationStore access as the place to centralize this cross-process-safe
claim.
In `@app/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.kt`:
- Around line 1719-1739: The batch traffic update lookup is happening off the
main thread in onUpdated, but it reads configurationIdList and touches
layoutManager.findViewByPosition() and
configurationListView.getChildViewHolder(), which must be done on the UI thread.
Refactor ConfigurationFragment.onUpdated so the index resolution and holder
lookup happen inside onMainDispatcher { ... } or configurationListView.post {
... }, then bind the collected ConfigurationHolder/TrafficData pairs there to
avoid stale positions or recycled holders.
In `@buildScript/lib/mieru.sh`:
- Around line 22-24: The fetch logic in mieru.sh still uses the raw MIERU_COMMIT
SHA, which can fail when the commit is not advertised or the tag moves; update
the checkout flow around the git fetch call to fetch and use the release tag or
another stable ref instead of the bare SHA. Keep MIERU_COMMIT as the pinned
revision source if needed, but make the actual fetch in the MIERU version
handling code resolve via the tag/ref so the build does not depend on GitHub
accepting raw-SHA fetches.
---
Nitpick comments:
In @.github/workflows/build.yml:
- Around line 21-22: The Checkout step in the workflow is still persisting git
credentials, which weakens the CI hardening. Update the `actions/checkout`
usages in the workflow to set `persist-credentials: false` so credentials are
not left in local git config for later steps. Apply this to the relevant
Checkout steps identified by the `actions/checkout` entries in
`.github/workflows/build.yml`.
In @.github/workflows/ci.yml:
- Line 19: The checkout steps in the CI workflow still persist the git token in
local config, which conflicts with the hardening intent. Update each
actions/checkout usage in the workflow to disable credential persistence by
setting persist-credentials to false, including the additional checkout instance
referenced in the same job.
In @.github/workflows/release.yml:
- Around line 23-24: Add persist-credentials: false to every actions/checkout
step in this workflow so the GitHub token is not left in git config for later
steps. Update each Checkout job block in release.yml that uses actions/checkout
(the repeated checkout invocations in the workflow) to include this setting
alongside the existing uses reference.
🪄 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: ceb6d937-1eb6-46b4-a963-97ca24f98f40
📒 Files selected for processing (24)
.depot/workflows/android-instrumented.yml.depot/workflows/build-apk.yml.depot/workflows/guard.yml.depot/workflows/lint.yml.depot/workflows/unit-tests.yml.github/workflows/build.yml.github/workflows/ci.yml.github/workflows/preview.yml.github/workflows/release.ymlapp/src/main/aidl/io/nekohasekai/sagernet/aidl/ISagerNetServiceCallback.aidlapp/src/main/java/io/nekohasekai/sagernet/Constants.ktapp/src/main/java/io/nekohasekai/sagernet/bg/BaseService.ktapp/src/main/java/io/nekohasekai/sagernet/bg/SagerConnection.ktapp/src/main/java/io/nekohasekai/sagernet/bg/proto/TrafficLooper.ktapp/src/main/java/io/nekohasekai/sagernet/database/DataStore.ktapp/src/main/java/io/nekohasekai/sagernet/database/ProfileManager.ktapp/src/main/java/io/nekohasekai/sagernet/fmt/ConfigBuilder.ktapp/src/main/java/io/nekohasekai/sagernet/plugin/PluginManager.ktapp/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.ktapp/src/main/java/io/nekohasekai/sagernet/ui/MainActivity.ktapp/src/main/java/io/nekohasekai/sagernet/ui/WebviewFragment.ktapp/src/main/java/io/nekohasekai/sagernet/utils/Commandline.ktbuildScript/lib/mieru.shlibcore/init.sh
| val clashApiSecret: String | ||
| @Synchronized get() { | ||
| var s = configurationStore.getString(Key.CLASH_API_SECRET) | ||
| if (s.isNullOrEmpty()) { | ||
| s = java.util.UUID.randomUUID().toString().replace("-", "") | ||
| configurationStore.putString(Key.CLASH_API_SECRET, s) | ||
| } | ||
| return s | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files and usages first
git ls-files | rg '^(app/src/main/java/io/nekohasekai/sagernet/database/DataStore\.kt|app/src/main/java/io/nekohasekai/sagernet/.*(ConfigBuilder|WebviewFragment)|.*configurationStore.*|.*Key\.kt)$' || true
echo '--- DataStore.kt ---'
sed -n '1,260p' app/src/main/java/io/nekohasekai/sagernet/database/DataStore.kt
echo '--- configurationStore usages ---'
rg -n "configurationStore|CLASH_API_SECRET|clashApiSecret" app/src/main/java -g '!**/build/**'
echo '--- Key definition ---'
rg -n "CLASH_API_SECRET" app/src/main/java
echo '--- storage backend clues ---'
rg -n "SharedPreferences|DataStore|MMKV|Room|ObjectBox|Preferences" app/src/main/javaRepository: hawkff/NekoBoxForAndroid
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- RoomPreferenceDataStore.kt (outline) ---'
ast-grep outline app/src/main/java/io/nekohasekai/sagernet/database/preference/RoomPreferenceDataStore.kt --view expanded || true
echo '--- RoomPreferenceDataStore.kt (relevant slices) ---'
sed -n '1,220p' app/src/main/java/io/nekohasekai/sagernet/database/preference/RoomPreferenceDataStore.kt
echo '---'
sed -n '220,520p' app/src/main/java/io/nekohasekai/sagernet/database/preference/RoomPreferenceDataStore.kt
echo '--- KeyValuePair.kt (DAO / conflict strategy) ---'
sed -n '1,220p' app/src/main/java/io/nekohasekai/sagernet/database/preference/KeyValuePair.kt
echo '--- startup / secret consumers ---'
sed -n '1,160p' app/src/main/java/io/nekohasekai/sagernet/SagerNet.kt
echo '---'
sed -n '220,280p' app/src/main/java/io/nekohasekai/sagernet/fmt/ConfigBuilder.kt
echo '---'
sed -n '60,120p' app/src/main/java/io/nekohasekai/sagernet/ui/WebviewFragment.ktRepository: hawkff/NekoBoxForAndroid
Length of output: 39838
Make the initial Clash API secret claim atomic across processes. @Synchronized only serializes one process, but configurationStore is shared and refreshed asynchronously. On a fresh install, main and bg can both generate different UUIDs before either sees the other's write, so the core and dashboard can end up using different secrets. Use a single insert-if-absent/transactional claim that returns the stored value.
🤖 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/database/DataStore.kt` around lines
187 - 195, The clashApiSecret getter in DataStore currently generates and stores
a UUID under `@Synchronized`, but that only protects one process and can race with
other processes reading the shared configurationStore. Replace the
read-then-generate flow with a single atomic insert-if-absent or transactional
claim that returns the already-stored value if present, and only creates one new
secret when missing. Use the existing clashApiSecret property and
configurationStore access as the place to centralize this cross-process-safe
claim.
| override suspend fun onUpdated(data: List<TrafficData>) { | ||
| try { | ||
| val positions = HashMap<Long, Int>(configurationIdList.size) | ||
| configurationIdList.forEachIndexed { index, id -> positions[id] = index } | ||
| val updates = ArrayList<Pair<ConfigurationHolder, TrafficData>>() | ||
| for (item in data) { | ||
| val index = positions[item.id] ?: continue | ||
| val holder = layoutManager.findViewByPosition(index) | ||
| ?.let { configurationListView.getChildViewHolder(it) } as ConfigurationHolder? | ||
| if (holder != null) updates.add(holder to item) | ||
| } | ||
| if (updates.isNotEmpty()) { | ||
| onMainDispatcher { | ||
| for ((holder, item) in updates) holder.bind(holder.entity, item) | ||
| } | ||
| } | ||
| } catch (e: Exception) { | ||
| Logs.w(e) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="app/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.kt"
echo "== File outline =="
ast-grep outline "$FILE" --view expanded || true
echo
echo "== Relevant line ranges =="
sed -n '1650,1765p' "$FILE" | cat -n
echo
echo "== Search for configurationIdList mutations and traffic update callbacks =="
rg -n "configurationIdList|cbTrafficUpdateList|postUpdate|runOnDefaultDispatcher|onUpdated\(data: List<TrafficData>\)|onUpdated\(data: TrafficData\)" app/src/main/java -SRepository: hawkff/NekoBoxForAndroid
Length of output: 30999
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Async dispatcher helpers =="
sed -n '1,120p' app/src/main/java/io/nekohasekai/sagernet/ktx/Asyncs.kt | cat -n
echo
echo "== ProfileManager traffic update dispatch =="
sed -n '1,240p' app/src/main/java/io/nekohasekai/sagernet/database/ProfileManager.kt | cat -n
echo
echo "== MainActivity traffic callbacks =="
sed -n '410,450p' app/src/main/java/io/nekohasekai/sagernet/ui/MainActivity.kt | cat -n
echo
echo "== ConfigurationFragment mutations around the reported area =="
sed -n '1550,1755p' app/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.kt | cat -nRepository: hawkff/NekoBoxForAndroid
Length of output: 22847
Move the batch traffic update lookup onto the main thread.
onUpdated(List<TrafficData>) runs on Dispatchers.Default via cbTrafficUpdateList -> ProfileManager.postUpdate, but it reads configurationIdList and calls layoutManager.findViewByPosition() / getChildViewHolder(). Those accesses should stay on the main thread; otherwise a concurrent post {} mutation can produce a stale index or recycled holder. Move the lookup into onMainDispatcher { ... } or configurationListView.post { ... } before binding.
🤖 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/ConfigurationFragment.kt` around
lines 1719 - 1739, The batch traffic update lookup is happening off the main
thread in onUpdated, but it reads configurationIdList and touches
layoutManager.findViewByPosition() and
configurationListView.getChildViewHolder(), which must be done on the UI thread.
Refactor ConfigurationFragment.onUpdated so the index resolution and holder
lookup happen inside onMainDispatcher { ... } or configurationListView.post {
... }, then bind the collected ConfigurationHolder/TrafficData pairs there to
avoid stale positions or recycled holders.
| # Immutable commit that MIERU_VERSION points to (pinned for reproducible builds; | ||
| # update together with MIERU_VERSION on any bump). | ||
| MIERU_COMMIT="${MIERU_COMMIT:-1532c85cc8ca08dff469326f35a3f027697c6950}" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does GitHub reliably support git fetch --depth 1 origin for a commit that is not the tip of any branch or tag?
💡 Result:
No, GitHub does not reliably support git fetch --depth 1 origin for commits that are not advertised as branch tips or tags [1][2]. In the Git protocol, servers generally only allow clients to fetch objects that are reachable from advertised references (like branches and tags) to prevent the exposure of non-public history and to manage performance costs [3][4][2]. While the underlying Git software supports configuration flags—such as uploadpack.allowReachableSHA1InWant—that could theoretically enable this, GitHub does not expose such arbitrary commit fetching via the Git protocol [1][4][2]. If you attempt to run git fetch --depth 1 origin on GitHub for a non-advertised commit, the command will typically fail [1][2]. To obtain a specific commit from a GitHub repository, recommended alternatives include: 1. Using the GitHub REST API or GraphQL API: You can use the GitHub API to download a tarball or zipball of a specific commit SHA, which bypasses the Git protocol entirely [5][2]. 2. Progressive Deepening: If you have a shallow clone of the repository, you can perform a shallow clone of the default branch and then use git fetch --deepen repeatedly to pull more history until the desired commit is reached [6][2]. 3. Full Clone: If the repository size permits, a full git clone remains the most reliable way to access any arbitrary commit in the history [2].
Citations:
- 1: GitSource cannot fetch an older commit SHA with --depth=1 knative/build#212
- 2: https://nikhedonia.net/articles/how-to-find-a-deadbeef
- 3: https://stackoverflow.com/questions/14872486/retrieve-specific-commit-from-a-remote-git-repository
- 4: Allow fetching commit by id isaacs/github#436
- 5: feat: support installing skills pinned to a commit SHA vercel-labs/skills#1439
- 6: https://stackoverflow.com/questions/48425221/fetch-specific-remote-commit-and-its-depth-in-shallow-clone-using-github-api
🌐 Web query:
GitHub git fetch by raw commit SHA shallow clone non-tip commit documented support uploadpack.allowReachableSHA1InWant
💡 Result:
The ability to fetch a specific commit SHA that is not at the tip of a branch (a non-tip commit) via a shallow clone is controlled by the server-side configuration uploadpack.allowReachableSHA1InWant [1][2][3]. When this setting is enabled on a Git server, it permits the server to accept a fetch request for any object (such as a specific commit SHA) that is reachable from any of the repository's ref tips, even if that commit is not explicitly advertised [2][4][5]. This functionality was introduced to allow clients to fetch specific commits—useful for submodules or CI/CD workflows—without needing to clone or fetch the entire repository history [2][5]. Key points regarding this feature include: 1. Server-Side Requirement: The Git server must explicitly set uploadpack.allowReachableSHA1InWant to true [2][5]. By default, this value is false [1][3]. Whether this is enabled on platforms like GitHub can vary depending on the specific infrastructure or configuration (e.g., GitHub Enterprise instances may default to disabled) [6]. 2. Functionality: When enabled, a client can perform a shallow fetch (e.g., git fetch --depth 1 origin <SHA>) for a commit that is not a branch or tag tip [2][7]. The server performs a reachability check to ensure the requested SHA is part of the repository history, which carries a computational cost [1][2][3]. 3. Documentation: This configuration is officially documented in the Git configuration documentation for upload-pack [1][3][8]. It is separate from uploadpack.allowTipSHA1InWant, which only allows fetching objects that are currently at the tips of refs [1][5]. 4. Client Usage: While modern Git clients support requesting these SHAs, the successful execution of such a request depends entirely on the server's policy [2][9]. If the server does not have this setting enabled, a fetch request for a non-advertised SHA will typically be rejected, forcing clients to fall back to fetching more history or the entire repository [4][6].
Citations:
- 1: https://github.com/git/git/blob/6dd3456a8ccbad6a0a6a31bc7192025617114243/Documentation/config/uploadpack.txt
- 2: git/git@68ee628
- 3: https://code.googlesource.com/git/+/refs/tags/v2.41.0/Documentation/config/uploadpack.txt
- 4: http://public-inbox.org/git/CAPig+cQe3=Y15+RBZCxry7-78kbNL6H0PfdYs=OV_DMifOWxaA@mail.gmail.com/T/
- 5: https://stackoverflow.com/questions/14872486/retrieve-specific-commit-from-a-remote-git-repository
- 6: Fetching for git_repository with a commit sha doesn't take the fast, shallow path bazelbuild/bazel#12174
- 7: https://github.com/robherley/shallow-fetch-sha
- 8: https://gitlab.utu.fi/taahol/git/-/blob/v2.46.3/Documentation/config/uploadpack.txt?ref_type=tags
- 9: Support SHA1 in source of RefSpec for fetch src-d/go-git#628
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files buildScript/lib/mieru.sh
wc -l buildScript/lib/mieru.sh
cat -n buildScript/lib/mieru.sh | sed -n '1,120p'Repository: hawkff/NekoBoxForAndroid
Length of output: 3529
Fetch the release tag instead of the raw commit SHA in buildScript/lib/mieru.sh:46-58 git fetch --depth 1 origin "$MIERU_COMMIT" still depends on GitHub accepting raw-SHA fetches, which is not guaranteed for non-tip commits. Even with this pin on a release-tag commit, the build can break if the tag is moved or the SHA is no longer advertised.
🤖 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 `@buildScript/lib/mieru.sh` around lines 22 - 24, The fetch logic in mieru.sh
still uses the raw MIERU_COMMIT SHA, which can fail when the commit is not
advertised or the tag moves; update the checkout flow around the git fetch call
to fetch and use the release tag or another stable ref instead of the bare SHA.
Keep MIERU_COMMIT as the pinned revision source if needed, but make the actual
fetch in the MIERU version handling code resolve via the tag/ref so the build
does not depend on GitHub accepting raw-SHA fetches.
The per-install token is for the local controller only; a config exported for sharing must not embed it. Mirrors the existing export gating in the selector path.
1 participant
Summary
Performance improvements on hot paths plus supply-chain and local-service-hardening
changes. Stacked on the bug-fix PR (merge that first).
Performance
of building a full throwaway configuration on every switch (mirrors the existing
builder logic; a keep-in-sync comment references the source lines).
of one write plus one listener round per profile; the existing full-group refresh
that follows delivers the UI update.
tick (one IPC + one main-thread hop + an O(1) lookup) instead of one call per
profile with an O(n) scan. The single-item callback is retained for the
selector-switch path; new interface methods have default bodies so existing
implementers are unaffected.
Security / supply chain
test workflows to a full commit SHA with a version comment (matching the one action
already pinned this way). No job logic changes.
and the bundled sidecar source (previously a moving tag) to exact commits, matching
the pinning the sibling sources already use. Fresh checkout dir + fail-fast on git
errors so an interrupted prior run can’t build from stale state.
externally installed provider, so an external package can’t shadow a binary the app
ships (external providers are still used for anything the app doesn’t bundle).
API (off by default) and pre-authenticate the bundled dashboard for the local
endpoint only. The token is added to the log-redaction list.
Testing
build-script syntax checked. Compilation, lint, and unit gates run in CI.
unauthenticated requests are rejected) before merge — to be verified on Android.
Follow-up (not in this PR)
repointing is tracked separately (needs an org-mirror step + lockfile regeneration).
Greptile Summary
This PR delivers batched-IPC performance improvements on hot paths (traffic updates, test-result writes, selector-reload checks) and a suite of supply-chain / local-API security hardening changes on top of the bug-fix base branch. Previously identified issues — the per-install Clash API token leaking into exported configs (
ConfigBuilder.kt) and the WebviewFragment URL-injection bugs (fragment-blinding, prefix ambiguity) — are fully addressed here.cbTrafficUpdateListis added to the AIDL interface with a default fallback body, so existing implementers are unaffected;TrafficLooperandConfigurationFragmentuse the new batch path to reduce IPC round-trips per tick.mieru.shandlibcore/init.shnow fetch by commit hash instead of a mutable tag/branch, withset -e/ explicit|| exit 1providing fail-fast guarantees.DataStore, redacted from logs viaCommandline.kt, injected into configs only when not exporting, and pre-authenticated into the embedded dashboard URL via properUri.buildUpon()query construction.Confidence Score: 5/5
Safe to merge after the on-device dashboard auth check noted in the PR description is confirmed.
All previously flagged issues are addressed: the Clash API secret is now guarded by !forExport in ConfigBuilder, and WebviewFragment uses Uri.buildUpon() with an exact host+port match to inject credentials safely. The new batch traffic path mirrors the pre-existing single-item pattern. Build-script changes are protected by set -e (mieru.sh) and explicit || exit 1 (libcore/init.sh). No new correctness or security issues were found in the diff.
No files require special attention beyond the on-device dashboard authentication verification called out in the PR description.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant TL as TrafficLooper participant Binder as ISagerNetServiceCallback (AIDL) participant SC as SagerConnection participant MA as MainActivity participant PM as ProfileManager participant CF as ConfigurationFragment.Listener Note over TL: Per-tick (1 s) TL->>TL: Build ArrayList(traffic.values) TL->>Binder: cbTrafficUpdateList(batch) Binder->>SC: cbTrafficUpdateList(stats) SC->>SC: runOnMainDispatcher SC->>MA: cbTrafficUpdateList(data) MA->>MA: runOnDefaultDispatcher MA->>PM: postUpdate(data) PM->>CF: "onUpdated(List<TrafficData>)" CF->>CF: Build positions map (O(n)) CF->>CF: Lookup visible holders (background) CF->>CF: onMainDispatcher CF->>CF: bind each holder (main thread) Note over TL: Selector-switch path (unchanged) TL->>Binder: cbTrafficUpdate(single) Binder->>SC: cbTrafficUpdate SC->>MA: cbTrafficUpdate MA->>PM: postUpdate(single) PM->>CF: onUpdated(TrafficData)%%{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"}}}%% sequenceDiagram participant TL as TrafficLooper participant Binder as ISagerNetServiceCallback (AIDL) participant SC as SagerConnection participant MA as MainActivity participant PM as ProfileManager participant CF as ConfigurationFragment.Listener Note over TL: Per-tick (1 s) TL->>TL: Build ArrayList(traffic.values) TL->>Binder: cbTrafficUpdateList(batch) Binder->>SC: cbTrafficUpdateList(stats) SC->>SC: runOnMainDispatcher SC->>MA: cbTrafficUpdateList(data) MA->>MA: runOnDefaultDispatcher MA->>PM: postUpdate(data) PM->>CF: "onUpdated(List<TrafficData>)" CF->>CF: Build positions map (O(n)) CF->>CF: Lookup visible holders (background) CF->>CF: onMainDispatcher CF->>CF: bind each holder (main thread) Note over TL: Selector-switch path (unchanged) TL->>Binder: cbTrafficUpdate(single) Binder->>SC: cbTrafficUpdate SC->>MA: cbTrafficUpdate MA->>PM: postUpdate(single) PM->>CF: onUpdated(TrafficData)Comments Outside Diff (1)
app/src/main/java/io/nekohasekai/sagernet/fmt/ConfigBuilder.kt, line 239-244 (link)buildConfigis also called withforExport = truefromProxyEntity.exportConfig(), which serialises the result to a sharable JSON file. Because theDataStore.enableClashAPIblock does not guard onforExport, the real per-install secret is written verbatim into every exported config. Anyone who receives the exported file (e.g. for troubleshooting) gains knowledge of the local Clash API token for that device. While the API is bound to loopback today, the token should be treated as a local capability credential and not exported.Reviews (6): Last reviewed commit: "security(config): omit the local API tok..." | Re-trigger Greptile