mirror of
https://github.com/openai/codex.git
synced 2026-10-22 17:58:46 +00:00
Move workspace roots onto thread/session state and stop using active permission profile modifications as an overlay for writable roots. Existing app-server threads now preserve their persisted PermissionProfile value across resume, fork, and turn updates; permissions requests on existing threads only update the active named profile after validating it exists. Workspace roots can be updated independently, and SandboxPolicy::WorkspaceWrite no longer stores its own writable_roots.
This commit is contained in:
@@ -335,21 +335,6 @@ pub struct ActivePermissionProfile {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub extends: Option<String>,
|
||||
|
||||
/// Bounded user-requested modifications applied on top of the named
|
||||
/// profile, if any.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub modifications: Vec<ActivePermissionProfileModification>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Eq, PartialEq, Deserialize, Serialize, JsonSchema, TS)]
|
||||
#[serde(tag = "type", rename_all = "snake_case")]
|
||||
#[ts(tag = "type")]
|
||||
pub enum ActivePermissionProfileModification {
|
||||
/// Additional concrete directory that should be writable.
|
||||
#[serde(rename_all = "snake_case")]
|
||||
#[ts(rename_all = "snake_case")]
|
||||
AdditionalWritableRoot { path: AbsolutePathBuf },
|
||||
}
|
||||
|
||||
impl ActivePermissionProfile {
|
||||
@@ -357,17 +342,8 @@ impl ActivePermissionProfile {
|
||||
Self {
|
||||
id: id.into(),
|
||||
extends: None,
|
||||
modifications: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn with_modifications(
|
||||
mut self,
|
||||
modifications: Vec<ActivePermissionProfileModification>,
|
||||
) -> Self {
|
||||
self.modifications = modifications;
|
||||
self
|
||||
}
|
||||
}
|
||||
|
||||
impl Default for PermissionProfile {
|
||||
@@ -539,6 +515,20 @@ impl PermissionProfile {
|
||||
self.network_sandbox_policy(),
|
||||
)
|
||||
}
|
||||
|
||||
pub fn materialize_project_roots_with_workspace_roots(
|
||||
&self,
|
||||
workspace_roots: &[AbsolutePathBuf],
|
||||
) -> Self {
|
||||
let (file_system_sandbox_policy, network_sandbox_policy) = self.to_runtime_permissions();
|
||||
let file_system_sandbox_policy = file_system_sandbox_policy
|
||||
.materialize_project_roots_with_workspace_roots(workspace_roots);
|
||||
Self::from_runtime_permissions_with_enforcement(
|
||||
self.enforcement(),
|
||||
&file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize)]
|
||||
|
||||
@@ -538,27 +538,16 @@ impl FileSystemSandboxPolicy {
|
||||
/// into split filesystem policy.
|
||||
pub fn from_legacy_sandbox_policy_for_cwd(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Self {
|
||||
let mut file_system_policy = Self::from(sandbox_policy);
|
||||
if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = sandbox_policy {
|
||||
if let Ok(cwd_root) = AbsolutePathBuf::from_absolute_path(cwd) {
|
||||
for protected_path in default_read_only_subpaths_for_writable_root(
|
||||
&cwd_root, /*protect_missing_dot_codex*/ true,
|
||||
) {
|
||||
append_default_read_only_path_if_no_explicit_rule(
|
||||
&mut file_system_policy.entries,
|
||||
protected_path,
|
||||
);
|
||||
}
|
||||
}
|
||||
for writable_root in writable_roots {
|
||||
for protected_path in default_read_only_subpaths_for_writable_root(
|
||||
writable_root,
|
||||
/*protect_missing_dot_codex*/ false,
|
||||
) {
|
||||
append_default_read_only_path_if_no_explicit_rule(
|
||||
&mut file_system_policy.entries,
|
||||
protected_path,
|
||||
);
|
||||
}
|
||||
if let SandboxPolicy::WorkspaceWrite { .. } = sandbox_policy
|
||||
&& let Ok(cwd_root) = AbsolutePathBuf::from_absolute_path(cwd)
|
||||
{
|
||||
for protected_path in default_read_only_subpaths_for_writable_root(
|
||||
&cwd_root, /*protect_missing_dot_codex*/ true,
|
||||
) {
|
||||
append_default_read_only_path_if_no_explicit_rule(
|
||||
&mut file_system_policy.entries,
|
||||
protected_path,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -679,6 +668,38 @@ impl FileSystemSandboxPolicy {
|
||||
self
|
||||
}
|
||||
|
||||
/// Replaces symbolic `:project_roots` entries with concrete entries for
|
||||
/// each workspace root.
|
||||
pub fn materialize_project_roots_with_workspace_roots(
|
||||
mut self,
|
||||
workspace_roots: &[AbsolutePathBuf],
|
||||
) -> Self {
|
||||
let mut entries = Vec::with_capacity(self.entries.len());
|
||||
for entry in self.entries {
|
||||
let FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::ProjectRoots { subpath },
|
||||
} = &entry.path
|
||||
else {
|
||||
entries.push(entry);
|
||||
continue;
|
||||
};
|
||||
|
||||
entries.extend(workspace_roots.iter().map(|root| FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: match subpath.as_ref() {
|
||||
Some(subpath) => {
|
||||
AbsolutePathBuf::resolve_path_against_base(subpath, root.as_path())
|
||||
}
|
||||
None => root.clone(),
|
||||
},
|
||||
},
|
||||
access: entry.access,
|
||||
}));
|
||||
}
|
||||
self.entries = entries;
|
||||
self
|
||||
}
|
||||
|
||||
pub fn with_additional_readable_roots(
|
||||
mut self,
|
||||
cwd: &Path,
|
||||
@@ -994,10 +1015,10 @@ impl FileSystemSandboxPolicy {
|
||||
let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok();
|
||||
let has_full_disk_write_access = self.has_full_disk_write_access();
|
||||
let mut workspace_root_writable = false;
|
||||
let mut writable_roots = Vec::new();
|
||||
let mut tmpdir_writable = false;
|
||||
let mut slash_tmp_writable = false;
|
||||
let mut unbridgeable_root_write = false;
|
||||
let mut unbridgeable_external_write = false;
|
||||
|
||||
for entry in &self.entries {
|
||||
match &entry.path {
|
||||
@@ -1007,7 +1028,7 @@ impl FileSystemSandboxPolicy {
|
||||
if cwd_absolute.as_ref().is_some_and(|cwd| cwd == path) {
|
||||
workspace_root_writable = true;
|
||||
} else {
|
||||
writable_roots.push(path.clone());
|
||||
unbridgeable_external_write = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1023,11 +1044,8 @@ impl FileSystemSandboxPolicy {
|
||||
FileSystemSpecialPath::ProjectRoots { subpath } => {
|
||||
if subpath.is_none() && entry.access.can_write() {
|
||||
workspace_root_writable = true;
|
||||
} else if let Some(path) =
|
||||
resolve_file_system_special_path(value, cwd_absolute.as_ref())
|
||||
&& entry.access.can_write()
|
||||
{
|
||||
writable_roots.push(path);
|
||||
} else if entry.access.can_write() {
|
||||
unbridgeable_external_write = true;
|
||||
}
|
||||
}
|
||||
FileSystemSpecialPath::Tmpdir => {
|
||||
@@ -1057,16 +1075,12 @@ impl FileSystemSandboxPolicy {
|
||||
|
||||
if workspace_root_writable {
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: dedup_absolute_paths(
|
||||
writable_roots,
|
||||
/*normalize_effective_paths*/ false,
|
||||
),
|
||||
network_access: network_policy.is_enabled(),
|
||||
exclude_tmpdir_env_var: !tmpdir_writable,
|
||||
exclude_slash_tmp: !slash_tmp_writable,
|
||||
}
|
||||
} else if unbridgeable_root_write
|
||||
|| !writable_roots.is_empty()
|
||||
|| unbridgeable_external_write
|
||||
|| tmpdir_writable
|
||||
|| slash_tmp_writable
|
||||
{
|
||||
@@ -1135,12 +1149,11 @@ impl From<&SandboxPolicy> for FileSystemSandboxPolicy {
|
||||
}])
|
||||
}
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots,
|
||||
exclude_tmpdir_env_var,
|
||||
exclude_slash_tmp,
|
||||
..
|
||||
} => FileSystemSandboxPolicy::workspace_write(
|
||||
writable_roots,
|
||||
&[],
|
||||
*exclude_tmpdir_env_var,
|
||||
*exclude_slash_tmp,
|
||||
),
|
||||
@@ -1447,7 +1460,6 @@ fn legacy_runtime_file_system_policy_for_cwd(
|
||||
cwd: &Path,
|
||||
) -> FileSystemSandboxPolicy {
|
||||
let SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots,
|
||||
exclude_tmpdir_env_var,
|
||||
exclude_slash_tmp,
|
||||
..
|
||||
@@ -1487,16 +1499,6 @@ fn legacy_runtime_file_system_policy_for_cwd(
|
||||
access: FileSystemAccessMode::Write,
|
||||
});
|
||||
}
|
||||
entries.extend(
|
||||
writable_roots
|
||||
.iter()
|
||||
.cloned()
|
||||
.map(|path| FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path { path },
|
||||
access: FileSystemAccessMode::Write,
|
||||
}),
|
||||
);
|
||||
|
||||
if let Ok(cwd_root) = AbsolutePathBuf::from_absolute_path(cwd) {
|
||||
for protected_path in default_read_only_subpaths_for_writable_root(
|
||||
&cwd_root, /*protect_missing_dot_codex*/ true,
|
||||
@@ -1504,14 +1506,6 @@ fn legacy_runtime_file_system_policy_for_cwd(
|
||||
append_default_read_only_path_if_no_explicit_rule(&mut entries, protected_path);
|
||||
}
|
||||
}
|
||||
for writable_root in writable_roots {
|
||||
for protected_path in default_read_only_subpaths_for_writable_root(
|
||||
writable_root,
|
||||
/*protect_missing_dot_codex*/ false,
|
||||
) {
|
||||
append_default_read_only_path_if_no_explicit_rule(&mut entries, protected_path);
|
||||
}
|
||||
}
|
||||
|
||||
FileSystemSandboxPolicy::restricted(entries)
|
||||
}
|
||||
@@ -1800,7 +1794,6 @@ mod tests {
|
||||
#[test]
|
||||
fn legacy_workspace_write_projection_preserves_symbolic_project_root() {
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: Vec::new(),
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
@@ -1956,7 +1949,6 @@ mod tests {
|
||||
)
|
||||
.expect("absolute root");
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
@@ -2472,7 +2464,6 @@ mod tests {
|
||||
fn legacy_projection_runtime_enforcement_ignores_entry_order() {
|
||||
let cwd = TempDir::new().expect("tempdir");
|
||||
let legacy_policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: Vec::new(),
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
@@ -2499,7 +2490,6 @@ mod tests {
|
||||
fn missing_symbolic_metadata_carveouts_need_direct_runtime_enforcement() {
|
||||
let cwd = TempDir::new().expect("tempdir");
|
||||
let legacy_policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: Vec::new(),
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
|
||||
@@ -466,6 +466,10 @@ pub enum Op {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
cwd: Option<PathBuf>,
|
||||
|
||||
/// Updated workspace roots for sandbox/tool calls.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
workspace_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
|
||||
/// Updated command approval policy.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
approval_policy: Option<AskForApproval>,
|
||||
@@ -1051,14 +1055,9 @@ pub enum SandboxPolicy {
|
||||
},
|
||||
|
||||
/// Same as `ReadOnly` but additionally grants write access to the current
|
||||
/// working directory ("workspace").
|
||||
/// thread workspace roots.
|
||||
#[serde(rename = "workspace-write")]
|
||||
WorkspaceWrite {
|
||||
/// Additional folders (beyond cwd and possibly TMPDIR) that should be
|
||||
/// writable from within the sandbox.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
writable_roots: Vec<AbsolutePathBuf>,
|
||||
|
||||
/// When set to `true`, outbound network access is allowed. `false` by
|
||||
/// default.
|
||||
#[serde(default)]
|
||||
@@ -1168,7 +1167,6 @@ impl SandboxPolicy {
|
||||
/// not allow network access.
|
||||
pub fn new_workspace_write_policy() -> Self {
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
@@ -1206,13 +1204,11 @@ impl SandboxPolicy {
|
||||
SandboxPolicy::ExternalSandbox { .. } => Vec::new(),
|
||||
SandboxPolicy::ReadOnly { .. } => Vec::new(),
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots,
|
||||
exclude_tmpdir_env_var,
|
||||
exclude_slash_tmp,
|
||||
network_access: _,
|
||||
} => {
|
||||
// Start from explicitly configured writable roots.
|
||||
let mut roots: Vec<AbsolutePathBuf> = writable_roots.clone();
|
||||
let mut roots: Vec<AbsolutePathBuf> = Vec::new();
|
||||
|
||||
// Always include defaults: cwd, /tmp (if present on Unix), and
|
||||
// on macOS, the per-user TMPDIR unless explicitly excluded.
|
||||
@@ -2724,6 +2720,8 @@ pub struct SessionMeta {
|
||||
pub forked_from_id: Option<ThreadId>,
|
||||
pub timestamp: String,
|
||||
pub cwd: PathBuf,
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub workspace_roots: Vec<AbsolutePathBuf>,
|
||||
pub originator: String,
|
||||
pub cli_version: String,
|
||||
#[serde(default)]
|
||||
@@ -2755,6 +2753,7 @@ impl Default for SessionMeta {
|
||||
forked_from_id: None,
|
||||
timestamp: String::new(),
|
||||
cwd: PathBuf::new(),
|
||||
workspace_roots: Vec::new(),
|
||||
originator: String::new(),
|
||||
cli_version: String::new(),
|
||||
source: SessionSource::default(),
|
||||
@@ -2824,6 +2823,8 @@ pub struct TurnContextItem {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub trace_id: Option<String>,
|
||||
pub cwd: PathBuf,
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub workspace_roots: Vec<AbsolutePathBuf>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub current_date: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
@@ -3500,6 +3501,10 @@ pub struct SessionConfiguredEvent {
|
||||
/// session.
|
||||
pub cwd: AbsolutePathBuf,
|
||||
|
||||
/// Workspace roots used to realize symbolic `:project_roots` entries.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub workspace_roots: Vec<AbsolutePathBuf>,
|
||||
|
||||
/// The effort the model is putting into reasoning about the user's request.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub reasoning_effort: Option<ReasoningEffortConfig>,
|
||||
@@ -3550,6 +3555,8 @@ impl<'de> Deserialize<'de> for SessionConfiguredEvent {
|
||||
#[serde(default)]
|
||||
active_permission_profile: Option<ActivePermissionProfile>,
|
||||
cwd: AbsolutePathBuf,
|
||||
#[serde(default)]
|
||||
workspace_roots: Vec<AbsolutePathBuf>,
|
||||
reasoning_effort: Option<ReasoningEffortConfig>,
|
||||
history_log_id: u64,
|
||||
history_entry_count: usize,
|
||||
@@ -3569,6 +3576,11 @@ impl<'de> Deserialize<'de> for SessionConfiguredEvent {
|
||||
return Err(serde::de::Error::missing_field("permission_profile"));
|
||||
}
|
||||
};
|
||||
let workspace_roots = if wire.workspace_roots.is_empty() {
|
||||
vec![wire.cwd.clone()]
|
||||
} else {
|
||||
wire.workspace_roots
|
||||
};
|
||||
|
||||
Ok(Self {
|
||||
session_id: wire.session_id,
|
||||
@@ -3582,6 +3594,7 @@ impl<'de> Deserialize<'de> for SessionConfiguredEvent {
|
||||
permission_profile,
|
||||
active_permission_profile: wire.active_permission_profile,
|
||||
cwd: wire.cwd,
|
||||
workspace_roots,
|
||||
reasoning_effort: wire.reasoning_effort,
|
||||
history_log_id: wire.history_log_id,
|
||||
history_entry_count: wire.history_entry_count,
|
||||
@@ -4563,7 +4576,6 @@ mod tests {
|
||||
#[test]
|
||||
fn legacy_sandbox_policy_semantics_survive_split_bridge() {
|
||||
let cwd = TempDir::new().expect("tempdir");
|
||||
let writable_root = AbsolutePathBuf::resolve_path_against_base("writable", cwd.path());
|
||||
let policies = [
|
||||
SandboxPolicy::DangerFullAccess,
|
||||
SandboxPolicy::ExternalSandbox {
|
||||
@@ -4576,13 +4588,11 @@ mod tests {
|
||||
network_access: false,
|
||||
},
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
},
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![writable_root],
|
||||
network_access: true,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: true,
|
||||
@@ -5266,6 +5276,7 @@ mod tests {
|
||||
turn_id: None,
|
||||
trace_id: None,
|
||||
cwd: test_path_buf("/tmp"),
|
||||
workspace_roots: Vec::new(),
|
||||
current_date: None,
|
||||
timezone: None,
|
||||
approval_policy: AskForApproval::Never,
|
||||
@@ -5340,6 +5351,7 @@ mod tests {
|
||||
permission_profile: permission_profile.clone(),
|
||||
active_permission_profile: None,
|
||||
cwd: test_path_buf("/home/user/project").abs(),
|
||||
workspace_roots: Vec::new(),
|
||||
reasoning_effort: Some(ReasoningEffortConfig::default()),
|
||||
history_log_id: 0,
|
||||
history_entry_count: 0,
|
||||
|
||||
Reference in New Issue
Block a user