mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Report managed filesystem policy in codex doctor (#42821)
## What changed - Load doctor configuration through the cloud-aware CLI configuration path while preserving invocation overrides such as `--cd`. - Report configuration load time and clarify that diagnostics cover invocation configuration, including cloud-managed policy, but not active thread overrides. - Add redacted filesystem sandbox diagnostics for denied-read rule counts, glob rule counts, glob scan depth, and the managed policy source. ## Testing Add an integration test that verifies valid cloud-managed filesystem requirements are reported without exposing denied paths, and invalid requirements fail the configuration check. GitOrigin-RevId: 65c3c9ad3fd42c1579ca0c9d8e209066f98e5407
This commit is contained in:
@@ -5,6 +5,7 @@ use codex_config::CloudConfigBundleLoader;
|
||||
use codex_config::ConfigLoadOptions;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::ConfigBuilder;
|
||||
use codex_core::config::ConfigOverrides;
|
||||
use codex_core::config::LoaderOverrides;
|
||||
use codex_core::config::bootstrap_auth_config;
|
||||
use codex_core::config::find_codex_home;
|
||||
@@ -16,22 +17,31 @@ pub(crate) async fn load_config(
|
||||
config_overrides: &CliConfigOverrides,
|
||||
loader_overrides: LoaderOverrides,
|
||||
) -> Result<Config> {
|
||||
config_builder(config_overrides, loader_overrides)
|
||||
.await?
|
||||
.build()
|
||||
.await
|
||||
.context("failed to load configuration")
|
||||
config_builder(
|
||||
config_overrides,
|
||||
loader_overrides,
|
||||
ConfigOverrides::default(),
|
||||
)
|
||||
.await?
|
||||
.build()
|
||||
.await
|
||||
.context("failed to load configuration")
|
||||
}
|
||||
|
||||
pub(crate) async fn config_builder(
|
||||
config_overrides: &CliConfigOverrides,
|
||||
loader_overrides: LoaderOverrides,
|
||||
harness_overrides: ConfigOverrides,
|
||||
) -> Result<ConfigBuilder> {
|
||||
let cli_overrides = config_overrides
|
||||
.parse_overrides()
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
let codex_home = find_codex_home().context("failed to resolve CODEX_HOME")?;
|
||||
let cwd = AbsolutePathBuf::current_dir().context("failed to resolve current directory")?;
|
||||
let cwd = match harness_overrides.cwd.as_deref() {
|
||||
Some(cwd) => AbsolutePathBuf::relative_to_current_dir(cwd),
|
||||
None => AbsolutePathBuf::current_dir(),
|
||||
}
|
||||
.context("failed to resolve current directory")?;
|
||||
let bootstrap_config = load_config_toml_with_layer_stack(
|
||||
codex_home.as_path(),
|
||||
Some(&cwd),
|
||||
@@ -56,6 +66,7 @@ pub(crate) async fn config_builder(
|
||||
.codex_home(codex_home.to_path_buf())
|
||||
.cli_overrides(cli_overrides)
|
||||
.loader_overrides(loader_overrides)
|
||||
.harness_overrides(harness_overrides)
|
||||
.cloud_config_bundle(cloud_config_bundle)
|
||||
.fallback_cwd(Some(cwd.to_path_buf())))
|
||||
}
|
||||
|
||||
+27
-16
@@ -35,8 +35,8 @@ use codex_arg0::Arg0DispatchPaths;
|
||||
use codex_config::types::McpServerConfig;
|
||||
use codex_config::types::McpServerTransportConfig;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::ConfigBuilder;
|
||||
use codex_core::config::ConfigOverrides;
|
||||
use codex_core::config::LoaderOverrides;
|
||||
use codex_core::config::find_codex_home;
|
||||
use codex_features::FEATURES;
|
||||
use codex_http_client::ClientRouteClass;
|
||||
@@ -358,7 +358,9 @@ async fn build_report(
|
||||
checks.push(run_sync_check("search", progress.clone(), search_check));
|
||||
|
||||
progress.begin("config");
|
||||
let config_started = Instant::now();
|
||||
let config_result = load_config(root_config_overrides, interactive, arg0_paths).await;
|
||||
let config_duration = config_started.elapsed();
|
||||
let cwd = config_result
|
||||
.as_ref()
|
||||
.map(|config| config.cwd.as_path().to_path_buf())
|
||||
@@ -403,7 +405,14 @@ async fn build_report(
|
||||
background_server_check,
|
||||
reachability_check,
|
||||
) = tokio::join!(
|
||||
async { run_sync_check("config", progress.clone(), || config_check(config)) },
|
||||
async {
|
||||
run_sync_check("config", progress.clone(), || {
|
||||
config_check(config).detail(format!(
|
||||
"configuration load ms: {}",
|
||||
config_duration.as_millis()
|
||||
))
|
||||
})
|
||||
},
|
||||
async {
|
||||
run_sync_check("auth", progress.clone(), || match &auth_manager_result {
|
||||
Ok(_) => auth_check(config),
|
||||
@@ -562,18 +571,14 @@ async fn build_report(
|
||||
}
|
||||
|
||||
async fn load_config(
|
||||
root_config_overrides: CliConfigOverrides,
|
||||
mut root_config_overrides: CliConfigOverrides,
|
||||
interactive: &TuiCli,
|
||||
arg0_paths: &Arg0DispatchPaths,
|
||||
) -> anyhow::Result<Config> {
|
||||
let mut cli_kv_overrides = root_config_overrides
|
||||
.parse_overrides()
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
if interactive.web_search {
|
||||
cli_kv_overrides.push((
|
||||
"web_search".to_string(),
|
||||
toml::Value::String("live".to_string()),
|
||||
));
|
||||
root_config_overrides
|
||||
.raw_overrides
|
||||
.push("web_search=\"live\"".to_string());
|
||||
}
|
||||
|
||||
let overrides = ConfigOverrides {
|
||||
@@ -581,12 +586,15 @@ async fn load_config(
|
||||
..config_overrides_from_interactive(interactive, arg0_paths)
|
||||
};
|
||||
|
||||
ConfigBuilder::default()
|
||||
.cli_overrides(cli_kv_overrides)
|
||||
.harness_overrides(overrides)
|
||||
.build()
|
||||
.await
|
||||
.context("failed to load Codex config")
|
||||
crate::cloud_config::config_builder(
|
||||
&root_config_overrides,
|
||||
LoaderOverrides::default(),
|
||||
overrides,
|
||||
)
|
||||
.await?
|
||||
.build()
|
||||
.await
|
||||
.context("failed to load Codex config")
|
||||
}
|
||||
|
||||
fn config_overrides_from_interactive(
|
||||
@@ -1053,6 +1061,9 @@ fn codex_path_entries() -> Vec<String> {
|
||||
|
||||
fn config_check(config: &Config) -> DoctorCheck {
|
||||
let mut details = Vec::new();
|
||||
details
|
||||
.push("configuration scope: invocation config, including cloud-managed policy".to_string());
|
||||
details.push("active thread overrides: not inspected".to_string());
|
||||
details.push(format!("CODEX_HOME: {}", config.codex_home.display()));
|
||||
details.push(format!("cwd: {}", config.cwd.display()));
|
||||
details.push(format!(
|
||||
|
||||
@@ -4,11 +4,14 @@ use std::fs;
|
||||
use std::io;
|
||||
|
||||
use codex_arg0::Arg0DispatchPaths;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_core::config::Config;
|
||||
#[cfg(target_os = "windows")]
|
||||
use codex_core::windows_sandbox::WindowsSandboxLevelExt;
|
||||
#[cfg(target_os = "windows")]
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
#[cfg(target_os = "windows")]
|
||||
use codex_sandboxing::windows_sandbox_uses_elevated_backend;
|
||||
#[cfg(target_os = "windows")]
|
||||
@@ -49,6 +52,45 @@ pub(super) fn sandbox_check(config: &Config, arg0_paths: &Arg0DispatchPaths) ->
|
||||
));
|
||||
let file_system_sandbox = config.permissions.file_system_sandbox_policy();
|
||||
details.push(format!("filesystem sandbox: {}", file_system_sandbox.kind));
|
||||
let denied_entries = file_system_sandbox
|
||||
.entries
|
||||
.iter()
|
||||
.filter(|entry| entry.access == FileSystemAccessMode::Deny);
|
||||
details.push(format!(
|
||||
"denied-read rules: {}",
|
||||
denied_entries.clone().count()
|
||||
));
|
||||
details.push(format!(
|
||||
"denied-read glob rules: {}",
|
||||
denied_entries
|
||||
.filter(|entry| matches!(entry.path, FileSystemPath::GlobPattern { .. }))
|
||||
.count()
|
||||
));
|
||||
details.push(format!(
|
||||
"glob scan max depth: {}",
|
||||
file_system_sandbox
|
||||
.glob_scan_max_depth
|
||||
.map_or_else(|| "unbounded".to_string(), |depth| depth.to_string())
|
||||
));
|
||||
details.push(format!(
|
||||
"managed filesystem source: {}",
|
||||
match config
|
||||
.config_layer_stack
|
||||
.requirements()
|
||||
.filesystem
|
||||
.as_ref()
|
||||
.map(|value| &value.source)
|
||||
{
|
||||
None => "none",
|
||||
Some(RequirementSource::Unknown) => "unknown",
|
||||
Some(RequirementSource::EnterpriseManaged { .. }) => "cloud",
|
||||
Some(RequirementSource::Composite { .. }) => "multiple layers",
|
||||
Some(RequirementSource::MdmManagedPreferences { .. }) => "MDM",
|
||||
Some(RequirementSource::SystemRequirementsToml { .. }) => "system",
|
||||
Some(RequirementSource::LegacyManagedConfigTomlFromFile { .. }) => "legacy file",
|
||||
Some(RequirementSource::LegacyManagedConfigTomlFromMdm) => "legacy MDM",
|
||||
}
|
||||
));
|
||||
details.push(format!(
|
||||
"network sandbox: {}",
|
||||
config.permissions.network_sandbox_policy()
|
||||
@@ -88,10 +130,7 @@ pub(super) fn sandbox_check(config: &Config, arg0_paths: &Arg0DispatchPaths) ->
|
||||
} else {
|
||||
configured_backend
|
||||
};
|
||||
let denied = !config
|
||||
.permissions
|
||||
.file_system_sandbox_policy()
|
||||
.has_full_disk_read_access();
|
||||
let denied = !file_system_sandbox.has_full_disk_read_access();
|
||||
|
||||
check.details.push(format!("sandbox backend: {backend}"));
|
||||
check
|
||||
|
||||
@@ -3,6 +3,7 @@ use anyhow::Result;
|
||||
use anyhow::bail;
|
||||
use clap::Parser;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::ConfigOverrides;
|
||||
use codex_core::config::LoaderOverrides;
|
||||
use codex_core::plugins_manager_for_config;
|
||||
use codex_core_plugins::PluginMarketplaceUpgradeOutcome;
|
||||
@@ -128,8 +129,12 @@ impl MarketplaceCli {
|
||||
subcommand,
|
||||
} = self;
|
||||
|
||||
let builder =
|
||||
cloud_config::config_builder(&config_overrides, LoaderOverrides::default()).await?;
|
||||
let builder = cloud_config::config_builder(
|
||||
&config_overrides,
|
||||
LoaderOverrides::default(),
|
||||
ConfigOverrides::default(),
|
||||
)
|
||||
.await?;
|
||||
let config = builder.clone().build().await?;
|
||||
|
||||
match subcommand {
|
||||
|
||||
@@ -5,10 +5,11 @@ use std::process::Stdio;
|
||||
|
||||
use anyhow::Context as _;
|
||||
use anyhow::Result;
|
||||
#[cfg(target_os = "macos")]
|
||||
use app_test_support::ChatGptAuthFixture;
|
||||
use app_test_support::write_chatgpt_auth;
|
||||
use codex_config::types::AuthCredentialsStoreMode;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
#[cfg(target_os = "macos")]
|
||||
use serde_json::json;
|
||||
use tempfile::TempDir;
|
||||
use wiremock::Mock;
|
||||
@@ -17,6 +18,124 @@ use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn doctor_reports_cloud_filesystem_policy_and_rejects_invalid_requirements() -> Result<()> {
|
||||
for valid_requirements in [true, false] {
|
||||
let server = MockServer::start().await;
|
||||
let codex_home = TempDir::new()?;
|
||||
let workspace = TempDir::new()?;
|
||||
let workspace_key = serde_json::to_string(workspace.path())?;
|
||||
let private_path = workspace.path().join("private-doctor-control");
|
||||
let glob = format!("{}/**/*.doctor-secret", workspace.path().display());
|
||||
let requirements = if valid_requirements {
|
||||
format!("[permissions.filesystem]\ndeny_read = [{private_path:?}, {glob:?}]\n")
|
||||
} else {
|
||||
"[permissions.filesystem]\ndeny_read = false\n".to_string()
|
||||
};
|
||||
std::fs::write(
|
||||
codex_home.path().join("config.toml"),
|
||||
format!(
|
||||
r#"
|
||||
cli_auth_credentials_store = "ephemeral"
|
||||
chatgpt_base_url = "{}/backend-api"
|
||||
model_provider = "local"
|
||||
[model_providers.local]
|
||||
name = "local"
|
||||
base_url = "{}/v1"
|
||||
wire_api = "responses"
|
||||
[windows]
|
||||
sandbox = "elevated"
|
||||
[projects.{workspace_key}]
|
||||
trust_level = "trusted"
|
||||
"#,
|
||||
server.uri(),
|
||||
server.uri(),
|
||||
),
|
||||
)?;
|
||||
// Cloud authentication must use the project selected by --cd.
|
||||
std::fs::create_dir(workspace.path().join(".codex"))?;
|
||||
std::fs::write(
|
||||
workspace.path().join(".codex/config.toml"),
|
||||
"cli_auth_credentials_store = \"file\"\n",
|
||||
)?;
|
||||
write_chatgpt_auth(
|
||||
codex_home.path(),
|
||||
ChatGptAuthFixture::new("doctor-test-token")
|
||||
.account_id("doctor-workspace")
|
||||
.chatgpt_account_id("doctor-workspace")
|
||||
.chatgpt_user_id("doctor-user")
|
||||
.plan_type("enterprise"),
|
||||
AuthCredentialsStoreMode::File,
|
||||
)?;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/backend-api/wham/config/bundle"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||
"requirements_toml": {
|
||||
"enterprise_managed": [{
|
||||
"id": "doctor-policy",
|
||||
"name": "Doctor policy fixture",
|
||||
"contents": requirements,
|
||||
}],
|
||||
},
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let output = Command::new(codex_utils_cargo_bin::cargo_bin("codex")?)
|
||||
.current_dir(codex_home.path())
|
||||
.env("CODEX_HOME", codex_home.path())
|
||||
.env("NO_PROXY", "127.0.0.1,localhost")
|
||||
.env("no_proxy", "127.0.0.1,localhost")
|
||||
.env_remove("CODEX_ACCESS_TOKEN")
|
||||
.env_remove("CODEX_API_KEY")
|
||||
.env_remove("OPENAI_API_KEY")
|
||||
.arg("--cd")
|
||||
.arg(workspace.path())
|
||||
.args(["doctor", "--json"])
|
||||
.stdin(Stdio::null())
|
||||
.output()?;
|
||||
let report: Value = serde_json::from_slice(&output.stdout)?;
|
||||
if valid_requirements {
|
||||
let config = &report["checks"]["config.load"];
|
||||
let sandbox = &report["checks"]["sandbox.helpers"]["details"];
|
||||
assert_eq!(config["status"], "ok", "{config:#}");
|
||||
assert_eq!(
|
||||
config["details"]["cwd"],
|
||||
workspace.path().display().to_string()
|
||||
);
|
||||
insta::assert_snapshot!(
|
||||
serde_json::to_string_pretty(&json!({
|
||||
"scope": config["details"]["configuration scope"],
|
||||
"activeThreadOverrides": config["details"]["active thread overrides"],
|
||||
"denyRules": sandbox["denied-read rules"],
|
||||
"denyGlobs": sandbox["denied-read glob rules"],
|
||||
"scanDepth": sandbox["glob scan max depth"],
|
||||
"managedFilesystemSource": sandbox["managed filesystem source"],
|
||||
}))?,
|
||||
@r#"
|
||||
{
|
||||
"scope": "invocation config, including cloud-managed policy",
|
||||
"activeThreadOverrides": "not inspected",
|
||||
"denyRules": "2",
|
||||
"denyGlobs": "1",
|
||||
"scanDepth": "unbounded",
|
||||
"managedFilesystemSource": "cloud"
|
||||
}
|
||||
"#
|
||||
);
|
||||
let stdout = String::from_utf8(output.stdout)?;
|
||||
assert!(!stdout.contains("doctor-secret"));
|
||||
assert!(!stdout.contains("private-doctor-control"));
|
||||
} else {
|
||||
assert_eq!(report["checks"]["config.load"]["status"], "fail");
|
||||
assert!(report["checks"].get("sandbox.helpers").is_none());
|
||||
}
|
||||
server.verify().await;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn invalid_custom_ca_falls_back_to_system_roots() -> Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
|
||||
Reference in New Issue
Block a user