mirror of
https://github.com/openai/codex.git
synced 2026-10-26 18:38:47 +00:00
Fail closed for required OTEL exporters
This commit is contained in:
@@ -53,6 +53,7 @@ use codex_config::ThreadConfigLoader;
|
||||
use codex_config::config_toml::ConfigToml;
|
||||
use codex_core::config::Config;
|
||||
pub use codex_core::otel_init::build_provider as build_otel_provider;
|
||||
pub use codex_core::otel_init::has_required_exporter as has_required_otel_exporter;
|
||||
use codex_core::personality_migration::PersonalityMigrationStatus;
|
||||
use codex_core::personality_migration::maybe_migrate_personality;
|
||||
pub use codex_exec_server::EnvironmentManager;
|
||||
|
||||
@@ -11105,6 +11105,35 @@ async fn invalid_required_otel_trace_metadata_fails_closed() -> std::io::Result<
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn invalid_required_otel_exporter_headers_fail_closed() -> std::io::Result<()> {
|
||||
for (exporter, header) in [
|
||||
(
|
||||
"otlp-http",
|
||||
r#"headers = { "invalid header" = "secret" }, protocol = "binary""#,
|
||||
),
|
||||
("otlp-grpc", r#"headers = { valid = "\u007f" }"#),
|
||||
] {
|
||||
let codex_home = TempDir::new()?;
|
||||
let requirements = format!(
|
||||
r#"
|
||||
[otel]
|
||||
exporter = {{ {exporter} = {{ endpoint = "https://otel.example", {header} }} }}
|
||||
"#,
|
||||
);
|
||||
let err = load_with_enterprise_requirement(&codex_home, requirements)
|
||||
.await
|
||||
.expect_err("invalid managed OTEL headers should fail startup");
|
||||
|
||||
assert_eq!(err.kind(), ErrorKind::InvalidInput);
|
||||
assert!(
|
||||
err.to_string()
|
||||
.contains("invalid required `otel.exporter` header")
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn required_otel_tracestate_survives_invalid_user_merge() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
@@ -7,6 +7,8 @@ use codex_config::types::DEFAULT_OTEL_ENVIRONMENT;
|
||||
use codex_config::types::OtelConfig;
|
||||
use codex_config::types::OtelConfigToml;
|
||||
use codex_config::types::OtelExporterKind;
|
||||
use http::HeaderName;
|
||||
use http::HeaderValue;
|
||||
|
||||
pub(crate) fn resolve_config(
|
||||
config: OtelConfigToml,
|
||||
@@ -119,6 +121,13 @@ pub(super) fn apply_requirement(
|
||||
}
|
||||
|
||||
fn validate_requirement(requirement: &OtelConfigToml) -> std::io::Result<()> {
|
||||
for (field, exporter) in [
|
||||
("exporter", requirement.exporter.as_ref()),
|
||||
("trace_exporter", requirement.trace_exporter.as_ref()),
|
||||
("metrics_exporter", requirement.metrics_exporter.as_ref()),
|
||||
] {
|
||||
validate_exporter_headers(field, exporter)?;
|
||||
}
|
||||
if let Some(span_attributes) = requirement.span_attributes.as_ref() {
|
||||
codex_otel::validate_span_attributes(span_attributes).map_err(|err| {
|
||||
std::io::Error::new(
|
||||
@@ -138,6 +147,27 @@ fn validate_requirement(requirement: &OtelConfigToml) -> std::io::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_exporter_headers(
|
||||
field: &str,
|
||||
exporter: Option<&OtelExporterKind>,
|
||||
) -> std::io::Result<()> {
|
||||
let headers = match exporter {
|
||||
Some(OtelExporterKind::OtlpHttp { headers, .. })
|
||||
| Some(OtelExporterKind::OtlpGrpc { headers, .. }) => headers,
|
||||
Some(OtelExporterKind::None | OtelExporterKind::Statsig) | None => return Ok(()),
|
||||
};
|
||||
for (name, value) in headers {
|
||||
if HeaderName::from_bytes(name.as_bytes()).is_err() || HeaderValue::from_str(value).is_err()
|
||||
{
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
format!("invalid required `otel.{field}` header `{name}`"),
|
||||
));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Merges user-configured tracestate into a validated required base.
|
||||
///
|
||||
/// Required member fields always win. Other configured fields are admitted one
|
||||
|
||||
@@ -10,6 +10,19 @@ use codex_otel::OtelSettings;
|
||||
use codex_otel::OtelTlsConfig as OtelTlsSettings;
|
||||
use std::error::Error;
|
||||
|
||||
pub fn has_required_exporter(config: &Config) -> bool {
|
||||
config
|
||||
.config_layer_stack
|
||||
.requirements()
|
||||
.otel
|
||||
.as_ref()
|
||||
.is_some_and(|requirement| {
|
||||
requirement.value.exporter.is_some()
|
||||
|| requirement.value.trace_exporter.is_some()
|
||||
|| requirement.value.metrics_exporter.is_some()
|
||||
})
|
||||
}
|
||||
|
||||
/// Build an OpenTelemetry provider from the app Config.
|
||||
///
|
||||
/// Returns `None` when OTEL export is disabled.
|
||||
|
||||
@@ -484,6 +484,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
|
||||
std::process::exit(1);
|
||||
}
|
||||
|
||||
let required_otel_exporter = codex_core::otel_init::has_required_exporter(&config);
|
||||
let otel = match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
|
||||
codex_core::otel_init::build_provider(
|
||||
&config,
|
||||
@@ -493,10 +494,18 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
|
||||
)
|
||||
})) {
|
||||
Ok(Ok(otel)) => otel,
|
||||
Ok(Err(e)) if required_otel_exporter => {
|
||||
eprintln!("Could not create required otel exporter: {e}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
Ok(Err(e)) => {
|
||||
eprintln!("Could not create otel exporter: {e}");
|
||||
None
|
||||
}
|
||||
Err(_) if required_otel_exporter => {
|
||||
eprintln!("Could not create required otel exporter: panicked during initialization");
|
||||
std::process::exit(1);
|
||||
}
|
||||
Err(_) => {
|
||||
eprintln!("Could not create otel exporter: panicked during initialization");
|
||||
None
|
||||
|
||||
@@ -1060,6 +1060,7 @@ pub async fn run_main(
|
||||
remove_legacy_tui_log_file(config.codex_home.as_path());
|
||||
|
||||
let otel_originator = originator().value;
|
||||
let required_otel_exporter = codex_app_server_client::has_required_otel_exporter(&config);
|
||||
let otel = match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
|
||||
codex_app_server_client::build_otel_provider(
|
||||
&config,
|
||||
@@ -1069,6 +1070,13 @@ pub async fn run_main(
|
||||
)
|
||||
})) {
|
||||
Ok(Ok(otel)) => otel,
|
||||
Ok(Err(e)) if required_otel_exporter => {
|
||||
#[allow(clippy::print_stderr)]
|
||||
{
|
||||
eprintln!("Could not create required otel exporter: {e}");
|
||||
}
|
||||
std::process::exit(1);
|
||||
}
|
||||
Ok(Err(e)) => {
|
||||
#[allow(clippy::print_stderr)]
|
||||
{
|
||||
@@ -1076,6 +1084,15 @@ pub async fn run_main(
|
||||
}
|
||||
None
|
||||
}
|
||||
Err(_) if required_otel_exporter => {
|
||||
#[allow(clippy::print_stderr)]
|
||||
{
|
||||
eprintln!(
|
||||
"Could not create required otel exporter: panicked during initialization"
|
||||
);
|
||||
}
|
||||
std::process::exit(1);
|
||||
}
|
||||
Err(_) => {
|
||||
#[allow(clippy::print_stderr)]
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user