mirror of
https://github.com/openai/codex.git
synced 2026-11-13 21:30:47 +00:00
Remove redundant legacy permission helpers (#61194)
## What changed Remove `PermissionProfile::from_legacy_sandbox_policy` and the filesystem helpers for materializing workspace roots and adding legacy writable roots. Keep legacy template conversion self-contained and update round-trip tests to use the conversion that accepts a working directory. ## Testing Update preset tests to compare converted legacy policies, and move extra-root metadata protection coverage to `FileSystemSandboxPolicy::workspace_write`. GitOrigin-RevId: d545b329c13b2f09609fd2cbf42f0adbbda5386a
This commit is contained in:
@@ -655,14 +655,6 @@ impl PermissionProfile {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn from_legacy_sandbox_policy(sandbox_policy: &SandboxPolicy) -> Self {
|
||||
Self::from_runtime_permissions_with_enforcement(
|
||||
SandboxEnforcement::from_legacy_sandbox_policy(sandbox_policy),
|
||||
&FileSystemSandboxPolicy::from(sandbox_policy),
|
||||
NetworkSandboxPolicy::from(sandbox_policy),
|
||||
)
|
||||
}
|
||||
|
||||
/// Decodes a portable legacy template without probing workspace metadata.
|
||||
pub fn from_legacy_sandbox_policy_template(sandbox_policy: &SandboxPolicy) -> Self {
|
||||
if let SandboxPolicy::WorkspaceWrite {
|
||||
@@ -681,7 +673,11 @@ impl PermissionProfile {
|
||||
NetworkSandboxPolicy::from(sandbox_policy),
|
||||
)
|
||||
} else {
|
||||
Self::from_legacy_sandbox_policy(sandbox_policy)
|
||||
Self::from_runtime_permissions_with_enforcement(
|
||||
SandboxEnforcement::from_legacy_sandbox_policy(sandbox_policy),
|
||||
&FileSystemSandboxPolicy::from(sandbox_policy),
|
||||
NetworkSandboxPolicy::from(sandbox_policy),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3138,24 +3134,26 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permission_profile_presets_match_legacy_defaults() {
|
||||
fn permission_profile_presets_match_legacy_defaults() -> Result<()> {
|
||||
let cwd = tempdir()?;
|
||||
assert_eq!(
|
||||
PermissionProfile::read_only(),
|
||||
PermissionProfile::from_legacy_sandbox_policy(&SandboxPolicy::new_read_only_policy())
|
||||
PermissionProfile::read_only().to_legacy_sandbox_policy(cwd.path())?,
|
||||
SandboxPolicy::new_read_only_policy()
|
||||
);
|
||||
assert_eq!(
|
||||
PermissionProfile::workspace_write(),
|
||||
PermissionProfile::from_legacy_sandbox_policy(
|
||||
&SandboxPolicy::new_workspace_write_policy()
|
||||
)
|
||||
PermissionProfile::workspace_write().to_legacy_sandbox_policy(cwd.path())?,
|
||||
SandboxPolicy::new_workspace_write_policy()
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permission_profile_round_trip_preserves_disabled_sandbox() -> Result<()> {
|
||||
let cwd = tempdir()?;
|
||||
let permission_profile =
|
||||
PermissionProfile::from_legacy_sandbox_policy(&SandboxPolicy::DangerFullAccess);
|
||||
let permission_profile = PermissionProfile::from_legacy_sandbox_policy_for_cwd(
|
||||
&SandboxPolicy::DangerFullAccess,
|
||||
cwd.path(),
|
||||
);
|
||||
|
||||
assert_eq!(permission_profile, PermissionProfile::Disabled);
|
||||
assert_eq!(
|
||||
@@ -3237,7 +3235,8 @@ mod tests {
|
||||
let sandbox_policy = SandboxPolicy::ExternalSandbox {
|
||||
network_access: crate::protocol::NetworkAccess::Restricted,
|
||||
};
|
||||
let permission_profile = PermissionProfile::from_legacy_sandbox_policy(&sandbox_policy);
|
||||
let permission_profile =
|
||||
PermissionProfile::from_legacy_sandbox_policy_for_cwd(&sandbox_policy, cwd.path());
|
||||
|
||||
assert_eq!(
|
||||
permission_profile,
|
||||
|
||||
@@ -1339,19 +1339,6 @@ impl FileSystemSandboxPolicy {
|
||||
Some(self)
|
||||
}
|
||||
|
||||
/// Preserves symbolic `:workspace_roots` entries while also adding concrete
|
||||
/// entries for each provided workspace root.
|
||||
pub fn with_materialized_project_roots_for_workspace_roots(
|
||||
self,
|
||||
workspace_roots: &[AbsolutePathBuf],
|
||||
) -> Self {
|
||||
let roots = workspace_roots
|
||||
.iter()
|
||||
.map(PathUri::from_abs_path)
|
||||
.collect::<Vec<_>>();
|
||||
self.with_materialized_project_roots_for_path_uris(&roots)
|
||||
}
|
||||
|
||||
pub fn with_additional_readable_roots(
|
||||
mut self,
|
||||
cwd: &Path,
|
||||
@@ -1394,44 +1381,6 @@ impl FileSystemSandboxPolicy {
|
||||
self
|
||||
}
|
||||
|
||||
/// Add roots using legacy `WorkspaceWrite` behavior.
|
||||
///
|
||||
/// Unlike [`Self::with_additional_writable_roots`], this mirrors legacy
|
||||
/// writable-roots semantics by adding exact roots even when they are
|
||||
/// already writable through `:workspace_roots`, and by adding the default
|
||||
/// read-only protected subpaths for each new root.
|
||||
pub fn with_additional_legacy_workspace_writable_roots(
|
||||
mut self,
|
||||
additional_writable_roots: &[AbsolutePathBuf],
|
||||
) -> Self {
|
||||
if !matches!(self.kind, FileSystemSandboxKind::Restricted) {
|
||||
return self;
|
||||
}
|
||||
|
||||
for path in additional_writable_roots {
|
||||
if !self.entries.iter().any(|entry| {
|
||||
entry.access.can_write()
|
||||
&& matches!(&entry.path, FileSystemPath::Path { path: existing } if existing == &PathUri::from_abs_path(path))
|
||||
}) {
|
||||
self.entries.push(FileSystemSandboxEntry::new(
|
||||
path.clone().into(),
|
||||
FileSystemAccessMode::Write,
|
||||
));
|
||||
}
|
||||
|
||||
for protected_path in default_read_only_subpaths_for_writable_root(
|
||||
path, /*protect_missing_dot_codex*/ false,
|
||||
) {
|
||||
append_default_read_only_path_if_no_explicit_rule(
|
||||
&mut self.entries,
|
||||
protected_path,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
self
|
||||
}
|
||||
|
||||
pub fn needs_direct_runtime_enforcement(
|
||||
&self,
|
||||
network_policy: NetworkSandboxPolicy,
|
||||
|
||||
@@ -437,9 +437,6 @@ fn materializing_legacy_home_relative_workspace_denials_removes_all_grants() {
|
||||
&native_root,
|
||||
)),
|
||||
);
|
||||
materialized.push(policy.with_materialized_project_roots_for_workspace_roots(
|
||||
std::slice::from_ref(&native_root),
|
||||
));
|
||||
}
|
||||
for policy in materialized {
|
||||
assert_eq!(policy, FileSystemSandboxPolicy::restricted(Vec::new()));
|
||||
|
||||
@@ -1748,25 +1748,26 @@ fn materialize_project_roots_with_workspace_roots_expands_exact_and_glob_entries
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn with_additional_legacy_workspace_writable_roots_protects_metadata() {
|
||||
fn workspace_write_with_extra_root_protects_metadata() {
|
||||
let temp_dir = TempDir::new().expect("tempdir");
|
||||
let extra = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("extra"))
|
||||
.expect("resolve extra root");
|
||||
std::fs::create_dir_all(extra.join(".git")).expect("create .git dir");
|
||||
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
missing_path_behavior: None,
|
||||
}]);
|
||||
|
||||
let actual =
|
||||
policy.with_additional_legacy_workspace_writable_roots(std::slice::from_ref(&extra));
|
||||
let actual = FileSystemSandboxPolicy::workspace_write(
|
||||
std::slice::from_ref(&extra),
|
||||
/*exclude_tmpdir_env_var*/ true,
|
||||
/*exclude_slash_tmp*/ true,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
actual,
|
||||
FileSystemSandboxPolicy::restricted(vec![
|
||||
FileSystemSandboxEntry::new(
|
||||
FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Root
|
||||
},
|
||||
FileSystemAccessMode::Read,
|
||||
),
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
|
||||
@@ -1781,6 +1782,30 @@ fn with_additional_legacy_workspace_writable_roots_protects_metadata() {
|
||||
access: FileSystemAccessMode::Write,
|
||||
missing_path_behavior: None,
|
||||
},
|
||||
FileSystemSandboxEntry::skip_missing_path(
|
||||
FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::project_roots(Some(".git".to_string())),
|
||||
},
|
||||
FileSystemAccessMode::Read,
|
||||
),
|
||||
FileSystemSandboxEntry::skip_missing_path(
|
||||
FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::project_roots(Some(".agents".to_string())),
|
||||
},
|
||||
FileSystemAccessMode::Read,
|
||||
),
|
||||
FileSystemSandboxEntry::skip_missing_path(
|
||||
FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::project_roots(Some(".codex".to_string())),
|
||||
},
|
||||
FileSystemAccessMode::Read,
|
||||
),
|
||||
FileSystemSandboxEntry::skip_missing_path(
|
||||
FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::project_roots(Some(".aws".to_string())),
|
||||
},
|
||||
FileSystemAccessMode::Read,
|
||||
),
|
||||
FileSystemSandboxEntry::skip_missing_path(
|
||||
FileSystemPath::Path {
|
||||
path: extra.join(".git").into()
|
||||
|
||||
Reference in New Issue
Block a user