fix(sandbox): writable roots Codex dérivées des permissions effectives du menu agent — #197 (QA verte)

This commit is contained in:
2026-08-14 15:05:15 +02:00
parent 6a98752cb0
commit a60fb3ed92
6 changed files with 482 additions and 146 deletions

View File

@ -25,8 +25,8 @@ use domain::events::DomainEvent;
use domain::ids::{AgentId, ProfileId, ProjectId};
use domain::markdown::MarkdownDoc;
use domain::permission::{
EffectivePermissions, PermissionProjection, PermissionProjector, Posture, ProjectedFile,
ProjectionContext, ProjectorKey,
Capability, Effect, EffectivePermissions, PathScope, PermissionProjection, PermissionProjector,
PermissionRule, Posture, ProjectedFile, ProjectionContext, ProjectorKey,
};
use domain::ports::{
AgentContextStore, AgentRuntime, AgentSession, AgentSessionError, AgentSessionFactory, Clock,
@ -3744,6 +3744,19 @@ fn perm_doc(posture: Posture) -> ProjectPermissions {
ProjectPermissions::new(Some(PermissionSet::new(vec![], posture)), Vec::new())
}
fn perm_doc_with_rules(rules: Vec<PermissionRule>, posture: Posture) -> ProjectPermissions {
ProjectPermissions::new(Some(PermissionSet::new(rules, posture)), Vec::new())
}
fn file_rule(capability: Capability, effect: Effect, patterns: &[&str]) -> PermissionRule {
PermissionRule::file(
capability,
effect,
PathScope::new(patterns.iter().map(|pattern| (*pattern).to_owned())).unwrap(),
)
.unwrap()
}
/// Builds a `codex` profile (convention `AGENTS.md`) with the given customiser, so
/// the structured-adapter / projector / mcp variants can be exercised.
fn codex_profile() -> AgentProfile {
@ -4179,8 +4192,7 @@ async fn launch_codex_structured_for_project(
(factory.clone(), factory.policies(), factory.sandboxes())
}
fn assert_codex_git_roots(
project_root: &Path,
fn assert_codex_writable_roots(
policies: Vec<Option<domain::ports::StructuredProviderLaunchPolicy>>,
sandboxes: Vec<Option<domain::sandbox::SandboxPlan>>,
expected_roots: Vec<String>,
@ -4197,11 +4209,7 @@ fn assert_codex_git_roots(
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"
"Codex writable_roots must mirror resolved write permission roots"
);
assert_eq!(sandboxes.len(), 1, "exactly one structured sandbox plan");
@ -4213,67 +4221,67 @@ fn assert_codex_git_roots(
.collect::<Vec<_>>();
assert_eq!(
sandbox_roots, writable_roots,
"OS sandbox Git enrichment must match Codex writable_roots"
"OS sandbox write roots 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"
"Codex writable roots must be added as write grants"
);
}
#[tokio::test]
async fn codex_git_enrichment_adds_standard_git_dir_without_project_root_blanket() {
async fn codex_structured_project_write_grant_includes_project_root() {
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,
assert_codex_writable_roots(
policies,
sandboxes,
vec![expected_git.to_string_lossy().into_owned()],
vec![root.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() {
async fn codex_structured_project_write_grant_does_not_require_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());
assert_codex_writable_roots(
policies,
sandboxes,
vec![root.to_string_lossy().into_owned()],
);
let _ = std::fs::remove_dir_all(&root);
}
#[tokio::test]
async fn codex_git_enrichment_resolves_worktree_gitdir_file() {
async fn codex_structured_project_write_grant_does_not_add_worktree_gitdir() {
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,
assert_codex_writable_roots(
policies,
sandboxes,
vec![expected_git.to_string_lossy().into_owned()],
vec![root.to_string_lossy().into_owned()],
);
let _ = std::fs::remove_dir_all(&base);
}
#[tokio::test]
async fn codex_git_enrichment_canonicalizes_and_avoids_parent_grants() {
async fn codex_structured_project_write_grant_uses_launch_root_without_parent_grants() {
let base = fresh_temp_dir("canonical");
let real_root = base.join("real");
let linked_root = base.join("linked");
@ -4284,33 +4292,218 @@ async fn codex_git_enrichment_canonicalizes_and_avoids_parent_grants() {
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,
assert_codex_writable_roots(
policies.clone(),
sandboxes,
vec![expected_git.to_string_lossy().into_owned()],
vec![launch_root.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_eq!(
writable_roots.len(),
1,
"permission 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"
"canonical repo parent must not be granted when the launch root differs"
);
let _ = std::fs::remove_dir_all(&base);
}
#[tokio::test]
async fn codex_structured_fallback_deny_with_explicit_write_allow_grants_only_subtree() {
let root = fresh_temp_dir("explicit-write-subtree");
let allowed = root.join("out");
std::fs::create_dir_all(&allowed).unwrap();
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_with_rules(
vec![file_rule(Capability::Write, Effect::Allow, &["out/**"])],
Posture::Deny,
)),
);
let factory = FakeStructuredFactory::new(trace(), sid(892));
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, &root))
.await
.expect("structured launch");
assert!(
pty.spawns().is_empty(),
"structured route must not spawn PTY"
);
assert_codex_writable_roots(
factory.policies(),
factory.sandboxes(),
vec![allowed.to_string_lossy().into_owned()],
);
let _ = std::fs::remove_dir_all(&root);
}
#[tokio::test]
async fn codex_structured_broad_write_with_subpath_deny_does_not_grant_project_root() {
let root = fresh_temp_dir("broad-write-subpath-deny");
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_with_rules(
vec![
file_rule(Capability::Write, Effect::Allow, &["**"]),
file_rule(Capability::Write, Effect::Deny, &["secret/**"]),
],
Posture::Deny,
)),
);
let factory = FakeStructuredFactory::new(trace(), sid(893));
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, &root))
.await
.expect("structured launch");
assert!(
pty.spawns().is_empty(),
"structured route must not spawn PTY"
);
assert_eq!(
factory.policies(),
vec![Some(domain::ports::StructuredProviderLaunchPolicy::Codex {
sandbox_mode: "read-only".to_owned(),
writable_roots: Vec::new(),
network_access: false,
})],
"Codex must not receive project-root write when a subpath write deny cannot be carved out"
);
let sandbox = factory.sandboxes().remove(0).expect("sandbox plan");
assert!(
sandbox.allowed.is_empty(),
"OS sandbox must not grant a broad root that would include the denied subpath"
);
let _ = std::fs::remove_dir_all(&root);
}
#[tokio::test]
async fn codex_structured_fallback_write_with_subpath_deny_fails_closed() {
let root = fresh_temp_dir("fallback-write-subpath-deny");
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_with_rules(
vec![file_rule(Capability::Write, Effect::Deny, &["secret/**"])],
Posture::Allow,
)),
);
let factory = FakeStructuredFactory::new(trace(), sid(894));
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, &root))
.await
.expect("structured launch");
assert!(
pty.spawns().is_empty(),
"structured route must not spawn PTY"
);
assert_eq!(
factory.policies(),
vec![Some(domain::ports::StructuredProviderLaunchPolicy::Codex {
sandbox_mode: "read-only".to_owned(),
writable_roots: Vec::new(),
network_access: false,
})],
"fallback-wide write plus a subpath deny cannot be represented by Codex writable roots"
);
let _ = std::fs::remove_dir_all(&root);
}
#[tokio::test]
async fn codex_structured_disjoint_write_allow_survives_unrelated_subpath_deny() {
let root = fresh_temp_dir("disjoint-write-deny");
let allowed = root.join("out");
std::fs::create_dir_all(&allowed).unwrap();
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_with_rules(
vec![
file_rule(Capability::Write, Effect::Allow, &["out/**"]),
file_rule(Capability::Write, Effect::Deny, &["secret/**"]),
],
Posture::Deny,
)),
);
let factory = FakeStructuredFactory::new(trace(), sid(895));
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, &root))
.await
.expect("structured launch");
assert!(
pty.spawns().is_empty(),
"structured route must not spawn PTY"
);
assert_codex_writable_roots(
factory.policies(),
factory.sandboxes(),
vec![allowed.to_string_lossy().into_owned()],
);
let _ = std::fs::remove_dir_all(&root);
}
#[tokio::test]
async fn managed_temp_exposure_enriches_codex_policy_and_os_sandbox_from_same_resolution() {
let root = fresh_temp_dir("managed-temp-exposure");
@ -4368,18 +4561,24 @@ async fn managed_temp_exposure_enriches_codex_policy_and_os_sandbox_from_same_re
factory.policies(),
vec![Some(domain::ports::StructuredProviderLaunchPolicy::Codex {
sandbox_mode: "workspace-write".to_owned(),
writable_roots: vec![managed_root.clone()],
writable_roots: vec![root.to_string_lossy().into_owned(), managed_root.clone()],
network_access: false,
})],
"Codex writable_roots must come from the resolved managed-temp path"
"Codex writable_roots must include permission roots plus the resolved managed-temp path"
);
let sandbox = factory.sandboxes().remove(0).expect("sandbox plan");
assert_eq!(
sandbox.allowed,
vec![domain::sandbox::PathGrant {
abs_root: managed_root,
access: domain::sandbox::PathAccess::RW,
}],
vec![
domain::sandbox::PathGrant {
abs_root: root.to_string_lossy().into_owned(),
access: domain::sandbox::PathAccess::RW,
},
domain::sandbox::PathGrant {
abs_root: managed_root,
access: domain::sandbox::PathAccess::RW,
},
],
"OS sandbox grants must match the Codex writable roots"
);
let _ = std::fs::remove_dir_all(&root);

