Repository navigation
Description
Record of a pre-existing production defect in buzz-terminal's Unix shell lookup, surfaced by a hosted CI failure. It is not a regression from PR #7192 (that PR changes no buzz-terminal/terminal-path files); recording it separately so the residual race stays visible regardless of CI rerun outcomes.
Unsafe code (exact-head permalink)
desktop/src-tauri/crates/buzz-terminal/src/shell.rs L103–L116 @ 63a1c47a
passwd_shell() calls non-reentrant getpwuid() and copies pw_shell out of the libc-owned static passwd struct. The SAFETY comment promises no intervening call in the current thread, but Linux documents getpwuid as MT-Unsafe race:pwuid: the returned storage may be overwritten by a passwd-database call from any thread. glibc's getXXbyYY.c unlocks its internal NSS lock before returning the pointer, so a concurrent getpwuid*/getpwnam* call can rewrite the struct between libc's return and our copy — the copy itself is the race window. resolve_shell also evaluates passwd_shell() eagerly even when an explicit valid shell was passed (L82), so every concurrent resolver use exercises the unsafe lifetime. Production caller: desktop/src-tauri/src/terminal_runtime.rs:421.
This code is unchanged between integration base 7f8c2e5c and head 63a1c47a (git diff --exit-code base head -- desktop/src-tauri is clean; the PR diff is 11 Welcome/E2E files) — the defect predates that PR's diff.
Observed CI failure (attempt 1, preserved)
- Run: https://github.com/block/buzz/actions/runs/34631406730 (attempt 1) — job Desktop Core
103369033534, Ubuntu 24.04, step "Desktop Tauri compiled-flag verification" (just desktop-tauri-test-compiled-flags→ finalBUZZ_BUILD_DEMO_SLUG=… cargo test --workspacesweep, exit 101). - Earlier in the same job, the same displayed test binary
deps/buzz_terminal-bd20f73363094eaepassed all 27/27 library tests; the final workspace sweep then passed 26/27. - Failing test:
env_fence_tests::resolve_shell_falls_through_to_passwd_not_the_default, panic atcrates/buzz-terminal/src/env_fence_tests.rs:237—assertion left == right failed: an unset $SHELL did not resolve to the passwd entry — left: "/bin/bash", right: "/back".
Failure model (concrete)
The failing test first reads passwd_shell() into an owned String, then calls resolve_shell(None) — two separate getpwuid calls in a concurrently-running test binary (default parallel test threads; sibling tests in the same binary spawn children and call the same resolver). Any concurrent passwd-database call can rewrite the static result during either window, so the two reads can disagree — the observed mismatched /back fragment is that signature. The right-hand value is a raw libc lookup, not a mocked fixture, so /back is not evidence of the runner's configured login shell.
Causation and repro limits (explicit)
- The static-storage MT-unsafe use is concrete and independently established from the Linux man-pages/glibc API contract (
getpwuid: MT-Unsaferace:pwuid;getpwuid_r: MT-Safe). - The exact interleaving that produced
/backin CI is not proved. The CI log cannot show which thread's call rewrote the storage, nor independently exclude an NSS/backend anomaly. This issue does not claim the CI interleaving was reproduced. - Local Linux repro is currently blocked:
cargo test -p buzz-terminal --offline --lockedfails at dependency resolution (uncachedmesh-llmgit dependency pulled in by workspace resolution). macOSgetpwuidreturns a thread-specific structure, so a Darwin stress pass would not clear the Linux defect, and no local Docker/Linux environment was available. No reproduction was manufactured.
Proposed correction (owner: native terminal/desktop Rust)
- Replace
getpwuidwithgetpwuid_rand caller-owned storage (passwdstruct + buffer,ERANGEhandled with bounded growth), validating pointers/encoding before copying; preserve lookup-error/absent-entry fallback and the existing executable-file validation and resolution-order assertions. A private mutex is insufficient — it cannot govern unrelated libc consumers in the same process. - Add a regression at the production lookup seam that discriminates old/new code under concurrent calls (not just a flaky stress pass), then run the full
cargo test -p buzz-terminalpackage suite on Linux/glibc — that full-package Linux evidence is still needed and does not exist yet. - Test-hygiene debt (process-env mutation without restoration/serialization in sibling tests) may deserve separate hardening; it is not a fix for the unsafe lookup.
A green CI rerun at any head does not fix or explain this race; the residual risk stays open until the reentrant correction lands with Linux validation.
Activity
fix(desktop): make the terminal shell lookup safe to call from any th…