From 3cc73f65f5b93f321c8378e89603ed416dfe5c4a Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:57:06 -0700 Subject: [PATCH] shell: process apps inherit an allow-list of the environment, never keys or tokens (G9) [skip ci] scrub_env removed only OCTOS_* and OCTOSENSE_*, so provider keys in the shell's environment (OPENAI_API_KEY, ANTHROPIC_API_KEY, ...) and any other token reached every process app. - scrub_env now clears the child's environment and passes on only inherited_var's allow-list: PATH, HOME, USER, the locale (LANG, LC_*), TERM, the temp dir, TZ, the display (DISPLAY, WAYLAND_DISPLAY, XDG_*, XAUTHORITY, the session bus, the GPU loader variables), Makepad's own (MAKEPAD_*, STUDIO_*), what a `cargo run` launch needs (CARGO_HOME, RUSTUP_*, the target dir, the compilers), and Windows' system variables. What the shell sets on the command itself is kept. - Whatever the list says, a secret-shaped name (OCTOS*, *_API_KEY, *_TOKEN, *_SECRET, *_PASSWORD, *_KEY, AWS_*, ...) is dropped, from inherited and explicit variables alike. - Tests: a real child started with a fake shell environment full of keys and tokens sees none of them (nor their values) and keeps the display, locale and Makepad variables; the list never admits a secret-shaped name. No real key is read or printed. Not kept on purpose: SSH_AUTH_SOCK and other agent sockets (the Terminal's login shell sets its own). Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/shell/src/sandbox/mod.rs | 80 ++++++++++++++++++++++++++----- crates/shell/src/sandbox/tests.rs | 65 +++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 13 deletions(-) diff --git a/crates/shell/src/sandbox/mod.rs b/crates/shell/src/sandbox/mod.rs index 4ae6127a..e2b0cd3b 100644 --- a/crates/shell/src/sandbox/mod.rs +++ b/crates/shell/src/sandbox/mod.rs @@ -29,10 +29,14 @@ //! **Child processes.** `processes: false`: no fork and no exec after the //! app's own start. //! -//! **Environment.** Whatever the policy, a process app never inherits the -//! kernel's descriptors or tokens: [`scrub_env`] removes every `OCTOS_*` -//! and `OCTOSENSE_*` variable (host and external tokens, the core and -//! kernel directories, the secrets vault) before the child starts, and on +//! **Environment.** Whatever the policy, a process app inherits only an +//! allow-list of the shell's environment ([`inherited_var`]: the path, the +//! home, the locale, the terminal, the temp dir, the display and Wayland +//! variables, Makepad's own, and what a `cargo run` build needs), never a +//! provider key (`OPENAI_API_KEY`, ...), a token, or the kernel's +//! descriptors: [`scrub_env`] clears everything else, and a name that looks +//! like a secret ([`is_secret_var`]: `*_API_KEY`, `*_TOKEN`, `OCTOS*`, ...) +//! is dropped even from the allow-list and from what the shell sets. On //! macOS a `cargo run` launch drops the ones the checkout's //! `.cargo/config.toml` `[env]` would hand back (build paths) in its //! runner. A process app reaches its agent only over the peer link @@ -257,6 +261,45 @@ pub fn is_host_secret_var(name: &str) -> bool { upper.starts_with("OCTOS_") || upper.starts_with("OCTOSENSE_") } +/// A name that holds, or looks like it holds, a secret: the host's own +/// (`OCTOS*`), provider keys and tokens of any kind. Never passed to a +/// process app, whatever else says so. +pub fn is_secret_var(name: &str) -> bool { + let upper = name.to_ascii_uppercase(); + if upper.starts_with("OCTOS") || is_host_secret_var(&upper) { + return true; + } + const PREFIXES: &[&str] = &["AWS_", "AZURE_", "GOOGLE_APPLICATION_CREDENTIALS", "GCLOUD_", "HF_", "HUGGING"]; + const MARKS: &[&str] = &["API_KEY", "APIKEY", "TOKEN", "SECRET", "PASSWORD", "PASSWD", "CREDENTIAL", "PRIVATE_KEY", "ACCESS_KEY", "AUTH_"]; + PREFIXES.iter().any(|p| upper.starts_with(p)) || MARKS.iter().any(|m| upper.contains(m)) || upper.ends_with("_KEY") +} + +/// The shell's variables a process app inherits (ADR 0004 §3): what a +/// Makepad app, a shell in the Terminal, or a `cargo run` build needs to +/// start and draw. Everything else stays with the shell. +pub fn inherited_var(name: &str) -> bool { + if is_secret_var(name) { + return false; + } + const EXACT: &[&str] = &[ + // Every platform. + "PATH", "HOME", "USER", "LOGNAME", "SHELL", "LANG", "LANGUAGE", "TERM", "COLORTERM", "TMPDIR", "TMP", "TEMP", "TZ", + "RUST_BACKTRACE", "RUST_LOG", + // The display: X11, Wayland and the session bus the GPU stack uses. + "DISPLAY", "WAYLAND_DISPLAY", "XAUTHORITY", "DBUS_SESSION_BUS_ADDRESS", "LD_LIBRARY_PATH", "VK_ICD_FILENAMES", "VK_DRIVER_FILES", + "__GLX_VENDOR_LIBRARY_NAME", "__EGL_VENDOR_LIBRARY_FILENAMES", + // A `cargo run` launch: the toolchain and where it builds. + "CARGO_HOME", "RUSTUP_HOME", "RUSTUP_TOOLCHAIN", "CARGO_TARGET_DIR", "CARGO_TERM_COLOR", "RUSTFLAGS", "CARGO_ENCODED_RUSTFLAGS", "RUSTC", + "RUSTC_WRAPPER", "CC", "CXX", "AR", "SDKROOT", "DEVELOPER_DIR", "MACOSX_DEPLOYMENT_TARGET", "PKG_CONFIG_PATH", + // Windows. + "SYSTEMROOT", "WINDIR", "USERPROFILE", "USERNAME", "APPDATA", "LOCALAPPDATA", "PROGRAMDATA", "PROGRAMFILES", "PROGRAMFILES(X86)", "COMSPEC", + "PATHEXT", "HOMEDRIVE", "HOMEPATH", "OS", "NUMBER_OF_PROCESSORS", "PROCESSOR_ARCHITECTURE", + ]; + const PREFIXES: &[&str] = &["LC_", "XDG_", "MAKEPAD_", "STUDIO_", "MESA_", "LIBGL_", "CARGO_BUILD_", "CARGO_PROFILE_"]; + let upper = name.to_ascii_uppercase(); + EXACT.contains(&upper.as_str()) || PREFIXES.iter().any(|p| upper.starts_with(p)) +} + /// The host variables (`OCTOS_*`, `OCTOSENSE_*`) a checkout's /// `.cargo/config.toml` `[env]` sets, which `cargo run` hands the app. pub fn cargo_env_host_vars(config: &str) -> Vec { @@ -281,18 +324,29 @@ pub fn cargo_env_host_vars(config: &str) -> Vec { out } -/// Remove every host variable from what `cmd` passes on (ADR 0004 §3: a -/// process app never connects to the kernel and never sees the host token). +/// What `cmd` passes on, from an allow-list (ADR 0004 §3: a process app +/// never connects to the kernel, never sees the host token, and never gets +/// the shell's provider keys): the shell's own variables that +/// [`inherited_var`] allows, then what the shell set on `cmd` itself, minus +/// anything that looks like a secret ([`is_secret_var`]). pub fn scrub_env(cmd: &mut Command) { - for (name, _) in std::env::vars_os() { - if name.to_str().is_some_and(is_host_secret_var) { - cmd.env_remove(&name); + scrub_env_from(cmd, std::env::vars_os()); +} + +/// [`scrub_env`] with `inherited` standing for the shell's environment. +pub fn scrub_env_from(cmd: &mut Command, inherited: impl IntoIterator) { + let explicit: Vec<(std::ffi::OsString, std::ffi::OsString)> = + cmd.get_envs().filter_map(|(k, v)| v.map(|v| (k.to_os_string(), v.to_os_string()))).collect(); + cmd.env_clear(); + for (name, value) in inherited { + if name.to_str().is_some_and(inherited_var) { + cmd.env(name, value); } } - let explicit: Vec = - cmd.get_envs().filter(|(k, v)| v.is_some() && k.to_str().is_some_and(is_host_secret_var)).map(|(k, _)| k.to_os_string()).collect(); - for name in explicit { - cmd.env_remove(name); + for (name, value) in explicit { + if name.to_str().is_some_and(|n| !is_secret_var(n)) { + cmd.env(name, value); + } } } diff --git a/crates/shell/src/sandbox/tests.rs b/crates/shell/src/sandbox/tests.rs index fdc7a8bc..394a52e9 100644 --- a/crates/shell/src/sandbox/tests.rs +++ b/crates/shell/src/sandbox/tests.rs @@ -314,3 +314,68 @@ fn landlock_splits_a_grant_around_the_private_dirs() { assert!(!paths.iter().any(|p| p.starts_with(&octo) || p == root), "{paths:?}"); assert!(!paths.contains(&root.join("sneaky")), "a link into a private dir gets nothing: {paths:?}"); } + +// ---------------------------------------------------------------- the environment (G9) + +fn secret_shaped(name: &str) -> bool { + let upper = name.to_ascii_uppercase(); + upper.ends_with("_API_KEY") || upper.ends_with("_TOKEN") || upper.starts_with("OCTOS") +} + +#[cfg(unix)] +#[test] +fn a_child_gets_only_the_allow_list_and_never_a_key_or_token() { + // As if the shell's environment held provider keys and tokens (fake + // values: no real key is ever read or printed here). + let fake = "fake-value-for-the-test"; + let shell_env: Vec<(std::ffi::OsString, std::ffi::OsString)> = [ + ("PATH", "/usr/bin:/bin"), + ("HOME", "/home/person"), + ("LANG", "en_US.UTF-8"), + ("TERM", "xterm-256color"), + ("TMPDIR", "/tmp"), + ("WAYLAND_DISPLAY", "wayland-0"), + ("DISPLAY", ":0"), + ("XAUTHORITY", "/home/person/.Xauthority"), + ("MAKEPAD_WM_THEME_SPLASH", "dark"), + ("OPENAI_API_KEY", fake), + ("ANTHROPIC_API_KEY", fake), + ("DEEPSEEK_API_KEY", fake), + ("GITHUB_TOKEN", fake), + ("HF_TOKEN", fake), + ("CARGO_REGISTRY_TOKEN", fake), + ("OCTOS_AUTH_TOKEN", fake), + ("OCTOS_APP_CORE_DIR", "/core"), + ("OCTOSENSE_SECRETS", "/vault"), + ("OCTOSENSE_HOME", "/home/person/.octosense"), + ("AWS_SECRET_ACCESS_KEY", fake), + ("SOME_TOOLS_PRIVATE_SETTING", "x"), + ] + .iter() + .map(|(k, v)| (k.into(), v.into())) + .collect(); + let mut cmd = Command::new("/usr/bin/env"); + cmd.env("STUDIO_HOST", "http://127.0.0.1:8765").env("MY_SERVICE_API_KEY", fake); + scrub_env_from(&mut cmd, shell_env); + let out = cmd.output().unwrap(); + let out = String::from_utf8_lossy(&out.stdout).to_string(); + let names: Vec<&str> = out.lines().map(|l| l.split('=').next().unwrap_or("")).collect(); + for name in &names { + assert!(!secret_shaped(name) && !is_secret_var(name), "the child sees {name}"); + } + assert!(!out.contains(fake), "no secret value reaches the child"); + for kept in ["PATH", "HOME", "LANG", "TERM", "TMPDIR", "WAYLAND_DISPLAY", "DISPLAY", "XAUTHORITY", "MAKEPAD_WM_THEME_SPLASH", "STUDIO_HOST"] { + assert!(names.contains(&kept), "{kept} is passed on: {names:?}"); + } + assert!(!names.contains(&"SOME_TOOLS_PRIVATE_SETTING"), "anything not on the list stays with the shell"); +} + +#[test] +fn the_allow_list_never_admits_a_secret_shaped_name() { + for name in ["OPENAI_API_KEY", "anthropic_api_key", "GITHUB_TOKEN", "OCTOS_HOST_EXTERNAL_TOKEN", "OCTOSENSE_WORKSPACE", "OCTOSX", "MAKEPAD_WM_TOKEN", "XDG_SECRET", "CARGO_BUILD_API_KEY"] { + assert!(is_secret_var(name) && !inherited_var(name), "{name}"); + } + for name in ["PATH", "HOME", "LANG", "LC_ALL", "TERM", "TMPDIR", "DISPLAY", "WAYLAND_DISPLAY", "XDG_RUNTIME_DIR", "XAUTHORITY", "MAKEPAD_WM_ROOT", "CARGO_HOME", "RUSTUP_TOOLCHAIN"] { + assert!(inherited_var(name), "{name}"); + } +}