mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: add secret auth storage configuration (#27504)
## Why Windows Credential Manager limits generic credential blobs to 2,560 bytes. The encrypted local secrets backend avoids storing large serialized auth payloads directly in the OS keyring, but selecting that backend needs an independently reviewable feature/config layer before the auth and secrets implementation is wired in. ## What Changed - Added the stable `secret_auth_storage` feature, enabled by default on Windows and disabled by default elsewhere. - Added `AuthKeyringBackendKind` and config resolution for full and bootstrap config loading. - Applied managed feature requirements when resolving the bootstrap auth backend. - Updated the generated config schema and added focused tests. This is the base PR for #17931. The auth, secrets, MCP, CLI, TUI, and app-server implementation remains in that follow-up PR. ## Validation - `just test -p codex-features` - `just test -p codex-config` - `just test -p codex-core resolve_bootstrap_auth_keyring_backend_kind_uses_secret_auth_storage_feature` - `just write-config-schema` - `just fix -p codex-core` The full `just test -p codex-core` run compiled successfully and ran 2,690 tests; 2,589 passed, one was flaky, and 101 environment-sensitive tests failed because this shell injects a `pyenv` rehash warning into command output or because sandboxed subprocesses timed out.
This commit is contained in:
committed by
GitHub
Unverified
parent
76d8f20241
commit
b724f5966e
@@ -113,6 +113,26 @@ pub enum OAuthCredentialsStoreMode {
|
||||
Keyring,
|
||||
}
|
||||
|
||||
/// Determine how auth credentials should use keyring-backed storage.
|
||||
#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
pub enum AuthKeyringBackendKind {
|
||||
/// Store the serialized auth payload directly in the OS keyring.
|
||||
Direct,
|
||||
/// Store auth payloads in the local encrypted secrets file, with the file key in the OS keyring.
|
||||
Secrets,
|
||||
}
|
||||
|
||||
impl Default for AuthKeyringBackendKind {
|
||||
fn default() -> Self {
|
||||
if cfg!(windows) {
|
||||
Self::Secrets
|
||||
} else {
|
||||
Self::Direct
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema)]
|
||||
#[serde(rename_all = "kebab-case")]
|
||||
pub enum WindowsSandboxModeToml {
|
||||
|
||||
@@ -599,6 +599,9 @@
|
||||
"search_tool": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"secret_auth_storage": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"shell_snapshot": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -4751,6 +4754,9 @@
|
||||
"search_tool": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"secret_auth_storage": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"shell_snapshot": {
|
||||
"type": "boolean"
|
||||
},
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
use super::Config;
|
||||
use super::ConfigTomlLoadResult;
|
||||
use super::ManagedFeatures;
|
||||
use codex_config::types::AuthKeyringBackendKind;
|
||||
use codex_features::Feature;
|
||||
use codex_features::FeatureConfigSource;
|
||||
use codex_features::FeatureOverrides;
|
||||
use codex_features::Features;
|
||||
|
||||
impl Config {
|
||||
pub fn auth_keyring_backend_kind(&self) -> AuthKeyringBackendKind {
|
||||
auth_keyring_backend_kind_from_secret_auth_storage(
|
||||
self.features.enabled(Feature::SecretAuthStorage),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve the auth keyring backend from a partially loaded bootstrap config.
|
||||
///
|
||||
/// This is intended for startup paths that must read auth before managed cloud
|
||||
/// requirements can be loaded and before a full [`Config`] exists.
|
||||
pub fn resolve_bootstrap_auth_keyring_backend_kind(
|
||||
bootstrap_config: &ConfigTomlLoadResult,
|
||||
) -> std::io::Result<AuthKeyringBackendKind> {
|
||||
let config_toml = &bootstrap_config.config_toml;
|
||||
let features = Features::from_sources(
|
||||
FeatureConfigSource {
|
||||
features: config_toml.features.as_ref(),
|
||||
experimental_use_unified_exec_tool: config_toml.experimental_use_unified_exec_tool,
|
||||
},
|
||||
FeatureConfigSource::default(),
|
||||
FeatureOverrides::default(),
|
||||
);
|
||||
let managed_features = ManagedFeatures::from_configured(
|
||||
features,
|
||||
bootstrap_config
|
||||
.config_layer_stack
|
||||
.requirements()
|
||||
.feature_requirements
|
||||
.clone(),
|
||||
)?;
|
||||
Ok(auth_keyring_backend_kind_from_secret_auth_storage(
|
||||
managed_features.enabled(Feature::SecretAuthStorage),
|
||||
))
|
||||
}
|
||||
|
||||
fn auth_keyring_backend_kind_from_secret_auth_storage(
|
||||
secret_auth_storage_enabled: bool,
|
||||
) -> AuthKeyringBackendKind {
|
||||
if secret_auth_storage_enabled {
|
||||
AuthKeyringBackendKind::Secrets
|
||||
} else {
|
||||
AuthKeyringBackendKind::Direct
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "auth_keyring_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,79 @@
|
||||
use super::*;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigRequirements;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_config::FeatureRequirementsToml;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_config::Sourced;
|
||||
use codex_config::config_toml::ConfigToml;
|
||||
use codex_features::FeaturesToml;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
#[test]
|
||||
fn resolve_bootstrap_auth_keyring_backend_kind_uses_secret_auth_storage_feature()
|
||||
-> std::io::Result<()> {
|
||||
let config_toml = ConfigToml {
|
||||
features: Some(FeaturesToml::from(BTreeMap::from([(
|
||||
"secret_auth_storage".to_string(),
|
||||
true,
|
||||
)]))),
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(
|
||||
resolve_bootstrap_auth_keyring_backend_kind(&config_toml_load_result(
|
||||
config_toml,
|
||||
/*feature_requirements*/ None,
|
||||
)?)?,
|
||||
AuthKeyringBackendKind::Secrets
|
||||
);
|
||||
|
||||
let config_toml = ConfigToml {
|
||||
features: Some(FeaturesToml::from(BTreeMap::from([(
|
||||
"secret_auth_storage".to_string(),
|
||||
false,
|
||||
)]))),
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(
|
||||
resolve_bootstrap_auth_keyring_backend_kind(&config_toml_load_result(
|
||||
config_toml.clone(),
|
||||
/*feature_requirements*/ None,
|
||||
)?)?,
|
||||
AuthKeyringBackendKind::Direct
|
||||
);
|
||||
|
||||
let requirements = Sourced::new(
|
||||
FeatureRequirementsToml {
|
||||
entries: BTreeMap::from([("secret_auth_storage".to_string(), true)]),
|
||||
},
|
||||
RequirementSource::Unknown,
|
||||
);
|
||||
assert_eq!(
|
||||
resolve_bootstrap_auth_keyring_backend_kind(&config_toml_load_result(
|
||||
config_toml,
|
||||
Some(requirements),
|
||||
)?)?,
|
||||
AuthKeyringBackendKind::Secrets
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn config_toml_load_result(
|
||||
config_toml: ConfigToml,
|
||||
feature_requirements: Option<Sourced<FeatureRequirementsToml>>,
|
||||
) -> std::io::Result<ConfigTomlLoadResult> {
|
||||
let requirements = ConfigRequirements {
|
||||
feature_requirements,
|
||||
..Default::default()
|
||||
};
|
||||
Ok(ConfigTomlLoadResult {
|
||||
config_toml,
|
||||
config_layer_stack: ConfigLayerStack::new(
|
||||
Vec::new(),
|
||||
requirements,
|
||||
ConfigRequirementsToml::default(),
|
||||
)?,
|
||||
})
|
||||
}
|
||||
@@ -4,8 +4,10 @@ use crate::config::edit::apply_blocking;
|
||||
use assert_matches::assert_matches;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::ConfigLayerEntry;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ProfileV2Name;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_config::Sourced;
|
||||
use codex_config::config_toml::AgentRoleToml;
|
||||
use codex_config::config_toml::AgentsToml;
|
||||
use codex_config::config_toml::AutoReviewToml;
|
||||
|
||||
@@ -137,6 +137,7 @@ use toml::Value as TomlValue;
|
||||
use toml_edit::DocumentMut;
|
||||
|
||||
pub(crate) mod agent_roles;
|
||||
mod auth_keyring;
|
||||
pub mod edit;
|
||||
mod managed_features;
|
||||
mod network_proxy_spec;
|
||||
@@ -145,6 +146,7 @@ mod permissions;
|
||||
mod resolved_permission_profile;
|
||||
#[cfg(test)]
|
||||
mod schema;
|
||||
pub use auth_keyring::resolve_bootstrap_auth_keyring_backend_kind;
|
||||
pub use codex_config::ConfigLoadOptions;
|
||||
pub use codex_config::Constrained;
|
||||
pub use codex_config::ConstraintError;
|
||||
@@ -1647,6 +1649,29 @@ pub async fn load_config_as_toml_with_cli_and_load_options(
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
options: impl Into<ConfigLoadOptions>,
|
||||
) -> std::io::Result<ConfigToml> {
|
||||
load_config_toml_with_layer_stack(codex_home, cwd, cli_overrides, options)
|
||||
.await
|
||||
.map(|result| result.config_toml)
|
||||
}
|
||||
|
||||
/// Partially loaded config plus the layer stack used to derive it.
|
||||
///
|
||||
/// This is intended for startup paths that must inspect raw config before a
|
||||
/// full [`Config`] can be constructed, but still need access to managed
|
||||
/// requirements loaded with the config layers.
|
||||
pub struct ConfigTomlLoadResult {
|
||||
pub config_toml: ConfigToml,
|
||||
pub config_layer_stack: ConfigLayerStack,
|
||||
}
|
||||
|
||||
/// Loads the partially merged config together with the layer stack used to
|
||||
/// derive it, before constructing a full [`Config`].
|
||||
pub async fn load_config_toml_with_layer_stack(
|
||||
codex_home: &Path,
|
||||
cwd: Option<&AbsolutePathBuf>,
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
options: impl Into<ConfigLoadOptions>,
|
||||
) -> std::io::Result<ConfigTomlLoadResult> {
|
||||
let config_layer_stack = load_config_layers_state(
|
||||
LOCAL_FS.as_ref(),
|
||||
codex_home,
|
||||
@@ -1663,7 +1688,10 @@ pub async fn load_config_as_toml_with_cli_and_load_options(
|
||||
e
|
||||
})?;
|
||||
|
||||
Ok(cfg)
|
||||
Ok(ConfigTomlLoadResult {
|
||||
config_toml: cfg,
|
||||
config_layer_stack,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn deserialize_config_toml_with_base(
|
||||
|
||||
@@ -81,6 +81,8 @@ pub enum Feature {
|
||||
ShellTool,
|
||||
/// Enable Claude-style lifecycle hooks loaded from hooks.json files.
|
||||
CodexHooks,
|
||||
/// Store CLI auth in the encrypted local secrets backend when keyring storage is selected.
|
||||
SecretAuthStorage,
|
||||
|
||||
// Experimental
|
||||
/// Enable JavaScript code mode backed by the in-process V8 runtime.
|
||||
@@ -748,6 +750,12 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::SecretAuthStorage,
|
||||
key: "secret_auth_storage",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: cfg!(windows),
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::UnifiedExec,
|
||||
key: "unified_exec",
|
||||
|
||||
@@ -189,6 +189,16 @@ fn tool_search_is_removed_and_disabled_by_default() {
|
||||
assert_eq!(feature_for_key("tool_search"), Some(Feature::ToolSearch));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn secret_auth_storage_defaults_to_windows_only() {
|
||||
assert_eq!(Feature::SecretAuthStorage.stage(), Stage::Stable);
|
||||
assert_eq!(Feature::SecretAuthStorage.default_enabled(), cfg!(windows));
|
||||
assert_eq!(
|
||||
feature_for_key("secret_auth_storage"),
|
||||
Some(Feature::SecretAuthStorage)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn browser_controls_are_stable_and_enabled_by_default() {
|
||||
assert_eq!(Feature::InAppBrowser.stage(), Stage::Stable);
|
||||
|
||||
Reference in New Issue
Block a user