mirror of
https://github.com/openai/codex.git
synced 2026-05-22 12:04:19 +00:00
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`
This commit is contained in:
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -2321,6 +2321,7 @@ dependencies = [
|
||||
"codex-login",
|
||||
"codex-otel",
|
||||
"codex-protocol",
|
||||
"codex-utils-absolute-path",
|
||||
"hmac",
|
||||
"pretty_assertions",
|
||||
"serde",
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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<ConfigRequirementsToml> {
|
||||
self.contents
|
||||
.as_deref()
|
||||
.and_then(|contents| parse_cloud_requirements(contents).ok().flatten())
|
||||
fn requirements(&self, requirements_base_dir: &Path) -> Option<ConfigRequirementsToml> {
|
||||
self.contents.as_deref().and_then(|contents| {
|
||||
parse_cloud_requirements(contents, requirements_base_dir)
|
||||
.ok()
|
||||
.flatten()
|
||||
})
|
||||
}
|
||||
}
|
||||
fn sign_cache_payload(payload_bytes: &[u8]) -> Option<String> {
|
||||
@@ -258,6 +262,7 @@ impl RequirementsFetcher for BackendRequirementsFetcher {
|
||||
struct CloudRequirementsService {
|
||||
auth_manager: Arc<AuthManager>,
|
||||
fetcher: Arc<dyn RequirementsFetcher>,
|
||||
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<Option<ConfigRequirementsToml>, 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<ConfigRequirementsToml> {
|
||||
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::<ConfigRequirementsToml>(&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,
|
||||
|
||||
Reference in New Issue
Block a user