Six small review-list items rolled together — all CI/dev-tooling, no
runtime behaviour change.
#12 Dockerfile: pin Rust 1.95.0 and cargo-ndk 4.1.2 (was floating
`stable` + latest cargo-ndk on monthly rebuild). Versions live
in ENV vars to make the next bump a one-line edit.
#13 Add shellcheck to lint job. SC2034/SC3043 excluded — Magisk
reads SKIPUNZIP externally; Android's /system/bin/sh (mksh on
Pixel) does support `local` despite POSIX. Verified locally
that the 11 .sh files (module-side + dev tooling) pass.
shellcheck baked into the CI image via apt; inline apt-get
fallback covers the window before image rebuild.
#24 ci.yml keystore.properties: replace heredoc with `printf '%s\n'`.
Heredoc without single-quoted EOF re-expands $, backticks and
backslashes in the password — printf takes the value verbatim.
#31 scripts/release.py::patch_file now hard-fails when a regex
pattern doesn't match (was silently leaving stale versions).
#32 Split rotate_fragments_into_history into rotate + delete steps
so release.py can save_json + write_md *before* unlinking the
fragment files. If anything in between fails, fragments are
still on disk and the run is retryable.
#37 codegen-interfaces.py: emit `assert!(matches_vpn(…), msg)` /
`assert!(!matches_vpn(…), msg)` instead of
`assert_eq!(matches_vpn(…), true/false, msg)` —
clippy::bool_assert_comparison was firing on every generated
row under `cargo clippy --tests`. Both generated test modules
regenerated. CI's clippy steps now also pass `--tests` so this
class of regression is caught.
#106 added \`cargo install uniffi-bindgen --version "^0.29" --locked\`,
which fails ci-image rebuilds:
error: could not find \`uniffi-bindgen\` in registry \`crates-io\`
with version \`^0.29\`
Two errors in the original change:
1. The crate Gobley installs is \`gobley-uniffi-bindgen\` (its own
fork on crates.io at 0.3.7), not upstream \`uniffi-bindgen\`.
2. Gobley installs the binary into \`app/build/gobley-tools-install/
uniffi-bindgen/\`, not \`~/.cargo/bin\`. A globally pre-installed
binary wouldn't satisfy the task's UP-TO-DATE check anyway.
\`org.gradle.caching=true\` from #106 already makes
\`installUniffiBindgen\` go UP-TO-DATE on warm runs (verified locally),
so the optimisation is in effect via the build cache instead.
Profiling the warm-cache run on PR #105 showed three remaining hot spots
in the Gradle phase:
installUniffiBindgen 52s ← cargo install on every CI build
cargoBuildAndroidArm64Debug 30s ← Rust crate compile
lintAnalyze* (3 variants) 43s ← AGP Lint × main + unit + androidTest
This PR cuts the first one entirely and trims the third.
- Dockerfile: pre-install uniffi-bindgen 0.29.x in the CI image so
Gobley's :app:installUniffiBindgen task finds it ready instead of
rebuilding it from sources on every run. Triggers a ci-image
rebuild on merge — wait for that workflow to finish before merging
consumers (or the first lint/lsposed run will still hit the old
image and behave as before).
- lsposed/gradle.properties: enable build cache + configuration
cache. Verified locally: `./gradlew :app:assembleDebug
--configuration-cache` reports "Configuration cache entry stored"
cleanly with Gobley 0.3.7 + AGP 8.9.3 + Kotlin 2.1.20.
- lsposed/app/build.gradle.kts: `lint { checkTestSources = false }`.
Skips lintAnalyzeDebugUnitTest / lintAnalyzeDebugAndroidTest. Test
sources here are pure JVM unit-test logic — functional bugs caught
by :app:testDebugUnitTest, no Android-lifecycle code to lint.
Deliberately leave `checkReleaseBuilds` at its default so ad-hoc
`./gradlew :app:lint` still catches R8/ProGuard issues.
- .github/workflows/ci.yml: `:app:lint` -> `:app:lintDebug`. Lints
the debug variant only on PRs; release-variant Lint stays
available locally / for future tag-time CI.
- docs/development.md: refresh local-lint snippet.
Expected effect on warm cache (cumulative on top of PR #105):
lint 286s -> ~190s (3m10s, -32%)
lsposed 227s -> ~130s (2m10s, -42%)
Repo had ~1800 lines of Python (kmod/build.py, scripts/*, zygisk/build.py,
portshide/build-zip.py) with no formatter or linter. Long-lived scripts
like scripts/release.py and scripts/codegen-interfaces.py benefit from
catching unused-import / undefined-name / outdated-syntax issues early.
pyproject.toml — ruff config, target-py312, line-length 100,
rules E F W I B UP SIM. Excludes zygisk/third_party,
target/, .claude/.
ci.yml — astral-sh/ruff-action@v4 for `format --check` and `check`,
ahead of the slow Rust/Gradle steps so it fails fast.
docs/development.md — add `uvx ruff …` to the local-lint snippet.
Cleanup applied (`ruff format` + `ruff check --fix`):
- reformat: kmod/build.py, scripts/{changelog_lib,codegen-interfaces,
release,stats}.py, zygisk/build.py
- I001: split multi-name imports onto separate lines after the
sys.path.insert prelude (kmod/build.py, zygisk/build.py)
- E501 manual: wrap one console.print line in scripts/release.py
Stdlib-only invariant from scripts/build_lib.py is preserved — ruff is
a dev/CI tool, not imported at runtime.
The two slowest CI jobs were both Gradle:
lint 6m41s (Android lint = 264s of it)
lsposed 6m44s (assembleRelease = 307s of it)
Cargo was already cached; Gradle was not.
Changes:
- gradle/actions/setup-gradle@v6 in lint and lsposed jobs. Caches
~/.gradle/caches, wrapper, configuration cache. cache-read-only on
PRs so only main pushes write it.
- lint job now has a cargo cache too (was missing). Combined key for
both Cargo.lock files.
- lint: Android lint and Kotlin unit tests run in one Gradle
invocation (./gradlew :app:lint :app:testDebugUnitTest). Saves a
second Gobley/AGP configuration phase + JVM startup.
- lsposed: assembleDebug for PRs and main pushes, assembleRelease only
for v* tags. R8/ProGuard runs only when the artifact actually goes
into a release.
- Drop --no-daemon: with one invocation per job (or one warm daemon
between two), keeping the daemon is cheaper than killing it.
- Drop the manual `export ANDROID_NDK_ROOT="$ANDROID_NDK_HOME"`. The
CI image's Dockerfile already sets ANDROID_NDK_ROOT (line 63), so
the workaround is redundant.
Two related changes that ship together because they touch the same
build-script + docs surface and were verified together on-device.
16 KiB alignment
- zygisk/build.rs: pass `-Wl,-z,max-page-size=16384` to lld so the
cdylib's LOAD segments line up on 16 KiB pages. NDK r28+ already
does this by default, but the flag keeps r27 builds compatible.
- lsposed/native/build.rs: new file, same flag, for libvpnhide_checks.so.
- docs/development.md: bumped the NDK requirement to r28+ and noted
the 16 KiB rationale.
Verified via `llvm-readelf -l`: both libvpnhide_zygisk.so and
libvpnhide_checks.so now show `Align 0x4000` on every LOAD segment.
Unified build entry points
- kmod/build.py replaces kmod/build-zip.py. Single script that
auto-detects whether to build natively (we're inside the DDK image
or `--kdir` was passed) or to spawn `ghcr.io/ylarod/ddk-min` via
podman/docker. CI uses the same script with `--inside-container`.
- zygisk/build-zip.py renamed to zygisk/build.py for symmetry; logic
unchanged.
- kmod/BUILDING.md rewritten — local build is now one command:
`./kmod/build.py --kmi android14-6.1` (or `--all`). The old
hand-rolled podman/docker recipes are gone.
- .github/workflows/ci.yml updated to call the new entry points.
The DDK image tag in CI now has a comment pointing at
`DDK_IMAGE_TAG` in kmod/build.py as the source of truth.
- README.{md,en.md}, kmod/README.md, zygisk/README.md, docs/releasing.md,
scripts/build_lib.py: reference updates.
- README.en.md: also fixes a "bacame" typo and tightens the Windows
zygisk-build note (the aux.rs / libgit2 issue is still real).
Verified end-to-end on Pixel 8 Pro (husky, android14-6.1, Android 16):
APK installs, kmod + zygisk modules load, all 26 self-checks PASS in
Enforcing, 22/26 PASS in Permissive (the same 4 by-design FAILs as
before — kmod doesn't cover those paths in Permissive).
Returning -EMSGSIZE from rtnl_fill_ifinfo for VPN devs hangs RTM_GETLINK
dumps on android14-6.1: the dump iterator retries the same entry forever
on an empty skb. inet6_fill_ifaddr in this same file already documented
and worked around the issue using skb_trim + return-0; mirror that for
rtnl_fill_ifinfo so RTM_GETLINK skips VPN ifaces cleanly.
Reproduced on Pixel 8 Pro (husky, android14-6.1) with SELinux Permissive:
the netlink_getlink check inside dev.okhsunrog.vpnhide hung on splash
screen. After the fix all three netlink-backed checks (getifaddrs,
netlink_getlink, netlink_getroute) PASS in Permissive in <1s.
`open_filtered_proc_net` used a fixed 65 536-byte stack array as the
read buffer. On devices with several hundred concurrent TCP sockets
(p2p clients, browsers with many tabs, dev devices with tethering),
`/proc/net/tcp6` exceeds 64 KiB — the previous code silently dropped
the tail and handed the truncated content back to the caller via
memfd. An app reading its own socket list could miss real entries,
which is a correctness bug, not a security one (the truncated tail
might contain non-VPN sockets the app needs).
Replace the stack array with a thread-local growable Vec<u8>:
* Initial capacity 64 KiB matches the previous fixed buffer, so the
first call (and most subsequent ones) does no reallocation.
* When the read fills capacity, `Vec::reserve(8 * 1024)` triggers
amortised doubling — large files cost O(log size) reallocations.
* `clear()` between calls keeps capacity intact: the second and later
calls in the same thread are zero-allocation.
* Bounded memory: per-thread overhead is `max-observed-size`. On
Android only one or two threads ever open /proc/net/* (network-
info worker, diagnostics probe), so the steady state is well under
a megabyte of RAM.
Trade-off: had to drop the `const { ... }` block on the thread_local
because `Vec::with_capacity` is not const fn. The per-`with()` lazy-
init flag check is negligible relative to the syscalls we're about
to issue.
Empirically: on the device used for testing /proc/net/tcp6 stays at
~15 KiB (no truncation triggered with the old buffer either). The
fix is preventive for power-user scenarios where the limit is
actually reached. SELinux on stock Android blocks /proc/net/* for
untrusted_app domain entirely, so the new path is exercised only on
permissive ROMs or by privileged callers — testing was therefore
limited to verifying no regression on the happy path.
The three files written under /data/system/ to coordinate state between
the LSPosed system_server hook and the app — vpnhide_uids.txt,
vpnhide_hidden_pkgs.txt, vpnhide_observer_uids.txt — were chmodded
0644 root:root. /data/system/ itself is mode 0775 system:system,
traversable by untrusted apps, so any "other"-readable file there is
both enumerable (`ls /data/system/`) and openable by name. Untrusted
apps could:
cat /data/system/vpnhide_uids.txt # all target UIDs
cat /data/system/vpnhide_hidden_pkgs.txt # the hide list
cat /data/system/vpnhide_observer_uids.txt # observer UIDs
If the reader's own UID is in vpnhide_uids.txt, that's a positive
"vpnhide is filtering me right now" detection — strictly stronger than
the presence-of-marker fingerprint we already closed for
vpnhide_hook_active in PR #100.
Switch every write site to mode 0640 + chown root:system. system_server
runs as UID 1000 with `system` (GID 1000) in its supplementary groups,
so it still gets read via the group bit. Untrusted apps fall to the
"other" octet (now ---) and get EACCES on open.
Empirically verified on Pixel:
before: 644 root:root → `cat` from untrusted shell succeeds
after: 640 root:system → untrusted shell EACCES;
`su system -c cat` (uid=1000) reads fine,
mirroring what system_server sees
Boot-time service.sh in both kmod and zygisk modules also include an
idempotent migration block that re-stamps any pre-PR files left at
0644 by an older version on the next boot. Closes#36 in REVIEW.
Workflow-level `contents: write` was granted to every job — lint,
zygisk build, lsposed build, portshide build, kmod matrix — even
though only the release job needs it (to create the draft GitHub
release via softprops/action-gh-release@v2). Tighten to the
least-privilege default of `contents: read` at the workflow level
and override with `permissions: contents: write` on the release job
alone. Reduces blast radius if any of the lint/build jobs ever runs
untrusted code from a PR.
The Diagnostics → Export debug bundle ran logcat with -s filtering
on VPNHideTest, VpnHide, and VpnHide-Dashboard tags only. zygisk
writes under `vpnhide-zygisk` (zygisk/src/lib.rs LOG_TAG), so the
zip the user ends up sharing was missing every native-side hook
trace — exactly the half that's hardest to reproduce locally.
`cp $ZYGISK_TARGETS $ZYGISK_MODULE_TARGETS 2>/dev/null; true` hid
both stderr and the exit code, so a read-only mount or SELinux
denial silently broke "edits in the app aren't picked up by zygisk
on next app launch" — the user had no signal.
Rewrite as `if [ -d ... ]; then cp ... 2>&1; fi`, capture exit and
output, and log a warn() only when both indicate a real failure
(non-zero exit + non-empty stderr). The dir-not-installed case
naturally produces exit=0 + empty output and stays quiet.
The else-branch in sanitizeLinkProperties' stacked-links loop set
`modified = true` whenever `stackedCopy !== value`, i.e. after any
successful clone. Cloning is the common case (the catch fallback is
just a defensive shim for a missing copy ctor), so the flag was
effectively always-true: every non-empty stacked map triggered the
trailing `stacked.clear() + putAll(filtered)`, even when no VPN data
was present and no modification happened.
Drop the `stackedCopy !== value` disjunct. `modified` now reflects
only real sanitization changes — the rewrite is skipped when there's
nothing to write back, and the clones in `filtered` are discarded
along with the never-mutated original entries staying in place. No
behavioural change for VPN-stripping callers; just less wasted work
on the non-VPN-bearing path.
inotify MODIFY fires on every write(2), not just on close. With the
current single-write `echo $b64 | base64 -d > $file` writers (see
ShellUtils.ensureSelfInTargets and the AppPicker save path) it
effectively always fires before CLOSE_WRITE, causing the hook to
invalidate the cache and re-read the file. If a writer ever switches
to a multi-write pattern (`echo line >> file` repeated, or anything
that flushes mid-buffer), the hook would see a partially-written
file. CLOSE_WRITE + MOVED_TO covers both single-write and atomic-
rename writers without the mid-write race.
`/data/system/vpnhide_hook_active` was being chmodded to 0644 via
`setReadable(true, false)` so the VPN Hide app could read it
directly. But the app already runs every other read of files under
/data/system/ via `suExec` (see DashboardData.kt's read of this
exact file), so the world-readable bit was redundant — and a
discoverable marker for anti-tamper SDKs that scan /data/system/
for known filenames.
Drop the line; the file stays at the default 0640 system:system
mode, readable by system_server (which writes it) and by anything
running as root (which is how the app reads it).
`c` iterates one character at a time over the test name string;
a single character is never the empty string, so the disjunct was
always False. The remaining `0x20 <= ord(c) < 0x7F` already
correctly rejects non-ASCII names, so behaviour is unchanged.
The function is called from hooks.rs (hooked_getifaddrs and
walk_getifaddrs_vpn). It's been used since the helper was added —
the allow attribute is leftover from before its callers landed.
clippy --release and cargo test both clean without it.
`proc_create()` returns NULL on failure (typically OOM at boot or
/proc not yet mounted). The previous code stored the NULL into
`targets_entry` and continued — `pr_info(": loaded")` fired, the
kretprobes were registered, but userspace had no way to write the
target UID list, so the module silently filtered nothing.
Treat /proc/vpnhide_targets failure as fatal: log an error,
unregister any probes that did succeed, and return -ENOMEM so
insmod surfaces the failure to the caller. /proc/vpnhide_debug
stays best-effort — losing the debug toggle just means no verbose
logging, the rest of the module is still useful.
Six places parsed `pm list packages -U` output with
`grep "^package:${pkg} "`, which treats `pkg` as a regex — dots in
package names cross-match, in theory mapping `com.x.y` to a
hypothetical `comXxXy` package. In practice Android won't let two
such packages coexist, so this has never bit anyone, but the fix is
free and unifies with the literal `awk '$1 == p'` pattern that
portshide/vpnhide_ports_apply.sh has been using all along.
Touched:
* kmod/module/service.sh, zygisk/module/service.sh — boot-time UID
resolution for kmod and lsposed/zygisk targets.
* lsposed/.../{AppPickerScreen,AppHidingScreen,ShellUtils}.kt — three
call-sites that build shell pipelines from Kotlin to resolve UIDs
for /proc/vpnhide_targets, the system_server hook uids file, and
the package-visibility observer uids file.
* lsposed/.../DashboardData.kt — the self-multi-profile detection
that warns when vpnhide is installed in more than one profile.
Six unrelated drift fixes that accumulated since they were last
synced. Each is independent of the rest:
* README{.en,}.md — kmod claim "filters /proc/net/*" trimmed to
/proc/net/route. The other /proc/net files are SELinux-blocked
for untrusted apps and the coverage table already says so.
* kmod/README.md — hook table and architecture note updated from
dev_ifconf to sock_ioctl. dev_ifconf gets inlined by Clang LTO
on GKI 5.10 so the kretprobe silently never fires; sock_ioctl
has been the actual hook target since the vpnhide_kmod.c fix.
* zygisk/README.md — five inline hooks now, not four (recv was
added separately because bionic's recv tail-calls recvfrom).
Also clarified pre_app_specialize runs in the forked child, not
zygote, matching the lifecycle block in lib.rs.
* docs/development.md — JDK requirement matches CI image (17, not
21); document ANDROID_NDK_ROOT quirk for Gobley; CI lint list
expanded to match what ci.yml actually runs.
* docs/development.md + lsposed/README.md — explain Gobley (the
Gradle plugin pair that builds lsposed/native/ and bundles the
.so + UniFFI Kotlin bindings into the APK). Previously absent
from all *.md.
The previous comments around `dir_fd` and `CACHED_TARGETS` were ambiguous
about execution context ("before zygote forks any app", "every app
forked after us"), reading as if the module ran in zygote. It doesn't —
NeoZygisk dlopen's the .so in the already-forked child, so every Rust
static is fresh per app launch and `targets.txt` is reread on every
force-stop + restart with no zygote-side cache to invalidate.
Replace with an explicit lifecycle block in the module doc citing the
canonical sources (Magisk api.hpp v2, zygisk-api-rs trait docs,
NeoZygisk loader) plus a section spelling out the implications for
state. Tighten the dir_fd and CACHED_TARGETS comments to match.
Both call sites duplicated the IN_GETIFADDRS guard wiring around
the real_getifaddrs lookup + ifaddrs walk. Extract the wiring so
a guard fix lands in one place — the kind of drift that hit the
closed PR #92.
It was added with comment "Temporary check to verify the recvfrom
hook works." The hook is verified in production; the probe carries
no diagnostic value beyond what check_netlink_getlink already gives.
Both forms came in with the codegen split (#91) but no [[vpn]]
rule has ever used them — the only `suffix=` rules are `digits`
(`wlan` test vector + `if` from #93). The grammar surface paid
for itself in ~150 lines of dead C/Rust helpers + their tests.
Drop them from VALID_KINDS, the parser, the C/Rust/Kotlin
emitters, and the helper test cases. If a future rule needs
either form, reintroduce alongside the rule that needs it.
Re-ran the codegen; tests pass for all four targets.
The zygisk job has had this for a while; lsposed/native was rebuilding
the uniffi/serde/quinn deps from scratch every run. Same shape as the
zygisk cache, separate cache key so the two jobs don't fight over a
shared `target/` (different crate, different artifacts).
Dashboard's `isVpnActiveSync` and `runJavaProtectionCheck` both
maintained their own `listOf("tun", "wg", "ppp", "tap", "ipsec",
"xfrm")` + `startsWith` checks, which missed names the kmod/zygisk
filter actually hides — `if<N>` from issue #86, `MyVPN`, `wg-client`,
substring catch-all. The dashboard would say "VPN not active" while
the filter was happily suppressing the renamed tunnel.
Move the `/sys/class/net + operstate` walk into a single
`isVpnActiveBlocking()` in ShellUtils.kt that uses
`IfaceLists.isVpnIface` (the same matcher fed to all three modules
from data/interfaces.toml). DiagnosticsScreen.isVpnActive becomes a
thin `withContext(IO)` wrapper around it. The link-properties
ifname check in `runJavaProtectionCheck` switches to the same
matcher.
The date line in changelog.d/fixed-notification-time-increased-to-make-it-fc9a.md
read `gi_2026-04-25_` instead of `_2026-04-25_`. _DATE_LINE in changelog_lib.py
anchors with `^_…_$` under re.MULTILINE, so parse_fragment would have raised
ValueError and blocked the next release. No effect on rendered output — date
isn't emitted into CHANGELOG.md, only used for fragment sort order.
Unauthenticated GitHub API hits rate-limit the moment you re-run; the
script then iterated the {"message": ...} error dict as a list of releases
and crashed on release["assets"]. Now uses GITHUB_TOKEN or `gh auth token`
when available and raise_for_status surfaces the real HTTP error.
Bar length was `count // 2`, which overflowed the terminal for any
non-trivial release and rich silently truncated with an ellipsis. Now
normalized: `count * bar_w // max_count` where bar_w is computed from
console.width minus the name and count columns plus padding.
- SIOCGIF{FLAGS,MTU,CONF} casts use `as _` so the host build
picks the right Ioctl type (matches PR #94).
- check_proc_file uses the codegen-backed `is_vpn_iface` over
whitespace tokens; the local VPN_PREFIXES array is gone now
that lib.rs imports `matches_vpn` from `generated::iface_lists`.
Real cause of the lsposed/lint NPE on CI: Gobley's
RustAndroidTarget.ndkToolchainDir resolves the NDK by checking, in
order, the explicit `ndkRoot` parameter, `<sdkRoot>/ndk/<latestVersion>`,
then `$ANDROID_NDK_ROOT`. The CI image installs the NDK as a separate
tree at /opt/android-ndk and exports `ANDROID_NDK_HOME`, not
`ANDROID_NDK_ROOT` — so all three lookups return null and Gobley's `!!`
produces a bare `NullPointerException` during `:app` configuration.
Locally my shell exports `ANDROID_NDK_ROOT` (Android Studio convention),
which is why the issue only surfaces in CI.
Bake `ANDROID_NDK_ROOT` into the CI Dockerfile and export it inline in
the lint / lsposed gradle steps so this PR's CI passes before the image
rebuilds. Revert the prior `rustup target add x86_64-unknown-linux-gnu`
and `--stacktrace` debug additions — that was a wrong-hypothesis
workaround (the host target is already installed by `rustup-init`).
Gobley's cargo plugin enumerates Kotlin targets at gradle configure
time and queries rustup for each one — including the JVM host target,
even though we never build for it (`androidUnitTest = false` skips
wiring the JVM cargo build into Android unit tests, but the build
entry is still created at configure time).
Without `x86_64-unknown-linux-gnu` installed, that lookup returns
null and `:app:lint` / `assembleRelease` die with a bare
`NullPointerException` during project configuration.
Add the target as a workflow step in the lint and lsposed jobs so
this PR's CI passes immediately, and bake it into the CI Dockerfile
so subsequent image rebuilds carry it.
