fix(sandbox): accès Git minimal pour Codex — writable roots restreints au .git résolu, sans blanket grant project (#195, QA verte)
Codex ne pouvait pas accéder aux métadonnées Git sous sandbox workspace-write. À l'inverse, la factory relayait jusqu'ici le project_root entier en --add-dir, soit un grant large non défendable. Change pour n'accorder en écriture (Codex --add-dir ET SandboxPlan OS) que le .git réellement résolu : - resolve_git_dir gère .git/ directory ET fichier worktree 'gitdir: <path>' (absolu ou relatif au parent du marker), puis canonicalize. - enrich_sandbox_plan_with_codex_git_roots ajoute/déduplique les roots en RW sur le SandboxPlan existant (préserve l'autorité EffectivePermissions). - factory: défaut writable_roots = Vec::new() en l'absence de policy explicite. Invariants conservés : - project_root n'est jamais injecté comme writable root. - aucun élargissement au parent commun (canonicalize + BTreeSet). - permissions MCP/globales inchangées (seul le plan runtime est enrichi). Tests (crates/application) : - standard .git/ dir -> root unique. - projet non-Git -> aucun root. - worktree (.git file gitdir:) -> résolution + canonicalization. - symlink -> dédup, pas de grant parent. - factory: défaut sans policy n'émet plus --add-dir project_root. Réserve QA: launch_conventionfile_injects_assigned_skills_in_order (agent_lifecycle) arbitré hors-scope/non bloquant pour #195.
This commit is contained in:
@ -16,6 +16,7 @@
|
||||
//! `LaunchAgent`: `prepare_invocation` → injection (fs write) → `pty.spawn`.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
use async_trait::async_trait;
|
||||
@ -615,6 +616,7 @@ struct FakeStructuredFactory {
|
||||
starts: Arc<Mutex<Vec<ProfileId>>>,
|
||||
efforts: Arc<Mutex<Vec<Option<String>>>>,
|
||||
envs: Arc<Mutex<Vec<Vec<(String, String)>>>>,
|
||||
sandboxes: Arc<Mutex<Vec<Option<domain::sandbox::SandboxPlan>>>>,
|
||||
policies: Arc<Mutex<Vec<Option<domain::ports::StructuredProviderLaunchPolicy>>>>,
|
||||
shutdowns: Arc<Mutex<Vec<SessionId>>>,
|
||||
next_session: SessionId,
|
||||
@ -627,6 +629,7 @@ impl FakeStructuredFactory {
|
||||
starts: Arc::new(Mutex::new(Vec::new())),
|
||||
efforts: Arc::new(Mutex::new(Vec::new())),
|
||||
envs: Arc::new(Mutex::new(Vec::new())),
|
||||
sandboxes: Arc::new(Mutex::new(Vec::new())),
|
||||
policies: Arc::new(Mutex::new(Vec::new())),
|
||||
shutdowns: Arc::new(Mutex::new(Vec::new())),
|
||||
next_session,
|
||||
@ -645,6 +648,10 @@ impl FakeStructuredFactory {
|
||||
self.envs.lock().unwrap().clone()
|
||||
}
|
||||
|
||||
fn sandboxes(&self) -> Vec<Option<domain::sandbox::SandboxPlan>> {
|
||||
self.sandboxes.lock().unwrap().clone()
|
||||
}
|
||||
|
||||
fn policies(&self) -> Vec<Option<domain::ports::StructuredProviderLaunchPolicy>> {
|
||||
self.policies.lock().unwrap().clone()
|
||||
}
|
||||
@ -677,6 +684,7 @@ impl AgentSessionFactory for FakeStructuredFactory {
|
||||
.unwrap()
|
||||
.push(profile.model_reasoning_effort.clone());
|
||||
self.envs.lock().unwrap().push(_env.to_vec());
|
||||
self.sandboxes.lock().unwrap().push(_sandbox.cloned());
|
||||
self.policies
|
||||
.lock()
|
||||
.unwrap()
|
||||
@ -738,11 +746,21 @@ fn sid(n: u128) -> SessionId {
|
||||
SessionId::from_uuid(Uuid::from_u128(n))
|
||||
}
|
||||
|
||||
fn fresh_temp_dir(label: &str) -> PathBuf {
|
||||
let dir = std::env::temp_dir().join(format!("idea-agent-lifecycle-{label}-{}", Uuid::new_v4()));
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
dir
|
||||
}
|
||||
|
||||
fn project() -> Project {
|
||||
project_at("/home/me/proj")
|
||||
}
|
||||
|
||||
fn project_at(root: &str) -> Project {
|
||||
Project::new(
|
||||
ProjectId::from_uuid(Uuid::from_u128(1000)),
|
||||
"demo",
|
||||
ProjectPath::new("/home/me/proj").unwrap(),
|
||||
ProjectPath::new(root).unwrap(),
|
||||
RemoteRef::local(),
|
||||
1_700_000_000_000,
|
||||
)
|
||||
@ -1183,8 +1201,16 @@ fn launch_fixture_with_profile_agent_and_recall(
|
||||
}
|
||||
|
||||
fn launch_input(agent_id: AgentId) -> LaunchAgentInput {
|
||||
launch_input_with_project(agent_id, project())
|
||||
}
|
||||
|
||||
fn launch_input_with_project_root(agent_id: AgentId, root: &Path) -> LaunchAgentInput {
|
||||
launch_input_with_project(agent_id, project_at(&root.to_string_lossy()))
|
||||
}
|
||||
|
||||
fn launch_input_with_project(agent_id: AgentId, project: Project) -> LaunchAgentInput {
|
||||
LaunchAgentInput {
|
||||
project: project(),
|
||||
project,
|
||||
agent_id,
|
||||
rows: 24,
|
||||
cols: 80,
|
||||
@ -4048,6 +4074,174 @@ async fn codex_structured_policy_carries_exec_safe_sandbox_roots_and_network() {
|
||||
);
|
||||
}
|
||||
|
||||
async fn launch_codex_structured_for_project(
|
||||
project_root: &Path,
|
||||
) -> (
|
||||
FakeStructuredFactory,
|
||||
Vec<Option<domain::ports::StructuredProviderLaunchPolicy>>,
|
||||
Vec<Option<domain::sandbox::SandboxPlan>>,
|
||||
) {
|
||||
let profile = codex_profile()
|
||||
.with_projector(ProjectorKey::Codex)
|
||||
.with_structured_adapter(StructuredAdapter::Codex);
|
||||
let (mut launch, agent, _fs, pty, _s) = launch_with_projection(
|
||||
profile,
|
||||
Some(ContextInjectionPlan::File {
|
||||
target: "AGENTS.md".to_owned(),
|
||||
}),
|
||||
Some(full_registry()),
|
||||
Some(perm_doc(Posture::Ask)),
|
||||
);
|
||||
let factory = FakeStructuredFactory::new(trace(), sid(890));
|
||||
let structured = Arc::new(StructuredSessions::new());
|
||||
launch = launch
|
||||
.with_structured_routing_mode(StructuredRoutingMode::RequireStructured)
|
||||
.with_structured(Arc::new(factory.clone()), Arc::clone(&structured));
|
||||
|
||||
launch
|
||||
.execute(launch_input_with_project_root(agent.id, project_root))
|
||||
.await
|
||||
.expect("structured launch");
|
||||
|
||||
assert!(
|
||||
pty.spawns().is_empty(),
|
||||
"structured route must not spawn PTY"
|
||||
);
|
||||
(factory.clone(), factory.policies(), factory.sandboxes())
|
||||
}
|
||||
|
||||
fn assert_codex_git_roots(
|
||||
project_root: &Path,
|
||||
policies: Vec<Option<domain::ports::StructuredProviderLaunchPolicy>>,
|
||||
sandboxes: Vec<Option<domain::sandbox::SandboxPlan>>,
|
||||
expected_roots: Vec<String>,
|
||||
) {
|
||||
assert_eq!(policies.len(), 1, "exactly one structured launch policy");
|
||||
let Some(domain::ports::StructuredProviderLaunchPolicy::Codex {
|
||||
sandbox_mode,
|
||||
writable_roots,
|
||||
network_access: _,
|
||||
}) = policies.into_iter().next().unwrap()
|
||||
else {
|
||||
panic!("expected a Codex structured launch policy");
|
||||
};
|
||||
assert_eq!(sandbox_mode, "workspace-write");
|
||||
assert_eq!(
|
||||
writable_roots, expected_roots,
|
||||
"Codex writable_roots must be the resolved Git roots only"
|
||||
);
|
||||
assert!(
|
||||
!writable_roots.contains(&project_root.to_string_lossy().into_owned()),
|
||||
"project root must not be granted as a Codex writable root"
|
||||
);
|
||||
|
||||
assert_eq!(sandboxes.len(), 1, "exactly one structured sandbox plan");
|
||||
let sandbox = sandboxes.into_iter().next().unwrap().expect("sandbox plan");
|
||||
let sandbox_roots = sandbox
|
||||
.allowed
|
||||
.iter()
|
||||
.map(|grant| grant.abs_root.clone())
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(
|
||||
sandbox_roots, writable_roots,
|
||||
"OS sandbox Git enrichment must match Codex writable_roots"
|
||||
);
|
||||
assert!(
|
||||
sandbox
|
||||
.allowed
|
||||
.iter()
|
||||
.all(|grant| grant.access == domain::sandbox::PathAccess::RW),
|
||||
"Git roots must be added as write grants only"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn codex_git_enrichment_adds_standard_git_dir_without_project_root_blanket() {
|
||||
let root = fresh_temp_dir("standard-git");
|
||||
std::fs::create_dir(root.join(".git")).unwrap();
|
||||
let expected_git = root.join(".git").canonicalize().unwrap();
|
||||
|
||||
let (_factory, policies, sandboxes) = launch_codex_structured_for_project(&root).await;
|
||||
|
||||
assert_codex_git_roots(
|
||||
&root,
|
||||
policies,
|
||||
sandboxes,
|
||||
vec![expected_git.to_string_lossy().into_owned()],
|
||||
);
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn codex_git_enrichment_adds_no_roots_for_non_git_project() {
|
||||
let root = fresh_temp_dir("non-git");
|
||||
|
||||
let (_factory, policies, sandboxes) = launch_codex_structured_for_project(&root).await;
|
||||
|
||||
assert_codex_git_roots(&root, policies, sandboxes, Vec::new());
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn codex_git_enrichment_resolves_worktree_gitdir_file() {
|
||||
let base = fresh_temp_dir("worktree");
|
||||
let root = base.join("worktree");
|
||||
let git_dir = base.join("common").join("worktrees").join("wt");
|
||||
std::fs::create_dir_all(&root).unwrap();
|
||||
std::fs::create_dir_all(&git_dir).unwrap();
|
||||
std::fs::write(root.join(".git"), "gitdir: ../common/worktrees/wt\n").unwrap();
|
||||
let expected_git = git_dir.canonicalize().unwrap();
|
||||
|
||||
let (_factory, policies, sandboxes) = launch_codex_structured_for_project(&root).await;
|
||||
|
||||
assert_codex_git_roots(
|
||||
&root,
|
||||
policies,
|
||||
sandboxes,
|
||||
vec![expected_git.to_string_lossy().into_owned()],
|
||||
);
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn codex_git_enrichment_canonicalizes_and_avoids_parent_grants() {
|
||||
let base = fresh_temp_dir("canonical");
|
||||
let real_root = base.join("real");
|
||||
let linked_root = base.join("linked");
|
||||
std::fs::create_dir_all(real_root.join(".git")).unwrap();
|
||||
#[cfg(unix)]
|
||||
std::os::unix::fs::symlink(&real_root, &linked_root).unwrap();
|
||||
#[cfg(not(unix))]
|
||||
std::fs::create_dir_all(&linked_root).unwrap();
|
||||
|
||||
let launch_root = if cfg!(unix) { &linked_root } else { &real_root };
|
||||
let expected_git = real_root.join(".git").canonicalize().unwrap();
|
||||
|
||||
let (_factory, policies, sandboxes) = launch_codex_structured_for_project(launch_root).await;
|
||||
|
||||
assert_codex_git_roots(
|
||||
launch_root,
|
||||
policies.clone(),
|
||||
sandboxes,
|
||||
vec![expected_git.to_string_lossy().into_owned()],
|
||||
);
|
||||
let Some(domain::ports::StructuredProviderLaunchPolicy::Codex { writable_roots, .. }) =
|
||||
policies.into_iter().next().unwrap()
|
||||
else {
|
||||
panic!("expected Codex policy");
|
||||
};
|
||||
assert_eq!(writable_roots.len(), 1, "Git roots must be deduplicated");
|
||||
assert!(
|
||||
!writable_roots.contains(&base.to_string_lossy().into_owned()),
|
||||
"common parent must not be granted"
|
||||
);
|
||||
assert!(
|
||||
!writable_roots.contains(&real_root.to_string_lossy().into_owned()),
|
||||
"repo parent must not be granted"
|
||||
);
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
}
|
||||
|
||||
/// (1d) A non-projectable profile (no projector, no CLAUDE.md, no Codex signal) ⇒
|
||||
/// no projection at all, even with a full registry and a posed policy.
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user