mirror of
https://github.com/openai/codex.git
synced 2026-11-03 08:22:30 +00:00
Make deny canonical for filesystem permission entries (#23493)
## Why Filesystem permission profiles used `none` for deny-read entries, which is less direct than the action the entry actually represents. This change makes `deny` the canonical filesystem permission spelling while preserving compatibility for older configs that still send `none`. ## What changed - rename `FileSystemAccessMode::None` to `Deny` - serialize and generate schemas with `deny` as the canonical value - retain `none` only as a legacy input alias for temporary config compatibility - update filesystem glob diagnostics and regression coverage to use the canonical spelling - refresh config and app-server schema fixtures to match the new wire shape ## Validation - `cargo test -p codex-protocol` - `cargo test -p codex-app-server-protocol` - `cargo test -p codex-core config_toml_deserializes_permission_profiles --lib` - `cargo test -p codex-core read_write_glob_patterns_still_reject_non_subpath_globs --lib` Earlier in the session, a broad `cargo test -p codex-core` run reached unrelated pre-existing failures in timing/snapshot/git-info tests under this environment; the targeted surfaces touched by this PR passed cleanly.
This commit is contained in:
1 parent
05b8ce4354
commit
3c76081876
44 files changed
+165
-148
No files matched your search
@@ -740,6 +740,7 @@ default_permissions = "workspace"
|
||||
|
||||
[permissions.workspace.filesystem]
|
||||
":minimal" = "read"
|
||||
"/tmp/secret.env" = "deny"
|
||||
|
||||
[permissions.workspace.filesystem.":workspace_roots"]
|
||||
"." = "write"
|
||||
@@ -777,6 +778,10 @@ allow_upstream_proxy = false
|
||||
":minimal".to_string(),
|
||||
FilesystemPermissionToml::Access(FileSystemAccessMode::Read),
|
||||
),
|
||||
(
|
||||
"/tmp/secret.env".to_string(),
|
||||
FilesystemPermissionToml::Access(FileSystemAccessMode::Deny),
|
||||
),
|
||||
(
|
||||
":workspace_roots".to_string(),
|
||||
FilesystemPermissionToml::Scoped(BTreeMap::from([
|
||||
@@ -1700,7 +1705,7 @@ async fn workspace_root_glob_none_compiles_to_filesystem_pattern_entry() -> std:
|
||||
":workspace_roots".to_string(),
|
||||
FilesystemPermissionToml::Scoped(BTreeMap::from([
|
||||
(".".to_string(), FileSystemAccessMode::Write),
|
||||
("**/*.env".to_string(), FileSystemAccessMode::None),
|
||||
("**/*.env".to_string(), FileSystemAccessMode::Deny),
|
||||
])),
|
||||
)]),
|
||||
}),
|
||||
@@ -1739,7 +1744,7 @@ async fn workspace_root_glob_none_compiles_to_filesystem_pattern_entry() -> std:
|
||||
path: FileSystemPath::GlobPattern {
|
||||
pattern: expected_pattern,
|
||||
},
|
||||
access: FileSystemAccessMode::None,
|
||||
access: FileSystemAccessMode::Deny,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
@@ -2031,7 +2031,7 @@ fn apply_managed_filesystem_constraints(
|
||||
path: codex_protocol::permissions::FileSystemPath::GlobPattern {
|
||||
pattern: deny_read.as_str().to_string(),
|
||||
},
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::None,
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::Deny,
|
||||
}
|
||||
} else {
|
||||
let Ok(path) = AbsolutePathBuf::try_from(deny_read.as_str()) else {
|
||||
@@ -2039,7 +2039,7 @@ fn apply_managed_filesystem_constraints(
|
||||
};
|
||||
codex_protocol::permissions::FileSystemSandboxEntry {
|
||||
path: codex_protocol::permissions::FileSystemPath::Path { path },
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::None,
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::Deny,
|
||||
}
|
||||
};
|
||||
if !file_system_sandbox_policy
|
||||
|
||||
@@ -243,7 +243,7 @@ pub(crate) fn compile_permission_profile(
|
||||
push_warning(
|
||||
startup_warnings,
|
||||
format!(
|
||||
"Filesystem glob `{pattern}` uses `read` or `write` access, which is not fully supported by this platform's sandboxing. Use an exact path or trailing `/**` subtree rule instead. `none` deny-read globs are supported."
|
||||
"Filesystem glob `{pattern}` uses `read` or `write` access, which is not fully supported by this platform's sandboxing. Use an exact path or trailing `/**` subtree rule instead. `deny` globs are supported."
|
||||
),
|
||||
);
|
||||
}
|
||||
@@ -414,7 +414,7 @@ fn compile_filesystem_permission(
|
||||
Some(FileSystemSpecialPath::ProjectRoots { .. }) | None => true,
|
||||
Some(_) => false,
|
||||
};
|
||||
if has_glob && *access == FileSystemAccessMode::None && can_compile_as_pattern {
|
||||
if has_glob && *access == FileSystemAccessMode::Deny && can_compile_as_pattern {
|
||||
// Scoped glob syntax is a first-class filesystem policy
|
||||
// pattern entry. Literal scoped paths continue through the
|
||||
// exact-path parser so existing path semantics stay intact.
|
||||
@@ -449,7 +449,7 @@ fn compile_filesystem_access_path(
|
||||
return compile_filesystem_path(path, startup_warnings);
|
||||
}
|
||||
|
||||
if access == FileSystemAccessMode::None {
|
||||
if access == FileSystemAccessMode::Deny {
|
||||
// At this point `path` is an unscoped filesystem table key. Top-level
|
||||
// glob deny entries still go through the absolute-path parser before
|
||||
// becoming policy patterns; relative project-root glob syntax is
|
||||
@@ -520,10 +520,10 @@ fn compile_scoped_filesystem_pattern(
|
||||
// Pattern entries currently mean deny-read only. Supporting broader access
|
||||
// modes here would imply glob-based read/write allow semantics that the
|
||||
// sandbox policy does not express yet.
|
||||
if access != FileSystemAccessMode::None {
|
||||
if access != FileSystemAccessMode::Deny {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
format!("filesystem glob subpath `{subpath}` only supports `none` access"),
|
||||
format!("filesystem glob subpath `{subpath}` only supports `deny` access"),
|
||||
));
|
||||
}
|
||||
let subpath = parse_relative_subpath(subpath)?;
|
||||
@@ -559,7 +559,7 @@ fn compile_read_write_glob_path(path: &str, access: FileSystemAccessMode) -> io:
|
||||
Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
format!(
|
||||
"filesystem glob path `{path}` only supports `none` access; use an exact path or trailing `/**` for `{access}` subtree access"
|
||||
"filesystem glob path `{path}` only supports `deny` access; use an exact path or trailing `/**` for `{access}` subtree access"
|
||||
),
|
||||
))
|
||||
}
|
||||
@@ -569,7 +569,7 @@ fn unsupported_read_write_glob_paths(filesystem: &FilesystemPermissionsToml) ->
|
||||
for (path, permission) in &filesystem.entries {
|
||||
match permission {
|
||||
FilesystemPermissionToml::Access(access) => {
|
||||
if *access != FileSystemAccessMode::None
|
||||
if *access != FileSystemAccessMode::Deny
|
||||
&& contains_glob_chars(remove_trailing_glob_suffix(path))
|
||||
{
|
||||
patterns.push(path.clone());
|
||||
@@ -577,7 +577,7 @@ fn unsupported_read_write_glob_paths(filesystem: &FilesystemPermissionsToml) ->
|
||||
}
|
||||
FilesystemPermissionToml::Scoped(scoped_entries) => {
|
||||
for (subpath, access) in scoped_entries {
|
||||
if *access != FileSystemAccessMode::None
|
||||
if *access != FileSystemAccessMode::Deny
|
||||
&& contains_glob_chars(remove_trailing_glob_suffix(subpath))
|
||||
{
|
||||
patterns.push(format!("{path}/{subpath}"));
|
||||
@@ -597,7 +597,7 @@ fn unbounded_unreadable_globstar_paths(filesystem: &FilesystemPermissionsToml) -
|
||||
let mut patterns = Vec::new();
|
||||
for (path, permission) in &filesystem.entries {
|
||||
match permission {
|
||||
FilesystemPermissionToml::Access(FileSystemAccessMode::None) => {
|
||||
FilesystemPermissionToml::Access(FileSystemAccessMode::Deny) => {
|
||||
if path.contains("**") {
|
||||
patterns.push(path.clone());
|
||||
}
|
||||
@@ -605,7 +605,7 @@ fn unbounded_unreadable_globstar_paths(filesystem: &FilesystemPermissionsToml) -
|
||||
FilesystemPermissionToml::Access(_) => {}
|
||||
FilesystemPermissionToml::Scoped(scoped_entries) => {
|
||||
for (subpath, access) in scoped_entries {
|
||||
if *access == FileSystemAccessMode::None && subpath.contains("**") {
|
||||
if *access == FileSystemAccessMode::Deny && subpath.contains("**") {
|
||||
patterns.push(format!("{path}/{subpath}"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -326,7 +326,7 @@ fn read_write_glob_warnings_skip_supported_deny_read_globs_and_trailing_subpaths
|
||||
(
|
||||
":workspace_roots".to_string(),
|
||||
FilesystemPermissionToml::Scoped(BTreeMap::from([
|
||||
("**/*.env".to_string(), FileSystemAccessMode::None),
|
||||
("**/*.env".to_string(), FileSystemAccessMode::Deny),
|
||||
("docs/**".to_string(), FileSystemAccessMode::Read),
|
||||
("src/**/*.rs".to_string(), FileSystemAccessMode::Write),
|
||||
])),
|
||||
@@ -340,7 +340,7 @@ fn read_write_glob_warnings_skip_supported_deny_read_globs_and_trailing_subpaths
|
||||
"/tmp/**/*.log".to_string(),
|
||||
":workspace_roots/src/**/*.rs".to_string()
|
||||
],
|
||||
"`none` glob patterns are supported as deny-read rules; only `read`/`write` globs should warn"
|
||||
"`deny` glob patterns are supported as deny-read rules; only `read`/`write` globs should warn"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -351,8 +351,8 @@ fn unreadable_globstar_warning_is_suppressed_when_scan_depth_is_configured() {
|
||||
entries: BTreeMap::from([(
|
||||
":workspace_roots".to_string(),
|
||||
FilesystemPermissionToml::Scoped(BTreeMap::from([
|
||||
("**/*.env".to_string(), FileSystemAccessMode::None),
|
||||
("*.pem".to_string(), FileSystemAccessMode::None),
|
||||
("**/*.env".to_string(), FileSystemAccessMode::Deny),
|
||||
("*.pem".to_string(), FileSystemAccessMode::Deny),
|
||||
])),
|
||||
)]),
|
||||
};
|
||||
@@ -435,7 +435,7 @@ fn read_write_glob_patterns_still_reject_non_subpath_globs() {
|
||||
assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput);
|
||||
assert!(
|
||||
err.to_string()
|
||||
.contains("filesystem glob path `src/**/*.rs` only supports `none` access"),
|
||||
.contains("filesystem glob path `src/**/*.rs` only supports `deny` access"),
|
||||
"{err}"
|
||||
);
|
||||
}
|
||||
@@ -698,7 +698,7 @@ fn windows_restricted_token_rejects_unreadable_split_carveouts() {
|
||||
},
|
||||
codex_protocol::permissions::FileSystemSandboxEntry {
|
||||
path: codex_protocol::permissions::FileSystemPath::Path { path: blocked },
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::None,
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::Deny,
|
||||
},
|
||||
]);
|
||||
|
||||
@@ -846,7 +846,7 @@ fn windows_elevated_supports_unreadable_split_carveouts() {
|
||||
path: codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&blocked)
|
||||
.expect("absolute blocked"),
|
||||
},
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::None,
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::Deny,
|
||||
},
|
||||
]);
|
||||
|
||||
@@ -908,7 +908,7 @@ fn windows_elevated_supports_unreadable_globs() {
|
||||
path: codex_protocol::permissions::FileSystemPath::GlobPattern {
|
||||
pattern: "**/*.env".to_string(),
|
||||
},
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::None,
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::Deny,
|
||||
},
|
||||
]);
|
||||
|
||||
|
||||
@@ -218,7 +218,7 @@ fn explicit_unreadable_paths_prevent_auto_approval_for_external_sandbox() {
|
||||
path: FileSystemPath::Path {
|
||||
path: blocked_absolute,
|
||||
},
|
||||
access: FileSystemAccessMode::None,
|
||||
access: FileSystemAccessMode::Deny,
|
||||
},
|
||||
]);
|
||||
|
||||
|
||||
@@ -3522,7 +3522,7 @@ async fn session_configuration_apply_permission_profile_preserves_existing_deny_
|
||||
path: FileSystemPath::GlobPattern {
|
||||
pattern: "**/*.env".to_string(),
|
||||
},
|
||||
access: FileSystemAccessMode::None,
|
||||
access: FileSystemAccessMode::Deny,
|
||||
};
|
||||
let mut existing_file_system_policy =
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(
|
||||
@@ -7137,7 +7137,7 @@ fn file_system_policy_with_unreadable_glob(turn_context: &TurnContext) -> FileSy
|
||||
path: FileSystemPath::GlobPattern {
|
||||
pattern: format!("{cwd_display}/**/*.env"),
|
||||
},
|
||||
access: FileSystemAccessMode::None,
|
||||
access: FileSystemAccessMode::Deny,
|
||||
});
|
||||
policy
|
||||
}
|
||||
|
||||
@@ -441,7 +441,7 @@ mod tests {
|
||||
path: FileSystemPath::GlobPattern {
|
||||
pattern: "**/*.env".to_string(),
|
||||
},
|
||||
access: FileSystemAccessMode::None,
|
||||
access: FileSystemAccessMode::Deny,
|
||||
},
|
||||
],
|
||||
glob_scan_max_depth: None,
|
||||
|
||||
@@ -2106,7 +2106,7 @@ async fn spawn_agent_reapplies_runtime_sandbox_after_role_config() {
|
||||
path: FileSystemPath::GlobPattern {
|
||||
pattern: "**/.env".to_string(),
|
||||
},
|
||||
access: FileSystemAccessMode::None,
|
||||
access: FileSystemAccessMode::Deny,
|
||||
});
|
||||
let expected_network_sandbox_policy = NetworkSandboxPolicy::from(&expected_sandbox);
|
||||
let expected_permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement(
|
||||
|
||||
@@ -281,7 +281,7 @@ fn shell_request_escalation_execution_is_explicit() {
|
||||
path: FileSystemPath::Path {
|
||||
path: AbsolutePathBuf::from_absolute_path("/tmp/secret").unwrap(),
|
||||
},
|
||||
access: FileSystemAccessMode::None,
|
||||
access: FileSystemAccessMode::Deny,
|
||||
},
|
||||
]);
|
||||
let network_sandbox_policy = NetworkSandboxPolicy::Restricted;
|
||||
|
||||
@@ -143,7 +143,7 @@ fn deny_read_blocks_explicit_escalation_but_preserves_policy_bypass() {
|
||||
path: FileSystemPath::GlobPattern {
|
||||
pattern: "**/*.env".to_string(),
|
||||
},
|
||||
access: FileSystemAccessMode::None,
|
||||
access: FileSystemAccessMode::Deny,
|
||||
}]);
|
||||
|
||||
assert_eq!(
|
||||
|
||||
Reference in new issue
Block a user