Bind symlinked rw/ro paths at the user-written destination

Canonicalizing rw/ro paths in the config layer resolved symlinks before
the sandbox was built, so a symlinked entry only appeared at its
target's location -- never at the path the user wrote. Stop
canonicalizing rw/ro at the config layer and instead resolve only the
source side of the bind in sandbox.rs.
This commit is contained in:
2026-04-07 17:45:38 +02:00
parent f0711f2894
commit 83bd4305c7
3 changed files with 91 additions and 11 deletions

View File

@@ -95,11 +95,20 @@ fn merge_paths(
let config_paths = globals.iter().chain(profile).cloned(); let config_paths = globals.iter().chain(profile).cloned();
let cli_paths = cli let cli_paths = cli
.into_iter() .into_iter()
.map(|p| std::fs::canonicalize(&p).map_err(|_| SandboxError::PathMissing(p))) .map(prepare_cli_bind_path)
.collect::<Result<Vec<_>, _>>()?; .collect::<Result<Vec<_>, _>>()?;
Ok(config_paths.chain(cli_paths).collect()) Ok(config_paths.chain(cli_paths).collect())
} }
fn prepare_cli_bind_path(path: PathBuf) -> Result<PathBuf, SandboxError> {
let absolute =
std::path::absolute(&path).map_err(|_| SandboxError::PathMissing(path.clone()))?;
if !absolute.exists() {
return Err(SandboxError::PathMissing(absolute));
}
Ok(absolute)
}
fn split_bwrap_args(raw: Vec<String>) -> Result<Vec<String>, SandboxError> { fn split_bwrap_args(raw: Vec<String>) -> Result<Vec<String>, SandboxError> {
let mut out = Vec::new(); let mut out = Vec::new();
for value in &raw { for value in &raw {
@@ -228,10 +237,10 @@ pub struct Options {
impl Options { impl Options {
fn validate_paths(&mut self) -> Result<(), SandboxError> { fn validate_paths(&mut self) -> Result<(), SandboxError> {
for p in &mut self.rw { for p in &mut self.rw {
*p = expand_and_canonicalize(p)?; *p = expand_and_require_existing(p)?;
} }
for p in &mut self.ro { for p in &mut self.ro {
*p = expand_and_canonicalize(p)?; *p = expand_and_require_existing(p)?;
} }
if let Some(ref chdir) = self.chdir { if let Some(ref chdir) = self.chdir {
self.chdir = Some(expand_and_canonicalize(chdir)?); self.chdir = Some(expand_and_canonicalize(chdir)?);
@@ -243,6 +252,14 @@ impl Options {
} }
} }
fn expand_and_require_existing(path: &Path) -> Result<PathBuf, SandboxError> {
let expanded = expand_and_require_absolute(path)?;
if !expanded.exists() {
return Err(SandboxError::PathMissing(expanded));
}
Ok(expanded)
}
fn expand_and_require_absolute(path: &Path) -> Result<PathBuf, SandboxError> { fn expand_and_require_absolute(path: &Path) -> Result<PathBuf, SandboxError> {
let expanded = expand_tilde(path)?; let expanded = expand_tilde(path)?;
require_absolute(&expanded)?; require_absolute(&expanded)?;
@@ -332,6 +349,8 @@ fn expand_and_canonicalize(path: &Path) -> Result<PathBuf, SandboxError> {
mod tests { mod tests {
use std::sync::LazyLock; use std::sync::LazyLock;
use tempfile::TempDir;
use super::*; use super::*;
const FULL_CONFIG_TOML: &str = r#" const FULL_CONFIG_TOML: &str = r#"
@@ -615,6 +634,45 @@ mod tests {
assert_eq!(config.extra_rw, vec![PathBuf::from(&home)]); assert_eq!(config.extra_rw, vec![PathBuf::from(&home)]);
} }
#[test]
fn build_preserves_symlinks_in_config_paths() {
let dir = TempDir::new().unwrap();
let target = dir.path().join("target");
let link = dir.path().join("link");
std::fs::write(&target, "x").unwrap();
std::os::unix::fs::symlink(&target, &link).unwrap();
let file_config = FileConfig {
options: Options {
ro: vec![link.clone()],
rw: vec![link.clone()],
..Options::default()
},
..FileConfig::default()
};
let config = build(args_with_command(), Some(file_config)).unwrap();
assert_eq!(config.extra_ro, vec![link.clone()]);
assert_eq!(config.extra_rw, vec![link]);
}
#[test]
fn build_preserves_symlinks_in_cli_paths() {
let dir = TempDir::new().unwrap();
let target = dir.path().join("target");
let link = dir.path().join("link");
std::fs::write(&target, "x").unwrap();
std::os::unix::fs::symlink(&target, &link).unwrap();
let args = Args {
extra_ro: vec![link.clone()],
extra_rw: vec![link.clone()],
..args_with_command()
};
let config = build(args, None).unwrap();
assert_eq!(config.extra_ro, vec![link.clone()]);
assert_eq!(config.extra_rw, vec![link]);
}
#[test] #[test]
fn build_relative_config_path_rejected() { fn build_relative_config_path_rejected() {
let file_config = FileConfig { let file_config = FileConfig {

View File

@@ -180,17 +180,17 @@ fn ro_bind_under_tmpfs(cmd: &mut Command, base: &str, paths: &[&str]) {
} }
fn add_rw_bind(cmd: &mut Command, path: &Path) -> Result<(), SandboxError> { fn add_rw_bind(cmd: &mut Command, path: &Path) -> Result<(), SandboxError> {
if !path.exists() { let source = resolve_bind_source(path)?;
return Err(SandboxError::PathMissing(path.to_path_buf())); cmd.arg("--bind").arg(source).arg(path);
}
cmd.arg("--bind").arg(path).arg(path);
Ok(()) Ok(())
} }
fn add_ro_bind(cmd: &mut Command, path: &Path) -> Result<(), SandboxError> { fn add_ro_bind(cmd: &mut Command, path: &Path) -> Result<(), SandboxError> {
if !path.exists() { let source = resolve_bind_source(path)?;
return Err(SandboxError::PathMissing(path.to_path_buf())); cmd.arg("--ro-bind").arg(source).arg(path);
}
cmd.arg("--ro-bind").arg(path).arg(path);
Ok(()) Ok(())
} }
fn resolve_bind_source(path: &Path) -> Result<PathBuf, SandboxError> {
std::fs::canonicalize(path).map_err(|_| SandboxError::PathMissing(path.to_path_buf()))
}

View File

@@ -796,6 +796,28 @@ fn config_command_replaced_by_passthrough() {
); );
} }
#[test]
fn whitelist_ro_symlink_visible_at_link_path() {
let dir = TempDir::new().unwrap();
let target = dir.path().join("target.txt");
let link = dir.path().join("link.txt");
fs::write(&target, "hello from target").expect("failed to write target");
std::os::unix::fs::symlink(&target, &link).expect("failed to create symlink");
let link_str = link.to_str().unwrap();
let output = sandbox(&["--whitelist", "--ro", link_str])
.args(["--", "cat", link_str])
.output()
.expect("agent-sandbox binary failed to execute");
let stdout = String::from_utf8_lossy(&output.stdout);
assert!(
stdout.contains("hello from target"),
"expected symlink path to be readable inside sandbox, got stdout: {stdout}, stderr: {}",
String::from_utf8_lossy(&output.stderr)
);
}
#[test] #[test]
fn mask_nonexistent_path_becomes_tmpfs() { fn mask_nonexistent_path_becomes_tmpfs() {
let dir = TempDir::new().unwrap(); let dir = TempDir::new().unwrap();