feat: olcRTC tunnel client (in-process gomobile sidecar)#110
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:
📝 WalkthroughWalkthroughAdds olcRTC as a new proxy type with URI parsing/serialization, database and settings wiring, sidecar process integration, a Go-sidecar build pipeline, and CI updates for building and verifying ChangesolcRTC Proxy Type
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 @.github/workflows/ci.yml:
- Around line 137-155: The olcrtc build job is missing the JDK setup required by
`buildScript/lib/olcrtc.sh` and the `Native Build` step that runs
`gomobile-matsuri bind`. Add the same `actions/setup-java` step used by the
other gomobile AAR build jobs before `Install Golang`/`Native Build`, so a cache
miss does not rely on a preinstalled JDK. Keep the fix scoped to the `olcrtc`
job in the CI workflow.
- Around line 139-147: The olcRTC cache key is based only on file contents, so
metadata-only changes to scripts can reuse a stale libolcrtc.aar. Update the
olcRTC Status step in the ci workflow to generate the status file using git
ls-files -s over the buildScript inputs instead of cat ... | sha1sum, matching
the existing metadata-aware pattern used elsewhere. Keep the olcRTC Cache step
keyed off that status file so cache invalidation reflects both content and mode
changes.
In `@app/src/main/java/io/nekohasekai/sagernet/bg/proto/BoxInstance.kt`:
- Around line 110-115: The olcRTC singleton check in BoxInstance’s OlcrtcBean
handling is only scoped to the current instance, so it can miss overlaps across
reloads or profile switches. Move the guard to shared process-wide state used by
BoxInstance/OlcrtcInstance, check that global state before creating a new olcRTC
instance, and clear it when OlcrtcInstance is torn down in close() so only one
process-wide olcRTC can exist at a time.
- Around line 301-313: The olcRTC readiness wait in BoxInstance should respect
strict mode instead of always enforcing a 60s minimum. In the block that gathers
OlcrtcInstance values and calls waitReady, change the timeout selection so it
uses the same strict timeout cap as the URL-test logic when strict = true, while
keeping the longer minimum only for non-strict behavior. Reference the existing
timeout handling near the OlcrtcInstance readiness path and reuse the same
strict-mode rule already used elsewhere in this class.
In `@app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt`:
- Around line 57-60: Reject unsupported carriers in OlcrtcFmt parsing by
validating the extracted carrier against the only supported values modeled by
the bean. In OlcrtcFmt.parseLink and the corresponding import path around the
carrier check, keep the non-empty guard but add an explicit allowlist for jitsi,
telemost, and wbstream so typoed links fail during import with a clear
invalid-link error instead of being accepted and breaking later.
- Around line 40-46: In OlcrtcFmt’s export/parsing flow, unnamed profiles with
roomId values containing $ are being emitted in a way that parseOlcrtc() later
splits at the last $ and truncates the room ID. Update the export logic for the
unnamed-profile path so room IDs are escaped or otherwise encoded before
concatenating with the comment delimiter, and make the corresponding import
handling in parseOlcrtc() decode it back correctly. Apply the same fix to the
related code path referenced by the other occurrence so both export sites
preserve $ characters in room IDs.
In
`@app/src/main/java/io/nekohasekai/sagernet/ui/profile/OlcrtcSettingsActivity.kt`:
- Around line 40-58: The OlcrtcBean.serialize() path currently saves clientId
from DataStore without validating it, which allows invalid profiles to be
persisted. Add a fail-fast require check for clientId alongside the existing
roomId and keyHex validation in OlcrtcSettingsActivity.serialize(), using a
clear error message that matches the existing style so blank CLIENT_HELLO device
IDs cannot be saved.
🪄 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: ca99f7c4-5f61-4982-a59a-4c104b684c11
📒 Files selected for processing (28)
.depot/workflows/android-instrumented.yml.depot/workflows/build-apk.yml.depot/workflows/lint.yml.depot/workflows/unit-tests.yml.github/workflows/ci.yml.gitignoreapp/src/androidTest/java/io/nekohasekai/sagernet/database/SagerDatabaseMigrationTest.ktapp/src/main/AndroidManifest.xmlapp/src/main/java/io/nekohasekai/sagernet/Constants.ktapp/src/main/java/io/nekohasekai/sagernet/bg/proto/BoxInstance.ktapp/src/main/java/io/nekohasekai/sagernet/bg/proto/OlcrtcInstance.ktapp/src/main/java/io/nekohasekai/sagernet/database/DataStore.ktapp/src/main/java/io/nekohasekai/sagernet/database/ProxyEntity.ktapp/src/main/java/io/nekohasekai/sagernet/database/SagerDatabase.ktapp/src/main/java/io/nekohasekai/sagernet/fmt/ConfigBuilder.ktapp/src/main/java/io/nekohasekai/sagernet/fmt/KryoConverters.javaapp/src/main/java/io/nekohasekai/sagernet/fmt/TypeMap.ktapp/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcBean.javaapp/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.ktapp/src/main/java/io/nekohasekai/sagernet/ktx/Formats.ktapp/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.ktapp/src/main/java/io/nekohasekai/sagernet/ui/profile/OlcrtcSettingsActivity.ktapp/src/main/res/menu/add_profile_menu.xmlapp/src/main/res/values/arrays.xmlapp/src/main/res/values/strings.xmlapp/src/main/res/xml/olcrtc_preferences.xmlapp/src/test/java/io/nekohasekai/sagernet/fmt/OlcrtcFmtTest.ktbuildScript/lib/olcrtc.sh
| - name: Checkout | ||
| uses: actions/checkout@v5 | ||
| - name: olcRTC Status | ||
| run: cat buildScript/lib/olcrtc.sh buildScript/init/env.sh buildScript/init/env_ndk.sh | sha1sum > olcrtc_status | ||
| - name: olcRTC Cache | ||
| id: cache | ||
| uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 | ||
| with: | ||
| path: | | ||
| app/libs/libolcrtc.aar | ||
| key: ${{ hashFiles('.github/workflows/*', 'olcrtc_status') }}-${{ env.OLCRTC_COMMIT }}-olcrtc-ci | ||
| - name: Install Golang | ||
| if: steps.cache.outputs.cache-hit != 'true' | ||
| uses: actions/setup-go@v6 | ||
| with: | ||
| go-version: '1.26.4' | ||
| - name: Native Build | ||
| if: steps.cache.outputs.cache-hit != 'true' | ||
| run: ./run lib olcrtc |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add the missing JDK setup to the olcrtc build job.
This is the only gomobile AAR build in this workflow without actions/setup-java. buildScript/lib/olcrtc.sh runs gomobile-matsuri bind, so a cache miss currently depends on Java already being present on the runner and can fail on a fresh image.
Proposed fix
olcrtc:
name: Native Build (olcRTC)
runs-on: namespace-profile-nekoyay
needs:
- guard
steps:
- name: Checkout
uses: actions/checkout@v5
+ - name: Setup Java
+ uses: actions/setup-java@v5
+ with:
+ distribution: 'temurin'
+ java-version: '17'
- name: olcRTC Status
run: cat buildScript/lib/olcrtc.sh buildScript/init/env.sh buildScript/init/env_ndk.sh | sha1sum > olcrtc_status📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Checkout | |
| uses: actions/checkout@v5 | |
| - name: olcRTC Status | |
| run: cat buildScript/lib/olcrtc.sh buildScript/init/env.sh buildScript/init/env_ndk.sh | sha1sum > olcrtc_status | |
| - name: olcRTC Cache | |
| id: cache | |
| uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 | |
| with: | |
| path: | | |
| app/libs/libolcrtc.aar | |
| key: ${{ hashFiles('.github/workflows/*', 'olcrtc_status') }}-${{ env.OLCRTC_COMMIT }}-olcrtc-ci | |
| - name: Install Golang | |
| if: steps.cache.outputs.cache-hit != 'true' | |
| uses: actions/setup-go@v6 | |
| with: | |
| go-version: '1.26.4' | |
| - name: Native Build | |
| if: steps.cache.outputs.cache-hit != 'true' | |
| run: ./run lib olcrtc | |
| - name: Checkout | |
| uses: actions/checkout@v5 | |
| - name: Setup Java | |
| uses: actions/setup-java@v5 | |
| with: | |
| distribution: 'temurin' | |
| java-version: '17' | |
| - name: olcRTC Status | |
| run: cat buildScript/lib/olcrtc.sh buildScript/init/env.sh buildScript/init/env_ndk.sh | sha1sum > olcrtc_status | |
| - name: olcRTC Cache | |
| id: cache | |
| uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 | |
| with: | |
| path: | | |
| app/libs/libolcrtc.aar | |
| key: ${{ hashFiles('.github/workflows/*', 'olcrtc_status') }}-${{ env.OLCRTC_COMMIT }}-olcrtc-ci | |
| - name: Install Golang | |
| if: steps.cache.outputs.cache-hit != 'true' | |
| uses: actions/setup-go@v6 | |
| with: | |
| go-version: '1.26.4' | |
| - name: Native Build | |
| if: steps.cache.outputs.cache-hit != 'true' | |
| run: ./run lib olcrtc |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 137-138: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 around lines 137 - 155, The olcrtc build job is
missing the JDK setup required by `buildScript/lib/olcrtc.sh` and the `Native
Build` step that runs `gomobile-matsuri bind`. Add the same `actions/setup-java`
step used by the other gomobile AAR build jobs before `Install Golang`/`Native
Build`, so a cache miss does not rely on a preinstalled JDK. Keep the fix scoped
to the `olcrtc` job in the CI workflow.
| is OlcrtcBean -> { | ||
| // olcRTC runs in-process (gomobile lib), not as a GuardedProcess. | ||
| // It is a process-global singleton, so reject a second olcRTC entry. | ||
| if (externalInstances.values.any { it is OlcrtcInstance }) { | ||
| error("olcRTC: only one olcRTC profile can run at a time") | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make the olcRTC singleton guard process-global.
Line 113 only checks externalInstances on the current BoxInstance. That does not protect against overlapping BoxInstance lifecycles during reload/profile switches, so two instances can still race the process-global Olcrtc singleton and later stop each other in close(). Move this guard to shared process-wide state and release it when the olcRTC instance is torn down.
🤖 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/bg/proto/BoxInstance.kt` around
lines 110 - 115, The olcRTC singleton check in BoxInstance’s OlcrtcBean handling
is only scoped to the current instance, so it can miss overlaps across reloads
or profile switches. Move the guard to shared process-wide state used by
BoxInstance/OlcrtcInstance, check that global state before creating a new olcRTC
instance, and clear it when OlcrtcInstance is torn down in close() so only one
process-wide olcRTC can exist at a time.
| // Trailing comment after the LAST '$' (room URLs do not contain '$'). | ||
| var comment = "" | ||
| val dollar = body.lastIndexOf('$') | ||
| if (dollar >= 0) { | ||
| comment = body.substring(dollar + 1) | ||
| body = body.substring(0, dollar) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve $ in room IDs when exporting unnamed profiles.
parseOlcrtc() treats the last $ as the comment delimiter. When name is blank, a roomId containing $ is exported raw and re-imports as a truncated room plus a synthetic comment.
Proposed fix
- if (name.isNotBlank()) sb.append('$').append(name)
+ if (name.isNotBlank() || '$' in roomId) sb.append('$').append(name)
return sb.toString()Also applies to: 138-139
🤖 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/fmt/olcrtc/OlcrtcFmt.kt` around
lines 40 - 46, In OlcrtcFmt’s export/parsing flow, unnamed profiles with roomId
values containing $ are being emitted in a way that parseOlcrtc() later splits
at the last $ and truncates the room ID. Update the export logic for the
unnamed-profile path so room IDs are escaped or otherwise encoded before
concatenating with the comment delimiter, and make the corresponding import
handling in parseOlcrtc() decode it back correctly. Apply the same fix to the
related code path referenced by the other occurrence so both export sites
preserve $ characters in room IDs.
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 `@app/src/main/java/io/nekohasekai/sagernet/bg/proto/BoxInstance.kt`:
- Around line 322-330: The readiness check in BoxInstance should not treat
olcRTC/MasterDnsVPN as a global flag for every pending port. Update the logic
around hasMasterDnsVpn, hasOlcrtc, and the pending-port handling near the
readiness timeout/failure path so the ports are tracked per sidecar/bean and
only those specific ports get the extended window or hard-fail behavior. Use the
existing config.externalIndex, chain.values, and requireBean() flow to associate
each pending port with its owning MasterDnsVpnBean or OlcrtcBean, and keep
strict mode as the only case that hard-fails all remaining pending ports.
- Around line 116-121: The olcRTC setup in BoxInstance.buildOlcrtcArgs currently
falls back to empty SOCKS credentials via creds?.first ?: "" and creds?.second
?: "", which can start the loopback proxy without auth; change this path to fail
closed when localProxyCredentials[port] is missing or incomplete. Ensure the
code either verifies creds is present before calling buildOlcrtcArgs or
throws/returns an error so olcRTC is not launched insecurely, and keep the fix
localized around the creds lookup and buildOlcrtcArgs call site.
In `@app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt`:
- Around line 165-176: The Olcrtc argument builder is missing the app readiness
deadline flag, so the sidecar always uses its default timeout. Update
buildOlcrtcArgs() in OlcrtcFmt.kt to emit -ready-timeout-ms using the same
timeout value that BoxInstance/DataStore.connectionTestTimeout relies on, so the
Go wrapper and app wait on the same deadline. Keep the change localized to the
args list construction and ensure the new flag is included alongside the
existing transport and DNS options.
In `@buildScript/lib/olcrtc-src/main.go`:
- Around line 40-67: The readiness wait in main is still hardcoded to the
default instead of using the user-configured timeout, so `mobile.WaitReady`
should be driven by the parsed `-ready-timeout-ms` flag. Update the startup flow
in `main` to pass the existing `readyMillis` value through the
`mobile.WaitReady` call, keeping the `StartWithTransport` and `WaitReady`
symbols aligned with the app’s readiness setting.
In `@buildScript/lib/olcrtc.sh`:
- Around line 74-80: The scratch module setup in olcrtc.sh still leaves the
build dependent on network access because `go mod tidy` resolves external
dependencies like `golang.org/x/sys` at build time. Replace the tidy step in the
`BUILD` scratch-module flow with a pinned dependency source, such as committing
and using a checked-in `go.sum` or vendored dependencies, so `go build` can run
offline after the `go.mod edit -replace` step.
🪄 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: 6928c91b-0314-449b-90b1-144396722bb4
📒 Files selected for processing (13)
.depot/workflows/android-instrumented.yml.depot/workflows/build-apk.yml.depot/workflows/lint.yml.depot/workflows/unit-tests.yml.github/workflows/ci.yml.gitignoreapp/src/main/java/io/nekohasekai/sagernet/bg/Executable.ktapp/src/main/java/io/nekohasekai/sagernet/bg/proto/BoxInstance.ktapp/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.ktapp/src/main/java/io/nekohasekai/sagernet/plugin/PluginManager.ktbuildScript/lib/olcrtc-src/go.modbuildScript/lib/olcrtc-src/main.gobuildScript/lib/olcrtc.sh
💤 Files with no reviewable changes (3)
- .depot/workflows/lint.yml
- .depot/workflows/unit-tests.yml
- .depot/workflows/android-instrumented.yml
✅ Files skipped from review due to trivial changes (2)
- buildScript/lib/olcrtc-src/go.mod
- .gitignore
| # Stage the wrapper in a scratch build dir and point it at the cloned olcRTC via a | ||
| # replace directive, so `go build` never has to resolve a pseudo-version online. | ||
| BUILD="$(pwd)/.olcrtc-wrapper" | ||
| rm -rf "$BUILD" | ||
| mkdir -p "$BUILD" | ||
| cp "$SRC/main.go" "$SRC/go.mod" "$BUILD/" | ||
| ( cd "$BUILD" && go mod edit -replace "github.com/openlibrecommunity/olcrtc=$WORK" && go mod tidy ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== staging logic =='
sed -n '74,90p' buildScript/lib/olcrtc.sh
echo
echo '== wrapper module files =='
fd -HI '^(go\.mod|go\.sum|vendor)$' buildScript/lib/olcrtc-src
echo
echo '== wrapper go.mod =='
sed -n '1,120p' buildScript/lib/olcrtc-src/go.modRepository: hawkff/NekoBoxForAndroid
Length of output: 1091
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== olcrtc wrapper directory listing =='
find buildScript/lib/olcrtc-src -maxdepth 2 -type f | sort
echo
echo '== any vendor directories in repo =='
find . -type d -name vendor | sort
echo
echo '== any go.sum files near wrapper =='
find buildScript/lib -name 'go.sum' -o -name 'vendor' | sort
echo
echo '== all references to golang.org/x/sys in repo =='
rg -n "golang\.org/x/sys" -S .Repository: hawkff/NekoBoxForAndroid
Length of output: 1448
go mod tidy still makes this build network-dependent. The scratch module has no checked-in go.sum or vendor tree, so a clean runner still has to resolve golang.org/x/sys at build time. Use a pinned lock/vendor set instead of tidying in the build.
🤖 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/olcrtc.sh` around lines 74 - 80, The scratch module setup in
olcrtc.sh still leaves the build dependent on network access because `go mod
tidy` resolves external dependencies like `golang.org/x/sys` at build time.
Replace the tidy step in the `BUILD` scratch-module flow with a pinned
dependency source, such as committing and using a checked-in `go.sum` or
vendored dependencies, so `go build` can run offline after the `go.mod edit
-replace` step.
| transport = DataStore.olcrtcTransport | ||
| vp8Fps = DataStore.olcrtcVp8Fps | ||
| vp8BatchSize = DataStore.olcrtcVp8Batch | ||
| dnsServer = DataStore.olcrtcDnsServer |
There was a problem hiding this comment.
DataStore.olcrtcDnsServer is declared as String? (no default), so it returns null when the field has never been written. This null is stored directly in OlcrtcBean.dnsServer. Later, buildOlcrtcArgs calls dnsServer.ifBlank { dnsFallback } on what Kotlin sees as a platform type String!; if the value is actually null that call throws NullPointerException at connect time, silently crashing the sidecar start. All other optional-string fields (carrier, transport) have default values in their DataStore declarations; dnsServer needs an explicit null-coalesce here instead.
| dnsServer = DataStore.olcrtcDnsServer | |
| dnsServer = DataStore.olcrtcDnsServer ?: "" |
20c1dc5 to
ae36a82
Compare
Add olcRTC profile import/export, settings, config generation, and sidecar launch support. Include URL-test readiness handling, sidecar CI wiring, and JVM coverage for sanitized diagnostics.
ae36a82 to
6b0471a
Compare
Summary
Adds support for connecting through olcRTC, an encrypted TCP-over-WebRTC tunnel carried over common meet services. Modeled as a profile that starts olcRTC's in-process loopback SOCKS5 listener (gomobile
libolcrtc.aar) which a sing-boxsocksoutbound dials.Built as its own gomobile .aar so olcRTC's heavy dependency graph (pion/webrtc, livekit, kcp, grpc, ...) stays out of libcore's pinned sing-box module graph. Clones upstream at an immutable pinned commit (no fork needed; the Android protect fd-hook is already upstream).
Changes
buildScript/lib/olcrtc.sh+./run lib olcrtc: gomobile bind./mobile->libolcrtc.aar(pinned olcRTC + pinned MatsuriDayo gomobile commits)olcrtcNamespace job + Depot build steps;libolcrtc-aar-ciwired into every :app-compiling jobOlcrtcBean/OlcrtcFmt(olcrtc://URI incl.&cid=client-id extension), settings UI, TypeMap/ProxyEntity/KryoConvertersOlcrtcInstance: in-process start/waitReady/stop, VpnService.protect fd-hook, single-instance guard, readiness gating, loopback SOCKS auth, non-Google resolver defaultolcrtcBeancolumn (+ 9->10 migration test)OlcrtcFmtround-trip unit testTested
spotlessKotlinCheckgreen locally (JVM-only; APK/AAR build is CI-only per repo policy)Greptile Summary
This PR adds olcRTC (encrypted TCP-over-WebRTC) as a new proxy profile type, shipping the client as a native sidecar binary (
libolcrtc.so) built from a Go wrapper around the olcRTC mobile package, following the same child-process pattern as mieru/naive/masterdnsvpn.OlcrtcBean,OlcrtcFmt(URI parser/emitter witholcrtc://scheme and&cid=extension), settings UI, DB v10 AutoMigration, TypeMap/KryoConverters, and a unit + migration test suite.BoxInstancewires loopback SOCKS credentials and a two-tier readiness timeout (15 s strict / 60 s live) matching the MasterDnsVPN pattern;ConfigBuilderforces the carrier signaling host todns-directto avoid fake-IP loops on fakeip VPNs.Commandlinegains flag-level and output-pattern redaction for key, room ID, clientId, and SOCKS credentials, with a dedicatedCommandlineTest.Confidence Score: 4/5
Safe to merge once the olcrtc.sh commit pin is aligned with the CI env value; all other integration points follow established patterns.
The olcrtc.sh script defaults to commit ca5828cf… while both CI workflow files pin OLCRTC_COMMIT=3e970ab8…. Developers who run ./run lib olcrtc locally without setting the env var build a different libolcrtc.so than what is shipped in released APKs, making local end-to-end testing of the sidecar against a production build unreliable. Everything else — DB migration, URI codec, sidecar startup, log redaction, and CI caching — is correctly implemented and well-tested.
buildScript/lib/olcrtc.sh (OLCRTC_COMMIT default out of sync with CI) and app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt (payloadEnd search edge case with '<' in room URL).
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant App as NekoBox App participant BI as BoxInstance participant CB as ConfigBuilder participant PM as PluginManager participant SC as libolcrtc.so sidecar participant olc as olcRTC mobile lib participant Carrier as Meet Service (Jitsi/etc) participant SB as sing-box (socks outbound) App->>CB: buildConfig(OlcrtcBean) CB->>CB: generate loopback SOCKS creds CB->>CB: force carrier host to dns-direct CB->>CB: NUL-join args into pluginConfigs[port] App->>BI: init() / startPlugin() BI->>PM: initPlugin(olcrtc-plugin) PM-->>BI: libolcrtc.so path BI->>SC: exec(libolcrtc.so -carrier jitsi -room ... -key ... -socks-port N ...) SC->>SC: installProtectedDefaults(protect_path, dns) SC->>olc: mobile.StartWithTransport(...) olc->>Carrier: WebRTC signaling (protected sockets) Carrier-->>olc: ICE / room join olc-->>SC: ready SC->>SC: bind 127.0.0.1:N SOCKS5 listener SC-->>olc: mobile.WaitReady() BI->>BI: waitForSidecars() (port probe) SB->>SC: SOCKS5 connect (with creds) SC->>olc: tunnel TCP over WebRTC%%{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 App as NekoBox App participant BI as BoxInstance participant CB as ConfigBuilder participant PM as PluginManager participant SC as libolcrtc.so sidecar participant olc as olcRTC mobile lib participant Carrier as Meet Service (Jitsi/etc) participant SB as sing-box (socks outbound) App->>CB: buildConfig(OlcrtcBean) CB->>CB: generate loopback SOCKS creds CB->>CB: force carrier host to dns-direct CB->>CB: NUL-join args into pluginConfigs[port] App->>BI: init() / startPlugin() BI->>PM: initPlugin(olcrtc-plugin) PM-->>BI: libolcrtc.so path BI->>SC: exec(libolcrtc.so -carrier jitsi -room ... -key ... -socks-port N ...) SC->>SC: installProtectedDefaults(protect_path, dns) SC->>olc: mobile.StartWithTransport(...) olc->>Carrier: WebRTC signaling (protected sockets) Carrier-->>olc: ICE / room join olc-->>SC: ready SC->>SC: bind 127.0.0.1:N SOCKS5 listener SC-->>olc: mobile.WaitReady() BI->>BI: waitForSidecars() (port probe) SB->>SC: SOCKS5 connect (with creds) SC->>olc: tunnel TCP over WebRTCComments Outside Diff (1)
app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt, line 1033-1060 (link)roomIdnot validated against the delimiter set before URI emissiontoUri()validatesclientIdandnameagainstDELIMITERSbefore embedding them, butroomIdis written verbatim into the URI. Jitsi room IDs arehttps://…URLs, and a URL with a query string containing$(percent-decoded) would corrupt the parser'slastIndexOf('$')comment-stripping step. While$is not legal unencoded in a URL path, a user might paste an already-decoded string. Addingrequire(roomId.none { it == '$' })(the only delimiter that can realistically appear in a URL and silently mangle the parse) makes the contract explicit and symmetric with thenamecheck.Reviews (11): Last reviewed commit: "feat(olcrtc): add Android tunnel client" | Re-trigger Greptile