diff --git a/crates/application/src/agent/lifecycle.rs b/crates/application/src/agent/lifecycle.rs index 93ab3933..0b180f5a 100644 --- a/crates/application/src/agent/lifecycle.rs +++ b/crates/application/src/agent/lifecycle.rs @@ -11,8 +11,7 @@ //! [`AgentRuntime`], [`PtyPort`], [`FileSystem`], [`EventBus`]); none knows about //! a concrete adapter or Tauri. -use std::collections::{BTreeSet, HashMap}; -use std::path::{Path, PathBuf}; +use std::collections::HashMap; use std::sync::Arc; use domain::ports::{ @@ -30,13 +29,16 @@ use domain::{ bound_handoff_summary, Agent, AgentId, AgentManifest, AgentOrigin, AgentProfile, ContextInjection, ConversationId, ConversationParty, DomainEvent, EffectivePermissions, Handoff, HandoffStore, ManifestEntry, MarkdownDoc, MemoryIndexEntry, MemoryType, NetworkPolicy, - NodeId, OrchestrationCapabilitySnapshot, PermissionProjector, Posture, ProfileId, Project, - ProjectPath, ProjectedFile, ProjectionContext, ProjectorKey, ProviderSessionStore, PtySize, - SessionId, SessionKind, SessionStatus, SkillKind, TerminalSession, HANDOFF_SUMMARY_MAX_CHARS, + NodeId, OrchestrationCapabilitySnapshot, PermissionProjector, ProfileId, Project, ProjectPath, + ProjectedFile, ProjectionContext, ProjectorKey, ProviderSessionStore, PtySize, SessionId, + SessionKind, SessionStatus, SkillKind, TerminalSession, HANDOFF_SUMMARY_MAX_CHARS, }; use domain::live_state::WorkStatus; +use crate::codex_permissions::{ + codex_permission_writable_roots, codex_sandbox_mode, compile_codex_launch_policy, +}; use crate::error::AppError; use crate::layout::{persist_doc, resolve_doc}; use crate::managed_temp::{ResolveManagedTemp, ResolveManagedTempInput}; @@ -1338,22 +1340,13 @@ fn select_projector_key(profile: &AgentProfile) -> Option { None } -fn codex_structured_sandbox_mode(permissions: Option<&EffectivePermissions>) -> &'static str { - match permissions.map(EffectivePermissions::fallback) { - Some(Posture::Deny) => "read-only", - Some(Posture::Ask | Posture::Allow) | None => "workspace-write", - } -} - -fn codex_network_access(network: Option) -> bool { - matches!(network, Some(NetworkPolicy::Allow)) -} - fn build_structured_launch_policy( profile: &AgentProfile, permissions: Option<&EffectivePermissions>, network: Option, - codex_git_writable_roots: &[String], + project_root: &str, + run_dir: &str, + extra_writable_roots: &[String], ) -> Option { if !matches!(profile.structured_adapter, Some(StructuredAdapter::Codex)) { return None; @@ -1362,70 +1355,13 @@ fn build_structured_launch_policy( return None; } - let sandbox_mode = codex_structured_sandbox_mode(permissions).to_owned(); - let writable_roots = if sandbox_mode == "workspace-write" { - codex_git_writable_roots.to_vec() - } else { - Vec::new() - }; - - Some(StructuredProviderLaunchPolicy::Codex { - sandbox_mode, - writable_roots, - network_access: codex_network_access(network), - }) -} - -fn codex_git_writable_roots(project_root: &ProjectPath) -> Vec { - git_writable_roots_for_project(Path::new(project_root.as_str())) -} - -fn git_writable_roots_for_project(project_root: &Path) -> Vec { - let git_marker = project_root.join(".git"); - let Some(git_dir) = resolve_git_dir(&git_marker) else { - return Vec::new(); - }; - let Ok(canonical) = git_dir.canonicalize() else { - return Vec::new(); - }; - if !canonical.is_dir() { - return Vec::new(); - } - - let mut roots = BTreeSet::new(); - roots.insert(canonical.to_string_lossy().into_owned()); - roots.into_iter().collect() -} - -fn resolve_git_dir(git_marker: &Path) -> Option { - if git_marker.is_dir() { - return Some(git_marker.to_path_buf()); - } - if !git_marker.is_file() { - return None; - } - - let contents = std::fs::read_to_string(git_marker).ok()?; - let gitdir = contents - .lines() - .find_map(|line| line.trim().strip_prefix("gitdir:").map(str::trim))?; - if gitdir.is_empty() { - return None; - } - - let path = PathBuf::from(gitdir); - if path.is_absolute() { - Some(path) - } else { - git_marker.parent().map(|parent| parent.join(path)) - } -} - -fn enrich_sandbox_plan_with_codex_git_roots( - plan: Option, - codex_git_writable_roots: &[String], -) -> Option { - enrich_sandbox_plan_with_rw_roots(plan, codex_git_writable_roots) + Some(compile_codex_launch_policy( + project_root, + run_dir, + permissions, + network, + extra_writable_roots, + )) } fn enrich_sandbox_plan_with_managed_temp_roots( @@ -1461,14 +1397,6 @@ fn enrich_sandbox_plan_with_rw_roots( Some(plan) } -fn append_unique_paths(target: &mut Vec, roots: &[String]) { - for root in roots { - if !target.iter().any(|existing| existing == root) { - target.push(root.clone()); - } - } -} - fn dedupe_paths(paths: impl IntoIterator) -> Vec { let mut roots = Vec::new(); for path in paths { @@ -1533,7 +1461,7 @@ fn append_codex_pty_managed_temp_roots( if managed_temp_roots.is_empty() || !is_codex_profile(profile) { return; } - if codex_structured_sandbox_mode(permissions) != "workspace-write" { + if codex_sandbox_mode(permissions, managed_temp_roots) != "workspace-write" { return; } for root in managed_temp_roots { @@ -2338,17 +2266,9 @@ impl LaunchAgent { // remplir `spec.sandbox`. `eff == None` (rien posé) ⇒ `None` : aucun plan, // comportement natif conservé (invariant produit). L'enforcer concret // (Landlock/Noop) est injecté côté PTY au composition root. - let codex_git_writable_roots = - if matches!(profile.structured_adapter, Some(StructuredAdapter::Codex)) { - codex_git_writable_roots(&input.project.root) - } else { - Vec::new() - }; let managed_temp_writable_roots = self .resolve_managed_temp_exposure(&input.managed_temp_exposure) .await?; - let mut codex_writable_roots = codex_git_writable_roots.clone(); - append_unique_paths(&mut codex_writable_roots, &managed_temp_writable_roots); spec.sandbox = compile_sandbox_plan( effective_permissions.as_ref(), &SandboxContext { @@ -2356,8 +2276,13 @@ impl LaunchAgent { run_dir: run_dir.as_str(), }, ); + let codex_permission_writable_roots = codex_permission_writable_roots( + input.project.root.as_str(), + run_dir.as_str(), + effective_permissions.as_ref(), + ); spec.sandbox = - enrich_sandbox_plan_with_codex_git_roots(spec.sandbox, &codex_git_writable_roots); + enrich_sandbox_plan_with_rw_roots(spec.sandbox, &codex_permission_writable_roots); spec.sandbox = enrich_sandbox_plan_with_managed_temp_roots(spec.sandbox, &managed_temp_writable_roots); append_codex_pty_managed_temp_roots( @@ -2370,7 +2295,9 @@ impl LaunchAgent { &profile, effective_permissions.as_ref(), network_permission, - &codex_writable_roots, + input.project.root.as_str(), + run_dir.as_str(), + &managed_temp_writable_roots, ); let resolved_effort = resolve_effort( profile.model_reasoning_effort.as_deref(), diff --git a/crates/application/src/codex_permissions.rs b/crates/application/src/codex_permissions.rs new file mode 100644 index 00000000..ce59359d --- /dev/null +++ b/crates/application/src/codex_permissions.rs @@ -0,0 +1,101 @@ +use domain::permission::{Capability, Effect, EffectivePermissions, Posture}; +use domain::ports::StructuredProviderLaunchPolicy; +use domain::sandbox::{compile_sandbox_plan, PathAccess, SandboxContext}; +use domain::NetworkPolicy; + +pub(crate) fn compile_codex_launch_policy( + project_root: &str, + run_dir: &str, + permissions: Option<&EffectivePermissions>, + network: Option, + extra_writable_roots: &[String], +) -> StructuredProviderLaunchPolicy { + let mut writable_roots = codex_permission_writable_roots(project_root, run_dir, permissions); + append_unique_paths(&mut writable_roots, extra_writable_roots); + let sandbox_mode = codex_sandbox_mode(permissions, &writable_roots).to_owned(); + if sandbox_mode != "workspace-write" { + writable_roots.clear(); + } + + StructuredProviderLaunchPolicy::Codex { + sandbox_mode, + writable_roots, + network_access: codex_network_access(network), + } +} + +pub(crate) fn codex_permission_writable_roots( + project_root: &str, + run_dir: &str, + permissions: Option<&EffectivePermissions>, +) -> Vec { + let Some(permissions) = permissions else { + return Vec::new(); + }; + + let mut roots = Vec::new(); + if !has_rw_deny(permissions) + && permissions.fallback() != Posture::Deny + && permissions.decide_file(Capability::Write, "") != Posture::Deny + { + roots.push(project_root.to_owned()); + } + + if let Some(plan) = compile_sandbox_plan( + Some(permissions), + &SandboxContext { + project_root, + run_dir, + }, + ) { + for grant in plan + .allowed + .iter() + .filter(|grant| grant.access.contains(PathAccess::RW)) + { + append_unique_path(&mut roots, &grant.abs_root); + } + } + roots.sort(); + roots +} + +pub(crate) fn codex_sandbox_mode( + permissions: Option<&EffectivePermissions>, + writable_roots: &[String], +) -> &'static str { + if !writable_roots.is_empty() { + return "workspace-write"; + } + if permissions.is_some_and(has_rw_deny) { + return "read-only"; + } + match permissions.map(EffectivePermissions::fallback) { + Some(Posture::Ask | Posture::Allow) | None => "workspace-write", + Some(Posture::Deny) => "read-only", + } +} + +fn has_rw_deny(permissions: &EffectivePermissions) -> bool { + permissions.rules().iter().any(|rule| { + rule.effect() == Effect::Deny + && matches!(rule.capability(), Capability::Write | Capability::Delete) + }) +} + +fn codex_network_access(network: Option) -> bool { + matches!(network, Some(NetworkPolicy::Allow)) +} + +fn append_unique_paths(target: &mut Vec, roots: &[String]) { + for root in roots { + append_unique_path(target, root); + } +} + +fn append_unique_path(target: &mut Vec, root: &str) { + if root.is_empty() || target.iter().any(|existing| existing == root) { + return; + } + target.push(root.to_owned()); +} diff --git a/crates/application/src/lib.rs b/crates/application/src/lib.rs index 66fec58d..f8c1c765 100644 --- a/crates/application/src/lib.rs +++ b/crates/application/src/lib.rs @@ -15,6 +15,7 @@ pub mod agent; pub mod background; pub mod chat_attachments; pub mod chat_drafts; +mod codex_permissions; pub mod conversation; pub mod device; pub mod diag; diff --git a/crates/application/src/ticket_assistant.rs b/crates/application/src/ticket_assistant.rs index 60be2529..a83d15bc 100644 --- a/crates/application/src/ticket_assistant.rs +++ b/crates/application/src/ticket_assistant.rs @@ -14,6 +14,7 @@ use domain::{ ProfileId, ProfileStore, Project, SessionId, }; +use crate::codex_permissions::compile_codex_launch_policy; use crate::terminal::StructuredSessions; use crate::AppError; @@ -229,11 +230,14 @@ impl OpenTicketAssistant { return Ok(environment.structured_policy.clone()); } let network = self.resolve_project_default_network(project).await?; - Ok(Some(StructuredProviderLaunchPolicy::Codex { - sandbox_mode: "workspace-write".to_owned(), - writable_roots: vec![project.root.as_str().to_owned()], - network_access: codex_network_access(network), - })) + let permissions = self.resolve_project_default_permissions(project).await?; + Ok(Some(compile_codex_launch_policy( + project.root.as_str(), + environment.cwd.as_str(), + permissions.as_ref(), + network, + &[], + ))) } async fn resolve_project_default_network( @@ -262,10 +266,6 @@ impl OpenTicketAssistant { } } -fn codex_network_access(network: Option) -> bool { - matches!(network, Some(NetworkPolicy::Allow)) -} - fn ticket_assistant_requester(project: &Project, issue_ref: IssueRef) -> String { format!( "ticket-assistant:{}:{}", diff --git a/crates/application/tests/agent_lifecycle.rs b/crates/application/tests/agent_lifecycle.rs index 05eacd48..2abe2192 100644 --- a/crates/application/tests/agent_lifecycle.rs +++ b/crates/application/tests/agent_lifecycle.rs @@ -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, 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>, sandboxes: Vec>, expected_roots: Vec, @@ -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::>(); 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); diff --git a/crates/application/tests/ticket_assistant.rs b/crates/application/tests/ticket_assistant.rs index 4651d408..19a84196 100644 --- a/crates/application/tests/ticket_assistant.rs +++ b/crates/application/tests/ticket_assistant.rs @@ -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) -> 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" + ); +}