shell env

This commit is contained in:
jaberjaber23
2026-05-12 15:34:39 +03:00
parent 6a1ce40d86
commit 5cc865e6e6
3 changed files with 153 additions and 3 deletions
@@ -35,6 +35,12 @@ pub const SAFE_ENV_VARS_WINDOWS: &[&str] = &[
/// - On Windows, the Windows-specific safe variables (`SAFE_ENV_VARS_WINDOWS`)
/// - Any additional variables the caller explicitly allows via `allowed_env_vars`
///
/// `allowed_env_vars` accepts either explicit variable names or the special
/// wildcard entry `"*"`, which forwards every variable present in the parent
/// process. Use the wildcard only when the operator has explicitly opted in
/// (e.g. `exec_policy.shell_env_passthrough = ["*"]`) — it will leak any
/// secret the parent holds into the child.
///
/// Variables that are not set in the current process environment are silently
/// skipped (rather than being set to empty strings).
pub fn sandbox_command(cmd: &mut tokio::process::Command, allowed_env_vars: &[String]) {
@@ -55,6 +61,14 @@ pub fn sandbox_command(cmd: &mut tokio::process::Command, allowed_env_vars: &[St
}
}
// Wildcard: forward every var from the parent process.
if allowed_env_vars.iter().any(|v| v == "*") {
for (key, val) in std::env::vars() {
cmd.env(key, val);
}
return;
}
// Re-add caller-specified allowed vars.
for var in allowed_env_vars {
if let Ok(val) = std::env::var(var) {
@@ -63,6 +77,22 @@ pub fn sandbox_command(cmd: &mut tokio::process::Command, allowed_env_vars: &[St
}
}
/// Merge two env-passthrough lists (hand-granted + exec-policy-granted),
/// deduplicating entries. If either contains `"*"`, the result is just `["*"]`
/// (wildcard subsumes anything else).
pub fn merge_env_passthrough(a: &[String], b: &[String]) -> Vec<String> {
if a.iter().any(|v| v == "*") || b.iter().any(|v| v == "*") {
return vec!["*".to_string()];
}
let mut out: Vec<String> = Vec::with_capacity(a.len() + b.len());
for v in a.iter().chain(b.iter()) {
if !out.iter().any(|existing| existing == v) {
out.push(v.clone());
}
}
out
}
/// Validates that an executable path does not contain directory traversal
/// components (`..`).
///
@@ -711,6 +741,40 @@ pub async fn wait_or_kill_with_idle(
mod tests {
use super::*;
// ── Env passthrough merge (issue #1169) ────────────────────────────
#[test]
fn test_merge_env_passthrough_empty() {
let merged = merge_env_passthrough(&[], &[]);
assert!(merged.is_empty());
}
#[test]
fn test_merge_env_passthrough_dedup() {
let a = vec!["TZ".to_string(), "HOME".to_string()];
let b = vec!["TZ".to_string(), "PATH".to_string()];
let merged = merge_env_passthrough(&a, &b);
assert_eq!(merged, vec!["TZ", "HOME", "PATH"]);
}
#[test]
fn test_merge_env_passthrough_wildcard_a() {
let merged = merge_env_passthrough(&["*".to_string()], &["TZ".to_string()]);
assert_eq!(merged, vec!["*"]);
}
#[test]
fn test_merge_env_passthrough_wildcard_b() {
let merged = merge_env_passthrough(&["TZ".to_string()], &["*".to_string()]);
assert_eq!(merged, vec!["*"]);
}
#[test]
fn test_exec_policy_default_has_empty_passthrough() {
let policy = openfang_types::config::ExecPolicy::default();
assert!(policy.shell_env_passthrough.is_empty());
}
#[test]
fn test_validate_path() {
// Clean paths should be accepted.
+19 -3
View File
@@ -332,7 +332,7 @@ pub async fn execute_tool(
"media_transcribe" => tool_media_transcribe(input, media_engine).await,
// Image generation tool
"image_generate" => tool_image_generate(input, workspace_root).await,
"image_generate" => tool_image_generate(input, workspace_root, media_engine).await,
// TTS/STT tools
"text_to_speech" => tool_text_to_speech(input, tts_engine, workspace_root).await,
@@ -1667,7 +1667,18 @@ async fn tool_shell_exec(
// SECURITY: Isolate environment to prevent credential leakage.
// Hand settings may grant access to specific provider API keys.
crate::subprocess_sandbox::sandbox_command(&mut cmd, allowed_env);
//
// Operators can also forward additional vars via
// `exec_policy.shell_env_passthrough` (issue #1169). This is the path
// Docker users hit: their container env (TZ, GOG_*, etc.) is present
// in PID 1 but `env_clear()` strips it. Listing names (or `"*"`) here
// re-adds them to the child.
let policy_env_passthrough: &[String] = exec_policy
.map(|p| p.shell_env_passthrough.as_slice())
.unwrap_or(&[]);
let merged_env =
crate::subprocess_sandbox::merge_env_passthrough(allowed_env, policy_env_passthrough);
crate::subprocess_sandbox::sandbox_command(&mut cmd, &merged_env);
// Ensure UTF-8 output on Windows
#[cfg(windows)]
@@ -3035,6 +3046,7 @@ async fn tool_media_transcribe(
async fn tool_image_generate(
input: &serde_json::Value,
workspace_root: Option<&Path>,
media_engine: Option<&crate::media_understanding::MediaEngine>,
) -> Result<String, String> {
let prompt = input["prompt"]
.as_str()
@@ -3064,7 +3076,11 @@ async fn tool_image_generate(
count,
};
let result = crate::image_gen::generate_image(&request).await?;
// Closes #1051: route to a local OpenAI-compatible image generation
// service when `media.image_gen_base_url` is set.
let base_url_override = media_engine
.and_then(|e| e.config().image_gen_base_url.as_deref());
let result = crate::image_gen::generate_image(&request, base_url_override).await?;
// Save images to workspace if available
let saved_paths = if let Some(workspace) = workspace_root {
+70
View File
@@ -969,6 +969,25 @@ pub struct ExecPolicy {
/// produce no stdout/stderr output for this duration. Default: 30.
#[serde(default = "default_no_output_timeout")]
pub no_output_timeout_secs: u64,
/// Environment variables to forward from the OpenFang process into
/// `shell_exec` subprocesses.
///
/// By default, subprocesses run with `env_clear()` and only receive a
/// minimal safe set (PATH, HOME, TMPDIR, LANG, TERM, etc. — see
/// `subprocess_sandbox::SAFE_ENV_VARS`). Anything else — including
/// user-defined variables present in the container/host environment —
/// is stripped. This list lets operators explicitly re-add specific
/// variables to the subprocess environment.
///
/// Each entry is an env var name. A single entry of `"*"` forwards
/// every variable present in the parent process. Use with care — `*`
/// will leak API keys and other secrets into child processes.
///
/// Aliases `env_passthrough` and `env_allowlist` are accepted for
/// backwards compatibility with users who configured these names
/// before the field existed (issue #1169).
#[serde(default, alias = "env_passthrough", alias = "env_allowlist")]
pub shell_env_passthrough: Vec<String>,
}
fn default_no_output_timeout() -> u64 {
@@ -990,6 +1009,7 @@ impl Default for ExecPolicy {
timeout_secs: 30,
max_output_bytes: 100 * 1024,
no_output_timeout_secs: default_no_output_timeout(),
shell_env_passthrough: Vec::new(),
}
}
}
@@ -4600,4 +4620,54 @@ mod tests {
let config: KernelConfig = toml::from_str(toml_str).unwrap();
assert_eq!(config.heartbeat.default_timeout_secs, 300);
}
// ── Issue #1169: shell_env_passthrough on ExecPolicy ──────────────
#[test]
fn test_exec_policy_passthrough_default_empty() {
let policy = ExecPolicy::default();
assert!(policy.shell_env_passthrough.is_empty());
}
#[test]
fn test_exec_policy_passthrough_deserializes() {
let toml_str = r#"
mode = "full"
shell_env_passthrough = ["TZ", "GOG_ACCOUNT"]
"#;
let policy: ExecPolicy = toml::from_str(toml_str).unwrap();
assert_eq!(policy.shell_env_passthrough, vec!["TZ", "GOG_ACCOUNT"]);
}
#[test]
fn test_exec_policy_passthrough_alias_env_passthrough() {
// Backwards-compat alias from the issue body (#1169).
let toml_str = r#"
mode = "full"
env_passthrough = ["TZ", "GOG_ACCOUNT"]
"#;
let policy: ExecPolicy = toml::from_str(toml_str).unwrap();
assert_eq!(policy.shell_env_passthrough, vec!["TZ", "GOG_ACCOUNT"]);
}
#[test]
fn test_exec_policy_passthrough_alias_env_allowlist() {
// Backwards-compat alias from the issue body (#1169).
let toml_str = r#"
mode = "full"
env_allowlist = ["TZ", "HOME"]
"#;
let policy: ExecPolicy = toml::from_str(toml_str).unwrap();
assert_eq!(policy.shell_env_passthrough, vec!["TZ", "HOME"]);
}
#[test]
fn test_exec_policy_passthrough_wildcard() {
let toml_str = r#"
mode = "full"
shell_env_passthrough = ["*"]
"#;
let policy: ExecPolicy = toml::from_str(toml_str).unwrap();
assert_eq!(policy.shell_env_passthrough, vec!["*"]);
}
}