mirror of
https://github.com/openai/codex.git
synced 2026-05-02 18:37:01 +00:00
permissions: make runtime config profile-backed (#19606)
## Why This supersedes #19391. During stack repair, GitHub marked #19391 as merged into a temporary stack branch rather than into `main`, so the runtime-config change needed a fresh PR. `PermissionProfile` is now the canonical permissions shape after #19231 because it can distinguish `Managed`, `Disabled`, and `External` enforcement while also carrying filesystem rules that legacy `SandboxPolicy` cannot represent cleanly. Core config and session state still needed to accept profile-backed permissions without forcing every profile through the strict legacy bridge, which rejected valid runtime profiles such as direct write roots. The unrelated CI/test hardening that previously rode along with this PR has been split into #19683 so this PR stays focused on the permissions model migration. ## What Changed - Adds `Permissions.permission_profile` and `SessionConfiguration.permission_profile` as constrained runtime state, while keeping `sandbox_policy` as a legacy compatibility projection. - Introduces profile setters that keep `PermissionProfile`, split filesystem/network policies, and legacy `SandboxPolicy` projections synchronized. - Uses a compatibility projection for requirement checks and legacy consumers instead of rejecting profiles that cannot round-trip through `SandboxPolicy` exactly. - Updates config loading, config overrides, session updates, turn context plumbing, prompt permission text, sandbox tags, and exec request construction to carry profile-backed runtime permissions. - Preserves configured deny-read entries and `glob_scan_max_depth` when command/session profiles are narrowed. - Adds `PermissionProfile::read_only()` and `PermissionProfile::workspace_write()` presets that match legacy defaults. ## Verification - `cargo test -p codex-core direct_write_roots` - `cargo test -p codex-core runtime_roots_to_legacy_projection` - `cargo test -p codex-app-server requested_permissions_trust_project_uses_permission_profile_intent` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19606). * #19395 * #19394 * #19393 * #19392 * __->__ #19606
This commit is contained in:
@@ -17,6 +17,9 @@ use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
use tokio::time::timeout;
|
||||
|
||||
#[cfg(windows)]
|
||||
const DEFAULT_TIMEOUT: Duration = Duration::from_secs(25);
|
||||
#[cfg(not(windows))]
|
||||
const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10);
|
||||
const INSTALLED_MARKETPLACES_DIR: &str = ".tmp/marketplaces";
|
||||
|
||||
@@ -63,13 +66,14 @@ fn commit_marketplace_marker(root: &Path, marker: &str) -> Result<String> {
|
||||
fn configured_git_marketplace_update<'a>(
|
||||
source: &'a str,
|
||||
last_revision: Option<&'a str>,
|
||||
ref_name: Option<&'a str>,
|
||||
) -> MarketplaceConfigUpdate<'a> {
|
||||
MarketplaceConfigUpdate {
|
||||
last_updated: "2026-04-13T00:00:00Z",
|
||||
last_revision,
|
||||
source_type: "git",
|
||||
source,
|
||||
ref_name: None,
|
||||
ref_name,
|
||||
sparse_paths: &[],
|
||||
}
|
||||
}
|
||||
@@ -90,12 +94,13 @@ fn record_git_marketplace(
|
||||
marketplace_name: &str,
|
||||
source: &Path,
|
||||
last_revision: &str,
|
||||
ref_name: Option<&str>,
|
||||
) -> Result<()> {
|
||||
let source = source.display().to_string();
|
||||
record_user_marketplace(
|
||||
codex_home,
|
||||
marketplace_name,
|
||||
&configured_git_marketplace_update(&source, Some(last_revision)),
|
||||
&configured_git_marketplace_update(&source, Some(last_revision), ref_name),
|
||||
)?;
|
||||
Ok(())
|
||||
}
|
||||
@@ -153,12 +158,14 @@ async fn marketplace_upgrade_all_configured_git_marketplaces() -> Result<()> {
|
||||
"debug",
|
||||
debug_source.path(),
|
||||
&debug_old_revision,
|
||||
Some(&debug_new_revision),
|
||||
)?;
|
||||
record_git_marketplace(
|
||||
codex_home.path(),
|
||||
"tools",
|
||||
tools_source.path(),
|
||||
&tools_old_revision,
|
||||
Some(&tools_new_revision),
|
||||
)?;
|
||||
disable_plugin_startup_tasks(codex_home.path())?;
|
||||
|
||||
@@ -205,12 +212,14 @@ async fn marketplace_upgrade_named_marketplace_only() -> Result<()> {
|
||||
"debug",
|
||||
debug_source.path(),
|
||||
&debug_old_revision,
|
||||
/*ref_name*/ None,
|
||||
)?;
|
||||
record_git_marketplace(
|
||||
codex_home.path(),
|
||||
"tools",
|
||||
tools_source.path(),
|
||||
&tools_old_revision,
|
||||
/*ref_name*/ None,
|
||||
)?;
|
||||
disable_plugin_startup_tasks(codex_home.path())?;
|
||||
|
||||
@@ -246,7 +255,13 @@ async fn marketplace_upgrade_returns_empty_roots_when_already_up_to_date() -> Re
|
||||
let source = TempDir::new()?;
|
||||
let old_revision = init_marketplace_repo(source.path(), "debug", "debug old")?;
|
||||
commit_marketplace_marker(source.path(), "debug new")?;
|
||||
record_git_marketplace(codex_home.path(), "debug", source.path(), &old_revision)?;
|
||||
record_git_marketplace(
|
||||
codex_home.path(),
|
||||
"debug",
|
||||
source.path(),
|
||||
&old_revision,
|
||||
/*ref_name*/ None,
|
||||
)?;
|
||||
disable_plugin_startup_tasks(codex_home.path())?;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
|
||||
@@ -749,13 +749,17 @@ async fn turn_start_rejects_combined_oversized_text_input() -> Result<()> {
|
||||
#[tokio::test]
|
||||
async fn turn_start_rejects_invalid_permission_profile_before_starting_turn() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let unsupported_write_root = TempDir::new()?;
|
||||
let disallowed_write_root = TempDir::new()?;
|
||||
create_config_toml(
|
||||
codex_home.path(),
|
||||
"http://localhost/unused",
|
||||
"never",
|
||||
&BTreeMap::from([(Feature::Personality, true)]),
|
||||
)?;
|
||||
std::fs::write(
|
||||
codex_home.path().join("managed_config.toml"),
|
||||
"sandbox_mode = \"read-only\"\n",
|
||||
)?;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
@@ -772,7 +776,7 @@ async fn turn_start_rejects_invalid_permission_profile_before_starting_turn() ->
|
||||
)
|
||||
.await??;
|
||||
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(thread_resp)?;
|
||||
let unsupported_write_root = AbsolutePathBuf::from_absolute_path(unsupported_write_root.path())
|
||||
let disallowed_write_root = AbsolutePathBuf::from_absolute_path(disallowed_write_root.path())
|
||||
.expect("tempdir path should be absolute");
|
||||
|
||||
let turn_req = mcp
|
||||
@@ -787,7 +791,7 @@ async fn turn_start_rejects_invalid_permission_profile_before_starting_turn() ->
|
||||
file_system: PermissionProfileFileSystemPermissions::Restricted {
|
||||
entries: vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: unsupported_write_root,
|
||||
path: disallowed_write_root,
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
}],
|
||||
@@ -806,9 +810,9 @@ async fn turn_start_rejects_invalid_permission_profile_before_starting_turn() ->
|
||||
assert_eq!(err.error.code, INVALID_REQUEST_ERROR_CODE);
|
||||
assert!(err.error.message.contains("invalid turn context override"));
|
||||
assert!(
|
||||
err.error
|
||||
.message
|
||||
.contains("filesystem writes outside the workspace root")
|
||||
err.error.message.contains("allowed set [ReadOnly]"),
|
||||
"unexpected error message: {}",
|
||||
err.error.message
|
||||
);
|
||||
let turn_started = tokio::time::timeout(
|
||||
std::time::Duration::from_millis(250),
|
||||
|
||||
Reference in New Issue
Block a user