valknar/llmx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Replace config.responses_originator_header_internal_override with CODEX_INTERNAL_ORIGINATOR_OVERRIDE_ENV_VAR (#3388)

Browse Source 
The previous config approach had a few issues:
1. It is part of the config but not designed to be used externally
2. It had to be wired through many places (look at the +/- on this PR
3. It wasn't guaranteed to be set consistently everywhere because we
don't have a super well defined way that configs stack. For example, the
extension would configure during newConversation but anything that
happened outside of that (like login) wouldn't get it.

This env var approach is cleaner and also creates one less thing we have
to deal with when coming up with a better holistic story around configs.

One downside is that I removed the unit test testing for the override
because I don't want to deal with setting the global env or spawning
child processes and figuring out how to introspect their originator
header. The new code is sufficiently simple and I tested it e2e that I
feel as if this is still worth it.
This commit is contained in:
Gabriel Peal
2025-09-09 14:23:23 -07:00
committed by GitHub
parent f656e192bf
commit 5eab4c7ab4
20 changed files with 92 additions and 206 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
codex-rs/core/src/auth.rs
View File

@@ -75,9 +75,8 @@ impl CodexAuth {
pub fn from_codex_home(
codex_home: &Path,
preferred_auth_method: AuthMode,
originator: &str,
) -> std::io::Result<Option<CodexAuth>> {
load_auth(codex_home, true, preferred_auth_method, originator)
load_auth(codex_home, true, preferred_auth_method)
}
pub async fn get_token_data(&self) -> Result<TokenData, std::io::Error> {
@@ -173,7 +172,7 @@ impl CodexAuth {
mode: AuthMode::ChatGPT,
auth_file: PathBuf::new(),
auth_dot_json,
client: crate::default_client::create_client("codex_cli_rs"),
client: crate::default_client::create_client(),
}
}
@@ -188,10 +187,7 @@ impl CodexAuth {
}
pub fn from_api_key(api_key: &str) -> Self {
Self::from_api_key_with_client(
api_key,
crate::default_client::create_client(crate::default_client::DEFAULT_ORIGINATOR),
)
Self::from_api_key_with_client(api_key, crate::default_client::create_client())
}
}
@@ -232,13 +228,12 @@ fn load_auth(
codex_home: &Path,
include_env_var: bool,
preferred_auth_method: AuthMode,
originator: &str,
) -> std::io::Result<Option<CodexAuth>> {
// First, check to see if there is a valid auth.json file. If not, we fall
// back to AuthMode::ApiKey using the OPENAI_API_KEY environment variable
// (if it is set).
let auth_file = get_auth_file(codex_home);
let client = crate::default_client::create_client(originator);
let client = crate::default_client::create_client();
let auth_dot_json = match try_read_auth_json(&auth_file) {
Ok(auth) => auth,
// If auth.json does not exist, try to read the OPENAI_API_KEY from the
@@ -473,7 +468,7 @@ mod tests {
auth_dot_json,
auth_file: _,
..
} = super::load_auth(codex_home.path(), false, AuthMode::ChatGPT, "codex_cli_rs")
} = super::load_auth(codex_home.path(), false, AuthMode::ChatGPT)
.unwrap()
.unwrap();
assert_eq!(None, api_key);
@@ -525,7 +520,7 @@ mod tests {
auth_dot_json,
auth_file: _,
..
} = super::load_auth(codex_home.path(), false, AuthMode::ChatGPT, "codex_cli_rs")
} = super::load_auth(codex_home.path(), false, AuthMode::ChatGPT)
.unwrap()
.unwrap();
assert_eq!(None, api_key);
@@ -576,7 +571,7 @@ mod tests {
auth_dot_json,
auth_file: _,
..
} = super::load_auth(codex_home.path(), false, AuthMode::ChatGPT, "codex_cli_rs")
} = super::load_auth(codex_home.path(), false, AuthMode::ChatGPT)
.unwrap()
.unwrap();
assert_eq!(Some("sk-test-key".to_string()), api_key);
@@ -596,7 +591,7 @@ mod tests {
)
.unwrap();
let auth = super::load_auth(dir.path(), false, AuthMode::ChatGPT, "codex_cli_rs")
let auth = super::load_auth(dir.path(), false, AuthMode::ChatGPT)
.unwrap()
.unwrap();
assert_eq!(auth.mode, AuthMode::ApiKey);
@@ -680,7 +675,6 @@ mod tests {
#[derive(Debug)]
pub struct AuthManager {
codex_home: PathBuf,
originator: String,
inner: RwLock<CachedAuth>,
}
@@ -689,13 +683,12 @@ impl AuthManager {
/// preferred auth method. Errors loading auth are swallowed; `auth()` will
/// simply return `None` in that case so callers can treat it as an
/// unauthenticated state.
pub fn new(codex_home: PathBuf, preferred_auth_mode: AuthMode, originator: String) -> Self {
let auth = CodexAuth::from_codex_home(&codex_home, preferred_auth_mode, &originator)
pub fn new(codex_home: PathBuf, preferred_auth_mode: AuthMode) -> Self {
let auth = CodexAuth::from_codex_home(&codex_home, preferred_auth_mode)
.ok()
.flatten();
Self {
codex_home,
originator,
inner: RwLock::new(CachedAuth {
preferred_auth_mode,
auth,
@@ -712,7 +705,6 @@ impl AuthManager {
};
Arc::new(Self {
codex_home: PathBuf::new(),
originator: "codex_cli_rs".to_string(),
inner: RwLock::new(cached),
})
}
@@ -734,7 +726,7 @@ impl AuthManager {
/// whether the auth value changed.
pub fn reload(&self) -> bool {
let preferred = self.preferred_auth_method();
let new_auth = CodexAuth::from_codex_home(&self.codex_home, preferred, &self.originator)
let new_auth = CodexAuth::from_codex_home(&self.codex_home, preferred)
.ok()
.flatten();
if let Ok(mut guard) = self.inner.write() {
@@ -755,12 +747,8 @@ impl AuthManager {
}
/// Convenience constructor returning an `Arc` wrapper.
pub fn shared(
codex_home: PathBuf,
preferred_auth_mode: AuthMode,
originator: String,
) -> Arc<Self> {
Arc::new(Self::new(codex_home, preferred_auth_mode, originator))
pub fn shared(codex_home: PathBuf, preferred_auth_mode: AuthMode) -> Arc<Self> {
Arc::new(Self::new(codex_home, preferred_auth_mode))
}
/// Attempt to refresh the current auth token (if any). On success, reload

2
codex-rs/core/src/client.rs
View File

@@ -84,7 +84,7 @@ impl ModelClient {
summary: ReasoningSummaryConfig,
conversation_id: ConversationId,
) -> Self {
let client = create_client(&config.responses_originator_header);
let client = create_client();
Self {
config,

17
codex-rs/core/src/config.rs
View File

@@ -40,8 +40,6 @@ pub(crate) const PROJECT_DOC_MAX_BYTES: usize = 32 * 1024; // 32 KiB
const CONFIG_TOML_FILE: &str = "config.toml";
const DEFAULT_RESPONSES_ORIGINATOR_HEADER: &str = "codex_cli_rs";
/// Application configuration loaded from disk and merged with overrides.
#[derive(Debug, Clone, PartialEq)]
pub struct Config {
@@ -169,9 +167,6 @@ pub struct Config {
pub tools_web_search_request: bool,
/// The value for the `originator` header included with Responses API requests.
pub responses_originator_header: String,
/// If set to `true`, the API key will be signed with the `originator` header.
pub preferred_auth_method: AuthMode,
@@ -478,9 +473,6 @@ pub struct ConfigToml {
pub experimental_use_exec_command_tool: Option<bool>,
/// The value for the `originator` header included with Responses API requests.
pub responses_originator_header_internal_override: Option<String>,
pub projects: Option<HashMap<String, ProjectConfig>>,
/// If set to `true`, the API key will be signed with the `originator` header.
@@ -773,10 +765,6 @@ impl Config {
Self::get_base_instructions(experimental_instructions_path, &resolved_cwd)?;
let base_instructions = base_instructions.or(file_base_instructions);
let responses_originator_header: String = cfg
.responses_originator_header_internal_override
.unwrap_or(DEFAULT_RESPONSES_ORIGINATOR_HEADER.to_owned());
let config = Self {
model,
model_family,
@@ -826,7 +814,6 @@ impl Config {
include_plan_tool: include_plan_tool.unwrap_or(false),
include_apply_patch_tool: include_apply_patch_tool.unwrap_or(false),
tools_web_search_request,
responses_originator_header,
preferred_auth_method: cfg.preferred_auth_method.unwrap_or(AuthMode::ChatGPT),
use_experimental_streamable_shell_tool: cfg
.experimental_use_exec_command_tool
@@ -1203,7 +1190,6 @@ model_verbosity = "high"
include_plan_tool: false,
include_apply_patch_tool: false,
tools_web_search_request: false,
responses_originator_header: "codex_cli_rs".to_string(),
preferred_auth_method: AuthMode::ChatGPT,
use_experimental_streamable_shell_tool: false,
include_view_image_tool: true,
@@ -1260,7 +1246,6 @@ model_verbosity = "high"
include_plan_tool: false,
include_apply_patch_tool: false,
tools_web_search_request: false,
responses_originator_header: "codex_cli_rs".to_string(),
preferred_auth_method: AuthMode::ChatGPT,
use_experimental_streamable_shell_tool: false,
include_view_image_tool: true,
@@ -1332,7 +1317,6 @@ model_verbosity = "high"
include_plan_tool: false,
include_apply_patch_tool: false,
tools_web_search_request: false,
responses_originator_header: "codex_cli_rs".to_string(),
preferred_auth_method: AuthMode::ChatGPT,
use_experimental_streamable_shell_tool: false,
include_view_image_tool: true,
@@ -1390,7 +1374,6 @@ model_verbosity = "high"
include_plan_tool: false,
include_apply_patch_tool: false,
tools_web_search_request: false,
responses_originator_header: "codex_cli_rs".to_string(),
preferred_auth_method: AuthMode::ChatGPT,
use_experimental_streamable_shell_tool: false,
include_view_image_tool: true,

62
codex-rs/core/src/default_client.rs
View File

@@ -1,11 +1,40 @@
pub const DEFAULT_ORIGINATOR: &str = "codex_cli_rs";
use reqwest::header::HeaderValue;
use std::sync::LazyLock;
pub fn get_codex_user_agent(originator: Option<&str>) -> String {
pub const CODEX_INTERNAL_ORIGINATOR_OVERRIDE_ENV_VAR: &str = "CODEX_INTERNAL_ORIGINATOR_OVERRIDE";
#[derive(Debug, Clone)]
pub struct Originator {
pub value: String,
pub header_value: HeaderValue,
}
pub static ORIGINATOR: LazyLock<Originator> = LazyLock::new(|| {
let default = "codex_cli_rs";
let value = std::env::var(CODEX_INTERNAL_ORIGINATOR_OVERRIDE_ENV_VAR)
.unwrap_or_else(|_| default.to_string());
match HeaderValue::from_str(&value) {
Ok(header_value) => Originator {
value,
header_value,
},
Err(e) => {
tracing::error!("Unable to turn originator override {value} into header value: {e}");
Originator {
value: default.to_string(),
header_value: HeaderValue::from_static(default),
}
}
}
});
pub fn get_codex_user_agent() -> String {
let build_version = env!("CARGO_PKG_VERSION");
let os_info = os_info::get();
format!(
"{}/{build_version} ({} {}; {}) {}",
originator.unwrap_or(DEFAULT_ORIGINATOR),
ORIGINATOR.value.as_str(),
os_info.os_type(),
os_info.version(),
os_info.architecture().unwrap_or("unknown"),
@@ -14,25 +43,19 @@ pub fn get_codex_user_agent(originator: Option<&str>) -> String {
}
/// Create a reqwest client with default `originator` and `User-Agent` headers set.
pub fn create_client(originator: &str) -> reqwest::Client {
pub fn create_client() -> reqwest::Client {
use reqwest::header::HeaderMap;
use reqwest::header::HeaderValue;
let mut headers = HeaderMap::new();
let originator_value = HeaderValue::from_str(originator)
.unwrap_or_else(|_| HeaderValue::from_static(DEFAULT_ORIGINATOR));
headers.insert("originator", originator_value);
let ua = get_codex_user_agent(Some(originator));
headers.insert("originator", ORIGINATOR.header_value.clone());
let ua = get_codex_user_agent();
match reqwest::Client::builder()
reqwest::Client::builder()
// Set UA via dedicated helper to avoid header validation pitfalls
.user_agent(ua)
.default_headers(headers)
.build()
{
Ok(client) => client,
Err(_) => reqwest::Client::new(),
}
.unwrap_or_else(|_| reqwest::Client::new())
}
#[cfg(test)]
@@ -41,7 +64,7 @@ mod tests {
#[test]
fn test_get_codex_user_agent() {
let user_agent = get_codex_user_agent(None);
let user_agent = get_codex_user_agent();
assert!(user_agent.starts_with("codex_cli_rs/"));
}
@@ -53,8 +76,7 @@ mod tests {
use wiremock::matchers::method;
use wiremock::matchers::path;
let originator = "test_originator";
let client = create_client(originator);
let client = create_client();
// Spin up a local mock server and capture a request.
let server = MockServer::start().await;
@@ -82,10 +104,10 @@ mod tests {
let originator_header = headers
.get("originator")
.expect("originator header missing");
assert_eq!(originator_header.to_str().unwrap(), originator);
assert_eq!(originator_header.to_str().unwrap(), "codex_cli_rs");
// User-Agent matches the computed Codex UA for that originator
let expected_ua = get_codex_user_agent(Some(originator));
let expected_ua = get_codex_user_agent();
let ua_header = headers
.get("user-agent")
.expect("user-agent header missing");
@@ -96,7 +118,7 @@ mod tests {
#[cfg(target_os = "macos")]
fn test_macos() {
use regex_lite::Regex;
let user_agent = get_codex_user_agent(None);
let user_agent = get_codex_user_agent();
let re = Regex::new(
r"^codex_cli_rs/\d+\.\d+\.\d+ \(Mac OS \d+\.\d+\.\d+; (x86_64|arm64)\) (\S+)$",
)