View File

@ -7,7 +7,9 @@ use application::{
StructuredSessions,
};
use async_trait::async_trait;
use domain::permission::{EffectivePermissions, Posture};
use domain::permission::{
Capability, Effect, EffectivePermissions, PathScope, PermissionRule, Posture,
};
use domain::ports::{
AgentSession, AgentSessionError, AgentSessionFactory, PermissionStore, ReplyStream,
RuntimeError, SessionPlan, StoreError, StructuredProviderLaunchPolicy,
@ -415,6 +417,15 @@ fn permission_doc(project_defaults: Option<PermissionSet>) -> ProjectPermissions
ProjectPermissions::new(project_defaults, Vec::new())
}
fn file_rule(capability: Capability, effect: Effect, patterns: &[&str]) -> PermissionRule {
PermissionRule::file(
capability,
effect,
PathScope::new(patterns.iter().map(|pattern| (*pattern).to_owned())).unwrap(),
)
.unwrap()
}
#[tokio::test]
async fn open_then_close_ticket_assistant_sets_policy_injects_context_and_emits_events() {
let issue = issue(7);
@ -642,6 +653,7 @@ async fn codex_ticket_assistant_uses_project_network_permission_for_structured_p
let factory = Arc::new(FakeFactory::default());
let system_doc =
ProjectSystemPermissions::new(Some(SystemPermissionSet::new(policy)), Vec::new());
let permission_doc = permission_doc(Some(PermissionSet::new(Vec::new(), Posture::Allow)));
let open = OpenTicketAssistant::new(
Arc::new(FakeIssues {
issue: issue.clone(),
@ -656,6 +668,7 @@ async fn codex_ticket_assistant_uses_project_network_permission_for_structured_p
policies,
events,
)
.with_permission_store(Arc::new(FakePermissionStore(permission_doc)))
.with_system_permission_store(Arc::new(FakeSystemPermissionStore(system_doc)));
open.execute(OpenTicketAssistantInput {
@ -678,3 +691,98 @@ async fn codex_ticket_assistant_uses_project_network_permission_for_structured_p
);
}
}
#[tokio::test]
async fn codex_ticket_assistant_uses_project_default_write_roots_without_overgranting() {
let issue = issue(7);
let profile_id = ProfileId::from_uuid(Uuid::from_u128(9));
let factory = Arc::new(FakeFactory::default());
let open = OpenTicketAssistant::new(
Arc::new(FakeIssues {
issue: issue.clone(),
}),
Arc::new(FakeProfiles {
profile: profile(profile_id).with_structured_adapter(StructuredAdapter::Codex),
}),
Arc::new(FakeAssistantContext::default()),
Arc::new(FakeEnvironmentPreparer::default()),
factory.clone(),
Arc::new(StructuredSessions::new()),
Arc::new(FakePolicies::default()),
Arc::new(SpyBus::default()),
)
.with_permission_store(Arc::new(FakePermissionStore(permission_doc(Some(
PermissionSet::new(
vec![file_rule(Capability::Write, Effect::Allow, &["out/**"])],
Posture::Deny,
),
)))));
open.execute(OpenTicketAssistantInput {
project: project(),
issue_ref: issue.reference(),
profile_id,
})
.await
.unwrap();
let starts = factory.starts.lock().unwrap();
assert_eq!(
starts[0].6,
Some(StructuredProviderLaunchPolicy::Codex {
sandbox_mode: "workspace-write".to_owned(),
writable_roots: vec!["/tmp/project/out".to_owned()],
network_access: false,
}),
"ticket assistant Codex policy must use permission-derived write roots"
);
}
#[tokio::test]
async fn codex_ticket_assistant_broad_write_with_subpath_deny_fails_closed() {
let issue = issue(7);
let profile_id = ProfileId::from_uuid(Uuid::from_u128(9));
let factory = Arc::new(FakeFactory::default());
let open = OpenTicketAssistant::new(
Arc::new(FakeIssues {
issue: issue.clone(),
}),
Arc::new(FakeProfiles {
profile: profile(profile_id).with_structured_adapter(StructuredAdapter::Codex),
}),
Arc::new(FakeAssistantContext::default()),
Arc::new(FakeEnvironmentPreparer::default()),
factory.clone(),
Arc::new(StructuredSessions::new()),
Arc::new(FakePolicies::default()),
Arc::new(SpyBus::default()),
)
.with_permission_store(Arc::new(FakePermissionStore(permission_doc(Some(
PermissionSet::new(
vec![
file_rule(Capability::Write, Effect::Allow, &["**"]),
file_rule(Capability::Write, Effect::Deny, &["secret/**"]),
],
Posture::Deny,
),
)))));
open.execute(OpenTicketAssistantInput {
project: project(),
issue_ref: issue.reference(),
profile_id,
})
.await
.unwrap();
let starts = factory.starts.lock().unwrap();
assert_eq!(
starts[0].6,
Some(StructuredProviderLaunchPolicy::Codex {
sandbox_mode: "read-only".to_owned(),
writable_roots: Vec::new(),
network_access: false,
}),
"ticket assistant must fail closed when Codex cannot represent a subpath write deny"
);
}