Conversation
Android's app seccomp policy denies openat2(2) and fchmodat2(2) with SECCOMP_RET_TRAP, killing the process with SIGSYS instead of returning ENOSYS, so the existing errno fallbacks never run and bun install dies while linking package bins under Termux. Detect Android at runtime (kernel release string or the ANDROID_ROOT/ANDROID_DATA env vars Android init sets) and take the existing fallbacks without issuing the syscalls: openat2_beneath fails with ENOSYS so bin linking uses its realpath containment check, openat2_in_root goes straight to plain openat, and lchmod uses an O_PATH + /proc/self/fd chmod emulation (libc's fchmodat AT_SYMLINK_NOFOLLOW can itself invoke fchmodat2 internally). Fixes #39060
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it adds a runtime platform-detection heuristic that gates syscall availability in bun_sys and introduces a new lchmod emulation path, a human look would still be worthwhile.
What was reviewed:
is_android_kernel()caching/detection and its inputs (cached_uname,c_field_bytes, the newANDROID_*env-var accessors) — helpers exist and signatures match.openat2_beneathENOSYS return traced to its only caller (src/install/bin.rs:1416), which already routes ENOSYS to the realpath-containment fallback.lchmod_no_fchmodat2: fd closed on every exit path;/proc/self/fd/Nbuffer sizing (32 bytes vs. ≤25 needed);ZStr::from_bufNUL-at-lenprecondition satisfied by the trailing\0in the format string.
Extended reasoning...
Overview
The PR touches three files: two new ANDROID_ROOT/ANDROID_DATA string env-var declarations in src/bun_core/env_var.rs, ~90 lines in src/sys/lib.rs (a cached is_android_kernel() runtime probe, guards on openat2_beneath/openat2_in_root/lchmod that skip openat2(2)/fchmodat2(2) when it returns true, and a new lchmod_no_fchmodat2 helper implementing musl's O_PATH + chmod("/proc/self/fd/N") fallback), and a new Linux-only test that installs an Android-equivalent seccomp filter and runs bun install under it.
Security risks
openat2_beneath is used by bin-target validation to enforce that a package's declared bin path resolves inside the package directory. On the Android path it now returns ENOSYS unconditionally, and the sole caller in src/install/bin.rs maps ENOSYS to None, which falls through to the pre-existing realpath containment check. That fallback already exists for pre-5.6 kernels, so no new containment gap is introduced — but a human should confirm they're comfortable with the env-var heuristic being able to force this weaker path on non-Android hosts (both ANDROID_ROOT and ANDROID_DATA must be non-empty, so accidental collision is unlikely). The lchmod emulation opens with O_PATH|O_NOFOLLOW and rejects symlinks with EOPNOTSUPP before chmod'ing, matching the documented libc semantics.
Level of scrutiny
Medium-high. This is the low-level syscall wrapper layer shared by install, directory routes, and node:fs, and the change is heuristic-driven (uname substring / env vars) rather than a compile-time gate. The mechanism, rationale, and rejected alternative (SIGSYS-handler probe unsafe under multi-threaded bin linking) are well-argued in the description, and the test genuinely reproduces the failure mode with a real seccomp filter. Still, this is not a mechanical change I'd want to land without a maintainer glancing at the detection heuristic and the /proc/self/fd fallback.
Other factors
The implementation follows house conventions (bun_core::strings, bun_core::env_var, ZStr::from_buf, bun_sys error tagging), closes the O_PATH fd on every branch, and the test drains stdout/stderr concurrently, gates on isLinux, distinguishes host-can't-build (skip) from source regression (throw), and asserts both exit status and the observable effect (symlink created, target executable). No prior human review comments are outstanding.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find bugs. The ENOSYS return from openat2_beneath correctly routes bin.rs:1416 to its realpath fallback, lchmod_no_fchmodat2 closes the O_PATH fd on every exit and the /proc path fits the 32-byte buffer, and the seccomp test asserts the bin is actually chmod'd rather than silently skipped. That said, this touches shared syscall wrappers and adds an env-var-triggered downgrade of the RESOLVE_BENEATH hardening, and the comment-cop bot is still flagging several doc comments after the consolidation commit — a human should confirm both the detection heuristic and whether the remaining comments are acceptable.
Extended reasoning...
Overview
The PR gates three Linux syscall sites in src/sys/lib.rs — openat2_beneath, openat2_in_root, and the raw fchmodat2 in lchmod — behind a new cached is_android_kernel() runtime check, so they are never issued on Android where seccomp delivers SIGSYS instead of ENOSYS. It adds a new lchmod_no_fchmodat2 fallback (O_PATH + chmod("/proc/self/fd/N"), musl's strategy), registers ANDROID_ROOT/ANDROID_DATA in env_var.rs, and adds a Linux-only test that compiles a seccomp wrapper mimicking Android's SECCOMP_RET_TRAP filter for syscalls 437/452 and runs bun install of a local dependency with a bin under it.
Security risks
openat2(RESOLVE_BENEATH) is the primary containment check that prevents a package's bin entry from escaping the package directory. On Android (or on any Linux host where a user sets both ANDROID_ROOT and ANDROID_DATA), this now short-circuits to ENOSYS and bin.rs falls back to its pre-existing realpath containment check — the same path already taken on kernels < 5.6, so it's not a new hole, but it is an env-var-controlled downgrade of a hardening layer. The new lchmod fallback rejects symlinks with EOPNOTSUPP before chmod'ing via /proc/self/fd, matching libc semantics, so no symlink-follow TOCTOU is introduced there.
Level of scrutiny
Medium-high. src/sys/lib.rs is core infrastructure used by install, directory routes, and node:fs; the new fallback has manual fd lifecycle across four exit paths; and the detection heuristic (uname release substring OR two env vars both set) is a design choice that affects which security path is taken on non-Android Linux. This is the kind of platform-gated FFI change REVIEW.md calls out for cross-platform review.
Other factors
The comment-cop github-actions bot left ten automated "paragraph-long comment" flags; the author responded with a "consolidate Android detection comments" commit, but the bot re-fired on five locations in the consolidated version (env_var.rs:43, sys/lib.rs:1956/1977/2873/2904). The remaining comments read to me as necessary context (they explain kernel behavior, not code hacks), but per the guidelines outstanding automated feedback should be resolved by a maintainer, not auto-approved past. The test is well-constructed (fails-right check via SIGSYS, asserts the bin was actually made executable, clean skips when cc/headers/seccomp are unavailable) but is Linux-only and was not run locally per the PR footer.
|
Updated 12:12 PM PT - Aug 15th, 2026
🔄 @robobun, the build for your commit |
|
CI status: the remaining failures (socket.test.ts keep-alive, shell mv FIFO timeout, inspect-error-leak, watch-mode kill-signal, bun-patch, napi 1_hello_world) are all flaky, they passed on retry or when run alone, and none touch this diff. Same set flaked on the previous run. The new seccomp test passes on all lanes. Ready for review. |
|
#39775 fixes this without Android detection. It installs a SIGSYS handler at startup that makes a trapped syscall return ENOSYS, so openat2_beneath takes its realpath fallback and lchmod takes its fchmodat fallback (the libc-internal fchmodat2 attempt gets ENOSYS from the same handler and libc emulates). The same handler covers close_range at startup (#30766, #30769) and pidfd_open. Its test traps 437 and 452 during bun install of a local package with a nested bin and checks the link and the mode. If #39775 lands, this PR is not needed. |
|
Checked this PR against #39775 at d6171ce (the handler that turns a seccomp trap into ENOSYS). I built that branch as is. It contains none of the
This PR stays open for now. #39403 carries it as commit 6dd1b74. The two changes touch different files and do not conflict.
|
This branch has not been deployed
1 participant
Problem
bun installis killed by SIGSYS (exit 159, "Bad system call") on Android/Termux while linking package bins (Android LD_PRELOAD workaround forbun installSIGSYScrashes caused byopenat2andfchmodat2#39060).openat2(2)andfchmodat2(2)withSECCOMP_RET_TRAP: the process gets SIGSYS instead of anENOSYSreturn, so the existing errno-based fallbacks never get a chance to run.openat2(RESOLVE_BENEATH)in bin-target validation (src/install/bin.rs:1416viasrc/sys/lib.rs), raw syscall 452 insys::lchmod(src/sys/lib.rs) when chmod'ing bin targets, andopenat2(RESOLVE_IN_ROOT)used by directory routes.Fix
bun_sys(is_android_kernel):cfg!(target_os = "android"), an "android" kernel release string (uname -r, all GKI kernels carry it), or theANDROID_ROOT/ANDROID_DATAenv vars Android init sets for every process.openat2_beneathfails withENOSYS, so bin linking takes its existing realpath containment check (same security property, noopenat2).openat2_in_rootgoes straight to the existing plain-openatfallback.lchmoduses anO_PATH+chmod("/proc/self/fd/N")emulation (musl's own fallback strategy). It cannot call libc'sfchmodat(.., AT_SYMLINK_NOFOLLOW)instead: current glibc and musl implement that flag by tryingfchmodat2(2)internally first, which would be SIGSYS-killed.test/cli/install/bun-install-android-seccomp.test.ts: compiles a small helper that installs the same seccomp filter Android uses (SECCOMP_RET_TRAPfor syscalls 437/452) and runsbun installof a local dependency with abinentry under it, withANDROID_ROOT/ANDROID_DATAset. Fails with SIGSYS before the fix, installs and chmods the bin after.test/cli/install/bun-install-native-binlink.test.ts(bin containment checks),test/js/bun/http/serve-directory-routes.test.ts(openat2_in_rootpath), andtest/js/node/fs/fs-stat-seccomp-linux.test.ts: all pass.Background
SECCOMP_RET_TRAP, which delivers SIGSYS to the process (fatal by default). Android uses the trap action for syscalls outside its allowlist, so "syscall not available" looks like a crash, not an error return.openat2(2)(Linux 5.6) isopenatplus resolve flags:RESOLVE_BENEATHfails path resolution that escapes the starting directory (bun uses it to validate that a package's bin target stays inside the package),RESOLVE_IN_ROOTtreats the starting directory as/(directory routes). Both uses already have fallbacks for kernels older than 5.6.fchmodat2(2)(Linux 6.6) isfchmodatwith a workingAT_SYMLINK_NOFOLLOW, i.e.lchmod: chmod that refuses to follow a final-component symlink. The emulation opens the file withO_PATH|O_NOFOLLOW(no permissions needed, does not follow a final symlink) and chmods it through the/proc/self/fd/Nmagic link.target_os = "linux"binaries that Termux runs on the Android kernel, so the detection must happen at runtime; compile-timecfggates can't catch this.Reproduction details
Reproduced on plain Linux with a seccomp wrapper installing the equivalent filter (
SECCOMP_RET_TRAPfor nrs 437/452), matching the reporter's strace exactly:Blocking each syscall individually kills the install either way, so handling only one of them is not enough. A SIGSYS-handler probe (as used for
close_rangein #30769) was considered and rejected here: these syscalls are first issued from bin linking, where multiple threads run, and a temporarily-installed process-wide SIGSYS handler thatsiglongjmps is only safe while single-threaded.no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/bun-install-android-seccomp.test.ts