refactor(secrets): extract codex-secrets and remove skills wiring

This commit is contained in:
viyatb-oai
2026-01-28 20:07:03 -08:00
parent 8783d5de60
commit f38f15ddca
20 changed files with 509 additions and 712 deletions

21
codex-rs/Cargo.lock generated
View File

@@ -1343,6 +1343,7 @@ dependencies = [
"codex-core",
"codex-exec",
"codex-execpolicy",
"codex-keyring-store",
"codex-login",
"codex-mcp-server",
"codex-protocol",
@@ -1452,7 +1453,6 @@ dependencies = [
name = "codex-core"
version = "0.0.0"
dependencies = [
"age",
"anyhow",
"arc-swap",
"assert_cmd",
@@ -1477,6 +1477,7 @@ dependencies = [
"codex-otel",
"codex-protocol",
"codex-rmcp-client",
"codex-secrets",
"codex-state",
"codex-utils-absolute-path",
"codex-utils-cargo-bin",
@@ -1944,6 +1945,24 @@ dependencies = [
"which",
]
[[package]]
name = "codex-secrets"
version = "0.0.0"
dependencies = [
"age",
"anyhow",
"base64 0.22.1",
"codex-keyring-store",
"pretty_assertions",
"rand 0.9.2",
"schemars 0.8.22",
"serde",
"serde_json",
"sha2",
"tempfile",
"tracing",
]
[[package]]
name = "codex-state"
version = "0.0.0"

View File

@@ -16,6 +16,7 @@ members = [
"cli",
"common",
"core",
"secrets",
"exec",
"exec-server",
"execpolicy",
@@ -76,6 +77,7 @@ codex-cli = { path = "cli"}
codex-client = { path = "codex-client" }
codex-common = { path = "common" }
codex-core = { path = "core" }
codex-secrets = { path = "secrets" }
codex-exec = { path = "exec" }
codex-execpolicy = { path = "execpolicy" }
codex-feedback = { path = "feedback" }

View File

@@ -57,6 +57,7 @@ codex_windows_sandbox = { package = "codex-windows-sandbox", path = "../windows-
[dev-dependencies]
assert_cmd = { workspace = true }
assert_matches = { workspace = true }
codex-keyring-store = { workspace = true }
codex-utils-cargo-bin = { workspace = true }
predicates = { workspace = true }
pretty_assertions = { workspace = true }

View File

@@ -31,6 +31,8 @@ pub struct SecretsCli {
pub enum SecretsSubcommand {
/// Store or update a secret value.
Set(SecretsSetArgs),
/// Update an existing secret value.
Edit(SecretsEditArgs),
/// List secret names (values are never displayed).
List(SecretsListArgs),
/// Delete a secret value.
@@ -61,6 +63,19 @@ pub struct SecretsSetArgs {
pub scope: SecretsScopeArgs,
}
#[derive(Debug, Parser)]
pub struct SecretsEditArgs {
/// Secret name (A-Z, 0-9, underscore only).
pub name: String,
/// New secret value. Prefer piping via stdin to avoid shell history.
#[arg(long)]
pub value: Option<String>,
#[clap(flatten)]
pub scope: SecretsScopeArgs,
}
#[derive(Debug, Parser)]
pub struct SecretsListArgs {
#[clap(flatten)]
@@ -82,6 +97,7 @@ impl SecretsCli {
let manager = SecretsManager::new(config.codex_home.clone(), config.secrets_backend);
match self.subcommand {
SecretsSubcommand::Set(args) => run_set(&config, &manager, args),
SecretsSubcommand::Edit(args) => run_edit(&config, &manager, args),
SecretsSubcommand::List(args) => run_list(&config, &manager, args),
SecretsSubcommand::Delete(args) => run_delete(&config, &manager, args),
}
@@ -100,12 +116,28 @@ async fn load_config(cli_config_overrides: CliConfigOverrides) -> Result<Config>
fn run_set(config: &Config, manager: &SecretsManager, args: SecretsSetArgs) -> Result<()> {
let name = SecretName::new(&args.name)?;
let scope = resolve_scope(config, &args.scope)?;
let value = resolve_value(&name, args.value)?;
let value = resolve_value(&args.name, args.value)?;
manager.set(&scope, &name, &value)?;
println!("Stored {name} in {}.", scope_label(&scope));
Ok(())
}
fn run_edit(config: &Config, manager: &SecretsManager, args: SecretsEditArgs) -> Result<()> {
let name = SecretName::new(&args.name)?;
let scope = resolve_scope(config, &args.scope)?;
let exists = manager.get(&scope, &name)?.is_some();
if !exists {
bail!(
"No secret named {name} found in {}. Use `codex secrets set {name}` to create it.",
scope_label(&scope)
);
}
let value = resolve_value(&args.name, args.value)?;
manager.set(&scope, &name, &value)?;
println!("Updated {name} in {}.", scope_label(&scope));
Ok(())
}
fn run_list(config: &Config, manager: &SecretsManager, args: SecretsListArgs) -> Result<()> {
let default_scope = default_environment_scope(config)?;
let scope_filter = match (args.scope.global, args.scope.environment_id.as_deref()) {
@@ -165,13 +197,13 @@ fn default_environment_scope(config: &Config) -> Result<SecretScope> {
SecretScope::environment(environment_id)
}
fn resolve_value(name: &SecretName, explicit: Option<String>) -> Result<String> {
fn resolve_value(display_name: &str, explicit: Option<String>) -> Result<String> {
if let Some(value) = explicit {
return Ok(value);
}
if std::io::stdin().is_terminal() {
return prompt_secret_value(name);
return prompt_secret_value(display_name);
}
let mut buf = String::new();
@@ -183,8 +215,8 @@ fn resolve_value(name: &SecretName, explicit: Option<String>) -> Result<String>
Ok(trimmed)
}
fn prompt_secret_value(name: &SecretName) -> Result<String> {
print!("Enter value for {name}: ");
fn prompt_secret_value(display_name: &str) -> Result<String> {
print!("Enter value for {display_name}: ");
std::io::stdout()
.flush()
.context("failed to flush stdout before prompt")?;
@@ -250,3 +282,90 @@ fn scope_label(scope: &SecretScope) -> String {
SecretScope::Environment(env_id) => format!("env/{env_id}"),
}
}
#[cfg(test)]
mod tests {
use super::*;
use codex_core::config::ConfigBuilder;
use codex_core::config::ConfigOverrides;
use codex_keyring_store::tests::MockKeyringStore;
use pretty_assertions::assert_eq;
use std::sync::Arc;
async fn test_config(codex_home: &std::path::Path, cwd: &std::path::Path) -> Result<Config> {
let overrides = ConfigOverrides {
cwd: Some(cwd.to_path_buf()),
..Default::default()
};
Ok(ConfigBuilder::default()
.codex_home(codex_home.to_path_buf())
.harness_overrides(overrides)
.build()
.await?)
}
#[tokio::test]
async fn edit_updates_existing_secret() -> Result<()> {
let codex_home = tempfile::tempdir().context("temp codex home")?;
let cwd = tempfile::tempdir().context("temp cwd")?;
let config = test_config(codex_home.path(), cwd.path()).await?;
let keyring = Arc::new(MockKeyringStore::default());
let manager = SecretsManager::new_with_keyring_store(
config.codex_home.clone(),
config.secrets_backend,
keyring,
);
let scope = SecretScope::Global;
let name = SecretName::new("TEST_SECRET")?;
manager.set(&scope, &name, "before")?;
run_edit(
&config,
&manager,
SecretsEditArgs {
name: "TEST_SECRET".to_string(),
value: Some("after".to_string()),
scope: SecretsScopeArgs {
global: true,
environment_id: None,
},
},
)?;
assert_eq!(manager.get(&scope, &name)?, Some("after".to_string()));
Ok(())
}
#[tokio::test]
async fn edit_missing_secret_errors() -> Result<()> {
let codex_home = tempfile::tempdir().context("temp codex home")?;
let cwd = tempfile::tempdir().context("temp cwd")?;
let config = test_config(codex_home.path(), cwd.path()).await?;
let keyring = Arc::new(MockKeyringStore::default());
let manager = SecretsManager::new_with_keyring_store(
config.codex_home.clone(),
config.secrets_backend,
keyring,
);
let err = run_edit(
&config,
&manager,
SecretsEditArgs {
name: "TEST_SECRET".to_string(),
value: Some("after".to_string()),
scope: SecretsScopeArgs {
global: true,
environment_id: None,
},
},
)
.expect_err("edit should fail when secret is missing");
let message = err.to_string();
assert!(message.contains("No secret named TEST_SECRET found in global."));
assert!(message.contains("codex secrets set TEST_SECRET"));
Ok(())
}
}

View File

@@ -17,7 +17,6 @@ path = "src/bin/config_schema.rs"
workspace = true
[dependencies]
age = { workspace = true }
anyhow = { workspace = true }
arc-swap = "1.8.0"
async-channel = { workspace = true }
@@ -38,6 +37,7 @@ codex-keyring-store = { workspace = true }
codex-otel = { workspace = true }
codex-protocol = { workspace = true }
codex-rmcp-client = { workspace = true }
codex-secrets = { workspace = true }
codex-state = { workspace = true }
codex-utils-absolute-path = { workspace = true }
codex-utils-pty = { workspace = true }

View File

@@ -155,7 +155,6 @@ use crate::rollout::RolloutRecorder;
use crate::rollout::RolloutRecorderParams;
use crate::rollout::map_session_init_error;
use crate::rollout::metadata;
use crate::secrets::resolve_skill_env_dependencies;
use crate::shell;
use crate::shell_snapshot::ShellSnapshot;
use crate::skills::SkillError;
@@ -1936,41 +1935,6 @@ impl Session {
state.record_mcp_dependency_prompted(names);
}
pub(crate) async fn skill_secret_prompted(&self) -> HashSet<String> {
let state = self.state.lock().await;
state.skill_secret_prompted()
}
pub(crate) async fn record_skill_secret_prompted<I>(&self, keys: I)
where
I: IntoIterator<Item = String>,
{
let mut state = self.state.lock().await;
state.record_skill_secret_prompted(keys);
}
pub(crate) async fn set_skill_env_overrides(
&self,
sub_id: String,
overrides: HashMap<String, String>,
) {
let mut state = self.state.lock().await;
state.set_skill_env_overrides(sub_id, overrides);
}
pub(crate) async fn skill_env_overrides(
&self,
sub_id: &str,
) -> Option<HashMap<String, String>> {
let state = self.state.lock().await;
state.skill_env_overrides(sub_id)
}
pub(crate) async fn clear_skill_env_overrides(&self, sub_id: &str) {
let mut state = self.state.lock().await;
state.clear_skill_env_overrides(sub_id);
}
pub(crate) async fn set_server_reasoning_included(&self, included: bool) {
let mut state = self.state.lock().await;
state.set_server_reasoning_included(included);
@@ -3214,20 +3178,6 @@ pub(crate) async fn run_turn(
)
.await;
let skill_secrets = resolve_skill_env_dependencies(
sess.as_ref(),
turn_context.as_ref(),
&cancellation_token,
&mentioned_skills,
)
.await;
sess.set_skill_env_overrides(turn_context.sub_id.clone(), skill_secrets.overrides)
.await;
for message in skill_secrets.warnings {
sess.send_event(&turn_context, EventMsg::Warning(WarningEvent { message }))
.await;
}
let otel_manager = turn_context.client.get_otel_manager();
let SkillInjections {
items: skill_items,

View File

@@ -236,6 +236,7 @@ fn candidate_filenames<'a>(config: &'a Config) -> Vec<&'a str> {
mod tests {
use super::*;
use crate::config::ConfigBuilder;
use crate::config::ConfigOverrides;
use crate::skills::load_skills;
use std::fs;
use std::path::PathBuf;
@@ -248,13 +249,16 @@ mod tests {
/// been configured.
async fn make_config(root: &TempDir, limit: usize, instructions: Option<&str>) -> Config {
let codex_home = TempDir::new().unwrap();
let overrides = ConfigOverrides {
cwd: Some(root.path().to_path_buf()),
..Default::default()
};
let mut config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.harness_overrides(overrides)
.build()
.await
.expect("defaults for test should always succeed");
config.cwd = root.path().to_path_buf();
config.project_doc_max_bytes = limit;
config.user_instructions = instructions.map(ToOwned::to_owned);

View File

@@ -1,211 +1,7 @@
use std::fmt;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use anyhow::Result;
use codex_keyring_store::DefaultKeyringStore;
use codex_keyring_store::KeyringStore;
use schemars::JsonSchema;
use serde::Deserialize;
use serde::Serialize;
use sha2::Digest;
use sha2::Sha256;
use crate::git_info::get_git_repo_root;
mod local;
mod skill_dependencies;
pub use local::LocalSecretsBackend;
pub(crate) use skill_dependencies::resolve_skill_env_dependencies;
const KEYRING_SERVICE: &str = "codex";
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct SecretName(String);
impl SecretName {
pub fn new(raw: &str) -> Result<Self> {
let trimmed = raw.trim();
anyhow::ensure!(!trimmed.is_empty(), "secret name must not be empty");
anyhow::ensure!(
trimmed
.chars()
.all(|ch| ch.is_ascii_uppercase() || ch.is_ascii_digit() || ch == '_'),
"secret name must contain only A-Z, 0-9, or _"
);
Ok(Self(trimmed.to_string()))
}
pub fn as_str(&self) -> &str {
self.0.as_str()
}
}
impl fmt::Display for SecretName {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.0)
}
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum SecretScope {
Global,
Environment(String),
}
impl SecretScope {
pub fn environment(environment_id: impl Into<String>) -> Result<Self> {
let env_id = environment_id.into();
let trimmed = env_id.trim();
anyhow::ensure!(!trimmed.is_empty(), "environment id must not be empty");
Ok(Self::Environment(trimmed.to_string()))
}
pub fn canonical_key(&self, name: &SecretName) -> String {
match self {
Self::Global => format!("global/{}", name.as_str()),
Self::Environment(environment_id) => {
format!("env/{environment_id}/{}", name.as_str())
}
}
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SecretListEntry {
pub scope: SecretScope,
pub name: SecretName,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "lowercase")]
pub enum SecretsBackendKind {
Local,
}
impl Default for SecretsBackendKind {
fn default() -> Self {
Self::Local
}
}
#[derive(Debug, Clone)]
pub struct SecretsManager {
backend: Arc<LocalSecretsBackend>,
}
impl SecretsManager {
pub fn new(codex_home: PathBuf, backend_kind: SecretsBackendKind) -> Self {
let keyring_store: Arc<dyn KeyringStore> = Arc::new(DefaultKeyringStore);
Self::new_with_keyring_store(codex_home, backend_kind, keyring_store)
}
pub fn new_with_keyring_store(
codex_home: PathBuf,
backend_kind: SecretsBackendKind,
keyring_store: Arc<dyn KeyringStore>,
) -> Self {
match backend_kind {
SecretsBackendKind::Local => Self {
backend: Arc::new(LocalSecretsBackend::new(codex_home, keyring_store)),
},
}
}
pub fn set(&self, scope: &SecretScope, name: &SecretName, value: &str) -> Result<()> {
self.backend.set(scope, name, value)
}
pub fn get(&self, scope: &SecretScope, name: &SecretName) -> Result<Option<String>> {
self.backend.get(scope, name)
}
pub fn delete(&self, scope: &SecretScope, name: &SecretName) -> Result<bool> {
self.backend.delete(scope, name)
}
pub fn list(&self, scope_filter: Option<&SecretScope>) -> Result<Vec<SecretListEntry>> {
self.backend.list(scope_filter)
}
}
pub fn environment_id_from_cwd(cwd: &Path) -> String {
if let Some(repo_root) = get_git_repo_root(cwd)
&& let Some(name) = repo_root.file_name()
{
let name = name.to_string_lossy().trim().to_string();
if !name.is_empty() {
return name;
}
}
let canonical = cwd
.canonicalize()
.unwrap_or_else(|_| cwd.to_path_buf())
.to_string_lossy()
.into_owned();
let mut hasher = Sha256::new();
hasher.update(canonical.as_bytes());
let digest = hasher.finalize();
let hex = format!("{digest:x}");
let short = hex.get(..12).unwrap_or(hex.as_str());
format!("cwd-{short}")
}
pub(crate) fn compute_keyring_account(codex_home: &Path) -> String {
let canonical = codex_home
.canonicalize()
.unwrap_or_else(|_| codex_home.to_path_buf())
.to_string_lossy()
.into_owned();
let mut hasher = Sha256::new();
hasher.update(canonical.as_bytes());
let digest = hasher.finalize();
let hex = format!("{digest:x}");
let short = hex.get(..16).unwrap_or(hex.as_str());
format!("secrets|{short}")
}
pub(crate) fn keyring_service() -> &'static str {
KEYRING_SERVICE
}
#[cfg(test)]
mod tests {
use super::*;
use codex_keyring_store::tests::MockKeyringStore;
use pretty_assertions::assert_eq;
#[test]
fn environment_id_fallback_has_cwd_prefix() {
let dir = tempfile::tempdir().expect("tempdir");
let env_id = environment_id_from_cwd(dir.path());
assert!(env_id.starts_with("cwd-"));
}
#[test]
fn manager_round_trips_local_backend() -> Result<()> {
let codex_home = tempfile::tempdir().expect("tempdir");
let keyring = Arc::new(MockKeyringStore::default());
let manager = SecretsManager::new_with_keyring_store(
codex_home.path().to_path_buf(),
SecretsBackendKind::Local,
keyring,
);
let scope = SecretScope::Global;
let name = SecretName::new("GITHUB_TOKEN")?;
manager.set(&scope, &name, "token-1")?;
assert_eq!(manager.get(&scope, &name)?, Some("token-1".to_string()));
let listed = manager.list(None)?;
assert_eq!(listed.len(), 1);
assert_eq!(listed[0].name, name);
assert!(manager.delete(&scope, &name)?);
assert_eq!(manager.get(&scope, &name)?, None);
Ok(())
}
}
pub use codex_secrets::LocalSecretsBackend;
pub use codex_secrets::SecretListEntry;
pub use codex_secrets::SecretName;
pub use codex_secrets::SecretScope;
pub use codex_secrets::SecretsBackendKind;
pub use codex_secrets::SecretsManager;
pub use codex_secrets::environment_id_from_cwd;

View File

@@ -1,382 +0,0 @@
use std::collections::BTreeMap;
use std::collections::HashMap;
use codex_protocol::request_user_input::RequestUserInputAnswer;
use codex_protocol::request_user_input::RequestUserInputArgs;
use codex_protocol::request_user_input::RequestUserInputQuestion;
use codex_protocol::request_user_input::RequestUserInputResponse;
use tokio_util::sync::CancellationToken;
use tracing::warn;
use crate::codex::Session;
use crate::codex::TurnContext;
use crate::secrets::SecretName;
use crate::secrets::SecretScope;
use crate::secrets::SecretsManager;
use crate::secrets::environment_id_from_cwd;
use crate::skills::SkillMetadata;
use crate::skills::model::SkillToolDependency;
const SKILL_SECRET_PROMPT_PREFIX: &str = "skill-secret";
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct SkillSecretsOutcome {
pub overrides: HashMap<String, String>,
pub warnings: Vec<String>,
}
#[derive(Debug, Clone)]
struct SecretDependencyContext {
name: SecretName,
skills: Vec<String>,
description: Option<String>,
}
#[derive(Debug, Clone)]
struct MissingSecret {
context: SecretDependencyContext,
canonical_key: String,
}
pub(crate) async fn resolve_skill_env_dependencies(
sess: &Session,
turn_context: &TurnContext,
cancellation_token: &CancellationToken,
mentioned_skills: &[SkillMetadata],
) -> SkillSecretsOutcome {
if mentioned_skills.is_empty() {
return SkillSecretsOutcome::default();
}
let config = turn_context.client.config();
let manager = SecretsManager::new(config.codex_home.clone(), config.secrets_backend);
let env_scope = match SecretScope::environment(environment_id_from_cwd(&turn_context.cwd)) {
Ok(scope) => scope,
Err(err) => {
return SkillSecretsOutcome {
overrides: HashMap::new(),
warnings: vec![format!("failed to resolve environment scope: {err}")],
};
}
};
let global_scope = SecretScope::Global;
let mut outcome = SkillSecretsOutcome::default();
let dependencies = collect_env_var_dependencies(mentioned_skills, &mut outcome.warnings);
if dependencies.is_empty() {
return outcome;
}
let mut missing = Vec::new();
for context in dependencies {
match resolve_secret_value(
&manager,
&env_scope,
&global_scope,
&context,
&mut outcome.warnings,
) {
Some(value) => {
outcome
.overrides
.insert(context.name.as_str().to_string(), value);
}
None => {
missing.push(MissingSecret {
canonical_key: env_scope.canonical_key(&context.name),
context,
});
}
}
}
if missing.is_empty() {
return outcome;
}
let prompted = sess.skill_secret_prompted().await;
let mut already_prompted = Vec::new();
let mut unprompted_missing = Vec::new();
for entry in missing {
if prompted.contains(&entry.canonical_key) {
already_prompted.push(entry);
} else {
unprompted_missing.push(entry);
}
}
for entry in already_prompted {
outcome
.warnings
.push(missing_secret_warning(&entry.context));
}
if unprompted_missing.is_empty() {
return outcome;
}
let responses = prompt_for_missing_secrets(
sess,
turn_context,
cancellation_token,
&env_scope,
&unprompted_missing,
)
.await;
let prompted_keys = unprompted_missing
.iter()
.map(|entry| entry.canonical_key.clone());
sess.record_skill_secret_prompted(prompted_keys).await;
let mut resolved_from_prompt = HashMap::new();
for entry in &unprompted_missing {
let Some(answer) = responses.answers.get(&entry.canonical_key) else {
outcome
.warnings
.push(missing_secret_warning(&entry.context));
continue;
};
let Some(value) = first_non_empty_answer(answer) else {
outcome
.warnings
.push(missing_secret_warning(&entry.context));
continue;
};
if let Err(err) = manager.set(&env_scope, &entry.context.name, &value) {
outcome.warnings.push(format!(
"failed to persist secret {}: {err}",
entry.context.name
));
}
resolved_from_prompt.insert(entry.context.name.as_str().to_string(), value);
}
outcome.overrides.extend(resolved_from_prompt);
outcome
}
fn collect_env_var_dependencies(
mentioned_skills: &[SkillMetadata],
warnings: &mut Vec<String>,
) -> Vec<SecretDependencyContext> {
let mut contexts: BTreeMap<String, SecretDependencyContext> = BTreeMap::new();
for skill in mentioned_skills {
let Some(dependencies) = skill.dependencies.as_ref() else {
continue;
};
for tool in &dependencies.tools {
if !tool.r#type.eq_ignore_ascii_case("env_var") {
continue;
}
add_dependency_context(&mut contexts, skill, tool, warnings);
}
}
contexts
.into_values()
.map(|mut context| {
context.skills.sort();
context.skills.dedup();
context
})
.collect()
}
fn add_dependency_context(
contexts: &mut BTreeMap<String, SecretDependencyContext>,
skill: &SkillMetadata,
tool: &SkillToolDependency,
warnings: &mut Vec<String>,
) {
let name = match SecretName::new(&tool.value) {
Ok(name) => name,
Err(err) => {
warnings.push(format!(
"skill {} declares invalid env_var dependency {}: {err}",
skill.name, tool.value
));
return;
}
};
let key = name.as_str().to_string();
let description = tool.description.clone();
contexts
.entry(key)
.and_modify(|context| {
context.skills.push(skill.name.clone());
if context.description.is_none() {
context.description = description.clone();
}
})
.or_insert_with(|| SecretDependencyContext {
name,
skills: vec![skill.name.clone()],
description,
});
}
fn resolve_secret_value(
manager: &SecretsManager,
env_scope: &SecretScope,
global_scope: &SecretScope,
context: &SecretDependencyContext,
warnings: &mut Vec<String>,
) -> Option<String> {
if let Some(env_value) = read_non_empty_env(context.name.as_str()) {
return Some(env_value);
}
match manager.get(env_scope, &context.name) {
Ok(Some(value)) if !value.trim().is_empty() => return Some(value),
Ok(Some(_)) | Ok(None) => {}
Err(err) => warnings.push(format!(
"failed to read secret {} from env scope: {err}",
context.name
)),
}
match manager.get(global_scope, &context.name) {
Ok(Some(value)) if !value.trim().is_empty() => Some(value),
Ok(Some(_)) | Ok(None) => None,
Err(err) => {
warnings.push(format!(
"failed to read secret {} from global scope: {err}",
context.name
));
None
}
}
}
fn read_non_empty_env(name: &str) -> Option<String> {
match std::env::var(name) {
Ok(value) if !value.trim().is_empty() => Some(value),
Ok(_) => None,
Err(std::env::VarError::NotPresent) => None,
Err(std::env::VarError::NotUnicode(_)) => {
warn!("environment variable {name} contains invalid unicode; treating as missing");
None
}
}
}
async fn prompt_for_missing_secrets(
sess: &Session,
turn_context: &TurnContext,
cancellation_token: &CancellationToken,
env_scope: &SecretScope,
missing: &[MissingSecret],
) -> RequestUserInputResponse {
let questions = missing
.iter()
.map(|entry| build_secret_question(env_scope, entry))
.collect();
let args = RequestUserInputArgs { questions };
let call_id = format!("{SKILL_SECRET_PROMPT_PREFIX}-{}", turn_context.sub_id);
let response_fut = sess.request_user_input(turn_context, call_id, args);
let sub_id = turn_context.sub_id.as_str();
tokio::select! {
biased;
_ = cancellation_token.cancelled() => {
let empty = RequestUserInputResponse { answers: HashMap::new() };
sess.notify_user_input_response(sub_id, empty.clone()).await;
empty
}
response = response_fut => response.unwrap_or_else(|| RequestUserInputResponse { answers: HashMap::new() }),
}
}
fn build_secret_question(
env_scope: &SecretScope,
missing: &MissingSecret,
) -> RequestUserInputQuestion {
let scope_label = match env_scope {
SecretScope::Global => "global".to_string(),
SecretScope::Environment(env_id) => format!("env/{env_id}"),
};
let skills = missing.context.skills.join(", ");
let description = missing
.context
.description
.as_deref()
.map(|text| format!(" {text}"))
.unwrap_or_default();
RequestUserInputQuestion {
id: missing.canonical_key.clone(),
header: format!("Missing secret: {}", missing.context.name),
question: format!(
"Skill(s) {skills} require {}.{description} Enter a value to store for {scope_label}.",
missing.context.name,
),
is_other: false,
options: None,
}
}
fn first_non_empty_answer(answer: &RequestUserInputAnswer) -> Option<String> {
answer
.answers
.iter()
.find(|value| !value.trim().is_empty())
.map(|value| value.to_string())
}
fn missing_secret_warning(context: &SecretDependencyContext) -> String {
let skills = context.skills.join(", ");
format!(
"Required secret {} is missing for skill(s) {skills}. Use `codex secrets set {}` to configure it.",
context.name, context.name
)
}
#[cfg(test)]
mod tests {
use super::*;
use anyhow::Context;
use pretty_assertions::assert_eq;
use std::path::PathBuf;
fn skill_with_env(name: &str, env_var: &str) -> SkillMetadata {
SkillMetadata {
name: name.to_string(),
description: name.to_string(),
short_description: None,
interface: None,
dependencies: Some(crate::skills::model::SkillDependencies {
tools: vec![crate::skills::model::SkillToolDependency {
r#type: "env_var".to_string(),
value: env_var.to_string(),
description: Some("token".to_string()),
transport: None,
command: None,
url: None,
}],
}),
path: PathBuf::from(name),
scope: codex_protocol::protocol::SkillScope::User,
}
}
#[tokio::test]
async fn resolves_from_environment_without_prompting() -> anyhow::Result<()> {
let (session, turn) = crate::codex::make_session_and_context().await;
let home = std::env::var("HOME").context("HOME must be set for tests")?;
let skills = vec![skill_with_env("skill", "HOME")];
let outcome =
resolve_skill_env_dependencies(&session, &turn, &CancellationToken::new(), &skills)
.await;
assert_eq!(outcome.warnings, Vec::<String>::new());
assert_eq!(outcome.overrides.get("HOME"), Some(&home));
Ok(())
}
}

View File

@@ -1,7 +1,6 @@
//! Session-wide mutable state.
use codex_protocol::models::ResponseItem;
use std::collections::HashMap;
use std::collections::HashSet;
use crate::codex::SessionConfiguration;
@@ -18,8 +17,6 @@ pub(crate) struct SessionState {
pub(crate) latest_rate_limits: Option<RateLimitSnapshot>,
pub(crate) server_reasoning_included: bool,
pub(crate) mcp_dependency_prompted: HashSet<String>,
pub(crate) skill_secret_prompted: HashSet<String>,
pub(crate) skill_env_overrides: HashMap<String, HashMap<String, String>>,
/// Whether the session's initial context has been seeded into history.
///
/// TODO(owen): This is a temporary solution to avoid updating a thread's updated_at
@@ -37,8 +34,6 @@ impl SessionState {
latest_rate_limits: None,
server_reasoning_included: false,
mcp_dependency_prompted: HashSet::new(),
skill_secret_prompted: HashSet::new(),
skill_env_overrides: HashMap::new(),
initial_context_seeded: false,
}
}
@@ -117,37 +112,6 @@ impl SessionState {
pub(crate) fn mcp_dependency_prompted(&self) -> HashSet<String> {
self.mcp_dependency_prompted.clone()
}
pub(crate) fn record_skill_secret_prompted<I>(&mut self, keys: I)
where
I: IntoIterator<Item = String>,
{
self.skill_secret_prompted.extend(keys);
}
pub(crate) fn skill_secret_prompted(&self) -> HashSet<String> {
self.skill_secret_prompted.clone()
}
pub(crate) fn set_skill_env_overrides(
&mut self,
sub_id: String,
overrides: HashMap<String, String>,
) {
if overrides.is_empty() {
self.skill_env_overrides.remove(&sub_id);
return;
}
self.skill_env_overrides.insert(sub_id, overrides);
}
pub(crate) fn skill_env_overrides(&self, sub_id: &str) -> Option<HashMap<String, String>> {
self.skill_env_overrides.get(sub_id).cloned()
}
pub(crate) fn clear_skill_env_overrides(&mut self, sub_id: &str) {
self.skill_env_overrides.remove(sub_id);
}
}
// Sometimes new snapshots don't include credits or plan information.

View File

@@ -191,7 +191,6 @@ impl Session {
false
};
drop(active);
self.clear_skill_env_overrides(&turn_context.sub_id).await;
if should_close_processes {
self.close_unified_exec_processes().await;
}

View File

@@ -101,10 +101,7 @@ impl SessionTask for UserShellCommandTask {
)
.await;
let mut env = create_env(&turn_context.shell_environment_policy);
if let Some(overrides) = session.skill_env_overrides(&turn_context.sub_id).await {
env.extend(overrides);
}
let env = create_env(&turn_context.shell_environment_policy);
let exec_env = ExecEnv {
command: exec_command.clone(),
cwd: cwd.clone(),

View File

@@ -216,7 +216,7 @@ impl ShellHandler {
async fn run_exec_like(args: RunExecLikeArgs) -> Result<ToolOutput, FunctionCallError> {
let RunExecLikeArgs {
tool_name,
mut exec_params,
exec_params,
prefix_rule,
session,
turn,
@@ -233,10 +233,6 @@ impl ShellHandler {
None
};
if let Some(overrides) = session.skill_env_overrides(&turn.sub_id).await {
exec_params.env.extend(overrides);
}
// Approval policy guard for explicit escalation in non-OnRequest modes.
if exec_params
.sandbox_permissions

View File

@@ -483,14 +483,7 @@ impl UnifiedExecProcessManager {
cwd: PathBuf,
context: &UnifiedExecContext,
) -> Result<UnifiedExecProcess, UnifiedExecError> {
let mut env = apply_unified_exec_env(create_env(&context.turn.shell_environment_policy));
if let Some(overrides) = context
.session
.skill_env_overrides(&context.turn.sub_id)
.await
{
env.extend(overrides);
}
let env = apply_unified_exec_env(create_env(&context.turn.shell_environment_policy));
let features = context.session.features();
let mut orchestrator = ToolOrchestrator::new();
let mut runtime = UnifiedExecRuntime::new(self);

View File

@@ -0,0 +1,24 @@
[package]
name = "codex-secrets"
version.workspace = true
edition.workspace = true
license.workspace = true
[lints]
workspace = true
[dependencies]
age = { workspace = true }
anyhow = { workspace = true }
base64 = { workspace = true }
codex-keyring-store = { workspace = true }
rand = { workspace = true }
schemars = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
sha2 = { workspace = true }
tracing = { workspace = true }
[dev-dependencies]
pretty_assertions = { workspace = true }
tempfile = { workspace = true }

223
codex-rs/secrets/src/lib.rs Normal file
View File

@@ -0,0 +1,223 @@
use std::fmt;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use anyhow::Result;
use codex_keyring_store::DefaultKeyringStore;
use codex_keyring_store::KeyringStore;
use schemars::JsonSchema;
use serde::Deserialize;
use serde::Serialize;
use sha2::Digest;
use sha2::Sha256;
mod local;
pub use local::LocalSecretsBackend;
const KEYRING_SERVICE: &str = "codex";
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct SecretName(String);
impl SecretName {
pub fn new(raw: &str) -> Result<Self> {
let trimmed = raw.trim();
anyhow::ensure!(!trimmed.is_empty(), "secret name must not be empty");
anyhow::ensure!(
trimmed
.chars()
.all(|ch| ch.is_ascii_uppercase() || ch.is_ascii_digit() || ch == '_'),
"secret name must contain only A-Z, 0-9, or _"
);
Ok(Self(trimmed.to_string()))
}
pub fn as_str(&self) -> &str {
self.0.as_str()
}
}
impl fmt::Display for SecretName {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.0)
}
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum SecretScope {
Global,
Environment(String),
}
impl SecretScope {
pub fn environment(environment_id: impl Into<String>) -> Result<Self> {
let env_id = environment_id.into();
let trimmed = env_id.trim();
anyhow::ensure!(!trimmed.is_empty(), "environment id must not be empty");
Ok(Self::Environment(trimmed.to_string()))
}
pub fn canonical_key(&self, name: &SecretName) -> String {
match self {
Self::Global => format!("global/{}", name.as_str()),
Self::Environment(environment_id) => {
format!("env/{environment_id}/{}", name.as_str())
}
}
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SecretListEntry {
pub scope: SecretScope,
pub name: SecretName,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "lowercase")]
pub enum SecretsBackendKind {
Local,
}
impl Default for SecretsBackendKind {
fn default() -> Self {
Self::Local
}
}
#[derive(Debug, Clone)]
pub struct SecretsManager {
backend: Arc<LocalSecretsBackend>,
}
impl SecretsManager {
pub fn new(codex_home: PathBuf, backend_kind: SecretsBackendKind) -> Self {
let keyring_store: Arc<dyn KeyringStore> = Arc::new(DefaultKeyringStore);
Self::new_with_keyring_store(codex_home, backend_kind, keyring_store)
}
pub fn new_with_keyring_store(
codex_home: PathBuf,
backend_kind: SecretsBackendKind,
keyring_store: Arc<dyn KeyringStore>,
) -> Self {
match backend_kind {
SecretsBackendKind::Local => Self {
backend: Arc::new(LocalSecretsBackend::new(codex_home, keyring_store)),
},
}
}
pub fn set(&self, scope: &SecretScope, name: &SecretName, value: &str) -> Result<()> {
self.backend.set(scope, name, value)
}
pub fn get(&self, scope: &SecretScope, name: &SecretName) -> Result<Option<String>> {
self.backend.get(scope, name)
}
pub fn delete(&self, scope: &SecretScope, name: &SecretName) -> Result<bool> {
self.backend.delete(scope, name)
}
pub fn list(&self, scope_filter: Option<&SecretScope>) -> Result<Vec<SecretListEntry>> {
self.backend.list(scope_filter)
}
}
pub fn environment_id_from_cwd(cwd: &Path) -> String {
if let Some(repo_root) = get_git_repo_root(cwd)
&& let Some(name) = repo_root.file_name()
{
let name = name.to_string_lossy().trim().to_string();
if !name.is_empty() {
return name;
}
}
let canonical = cwd
.canonicalize()
.unwrap_or_else(|_| cwd.to_path_buf())
.to_string_lossy()
.into_owned();
let mut hasher = Sha256::new();
hasher.update(canonical.as_bytes());
let digest = hasher.finalize();
let hex = format!("{digest:x}");
let short = hex.get(..12).unwrap_or(hex.as_str());
format!("cwd-{short}")
}
fn get_git_repo_root(base_dir: &Path) -> Option<PathBuf> {
let mut dir = base_dir.to_path_buf();
loop {
if dir.join(".git").exists() {
return Some(dir);
}
if !dir.pop() {
break;
}
}
None
}
pub(crate) fn compute_keyring_account(codex_home: &Path) -> String {
let canonical = codex_home
.canonicalize()
.unwrap_or_else(|_| codex_home.to_path_buf())
.to_string_lossy()
.into_owned();
let mut hasher = Sha256::new();
hasher.update(canonical.as_bytes());
let digest = hasher.finalize();
let hex = format!("{digest:x}");
let short = hex.get(..16).unwrap_or(hex.as_str());
format!("secrets|{short}")
}
pub(crate) fn keyring_service() -> &'static str {
KEYRING_SERVICE
}
#[cfg(test)]
mod tests {
use super::*;
use codex_keyring_store::tests::MockKeyringStore;
use pretty_assertions::assert_eq;
#[test]
fn environment_id_fallback_has_cwd_prefix() {
let dir = tempfile::tempdir().expect("tempdir");
let env_id = environment_id_from_cwd(dir.path());
assert!(env_id.starts_with("cwd-"));
}
#[test]
fn manager_round_trips_local_backend() -> Result<()> {
let codex_home = tempfile::tempdir().expect("tempdir");
let keyring = Arc::new(MockKeyringStore::default());
let manager = SecretsManager::new_with_keyring_store(
codex_home.path().to_path_buf(),
SecretsBackendKind::Local,
keyring,
);
let scope = SecretScope::Global;
let name = SecretName::new("GITHUB_TOKEN")?;
manager.set(&scope, &name, "token-1")?;
assert_eq!(manager.get(&scope, &name)?, Some("token-1".to_string()));
let listed = manager.list(None)?;
assert_eq!(listed.len(), 1);
assert_eq!(listed[0].name, name);
assert!(manager.delete(&scope, &name)?);
assert_eq!(manager.get(&scope, &name)?, None);
Ok(())
}
}

View File

@@ -205,10 +205,8 @@ fn parse_canonical_key(canonical_key: &str) -> Option<SecretListEntry> {
return None;
}
let name = SecretName::new(name).ok()?;
Some(SecretListEntry {
scope: SecretScope::Environment(environment_id.to_string()),
name,
})
let scope = SecretScope::environment(environment_id.to_string()).ok()?;
Some(SecretListEntry { scope, name })
}
_ => None,
}

View File

@@ -671,6 +671,10 @@ impl ChatComposer {
self.footer_hint_override = items;
}
pub(crate) fn set_mask_input(&mut self, mask_input: bool) {
self.textarea.set_mask_input(mask_input);
}
#[cfg(test)]
pub(crate) fn show_footer_flash(&mut self, line: Line<'static>, duration: Duration) {
let expires_at = Instant::now()

View File

@@ -32,6 +32,7 @@ use crate::render::renderable::Renderable;
use codex_core::protocol::Op;
use codex_protocol::request_user_input::RequestUserInputAnswer;
use codex_protocol::request_user_input::RequestUserInputEvent;
use codex_protocol::request_user_input::RequestUserInputQuestion;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_protocol::user_input::TextElement;
use unicode_width::UnicodeWidthStr;
@@ -375,6 +376,10 @@ impl RequestUserInputOverlay {
}
fn restore_current_draft(&mut self) {
let mask_input = self
.current_question()
.is_some_and(Self::is_secret_question);
self.composer.set_mask_input(mask_input);
self.composer
.set_placeholder_text(self.notes_placeholder().to_string());
self.composer.set_footer_hint_override(Some(Vec::new()));
@@ -401,6 +406,18 @@ impl RequestUserInputOverlay {
}
}
fn is_secret_question(question: &RequestUserInputQuestion) -> bool {
let has_options = question
.options
.as_ref()
.is_some_and(|options| !options.is_empty());
if has_options {
return false;
}
let id = question.id.as_str();
id.starts_with("global/") || id.starts_with("env/")
}
fn sync_composer_placeholder(&mut self) {
self.composer
.set_placeholder_text(self.notes_placeholder().to_string());
@@ -708,9 +725,14 @@ impl RequestUserInputOverlay {
};
let selected_label = selected_idx
.and_then(|selected_idx| Self::option_label_for_index(question, selected_idx));
let has_options = options.is_some_and(|opts| !opts.is_empty());
let mut answer_list = selected_label.into_iter().collect::<Vec<_>>();
if !notes.is_empty() {
answer_list.push(format!("user_note: {notes}"));
if has_options {
answer_list.push(format!("user_note: {notes}"));
} else {
answer_list.push(notes);
}
}
answers.insert(
question.id.clone(),
@@ -1976,6 +1998,61 @@ mod tests {
assert_eq!(answer.answers, Vec::<String>::new());
}
#[test]
fn freeform_submission_does_not_prefix_user_note() {
let (tx, mut rx) = test_sender();
let mut overlay = RequestUserInputOverlay::new(
request_event("turn-1", vec![question_without_options("q1", "Secret")]),
tx,
true,
false,
false,
);
overlay
.composer
.set_text_content("super-secret".to_string(), Vec::new(), Vec::new());
overlay.composer.move_cursor_to_end();
let draft = overlay.capture_composer_draft();
if let Some(answer) = overlay.current_answer_mut() {
answer.draft = draft;
answer.answer_committed = true;
}
overlay.submit_answers();
let event = rx.try_recv().expect("expected AppEvent");
let AppEvent::CodexOp(Op::UserInputAnswer { response, .. }) = event else {
panic!("expected UserInputAnswer");
};
let answer = response.answers.get("q1").expect("answer missing");
assert_eq!(answer.answers, vec!["super-secret".to_string()]);
}
#[test]
fn secret_questions_mask_rendered_input() {
let (tx, _rx) = test_sender();
let mut overlay = RequestUserInputOverlay::new(
request_event(
"turn-1",
vec![question_without_options("global/TEST_SECRET", "Secret")],
),
tx,
true,
false,
false,
);
overlay
.composer
.set_text_content("super-secret".to_string(), Vec::new(), Vec::new());
overlay.composer.move_cursor_to_end();
let snapshot = render_snapshot(&overlay, Rect::new(0, 0, 80, 12));
assert!(!snapshot.contains("super-secret"));
assert!(snapshot.contains("************"));
}
#[test]
fn notes_are_captured_for_selected_option() {
let (tx, mut rx) = test_sender();

View File

@@ -36,6 +36,7 @@ pub(crate) struct TextArea {
preferred_col: Option<usize>,
elements: Vec<TextElement>,
kill_buffer: String,
mask_input: bool,
}
#[derive(Debug, Clone)]
@@ -59,6 +60,7 @@ impl TextArea {
preferred_col: None,
elements: Vec::new(),
kill_buffer: String::new(),
mask_input: false,
}
}
@@ -72,6 +74,10 @@ impl TextArea {
self.set_text_inner(text, Some(elements));
}
pub fn set_mask_input(&mut self, mask_input: bool) {
self.mask_input = mask_input;
}
fn set_text_inner(&mut self, text: &str, elements: Option<&[UserTextElement]>) {
// Stage 1: replace the raw text and keep the cursor in a safe byte range.
self.text = text.to_string();
@@ -1157,8 +1163,15 @@ impl TextArea {
let r = &lines[idx];
let y = area.y + row as u16;
let line_range = r.start..r.end - 1;
let line_slice = &self.text[line_range.clone()];
if self.mask_input {
let masked_len = line_slice.graphemes(true).count();
let masked = "*".repeat(masked_len);
buf.set_string(area.x, y, masked, Style::default());
continue;
}
// Draw base line with default style.
buf.set_string(area.x, y, &self.text[line_range.clone()], Style::default());
buf.set_string(area.x, y, line_slice, Style::default());
// Overlay styled segments for elements that intersect this line.
for elem in &self.elements {