Read os.userInfo() from the passwd database - #33481
Conversation
|
Updated 9:56 PM PT - Jul 23rd, 2026
❌ @autofix-ci[bot], your commit 1101c82 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 33481That installs a local version of the PR into your bun-33481 --bun |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
WalkthroughThis PR changes ChangesuserInfo passwd rewrite
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@test/js/node/os/os.test.js`:
- Around line 205-208: The os.userInfo encoding coverage for the hex case is
incomplete because it asserts only username and homedir, unlike the buffer case.
Update the hex test in os.test.js to also verify hex.shell, using the same
pattern as the existing username and homedir assertions, so the hex path is
covered consistently.
- Around line 139-167: The subprocess-spawning tests in this block are still
running sequentially; update the affected `it(...)` cases that call
`userInfoWithPoisonedEnv` to run concurrently. Use `test.concurrent` or the
equivalent concurrent variant for the `is read from the operating system, not
the environment` and `reports the passwd entry of the effective uid` tests so
they can execute in parallel. Keep the existing assertions and `passwdEntry`
setup unchanged, only adjust the test declarations.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 94e08828-736e-47bd-bf40-ff48ade1c661
📒 Files selected for processing (8)
src/js/node/os.tssrc/libuv_sys/libuv.rssrc/runtime/hw_exports.rssrc/runtime/node/node_os.bind.tssrc/runtime/node/node_os.rssrc/runtime/node/types.rssrc/sys/lib.rstest/js/node/os/os.test.js
|
For reference: |
os.userInfo() built its result out of $USER and $SHELL, and discarded the options argument entirely. node reads the passwd entry of the effective uid (uv_os_get_passwd), so a caller could be handed whatever the parent process put in the environment. Replace it with getpwuid_r(geteuid()) on POSIX and uv_os_get_passwd on Windows, sharing the lookup loop with os.homedir(). Honor the encoding option, including encoding: "buffer". Match node's key order, its null-prototype result object, and its tolerance of a non-object options argument.
When the effective uid has no passwd entry (docker --user N, distroless, OpenShift arbitrary-uid), os.userInfo() and os.homedir() now throw a SystemError with code ERR_SYSTEM_ERROR and an info object, matching node's lib/os.js ERR_SYSTEM_ERROR(ctx) path. Previously they threw a plain Error with code ENOENT. Adds a setpriv-based regression test for the no-passwd-entry case.
ba1cb04 to
0f101bb
Compare
4a9caba to
1101c82
Compare
| fn throw_uv_error(global: &JSGlobalObject, err: bun_sys::Error) -> bun_jsc::JsError { | ||
| global.throw_value( | ||
| SystemError::from(err.to_system_error()).to_error_instance_with_info_object(global), | ||
| ) | ||
| } |
There was a problem hiding this comment.
🟡 throw_uv_error feeds err.to_system_error() into to_error_instance_with_info_object(), but those two disagree on what .message holds: to_system_error() produces the pre-formatted UVException string ("ENOENT: no such file or directory, uv_os_get_passwd"), while toErrorInstanceWithInfoObject expects the bare libuv description and wraps it again — yielding err.message = "A system error occurred: uv_os_get_passwd returned ENOENT (ENOENT: no such file or directory, uv_os_get_passwd)" and err.info.message = "ENOENT: no such file or directory, uv_os_get_passwd" instead of Node's "...(no such file or directory)" / "no such file or directory". Build the SystemError with just the bare label, the way the sibling get_priority path does (message: BunString::static_("no such process"), asserted byte-for-byte at the bottom of os.test.js).
Extended reasoning...
What the bug is. The new throw_uv_error helper (src/runtime/node/node_os.rs:788-792) composes two functions that have incompatible contracts on SystemError.message:
fn throw_uv_error(global: &JSGlobalObject, err: bun_sys::Error) -> bun_jsc::JsError {
global.throw_value(
SystemError::from(err.to_system_error()).to_error_instance_with_info_object(global),
)
}bun_sys::Error::to_system_error() (src/sys/Error.rs:409-469) writes .message in Node's UVException format — "{CODE}: {label}, {syscall}" — via the cursor at lines 419-439. Its doc comment says "format taken from Node.js 'exceptions.cc' … Local<Value> UVException". That format is meant to be used verbatim by to_error_instance() (the non-info-object path).
SystemError__toErrorInstanceWithInfoObject (src/jsc/bindings/bindings.cpp:2337-2372), on the other hand, treats err.message as the bare libuv description and wraps it at line 2347:
auto message = makeString("A system error occurred: "_s, syscallString, " returned "_s, codeString, " ("_s, messageString, ")"_s);and copies messageString verbatim into err.info.message at line 2366.
Step-by-step trace. Take the Ok(None) arm of user_info() on POSIX (no passwd entry for the effective uid — the docker run --user 12345 / distroless case the PR's own setpriv test exercises):
bun_sys::Error::from_code(ENOENT, Tag::uv_os_get_passwd)→throw_uv_error.to_system_error()runsfill_system_error_common(sets.code = "ENOENT",.syscall = "uv_os_get_passwd",.errno), then formats.message = "ENOENT: no such file or directory, uv_os_get_passwd"(the LIBUV_ERROR_MAP label for ENOENT is"no such file or directory").SystemError::from(...)is a straight field copy.to_error_instance_with_info_objectbuildserr.message = "A system error occurred: uv_os_get_passwd returned ENOENT (ENOENT: no such file or directory, uv_os_get_passwd)"anderr.info.message = "ENOENT: no such file or directory, uv_os_get_passwd".
Node.js produces err.message = "A system error occurred: uv_os_get_passwd returned ENOENT (no such file or directory)" and err.info.message = "no such file or directory" (Node's lib/internal/errors.js SystemError reads ctx.message = uvErrmapGet(ctx.errno)?.[1], which is the bare libuv description).
The same double-formatting applies to the Err(errno) arm of user_info, both arms of homedir (POSIX, when $HOME is unset), and the Windows uv_os_homedir / uv_os_get_passwd failure paths — all four call sites route through throw_uv_error.
Why existing code doesn't prevent it. The PR's own setpriv test only inspects e.name, e.code, e.syscall, and e.info?.code, never e.message or e.info.message, so it passes. Compare with the sibling get_priority path in the same file (node_os.rs:700-712), which hand-builds the SystemError with message: BunString::static_("no such process") — just the bare label — and whose output is asserted byte-for-byte in os.test.js (expect(err.message).toBe("A system error occurred: uv_os_getpriority returned ESRCH (no such process)") and expect(err.info).toEqual({ ..., message: "no such process", ... })). throw_uv_error is the first caller to feed to_system_error()'s output into the info-object path.
Impact. Only the human-readable err.message and err.info.message diverge; the structured fields callers actually branch on (name, code, syscall, errno, info.code, info.errno, info.syscall) are all correct. This is a rarely-hit error path (missing passwd entry / libuv failure). Filed as a nit — worth fixing given the PR description's "byte-identical to node" claim and REVIEW.md's "error messages are reviewed word-for-word", and to stay consistent with the neighboring getPriority path, but nothing functionally breaks.
Fix. Have throw_uv_error build the SystemError with .message set to just the libuv description label (what LIBUV_ERROR_MAP[errno] returns). Either hand-build it like get_priority/set_priority1 do, or add a helper on bun_sys::Error that returns the bare label alongside code/syscall/errno (analogous to to_shell_system_error but keyed by LIBUV_ERROR_MAP) — to_system_error() is the wrong producer for this consumer.
os.userInfo()was assembled from$USERand$SHELL, and itsoptionsargument was discarded (let _ = options; // TODO:). Any caller that trusts it for the real login shell or account name gets whatever the parent process put in the environment, and when the effective uid has no passwd entry (docker run --user 12345, distroless images, OpenShift arbitrary-uid) it fabricates{username: "unknown", shell: "unknown", ...}where node throws.Repro
Running as uid 0, whose real passwd row is
root ... /root /bin/bash:And under a uid with no passwd entry:
os.userInfo(42)also threw aTypeErrorwhere node ignores a non-objectoptions.Cause
node_os.rs'suser_info()readenv_var::USER/env_var::SHELL, tookhomedirfromos.homedir()(which checks$HOMEfirst), and never looked atoptions. node'sGetUserInfocallsuv_os_get_passwd(), which isgetpwuid_r(geteuid())on POSIX andGetUserProfileDirectoryW+GetUserNameWon Windows, then encodes each string field withStringBytes::Encode(..., encoding).Only
os.homedir()legitimately consults the environment;os.userInfo().homediris documented as coming from the OS.Fix
getpwuid_r(geteuid()), reusing theEINTR/ERANGEretry loop thatos.homedir()already had. It is extracted intowith_euid_passwd()so there is one copy.uid/gidnow come frompw_uid/pw_gid, andshellisnullwhen the entry has none, as inuv__getpwuid_r.uv_os_get_passwd()+uv_os_free_passwd(), sousernameandhomedirstop tracking%USERNAME%/%USERPROFILE%.uid/gidstay-1andshellstaysnull, matching node'sstatic_cast<int32_t>(pwd.uid & 0xFFFFFFFF).SystemErrorwithcode: 'ERR_SYSTEM_ERROR',syscall: 'uv_os_get_passwd'and aninfoobject carrying{errno, code: 'ENOENT', message, syscall}, matching node'slib/os.jsthrow new ERR_SYSTEM_ERROR(ctx)path.os.homedir()with$HOMEunset and no passwd entry gets the same treatment (it previously threw a plainErrorwithcode: 'ENOENT').encodingis honored forusername,homedir, andshell, including"buffer". An unrecognized or non-string encoding falls back to utf8, like node'sParseEncoding(..., UTF8).uid, gid, username, homedir, shell), and a non-objectoptionsis ignored instead of rejected.os.userInfo.lengthis now1, also matching node.bun_sys::Taggainsuv_os_get_passwdso the thrownerr.syscallreads the same as node's.Verification
test/js/node/os/os.test.jsgrows auserInfoblock. The load-bearing test spawns two children whose account environment variables disagree on everything (USER,LOGNAME,USERNAME,SHELL,HOME,USERPROFILE) and asserts they still report the same account; on Linux a second test pins the result against the effective uid's/etc/passwdrow. Twosetpriv-based tests (Linux + root only) cover the no-passwd-entry throw for bothuserInfo()andhomedir(). The rest cover the encoding matrix, the object shape, and the options-coercion cases.Seven of the eight new tests fail on the unfixed build and pass with the fix.
test/js/node/test/parallel/test-os.js,test-os-homedir-no-envvar.jsandtest-os-userinfo-handles-getter-errors.jsstill pass, and all 10 rust target triples compile (bun run rust:check-all).Output is now byte-identical to node v26.3.0 across every encoding and options shape
Run against the real passwd row while
$HOME,$SHELL,$USERand$LOGNAMEwere all poisoned.Not fixed here
os.userInfo({ encoding: "utf-16le" })still falls back to utf8. That spelling is missing from the sharedENCODING_MAPinsrc/runtime/node/types.rs(which instead carries autf16-lealias node rejects), sofs.readFileSync(f, "utf-16le")andCryptoHasherare wrong in the same way. Fixing the table touches those surfaces and wants its own tests, so it is left for a follow-up.ucs2,ucs-2andutf16leall work.#29248 also touches
os.homedir(); it is an independent fix for$HOMEbeing cached, and it calls out this WindowsuserInfodivergence as a follow-up.Fixes #25171