Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions sandbox/src/spawn/container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,19 +81,38 @@ impl Sandbox {
}))
}

/// The sandbox's space environment as the container spells each
/// directory, through the workspace mount.
fn container_space_environment(
&self,
runtime: &ContainerShellRuntime,
) -> Result<Vec<(String, String)>, SpawnError> {
self.inner
.space_environment
.iter()
.map(|(name, path)| {
let path = Self::container_path(runtime, path)?;
Ok((name.clone(), path.as_str().to_owned()))
})
.collect()
}

/// `script` as the container's managed shell runs it: the source's
/// environment exported, then the spec's, ahead of it, the programs'
/// directory first on its `PATH`.
/// environment exported, then the space's, then the spec's, ahead of
/// it, the programs' directory first on its `PATH`.
fn exec_script(
&self,
launch: &ContainerLaunch,
script: String,
space_env: Vec<(String, String)>,
env: Vec<(String, String)>,
info: &LaunchInfo,
) -> Result<String, SpawnError> {
let failed = |error: &dyn std::fmt::Display| SpawnError::Setup(error.to_string());
let spec_env: Arc<dyn EnvSource> = Arc::new(env);
let space_env: Arc<dyn EnvSource> = Arc::new(space_env);
let script = inject_env_exports(script, Some(&spec_env), info).map_err(|e| failed(&e))?;
let script = inject_env_exports(script, Some(&space_env), info).map_err(|e| failed(&e))?;
let script = inject_env_exports(script, self.inner.env_source.as_ref(), info)
.map_err(|e| failed(&e))?;
let script = launch.wrap_script(&script).map_err(|e| failed(&e))?;
Expand Down Expand Up @@ -241,15 +260,17 @@ impl Sandbox {
let confinement = self.container_confinement(runtime, container_grants)?;

// The exec channel accepts only a script, so the environment travels
// as exports ahead of it; the source's come first, so the spec's win.
// as exports ahead of it; the source's come first, then the space's,
// so the spec's win.
let script = match command {
SpawnCommand::Script(script) => script,
SpawnCommand::Program { program, args } => {
shell_words::join(std::iter::once(&program).chain(&args))
}
};
let script = script_reading(stdin, script)?;
let script = self.exec_script(launch, script, env, info)?;
let space_env = self.container_space_environment(runtime)?;
let script = self.exec_script(launch, script, space_env, env, info)?;
// A confined process runs in a sandbox rooted at the sandbox's own
// directory; the directory it was asked to start in is entered inside.
let script = match (&confinement, cwd.is_some()) {
Expand Down
19 changes: 14 additions & 5 deletions sandbox/src/spawn/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,19 +119,28 @@ impl Sandbox {
.unwrap_or_else(|_| self.inner.working_dir.clone())
}

/// The environment of a host spawn: the sandbox's own, its source's, then
/// the spec's, each overriding the one before.
/// The environment of a host spawn: the sandbox's own, its source's, its
/// space's, then the spec's, each overriding the one before.
fn host_environment(
&self,
spec_env: Vec<(String, String)>,
info: &LaunchInfo,
) -> Vec<(String, String)> {
) -> Result<Vec<(String, String)>, SpawnError> {
let mut env = self.inner.environment.clone();
if let Some(source) = &self.inner.env_source {
env.extend(source.env(info));
}
for (name, path) in &self.inner.space_environment {
let value = path.to_str().ok_or_else(|| {
SpawnError::Setup(format!(
"{} is not UTF-8, so the variable {name} cannot name it",
path.display()
))
})?;
env.push((name.clone(), value.to_owned()));
}
env.extend(spec_env);
env
Ok(env)
}

/// The grants a confined spawn carries: the embedder's fixed ones, every
Expand Down Expand Up @@ -267,7 +276,7 @@ impl Sandbox {
})
.collect::<Result<Vec<_>, _>>()?;
let marker = JobMarker::new();
let mut env = self.host_environment(env, &info);
let mut env = self.host_environment(env, &info)?;
env.push(marker.entry());
let launch = Launch {
program,
Expand Down
17 changes: 16 additions & 1 deletion sandbox/src/spawn/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,9 @@ struct SandboxInner {
fixed_grants: Vec<heel::Grant>,
environment: Vec<(String, String)>,
env_source: Option<Arc<dyn EnvSource>>,
/// Variables naming directories inside a subagent's own space, by their
/// host paths: the layer between the source and a spec's own variables.
space_environment: Vec<(String, PathBuf)>,
write_denylist: Option<heel::WriteDenylist>,
network: Option<CompiledNetworkPolicy>,
approver: Option<AnyDomainApprover>,
Expand Down Expand Up @@ -377,6 +380,7 @@ impl Sandbox {
fixed_grants: Vec::new(),
environment: Vec::new(),
env_source: None,
space_environment: Vec::new(),
write_denylist: None,
network: None,
approver: None,
Expand Down Expand Up @@ -427,6 +431,14 @@ impl Sandbox {
self.edit(|inner| inner.env_source = Some(Arc::new(source)))
}

/// The variables of a subagent's space: each names a directory inside
/// it, by its host path. They override the environment and its source
/// and yield to a spec's own variables; a container sees each directory
/// through its workspace mount, and an SSH server none.
pub(crate) fn with_space_environment(self, environment: Vec<(String, PathBuf)>) -> Self {
self.edit(|inner| inner.space_environment = environment)
}

/// The paths a spawn on the host must not write, however they are
/// granted. Not supported on Windows, whose backend refuses a denylist
/// rather than run weaker, so it is not applied there.
Expand Down Expand Up @@ -474,13 +486,16 @@ impl Sandbox {
/// grants [derived](Grants::derived) from this sandbox's with `own`, and
/// no agent label until [`with_agent`](Self::with_agent) gives one. It
/// cannot widen this sandbox: its grants never reach it, and a container
/// confines its processes to its own writable grants.
/// confines its processes to its own writable grants. The variables of
/// this sandbox's space are not the child's, which has a space of its
/// own.
#[must_use]
pub fn derived(&self, own: GrantSnapshot) -> Self {
let grants = self.inner.grants.derived(own);
self.clone().edit(|inner| {
inner.grants = grants;
inner.agent = None;
inner.space_environment = Vec::new();
})
}

Expand Down
122 changes: 122 additions & 0 deletions sandbox/src/spawn/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,49 @@ async fn environment_layers_resolve_spec_over_source_over_sandbox() {
assert_eq!(out, "sandbox source spec\n");
}

/// A space's variables lead the source's and the sandbox's and yield to the
/// spec's, and a sandbox derived from the space's carries none of them.
#[tokio::test]
async fn the_space_environment_leads_the_source_and_yields_to_the_spec() {
let (_work, work) = canonical_tempdir();
let (tmp, cache) = (work.join("tmp"), work.join("cache"));
let pair = |name: &str, value: &str| (name.to_string(), value.to_string());
let parent = host_sandbox(&work)
.await
.with_environment(vec![
pair("SPACE_TMP", "sandbox"),
pair("SPACE_CACHE", "sandbox"),
pair("SPACE_HOME", "sandbox"),
])
.with_env_source(vec![
pair("SPACE_CACHE", "source"),
pair("SPACE_HOME", "source"),
]);
let child = parent
.derived(GrantSnapshot::default())
.with_space_environment(vec![
("SPACE_TMP".to_string(), tmp.clone()),
("SPACE_CACHE".to_string(), cache.clone()),
("SPACE_HOME".to_string(), work.join("home")),
]);
let spec = || {
SpawnSpec::new("/bin/sh")
.args(["-c", "echo $SPACE_TMP $SPACE_CACHE $SPACE_HOME"])
.env([pair("SPACE_HOME", "spec")])
};
let (exit, out, _) = run_spec(&child, spec()).await;
assert!(exit.success(), "{exit:?}");
assert_eq!(out, format!("{} {} spec\n", tmp.display(), cache.display()));

let nested = child.derived(GrantSnapshot::default());
let (exit, out, _) = run_spec(&nested, spec()).await;
assert!(exit.success(), "{exit:?}");
assert_eq!(
out, "sandbox source spec\n",
"a derived sandbox has no space variables of its parent's"
);
}

/// A container exec that records its script, prints once and then waits for
/// the kill switch.
struct HangingExec {
Expand Down Expand Up @@ -600,6 +643,85 @@ async fn a_container_setup_runs_outside_the_sandbox_that_a_confined_child_gets()
);
}

/// A container child exports its space's variables with each directory as
/// the container spells it, after the source's exports and before the
/// spec's, and a sandbox derived from it exports none of them.
#[tokio::test]
async fn a_container_child_exports_its_space_environment_through_the_mount() {
let (_work, work) = canonical_tempdir();
let space = work.join("space");
let inner = space.join("inner");
std::fs::create_dir_all(&inner).unwrap();
let started = Arc::new(Mutex::new(Vec::new()));
let mount = crate::MountRoot::new(
work.clone(),
crate::ContainerPath::new("/workspace").unwrap(),
);
let pair = |name: &str, value: &str| (name.to_string(), value.to_string());
let parent = reporting_sandbox(
ReportingExec {
started: started.clone(),
killed: Vec::new(),
},
&work,
Some(mount),
)
.with_env_source(vec![pair("TMPDIR", "source-tmp")]);
let child = parent
.derived(GrantSnapshot {
writable: vec![space.clone()],
..GrantSnapshot::default()
})
.with_working_dir(&space)
.with_space_environment(vec![
("TMPDIR".to_string(), space.join("tmp")),
("XDG_CACHE_HOME".to_string(), space.join("cache")),
]);
let spec = || SpawnSpec::script("make").env([pair("TMPDIR", "spec-tmp")]);
let process = child.spawn(spec()).await.expect("the exec starts");
assert!(process.wait().await.success());
let nested = child
.derived(GrantSnapshot {
writable: vec![inner.clone()],
..GrantSnapshot::default()
})
.with_working_dir(&inner);
let process = nested.spawn(spec()).await.expect("the exec starts");
assert!(process.wait().await.success());

let scripts: Vec<String> = started
.lock()
.unwrap()
.iter()
.map(|(script, _)| script.clone())
.collect();
let script = &scripts[0];
let at = |needle: &str| {
script
.find(needle)
.unwrap_or_else(|| panic!("{needle} is exported: {script}"))
};
assert!(
at("source-tmp") < at("/workspace/space/tmp")
&& at("/workspace/space/tmp") < at("spec-tmp"),
"the space's exports follow the source's and precede the spec's: {script}"
);
assert!(
script.contains("export XDG_CACHE_HOME=") && script.contains("/workspace/space/cache"),
"{script}"
);
assert!(
!script.contains(&space.join("cache").display().to_string()),
"the container sees no host spelling: {script}"
);
let nested_script = &scripts[1];
assert!(
!nested_script.contains("XDG_CACHE_HOME")
&& !nested_script.contains("/workspace/space/tmp"),
"a derived sandbox exports no space variables of its parent's: {nested_script}"
);
}

/// A command a spawn's router offers.
struct Probe;

Expand Down
8 changes: 4 additions & 4 deletions sandbox/src/subagent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,15 +215,15 @@ pub enum SubagentGrantError {
/// The embedder's error.
source: SpacePreparationError,
},
/// The embedder named a working copy outside the space it prepared. The
/// space was removed.
/// The embedder named a working copy or an environment directory outside
/// the space it prepared. The space was removed.
#[error(
"the working copy {} prepared for a subagent is outside its space {}",
"the path {} prepared for a subagent is outside its space {}",
.path.display(),
.space.display()
)]
PreparedOutsideSpace {
/// The working copy.
/// The working copy or environment directory, by its canonical path.
path: PathBuf,
/// The space.
space: PathBuf,
Expand Down
Loading
Loading