From 0edcc4b94e63b208fbc18d4fabf3b5a7ca8b3e12 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 20 May 2026 12:15:44 -0700 Subject: [PATCH] fix(config): resolve cloud requirements deny-read globs (#23729) ## Why Cloud-managed `requirements.toml` contents were deserialized without an `AbsolutePathBuf` base directory. Relative managed `permissions.filesystem.deny_read` glob entries therefore failed while the equivalent local system requirements path succeeded under its `AbsolutePathBufGuard`. This follows the `codex_home` base path convention clarified in https://github.com/openai/codex/pull/15707. ## What changed - Resolve cloud requirements TOML under an `AbsolutePathBufGuard` rooted at `codex_home`. - Reuse the same base for cloud requirements loaded from the signed cache. - Add a regression test for a relative cloud-managed `deny_read` glob. ## Validation - `just fmt` - `cargo test -p codex-cloud-requirements` - `cargo clippy -p codex-cloud-requirements --all-targets --no-deps` - `just bazel-lock-update` - `just bazel-lock-check` - `git diff --check` --- codex-rs/Cargo.lock | 1 + codex-rs/cloud-requirements/Cargo.toml | 1 + codex-rs/cloud-requirements/src/lib.rs | 101 +++++++++++++++++++------ 3 files changed, 78 insertions(+), 25 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 72d395c273..823eb6d3c1 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2321,6 +2321,7 @@ dependencies = [ "codex-login", "codex-otel", "codex-protocol", + "codex-utils-absolute-path", "hmac", "pretty_assertions", "serde", diff --git a/codex-rs/cloud-requirements/Cargo.toml b/codex-rs/cloud-requirements/Cargo.toml index cc7aefc478..f4b2d2dbcb 100644 --- a/codex-rs/cloud-requirements/Cargo.toml +++ b/codex-rs/cloud-requirements/Cargo.toml @@ -17,6 +17,7 @@ codex-core = { workspace = true } codex-login = { workspace = true } codex-otel = { workspace = true } codex-protocol = { workspace = true } +codex-utils-absolute-path = { workspace = true } hmac = "0.12.1" serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } diff --git a/codex-rs/cloud-requirements/src/lib.rs b/codex-rs/cloud-requirements/src/lib.rs index 885e907ef8..b9740602dd 100644 --- a/codex-rs/cloud-requirements/src/lib.rs +++ b/codex-rs/cloud-requirements/src/lib.rs @@ -25,11 +25,13 @@ use codex_login::AuthManager; use codex_login::CodexAuth; use codex_login::RefreshTokenError; use codex_protocol::account::PlanType; +use codex_utils_absolute_path::AbsolutePathBufGuard; use hmac::Hmac; use hmac::Mac; use serde::Deserialize; use serde::Serialize; use sha2::Sha256; +use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::sync::Mutex; @@ -138,10 +140,12 @@ struct CloudRequirementsCacheSignedPayload { } impl CloudRequirementsCacheSignedPayload { - fn requirements(&self) -> Option { - self.contents - .as_deref() - .and_then(|contents| parse_cloud_requirements(contents).ok().flatten()) + fn requirements(&self, requirements_base_dir: &Path) -> Option { + self.contents.as_deref().and_then(|contents| { + parse_cloud_requirements(contents, requirements_base_dir) + .ok() + .flatten() + }) } } fn sign_cache_payload(payload_bytes: &[u8]) -> Option { @@ -258,6 +262,7 @@ impl RequirementsFetcher for BackendRequirementsFetcher { struct CloudRequirementsService { auth_manager: Arc, fetcher: Arc, + requirements_base_dir: PathBuf, cache_path: PathBuf, timeout: Duration, } @@ -272,6 +277,7 @@ impl CloudRequirementsService { Self { auth_manager, fetcher, + requirements_base_dir: codex_home.clone(), cache_path: codex_home.join(CLOUD_REQUIREMENTS_CACHE_FILENAME), timeout, } @@ -351,7 +357,7 @@ impl CloudRequirementsService { path = %self.cache_path.display(), "Using cached cloud requirements" ); - return Ok(signed_payload.requirements()); + return Ok(signed_payload.requirements(&self.requirements_base_dir)); } Err(cache_load_status) => { self.log_cache_load_status(&cache_load_status); @@ -482,24 +488,26 @@ impl CloudRequirementsService { }; let requirements = match contents.as_deref() { - Some(contents) => match parse_cloud_requirements(contents) { - Ok(requirements) => requirements, - Err(err) => { - tracing::error!(error = %err, "Failed to parse cloud requirements"); - emit_fetch_final_metric( - trigger, - "error", - "parse_error", - attempt, - last_status_code, - ); - return Err(CloudRequirementsLoadError::new( - CloudRequirementsLoadErrorCode::Parse, - /*status_code*/ None, - format_cloud_requirements_parse_failed_message(contents, &err), - )); + Some(contents) => { + match parse_cloud_requirements(contents, &self.requirements_base_dir) { + Ok(requirements) => requirements, + Err(err) => { + tracing::error!(error = %err, "Failed to parse cloud requirements"); + emit_fetch_final_metric( + trigger, + "error", + "parse_error", + attempt, + last_status_code, + ); + return Err(CloudRequirementsLoadError::new( + CloudRequirementsLoadErrorCode::Parse, + /*status_code*/ None, + format_cloud_requirements_parse_failed_message(contents, &err), + )); + } } - }, + } None => None, }; @@ -738,11 +746,13 @@ pub async fn cloud_requirements_loader_for_storage( fn parse_cloud_requirements( contents: &str, + requirements_base_dir: &Path, ) -> Result, toml::de::Error> { if contents.trim().is_empty() { return Ok(None); } + let _guard = AbsolutePathBufGuard::new(requirements_base_dir); let requirements: ConfigRequirementsToml = toml::from_str(contents)?; if requirements.is_empty() { Ok(None) @@ -1038,7 +1048,11 @@ mod tests { } fn parse_for_fetch(contents: Option<&str>) -> Option { - contents.and_then(|contents| parse_cloud_requirements(contents).ok().flatten()) + contents.and_then(|contents| { + parse_cloud_requirements(contents, &std::env::temp_dir()) + .ok() + .flatten() + }) } fn request_error() -> FetchAttemptError { @@ -1391,6 +1405,35 @@ mod tests { ); } + #[tokio::test] + async fn fetch_cloud_requirements_resolves_relative_deny_read_globs_from_codex_home() { + let codex_home = tempdir().expect("tempdir"); + let service = CloudRequirementsService::new( + auth_manager_with_plan("enterprise").await, + Arc::new(StaticFetcher { + contents: Some( + r#" +[permissions.filesystem] +deny_read = ["./sensitive/**/*.txt"] +"# + .to_string(), + ), + }), + codex_home.path().to_path_buf(), + CLOUD_REQUIREMENTS_TIMEOUT, + ); + let deny_read = format!("{}/sensitive/**/*.txt", codex_home.path().display()); + let expected = toml::from_str::(&format!( + r#" +[permissions.filesystem] +deny_read = [{deny_read:?}] +"# + )) + .expect("parse expected cloud requirements"); + + assert_eq!(service.fetch().await, Ok(Some(expected))); + } + #[tokio::test] async fn fetch_cloud_requirements_parses_apps_requirements_toml() { let result = parse_for_fetch(Some( @@ -2234,7 +2277,11 @@ command = "sample-mcp" .signed_payload .contents .as_deref() - .and_then(|contents| parse_cloud_requirements(contents).ok().flatten()), + .and_then(|contents| { + parse_cloud_requirements(contents, codex_home.path()) + .ok() + .flatten() + }), Some(ConfigRequirementsToml { allowed_approval_policies: Some(vec![AskForApproval::Never]), allowed_approvals_reviewers: None, @@ -2358,7 +2405,11 @@ command = "sample-mcp" .signed_payload .contents .as_deref() - .and_then(|contents| parse_cloud_requirements(contents).ok().flatten()), + .and_then(|contents| { + parse_cloud_requirements(contents, codex_home.path()) + .ok() + .flatten() + }), Some(ConfigRequirementsToml { allowed_approval_policies: Some(vec![AskForApproval::OnRequest]), allowed_approvals_reviewers: None,