windows sandbox: support multiple workspace roots (#6854)
The Windows sandbox did not previously support multiple workspace roots via config. Now it does
This commit is contained in:
parent
4fb714fb46
commit
cf57320b9f
8 changed files with 141 additions and 66 deletions
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
|
|
@ -1627,6 +1627,7 @@ name = "codex-windows-sandbox"
|
|||
version = "0.1.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"codex-protocol",
|
||||
"dirs-next",
|
||||
"dunce",
|
||||
"rand 0.8.5",
|
||||
|
|
|
|||
|
|
@ -138,11 +138,7 @@ async fn run_command_under_sandbox(
|
|||
{
|
||||
use codex_windows_sandbox::run_windows_sandbox_capture;
|
||||
|
||||
let policy_str = match &config.sandbox_policy {
|
||||
codex_core::protocol::SandboxPolicy::DangerFullAccess => "workspace-write",
|
||||
codex_core::protocol::SandboxPolicy::ReadOnly => "read-only",
|
||||
codex_core::protocol::SandboxPolicy::WorkspaceWrite { .. } => "workspace-write",
|
||||
};
|
||||
let policy_str = serde_json::to_string(&config.sandbox_policy)?;
|
||||
|
||||
let sandbox_cwd = sandbox_policy_cwd.clone();
|
||||
let cwd_clone = cwd.clone();
|
||||
|
|
@ -153,7 +149,7 @@ async fn run_command_under_sandbox(
|
|||
// Preflight audit is invoked elsewhere at the appropriate times.
|
||||
let res = tokio::task::spawn_blocking(move || {
|
||||
run_windows_sandbox_capture(
|
||||
policy_str,
|
||||
policy_str.as_str(),
|
||||
&sandbox_cwd,
|
||||
base_dir.as_path(),
|
||||
command_vec,
|
||||
|
|
|
|||
|
|
@ -182,12 +182,11 @@ async fn exec_windows_sandbox(
|
|||
..
|
||||
} = params;
|
||||
|
||||
let policy_str = match sandbox_policy {
|
||||
SandboxPolicy::DangerFullAccess => "workspace-write",
|
||||
SandboxPolicy::ReadOnly => "read-only",
|
||||
SandboxPolicy::WorkspaceWrite { .. } => "workspace-write",
|
||||
};
|
||||
|
||||
let policy_str = serde_json::to_string(sandbox_policy).map_err(|err| {
|
||||
CodexErr::Io(io::Error::other(format!(
|
||||
"failed to serialize Windows sandbox policy: {err}"
|
||||
)))
|
||||
})?;
|
||||
let sandbox_cwd = cwd.clone();
|
||||
let codex_home = find_codex_home().map_err(|err| {
|
||||
CodexErr::Io(io::Error::other(format!(
|
||||
|
|
@ -196,7 +195,7 @@ async fn exec_windows_sandbox(
|
|||
})?;
|
||||
let spawn_res = tokio::task::spawn_blocking(move || {
|
||||
run_windows_sandbox_capture(
|
||||
policy_str,
|
||||
policy_str.as_str(),
|
||||
&sandbox_cwd,
|
||||
codex_home.as_ref(),
|
||||
command,
|
||||
|
|
@ -444,7 +443,9 @@ async fn exec(
|
|||
stdout_stream: Option<StdoutStream>,
|
||||
) -> Result<RawExecToolCallOutput> {
|
||||
#[cfg(target_os = "windows")]
|
||||
if sandbox == SandboxType::WindowsRestrictedToken {
|
||||
if sandbox == SandboxType::WindowsRestrictedToken
|
||||
&& !matches!(sandbox_policy, SandboxPolicy::DangerFullAccess)
|
||||
{
|
||||
return exec_windows_sandbox(params, sandbox_policy).await;
|
||||
}
|
||||
let timeout = params.timeout_duration();
|
||||
|
|
|
|||
|
|
@ -12,6 +12,9 @@ anyhow = "1.0"
|
|||
serde = { version = "1.0", features = ["derive"] }
|
||||
serde_json = "1.0"
|
||||
dunce = "1.0"
|
||||
[dependencies.codex-protocol]
|
||||
package = "codex-protocol"
|
||||
path = "../protocol"
|
||||
[dependencies.rand]
|
||||
version = "0.8"
|
||||
default-features = false
|
||||
|
|
|
|||
|
|
@ -50,6 +50,7 @@ TIMEOUT_SEC = 20
|
|||
|
||||
WS_ROOT = Path(os.environ["USERPROFILE"]) / "sbx_ws_tests"
|
||||
OUTSIDE = Path(os.environ["USERPROFILE"]) / "sbx_ws_outside" # outside CWD for deny checks
|
||||
EXTRA_ROOT = Path(os.environ["USERPROFILE"]) / "WorkspaceRoot" # additional writable root
|
||||
|
||||
ENV_BASE = {} # extend if needed
|
||||
|
||||
|
|
@ -57,7 +58,13 @@ class CaseResult:
|
|||
def __init__(self, name: str, ok: bool, detail: str = ""):
|
||||
self.name, self.ok, self.detail = name, ok, detail
|
||||
|
||||
def run_sbx(policy: str, cmd_argv: List[str], cwd: Path, env_extra: Optional[dict] = None) -> Tuple[int, str, str]:
|
||||
def run_sbx(
|
||||
policy: str,
|
||||
cmd_argv: List[str],
|
||||
cwd: Path,
|
||||
env_extra: Optional[dict] = None,
|
||||
additional_root: Optional[Path] = None,
|
||||
) -> Tuple[int, str, str]:
|
||||
env = os.environ.copy()
|
||||
env.update(ENV_BASE)
|
||||
if env_extra:
|
||||
|
|
@ -68,7 +75,15 @@ def run_sbx(policy: str, cmd_argv: List[str], cwd: Path, env_extra: Optional[dic
|
|||
raise ValueError(f"unknown policy: {policy}")
|
||||
policy_flags: List[str] = ["--full-auto"] if policy == "workspace-write" else []
|
||||
|
||||
argv = [*CODEX_CMD, "sandbox", "windows", *policy_flags, "--", *cmd_argv]
|
||||
overrides: List[str] = []
|
||||
if policy == "workspace-write" and additional_root is not None:
|
||||
# Use config override to inject an additional writable root.
|
||||
overrides = [
|
||||
"-c",
|
||||
f'sandbox_workspace_write.writable_roots=["{additional_root.as_posix()}"]',
|
||||
]
|
||||
|
||||
argv = [*CODEX_CMD, "sandbox", "windows", *policy_flags, *overrides, "--", *cmd_argv]
|
||||
print(cmd_argv)
|
||||
cp = subprocess.run(argv, cwd=str(cwd), env=env,
|
||||
stdout=subprocess.PIPE, stderr=subprocess.PIPE,
|
||||
|
|
@ -134,6 +149,7 @@ def main() -> int:
|
|||
results: List[CaseResult] = []
|
||||
make_dir_clean(WS_ROOT)
|
||||
OUTSIDE.mkdir(exist_ok=True)
|
||||
EXTRA_ROOT.mkdir(exist_ok=True)
|
||||
# Environment probe: some hosts allow TEMP writes even under read-only
|
||||
# tokens due to ACLs and restricted SID semantics. Detect and adapt tests.
|
||||
probe_rc, _, _ = run_sbx(
|
||||
|
|
@ -165,6 +181,32 @@ def main() -> int:
|
|||
rc, out, err = run_sbx("workspace-write", ["cmd", "/c", f"echo nope > {outside_file}"], WS_ROOT)
|
||||
add("WS: write outside workspace denied", rc != 0 and assert_not_exists(outside_file), f"rc={rc}")
|
||||
|
||||
# 3b. WS: allow write in additional workspace root
|
||||
extra_target = EXTRA_ROOT / "extra_ok.txt"
|
||||
remove_if_exists(extra_target)
|
||||
rc, out, err = run_sbx(
|
||||
"workspace-write",
|
||||
["cmd", "/c", f"echo extra > {extra_target}"],
|
||||
WS_ROOT,
|
||||
additional_root=EXTRA_ROOT,
|
||||
)
|
||||
add("WS: write in additional root allowed", rc == 0 and assert_exists(extra_target), f"rc={rc}, err={err}")
|
||||
|
||||
# 3c. RO: deny write in additional workspace root
|
||||
ro_extra_target = EXTRA_ROOT / "extra_ro.txt"
|
||||
remove_if_exists(ro_extra_target)
|
||||
rc, out, err = run_sbx(
|
||||
"read-only",
|
||||
["cmd", "/c", f"echo nope > {ro_extra_target}"],
|
||||
WS_ROOT,
|
||||
additional_root=EXTRA_ROOT,
|
||||
)
|
||||
add(
|
||||
"RO: write in additional root denied",
|
||||
rc != 0 and assert_not_exists(ro_extra_target),
|
||||
f"rc={rc}",
|
||||
)
|
||||
|
||||
# 4. WS: allow TEMP write
|
||||
rc, out, err = run_sbx("workspace-write", ["cmd", "/c", "echo tempok > %TEMP%\\ws_temp_ok.txt"], WS_ROOT)
|
||||
add("WS: TEMP write allowed", rc == 0, f"rc={rc}")
|
||||
|
|
|
|||
|
|
@ -1,37 +1,83 @@
|
|||
use crate::policy::SandboxMode;
|
||||
use crate::policy::SandboxPolicy;
|
||||
use dunce::canonicalize;
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
pub fn compute_allow_paths(
|
||||
policy: &SandboxPolicy,
|
||||
_policy_cwd: &Path,
|
||||
policy_cwd: &Path,
|
||||
command_cwd: &Path,
|
||||
env_map: &HashMap<String, String>,
|
||||
) -> Vec<PathBuf> {
|
||||
let mut allow: Vec<PathBuf> = Vec::new();
|
||||
let mut seen = std::collections::HashSet::new();
|
||||
if matches!(policy.0, SandboxMode::WorkspaceWrite) {
|
||||
let abs = command_cwd.to_path_buf();
|
||||
if seen.insert(abs.to_string_lossy().to_string()) && abs.exists() {
|
||||
allow.push(abs);
|
||||
|
||||
let mut add_path = |p: PathBuf| {
|
||||
if seen.insert(p.to_string_lossy().to_string()) && p.exists() {
|
||||
allow.push(p);
|
||||
}
|
||||
};
|
||||
|
||||
if matches!(policy, SandboxPolicy::WorkspaceWrite { .. }) {
|
||||
add_path(command_cwd.to_path_buf());
|
||||
if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = policy {
|
||||
for root in writable_roots {
|
||||
let candidate = if root.is_absolute() {
|
||||
root.clone()
|
||||
} else {
|
||||
policy_cwd.join(root)
|
||||
};
|
||||
let canonical = canonicalize(&candidate).unwrap_or(candidate);
|
||||
add_path(canonical);
|
||||
}
|
||||
}
|
||||
}
|
||||
if !matches!(policy.0, SandboxMode::ReadOnly) {
|
||||
if !matches!(policy, SandboxPolicy::ReadOnly) {
|
||||
for key in ["TEMP", "TMP"] {
|
||||
if let Some(v) = env_map.get(key) {
|
||||
let abs = PathBuf::from(v);
|
||||
if seen.insert(abs.to_string_lossy().to_string()) && abs.exists() {
|
||||
allow.push(abs);
|
||||
}
|
||||
add_path(abs);
|
||||
} else if let Ok(v) = std::env::var(key) {
|
||||
let abs = PathBuf::from(v);
|
||||
if seen.insert(abs.to_string_lossy().to_string()) && abs.exists() {
|
||||
allow.push(abs);
|
||||
}
|
||||
add_path(abs);
|
||||
}
|
||||
}
|
||||
}
|
||||
allow
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::compute_allow_paths;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use std::collections::HashMap;
|
||||
use std::fs;
|
||||
use std::path::PathBuf;
|
||||
|
||||
#[test]
|
||||
fn includes_additional_writable_roots() {
|
||||
let command_cwd = PathBuf::from(r"C:\Workspace");
|
||||
let extra_root = PathBuf::from(r"C:\AdditionalWritableRoot");
|
||||
let _ = fs::create_dir_all(&command_cwd);
|
||||
let _ = fs::create_dir_all(&extra_root);
|
||||
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![extra_root.clone()],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
};
|
||||
|
||||
let allow = compute_allow_paths(&policy, &command_cwd, &command_cwd, &HashMap::new());
|
||||
|
||||
assert!(
|
||||
allow.iter().any(|p| p == &command_cwd),
|
||||
"command cwd should be allowed"
|
||||
);
|
||||
assert!(
|
||||
allow.iter().any(|p| p == &extra_root),
|
||||
"additional writable root should be allowed"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -40,7 +40,7 @@ mod windows_impl {
|
|||
use super::logging::log_failure;
|
||||
use super::logging::log_start;
|
||||
use super::logging::log_success;
|
||||
use super::policy::SandboxMode;
|
||||
use super::policy::parse_policy;
|
||||
use super::policy::SandboxPolicy;
|
||||
use super::token::convert_string_sid_to_sid;
|
||||
use super::winutil::format_last_error;
|
||||
|
|
@ -194,7 +194,7 @@ mod windows_impl {
|
|||
mut env_map: HashMap<String, String>,
|
||||
timeout_ms: Option<u64>,
|
||||
) -> Result<CaptureResult> {
|
||||
let policy = SandboxPolicy::parse(policy_json_or_preset)?;
|
||||
let policy = parse_policy(policy_json_or_preset)?;
|
||||
normalize_null_device_env(&mut env_map);
|
||||
ensure_non_interactive_pager(&mut env_map);
|
||||
apply_no_network_to_env(&mut env_map)?;
|
||||
|
|
@ -206,27 +206,32 @@ mod windows_impl {
|
|||
let logs_base_dir = Some(codex_home);
|
||||
log_start(&command, logs_base_dir);
|
||||
let cap_sid_path = cap_sid_file(codex_home);
|
||||
let is_workspace_write = matches!(&policy, SandboxPolicy::WorkspaceWrite { .. });
|
||||
|
||||
let (h_token, psid_to_use): (HANDLE, *mut c_void) = unsafe {
|
||||
match &policy.0 {
|
||||
SandboxMode::ReadOnly => {
|
||||
match &policy {
|
||||
SandboxPolicy::ReadOnly => {
|
||||
let caps = load_or_create_cap_sids(codex_home);
|
||||
ensure_dir(&cap_sid_path)?;
|
||||
fs::write(&cap_sid_path, serde_json::to_string(&caps)?)?;
|
||||
let psid = convert_string_sid_to_sid(&caps.readonly).unwrap();
|
||||
super::token::create_readonly_token_with_cap(psid)?
|
||||
}
|
||||
SandboxMode::WorkspaceWrite => {
|
||||
SandboxPolicy::WorkspaceWrite { .. } => {
|
||||
let caps = load_or_create_cap_sids(codex_home);
|
||||
ensure_dir(&cap_sid_path)?;
|
||||
fs::write(&cap_sid_path, serde_json::to_string(&caps)?)?;
|
||||
let psid = convert_string_sid_to_sid(&caps.workspace).unwrap();
|
||||
super::token::create_workspace_write_token_with_cap(psid)?
|
||||
}
|
||||
SandboxPolicy::DangerFullAccess => {
|
||||
anyhow::bail!("DangerFullAccess is not supported for sandboxing")
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
unsafe {
|
||||
if matches!(policy.0, SandboxMode::WorkspaceWrite) {
|
||||
if is_workspace_write {
|
||||
if let Ok(base) = super::token::get_current_token_for_restriction() {
|
||||
if let Ok(bytes) = super::token::get_logon_sid_bytes(base) {
|
||||
let mut tmp = bytes.clone();
|
||||
|
|
@ -238,7 +243,7 @@ mod windows_impl {
|
|||
}
|
||||
}
|
||||
|
||||
let persist_aces = matches!(policy.0, SandboxMode::WorkspaceWrite);
|
||||
let persist_aces = is_workspace_write;
|
||||
let allow = compute_allow_paths(&policy, sandbox_policy_cwd, ¤t_dir, &env_map);
|
||||
let mut guards: Vec<(PathBuf, *mut c_void)> = Vec::new();
|
||||
unsafe {
|
||||
|
|
|
|||
|
|
@ -1,36 +1,17 @@
|
|||
use anyhow::Result;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
pub use codex_protocol::protocol::SandboxPolicy;
|
||||
|
||||
#[derive(Clone, Debug, Serialize, Deserialize)]
|
||||
pub struct PolicyJson {
|
||||
pub mode: String,
|
||||
#[serde(default)]
|
||||
pub workspace_roots: Vec<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub enum SandboxMode {
|
||||
ReadOnly,
|
||||
WorkspaceWrite,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct SandboxPolicy(pub SandboxMode);
|
||||
|
||||
impl SandboxPolicy {
|
||||
pub fn parse(value: &str) -> Result<Self> {
|
||||
match value {
|
||||
"read-only" => Ok(SandboxPolicy(SandboxMode::ReadOnly)),
|
||||
"workspace-write" => Ok(SandboxPolicy(SandboxMode::WorkspaceWrite)),
|
||||
other => {
|
||||
let pj: PolicyJson = serde_json::from_str(other)?;
|
||||
Ok(match pj.mode.as_str() {
|
||||
"read-only" => SandboxPolicy(SandboxMode::ReadOnly),
|
||||
"workspace-write" => SandboxPolicy(SandboxMode::WorkspaceWrite),
|
||||
_ => SandboxPolicy(SandboxMode::ReadOnly),
|
||||
})
|
||||
pub fn parse_policy(value: &str) -> Result<SandboxPolicy> {
|
||||
match value {
|
||||
"read-only" => Ok(SandboxPolicy::ReadOnly),
|
||||
"workspace-write" => Ok(SandboxPolicy::new_workspace_write_policy()),
|
||||
"danger-full-access" => anyhow::bail!("DangerFullAccess is not supported for sandboxing"),
|
||||
other => {
|
||||
let parsed: SandboxPolicy = serde_json::from_str(other)?;
|
||||
if matches!(parsed, SandboxPolicy::DangerFullAccess) {
|
||||
anyhow::bail!("DangerFullAccess is not supported for sandboxing");
|
||||
}
|
||||
Ok(parsed)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue