Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion codex-rs/core/src/config/permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,10 @@ fn parse_special_path(path: &str) -> Option<FileSystemSpecialPath> {
match path {
":root" => Some(FileSystemSpecialPath::Root),
":minimal" => Some(FileSystemSpecialPath::Minimal),
":workspace_roots" => Some(FileSystemSpecialPath::project_roots(/*subpath*/ None)),
// `:project_roots` shipped before the canonical rename; keep it as an alias.
":project_roots" | ":workspace_roots" => {
Some(FileSystemSpecialPath::project_roots(/*subpath*/ None))
Comment on lines +783 to +784

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Canonicalize aliases before merging inherited profiles

When a child profile uses :workspace_roots to narrow a legacy parent's :project_roots rule, inheritance retains both map keys because merging happens before this alias is parsed. Both entries then compile to the same ProjectRoots target, where an inherited write beats the child's equally specific read, leaving the workspace writable despite the documented child override semantics. Normalize the legacy key before profile merging so semantically matching entries override each other.

AGENTS.md reference: AGENTS.md:L102-L110

Useful? React with 👍 / 👎.

Comment on lines +783 to +784

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Cover the legacy alias with an agent integration test

This changes effective filesystem enforcement for legacy permission profiles, but the added test stops at compile_permission_profile and resolve_access_with_cwd. Add a core/suite integration test using test_codex that loads legacy TOML and verifies actual command/file access, including denial and the read-only carveout, so config loading, workspace-root materialization, and sandbox execution are exercised as required for agent-logic changes.

AGENTS.md reference: AGENTS.md:L112-L118

Useful? React with 👍 / 👎.

}
":tmpdir" => Some(FileSystemSpecialPath::Tmpdir),
":slash_tmp" => Some(FileSystemSpecialPath::SlashTmp),
_ if path.starts_with(':') => {
Expand Down
50 changes: 50 additions & 0 deletions codex-rs/core/src/config/permissions_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,56 @@ fn compile_permission_profile_workspace_roots_resolves_enabled_entries() -> std:
Ok(())
}

#[test]
fn legacy_project_roots_restrictions_do_not_fail_open() -> std::io::Result<()> {
let permissions = toml::from_str::<PermissionsToml>(
r#"
[read_deny.filesystem]
":root" = "read"
":project_roots" = "none"

[write_deny.filesystem]
":root" = "write"
":project_roots" = "none"

[write_read.filesystem]
":root" = "write"

[write_read.filesystem.":project_roots"]
docs = "read"
"#,
)
.expect("legacy project roots profiles should deserialize");
let cwd = TempDir::new()?;
let docs = cwd.path().join("docs");
let mut startup_warnings = Vec::new();

let (read_deny_policy, _) =
compile_permission_profile(&permissions, "read_deny", &mut startup_warnings)?;
assert_eq!(
read_deny_policy.resolve_access_with_cwd(cwd.path(), cwd.path()),
FileSystemAccessMode::Deny
);

let (write_deny_policy, _) =
compile_permission_profile(&permissions, "write_deny", &mut startup_warnings)?;
assert!(!write_deny_policy.has_full_disk_write_access());
assert_eq!(
write_deny_policy.resolve_access_with_cwd(cwd.path(), cwd.path()),
FileSystemAccessMode::Deny
);

let (write_read_policy, _) =
compile_permission_profile(&permissions, "write_read", &mut startup_warnings)?;
assert!(!write_read_policy.has_full_disk_write_access());
assert_eq!(
write_read_policy.resolve_access_with_cwd(&docs, cwd.path()),
FileSystemAccessMode::Read
);

Ok(())
}

#[test]
fn read_write_glob_warnings_skip_supported_deny_read_globs_and_trailing_subpaths() {
let filesystem = FilesystemPermissionsToml {
Expand Down
Loading