Skip to content

buzz-terminal: shell lookup uses MT-unsafe getpwuid static storage (pre-existing; env_fence CI failure in run 34631406730) #7589

Description

@loganj

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 → final BUZZ_BUILD_DEMO_SLUG=… cargo test --workspace sweep, exit 101).
  • Earlier in the same job, the same displayed test binary deps/buzz_terminal-bd20f73363094eae passed 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 at crates/buzz-terminal/src/env_fence_tests.rs:237assertion 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-Unsafe race:pwuid; getpwuid_r: MT-Safe).
  • The exact interleaving that produced /back in 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 --locked fails at dependency resolution (uncached mesh-llm git dependency pulled in by workspace resolution). macOS getpwuid returns 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)

  1. Replace getpwuid with getpwuid_r and caller-owned storage (passwd struct + buffer, ERANGE handled 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.
  2. 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-terminal package suite on Linux/glibc — that full-package Linux evidence is still needed and does not exist yet.
  3. 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions