fix(supervisor): harden sandbox path startup repair

Signed-off-by: Evan Lezar <elezar@nvidia.com>

Author: elezar

crates/openshell-supervisor-process/src/child_env.rs

@@ -1,7 +1,7 @@
 // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-use std::io::Write;
+use std::io::{Read, Write};
 use std::path::Path;
 
 const LOCAL_NO_PROXY: &str = "127.0.0.1,localhost,::1";
@@ -11,6 +11,12 @@ const ENSURE_STANDARD_SBIN_PATHS_SCRIPT: &str = "for dir in /usr/local/sbin /usr
 const STARTUP_SNIPPET_MARKER: &str = "# OpenShell standard sbin PATH";
 const PROFILE_D_SNIPPET_PATH: &str = "/etc/profile.d/openshell-standard-sbin-path.sh";
 
+enum StartupFile {
+    Missing,
+    Regular(String),
+    Unsafe,
+}
+
 pub fn child_path_from_env() -> String {
     std::env::var("PATH")
         .ok()
@@ -71,34 +77,158 @@ fn startup_snippet() -> String {
 }
 
 fn write_profile_snippet(path: &Path) -> std::io::Result<()> {
-    let snippet = startup_snippet();
-    if std::fs::read_to_string(path).is_ok_and(|content| content == snippet) {
-        return Ok(());
-    }
     if let Some(parent) = path.parent() {
-        std::fs::create_dir_all(parent)?;
+        ensure_directory_without_symlink(parent)?;
+    }
+
+    let snippet = startup_snippet();
+    match read_startup_file(path)? {
+        StartupFile::Regular(content) if content == snippet => return Ok(()),
+        StartupFile::Regular(_) | StartupFile::Missing => {}
+        StartupFile::Unsafe => return Ok(()),
     }
-    std::fs::write(path, snippet)
+    write_startup_file(path, &snippet)
 }
 
 fn append_startup_snippet(path: &Path) -> std::io::Result<()> {
-    let existing = match std::fs::read_to_string(path) {
-        Ok(content) => content,
-        Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(()),
-        Err(error) => return Err(error),
+    if let Some(parent) = path.parent()
+        && !existing_directory_without_symlink(parent)?
+    {
+        return Ok(());
+    }
+
+    let existing = match read_startup_file(path)? {
+        StartupFile::Regular(content) => content,
+        StartupFile::Missing | StartupFile::Unsafe => return Ok(()),
     };
     if existing.contains(STARTUP_SNIPPET_MARKER) {
         return Ok(());
     }
 
     let snippet = startup_snippet();
-    let mut file = std::fs::OpenOptions::new().append(true).open(path)?;
+    let mut file = open_startup_file_for_append(path)?;
     if !existing.is_empty() && !existing.ends_with('\n') {
         file.write_all(b"\n")?;
     }
     file.write_all(snippet.as_bytes())
 }
 
+fn read_startup_file(path: &Path) -> std::io::Result<StartupFile> {
+    let metadata = match std::fs::symlink_metadata(path) {
+        Ok(metadata) => metadata,
+        Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
+            return Ok(StartupFile::Missing);
+        }
+        Err(error) => return Err(error),
+    };
+    let file_type = metadata.file_type();
+    if file_type.is_symlink() || !file_type.is_file() {
+        tracing::debug!(
+            path = %path.display(),
+            "skipping OpenShell PATH startup repair for non-regular file"
+        );
+        return Ok(StartupFile::Unsafe);
+    }
+
+    let mut content = String::new();
+    open_startup_file_for_read(path)?.read_to_string(&mut content)?;
+    Ok(StartupFile::Regular(content))
+}
+
+fn ensure_directory_without_symlink(path: &Path) -> std::io::Result<()> {
+    match std::fs::symlink_metadata(path) {
+        Ok(metadata) if metadata.file_type().is_symlink() => Err(std::io::Error::other(format!(
+            "directory '{}' is a symlink",
+            path.display()
+        ))),
+        Ok(metadata) if metadata.is_dir() => Ok(()),
+        Ok(_) => Err(std::io::Error::other(format!(
+            "'{}' is not a directory",
+            path.display()
+        ))),
+        Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
+            if let Some(parent) = path.parent() {
+                ensure_directory_without_symlink(parent)?;
+            }
+            std::fs::create_dir(path)
+        }
+        Err(error) => Err(error),
+    }
+}
+
+fn existing_directory_without_symlink(path: &Path) -> std::io::Result<bool> {
+    if let Some(parent) = path.parent()
+        && parent != path
+        && !parent.as_os_str().is_empty()
+        && !existing_directory_without_symlink(parent)?
+    {
+        return Ok(false);
+    }
+
+    match std::fs::symlink_metadata(path) {
+        Ok(metadata) if metadata.file_type().is_symlink() => {
+            tracing::debug!(
+                path = %path.display(),
+                "skipping OpenShell PATH startup repair through symlink directory"
+            );
+            Ok(false)
+        }
+        Ok(metadata) if metadata.is_dir() => Ok(true),
+        Ok(_) => {
+            tracing::debug!(
+                path = %path.display(),
+                "skipping OpenShell PATH startup repair through non-directory path"
+            );
+            Ok(false)
+        }
+        Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false),
+        Err(error) => Err(error),
+    }
+}
+
+fn write_startup_file(path: &Path, content: &str) -> std::io::Result<()> {
+    let mut file = no_follow_options()
+        .write(true)
+        .create(true)
+        .truncate(true)
+        .open(path)?;
+    ensure_opened_file_is_regular(&file, path)?;
+    file.write_all(content.as_bytes())
+}
+
+fn open_startup_file_for_read(path: &Path) -> std::io::Result<std::fs::File> {
+    let file = no_follow_options().read(true).open(path)?;
+    ensure_opened_file_is_regular(&file, path)?;
+    Ok(file)
+}
+
+fn open_startup_file_for_append(path: &Path) -> std::io::Result<std::fs::File> {
+    let file = no_follow_options().append(true).open(path)?;
+    ensure_opened_file_is_regular(&file, path)?;
+    Ok(file)
+}
+
+fn ensure_opened_file_is_regular(file: &std::fs::File, path: &Path) -> std::io::Result<()> {
+    let metadata = file.metadata()?;
+    if metadata.is_file() {
+        return Ok(());
+    }
+    Err(std::io::Error::other(format!(
+        "'{}' is not a regular file",
+        path.display()
+    )))
+}
+
+fn no_follow_options() -> std::fs::OpenOptions {
+    let mut options = std::fs::OpenOptions::new();
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::OpenOptionsExt as _;
+        options.custom_flags(libc::O_NOFOLLOW | libc::O_CLOEXEC);
+    }
+    options
+}
+
 pub fn proxy_env_vars(proxy_url: &str) -> [(&'static str, String); 9] {
     [
         ("ALL_PROXY", proxy_url.to_owned()),
@@ -211,6 +341,96 @@ mod tests {
         assert!(bashrc.contains("export PATH=\"/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin\""));
     }
 
+    #[cfg(unix)]
+    #[test]
+    fn profile_snippet_skips_symlink() {
+        use std::os::unix::fs::symlink;
+
+        let dir = tempfile::tempdir().expect("tempdir");
+        let target = dir.path().join("target");
+        std::fs::write(&target, "keep me").expect("write target");
+        let profile_path = dir.path().join("openshell-path.sh");
+        symlink(&target, &profile_path).expect("symlink profile");
+
+        write_profile_snippet(&profile_path).expect("skip symlink profile");
+
+        assert_eq!(
+            std::fs::read_to_string(&target).expect("read target"),
+            "keep me"
+        );
+        assert!(
+            std::fs::symlink_metadata(&profile_path)
+                .expect("profile metadata")
+                .file_type()
+                .is_symlink()
+        );
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn profile_snippet_rejects_symlink_parent() {
+        use std::os::unix::fs::symlink;
+
+        let dir = tempfile::tempdir().expect("tempdir");
+        let target_dir = dir.path().join("target-dir");
+        std::fs::create_dir(&target_dir).expect("target dir");
+        let parent = dir.path().join("profile.d");
+        symlink(&target_dir, &parent).expect("symlink parent");
+        let profile_path = parent.join("openshell-path.sh");
+
+        let error = write_profile_snippet(&profile_path).expect_err("reject symlink parent");
+
+        assert!(error.to_string().contains("symlink"));
+        assert!(!target_dir.join("openshell-path.sh").exists());
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn bashrc_snippet_skips_symlink() {
+        use std::os::unix::fs::symlink;
+
+        let dir = tempfile::tempdir().expect("tempdir");
+        let target = dir.path().join("target");
+        std::fs::write(&target, "keep me").expect("write target");
+        let bashrc_path = dir.path().join(".bashrc");
+        symlink(&target, &bashrc_path).expect("symlink bashrc");
+
+        append_startup_snippet(&bashrc_path).expect("skip symlink bashrc");
+
+        assert_eq!(
+            std::fs::read_to_string(&target).expect("read target"),
+            "keep me"
+        );
+        assert!(
+            std::fs::symlink_metadata(&bashrc_path)
+                .expect("bashrc metadata")
+                .file_type()
+                .is_symlink()
+        );
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn bashrc_snippet_skips_symlink_parent() {
+        use std::os::unix::fs::symlink;
+
+        let dir = tempfile::tempdir().expect("tempdir");
+        let target_dir = dir.path().join("target-dir");
+        std::fs::create_dir(&target_dir).expect("target dir");
+        std::fs::write(
+            target_dir.join(".bashrc"),
+            "export PATH=\"/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin\"\n",
+        )
+        .expect("target bashrc");
+        let home = dir.path().join("home");
+        symlink(&target_dir, &home).expect("symlink home");
+
+        append_startup_snippet(&home.join(".bashrc")).expect("skip symlink home");
+
+        let bashrc = std::fs::read_to_string(target_dir.join(".bashrc")).expect("read target");
+        assert!(!bashrc.contains(STARTUP_SNIPPET_MARKER));
+    }
+
     #[test]
     fn apply_proxy_env_includes_node_proxy_opt_in_and_local_bypass() {
         let mut cmd = Command::new("/usr/bin/env");

crates/openshell-supervisor-process/src/process.rs

@@ -190,7 +190,11 @@ fn inject_provider_env(cmd: &mut Command, provider_env: &HashMap<String, String>
         if is_supervisor_only_env_var(key) {
             continue;
         }
-        cmd.env(key, value);
+        if key == "PATH" {
+            cmd.env(key, child_env::path_with_standard_sbin_paths(value));
+        } else {
+            cmd.env(key, value);
+        }
     }
 }
 
@@ -3062,6 +3066,29 @@ mod tests {
         assert!(stdout.contains("ANTHROPIC_API_KEY=openshell:resolve:env:ANTHROPIC_API_KEY"));
     }
 
+    #[tokio::test]
+    async fn inject_provider_env_appends_standard_sbin_to_path() {
+        let mut cmd = Command::new("/usr/bin/env");
+        cmd.env_clear()
+            .stdin(StdStdio::null())
+            .stdout(StdStdio::piped())
+            .stderr(StdStdio::null());
+
+        let provider_env = HashMap::from([(
+            "PATH".to_string(),
+            "/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin".to_string(),
+        )]);
+
+        inject_provider_env(&mut cmd, &provider_env);
+
+        let output = cmd.output().await.expect("spawn env");
+        assert!(output.status.success());
+        let stdout = String::from_utf8(output.stdout).expect("utf8");
+        assert!(stdout.lines().any(|line| {
+            line == "PATH=/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin"
+        }));
+    }
+
     #[cfg(unix)]
     fn sandbox_policy_with_read_write(
         path: PathBuf,