openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-26 18:38:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: trust-gate project hooks and exec policies

Browse Source
This commit is contained in:
viyatb-oai
2026-03-26 18:06:27 -07:00
parent 81fa04783a
commit e0aaebaf16
9 changed files with 428 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/lib.rs
+11 -16
View File
@@ -275,21 +275,16 @@ fn project_config_warning(config: &Config) -> Option<ConfigWarningNotification>
ConfigLayerStackOrdering::LowestPrecedenceFirst,
/*include_disabled*/ true,
) {
if !matches!(layer.name, ConfigLayerSource::Project { .. })
|| layer.disabled_reason.is_none()
{
let ConfigLayerSource::Project { dot_codex_folder } = &layer.name else {
continue;
}
if let ConfigLayerSource::Project { dot_codex_folder } = &layer.name {
disabled_folders.push((
dot_codex_folder.as_path().display().to_string(),
layer
.disabled_reason
.as_ref()
.map(ToString::to_string)
.unwrap_or_else(|| "config.toml is disabled.".to_string()),
));
}
};
let Some(disabled_reason) = &layer.disabled_reason else {
continue;
};
disabled_folders.push((
dot_codex_folder.as_path().display().to_string(),
disabled_reason.clone(),
));
}
if disabled_folders.is_empty() {
@@ -297,8 +292,8 @@ fn project_config_warning(config: &Config) -> Option<ConfigWarningNotification>
}
let mut message = concat!(
"Project config.toml files are disabled in the following folders. ",
"Settings in those files are ignored, but skills and exec policies still load.\n",
"Project-local config, hooks, and exec policies are disabled in the following folders ",
"until the project is trusted, but skills still load.\n",
)
.to_string();
for (index, (folder, reason)) in disabled_folders.iter().enumerate() {
codex-rs/core/src/codex_tests.rs
+149
View File
@@ -17,6 +17,7 @@ use crate::tools::format_exec_output_str;
use codex_features::Features;
use codex_protocol::ThreadId;
use codex_protocol::config_types::TrustLevel;
use codex_protocol::models::FunctionCallOutputBody;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::permissions::FileSystemAccessMode;
@@ -277,6 +278,75 @@ fn developer_input_texts(items: &[ResponseItem]) -> Vec<&str> {
.collect()
}
fn write_project_hooks(dot_codex: &Path) -> std::io::Result<()> {
std::fs::create_dir_all(dot_codex)?;
std::fs::write(
dot_codex.join("hooks.json"),
r#"{
"hooks": {
"SessionStart": [
{
"hooks": [
{
"type": "command",
"command": "echo hello from hook"
}
]
}
]
}
}"#,
)
}
async fn write_project_trust_config(
codex_home: &Path,
trusted_projects: &[(&Path, TrustLevel)],
) -> std::io::Result<()> {
tokio::fs::write(
codex_home.join(codex_config::CONFIG_TOML_FILE),
toml::to_string(&crate::config::ConfigToml {
projects: Some(
trusted_projects
.iter()
.map(|(project, trust_level)| {
(
project.to_string_lossy().to_string(),
crate::config::ProjectConfig {
trust_level: Some(*trust_level),
},
)
})
.collect::<std::collections::HashMap<_, _>>(),
),
..Default::default()
})
.expect("serialize config"),
)
.await
}
async fn preview_session_start_hooks(
config: &crate::config::Config,
) -> std::io::Result<Vec<codex_protocol::protocol::HookRunSummary>> {
let hooks = Hooks::new(HooksConfig {
feature_enabled: true,
config_layer_stack: Some(config.config_layer_stack.clone()),
..HooksConfig::default()
});
Ok(
hooks.preview_session_start(&codex_hooks::SessionStartRequest {
session_id: ThreadId::new(),
cwd: config.cwd.clone(),
transcript_path: None,
model: "gpt-5".to_string(),
permission_mode: "default".to_string(),
source: codex_hooks::SessionStartSource::Startup,
}),
)
}
fn test_tool_runtime(session: Arc<Session>, turn_context: Arc<TurnContext>) -> ToolCallRuntime {
let router = Arc::new(ToolRouter::from_config(
&turn_context.tools_config,
@@ -5238,3 +5308,82 @@ async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request()
pretty_assertions::assert_eq!(output, expected);
}
#[tokio::test]
async fn session_start_hooks_only_load_from_trusted_project_layers() -> std::io::Result<()> {
let temp = tempfile::tempdir()?;
let codex_home = temp.path().join("home");
let project_root = temp.path().join("project");
let nested = project_root.join("nested");
let root_dot_codex = project_root.join(".codex");
let nested_dot_codex = nested.join(".codex");
std::fs::create_dir_all(&codex_home)?;
std::fs::create_dir_all(&nested_dot_codex)?;
std::fs::write(project_root.join(".git"), "gitdir: here")?;
write_project_hooks(&root_dot_codex)?;
write_project_hooks(&nested_dot_codex)?;
write_project_trust_config(&codex_home, &[(&nested, TrustLevel::Trusted)]).await?;
let config = ConfigBuilder::default()
.codex_home(codex_home)
.fallback_cwd(Some(nested))
.build()
.await?;
let preview = preview_session_start_hooks(&config).await?;
assert_eq!(
preview
.iter()
.map(|run| &run.source_path)
.collect::<Vec<_>>(),
vec![&nested_dot_codex.join("hooks.json")],
);
Ok(())
}
#[tokio::test]
async fn session_start_hooks_require_project_trust_without_config_toml() -> std::io::Result<()> {
let temp = tempfile::tempdir()?;
let project_root = temp.path().join("project");
let nested = project_root.join("nested");
let dot_codex = project_root.join(".codex");
std::fs::create_dir_all(&nested)?;
std::fs::write(project_root.join(".git"), "gitdir: here")?;
write_project_hooks(&dot_codex)?;
let cases = [
("unknown", Vec::<(&Path, TrustLevel)>::new(), 0_usize),
(
"untrusted",
vec![(&project_root as &Path, TrustLevel::Untrusted)],
0_usize,
),
(
"trusted",
vec![(&project_root as &Path, TrustLevel::Trusted)],
1_usize,
),
];
for (name, trust_entries, expected_hooks) in cases {
let codex_home = temp.path().join(format!("home_{name}"));
std::fs::create_dir_all(&codex_home)?;
write_project_trust_config(&codex_home, &trust_entries).await?;
let config = ConfigBuilder::default()
.codex_home(codex_home)
.fallback_cwd(Some(nested.clone()))
.build()
.await?;
assert_eq!(
preview_session_start_hooks(&config).await?.len(),
expected_hooks,
"unexpected hook count for {name}",
);
}
Ok(())
}
codex-rs/core/src/config_loader/mod.rs
+10 -22
View File
@@ -588,37 +588,35 @@ impl ProjectTrustContext {
}
}
fn disabled_reason_for_dir(&self, dir: &AbsolutePathBuf) -> Option<String> {
let decision = self.decision_for_dir(dir);
fn disabled_reason_for_decision(&self, decision: &ProjectTrustDecision) -> Option<String> {
if decision.is_trusted() {
return None;
}
let gated_features = "project-local config, hooks, and exec policies";
let trust_key = decision.trust_key.as_str();
let user_config_file = self.user_config_file.as_path().display();
match decision.trust_level {
Some(TrustLevel::Untrusted) => Some(format!(
"{trust_key} is marked as untrusted in {user_config_file}. To load config.toml, mark it trusted."
"{trust_key} is marked as untrusted in {user_config_file}. To load {gated_features}, mark it trusted."
)),
_ => Some(format!(
"To load config.toml, add {trust_key} as a trusted project in {user_config_file}."
"To load {gated_features}, add {trust_key} as a trusted project in {user_config_file}."
)),
}
}
}
fn project_layer_entry(
trust_context: &ProjectTrustContext,
dot_codex_folder: &AbsolutePathBuf,
layer_dir: &AbsolutePathBuf,
config: TomlValue,
config_toml_exists: bool,
disabled_reason: Option<String>,
) -> ConfigLayerEntry {
let source = ConfigLayerSource::Project {
dot_codex_folder: dot_codex_folder.clone(),
};
if config_toml_exists && let Some(reason) = trust_context.disabled_reason_for_dir(layer_dir) {
if let Some(reason) = disabled_reason {
ConfigLayerEntry::new_disabled(source, config, reason)
} else {
ConfigLayerEntry::new(source, config)
@@ -785,6 +783,7 @@ async fn load_project_layers(
let layer_dir = AbsolutePathBuf::from_absolute_path(dir)?;
let decision = trust_context.decision_for_dir(&layer_dir);
let disabled_reason = trust_context.disabled_reason_for_decision(&decision);
let dot_codex_abs = AbsolutePathBuf::from_absolute_path(&dot_codex)?;
let dot_codex_normalized =
normalize_path(dot_codex_abs.as_path()).unwrap_or_else(|_| dot_codex_abs.to_path_buf());
@@ -807,24 +806,16 @@ async fn load_project_layers(
));
}
layers.push(project_layer_entry(
trust_context,
&dot_codex_abs,
&layer_dir,
TomlValue::Table(toml::map::Map::new()),
/*config_toml_exists*/ true,
disabled_reason.clone(),
));
continue;
}
};
let config =
resolve_relative_paths_in_config_toml(config, dot_codex_abs.as_path())?;
let entry = project_layer_entry(
trust_context,
&dot_codex_abs,
&layer_dir,
config,
/*config_toml_exists*/ true,
);
let entry = project_layer_entry(&dot_codex_abs, config, disabled_reason.clone());
layers.push(entry);
}
Err(err) => {
@@ -833,11 +824,9 @@ async fn load_project_layers(
// for this project layer, as this may still have subfolders
// that are significant in the overall ConfigLayerStack.
layers.push(project_layer_entry(
trust_context,
&dot_codex_abs,
&layer_dir,
TomlValue::Table(toml::map::Map::new()),
/*config_toml_exists*/ false,
disabled_reason,
));
} else {
let config_file_display = config_file.as_path().display();
@@ -852,7 +841,6 @@ async fn load_project_layers(
Ok(layers)
}
/// The legacy mechanism for specifying admin-enforced configuration is to read
/// from a file like `/etc/codex/managed_config.toml` that has the same
/// structure as `config.toml` where fields like `approval_policy` can specify
codex-rs/core/src/config_loader/tests.rs
+58
View File
@@ -1348,6 +1348,64 @@ async fn invalid_project_config_ignored_when_untrusted_or_unknown() -> std::io::
Ok(())
}
#[tokio::test]
async fn project_layer_without_config_toml_is_disabled_when_untrusted_or_unknown()
-> std::io::Result<()> {
let tmp = tempdir()?;
let project_root = tmp.path().join("project");
let nested = project_root.join("child");
tokio::fs::create_dir_all(nested.join(".codex")).await?;
tokio::fs::write(project_root.join(".git"), "gitdir: here").await?;
let cwd = AbsolutePathBuf::from_absolute_path(&nested)?;
let cases = [
("untrusted", Some(TrustLevel::Untrusted), true),
("unknown", None, true),
("trusted", Some(TrustLevel::Trusted), false),
];
for (name, trust_level, expect_disabled) in cases {
let codex_home = tmp.path().join(format!("home_no_config_{name}"));
tokio::fs::create_dir_all(&codex_home).await?;
if let Some(trust_level) = trust_level {
make_config_for_test(&codex_home, &project_root, trust_level, None).await?;
}
let layers = load_config_layers_state(
&codex_home,
Some(cwd.clone()),
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
)
.await?;
let project_layers: Vec<_> = layers
.get_layers(
super::ConfigLayerStackOrdering::HighestPrecedenceFirst,
true,
)
.into_iter()
.filter(|layer| matches!(layer.name, super::ConfigLayerSource::Project { .. }))
.collect();
assert_eq!(
project_layers.len(),
1,
"expected one project layer for {name}"
);
assert_eq!(
project_layers[0].disabled_reason.is_some(),
expect_disabled,
"unexpected disabled state for {name}",
);
assert_eq!(
project_layers[0].config,
TomlValue::Table(toml::map::Map::new())
);
}
Ok(())
}
#[tokio::test]
async fn cli_overrides_with_relative_paths_do_not_break_trust_check() -> std::io::Result<()> {
let tmp = tempdir()?;
codex-rs/core/src/exec_policy.rs
+2
View File
@@ -485,6 +485,8 @@ async fn load_exec_policy_with_warning(
}
pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result<Policy, ExecPolicyError> {
// Disabled project layers already represent the trust decision, so hooks
// and exec-policy loading can reuse the normal trusted-layer view.
// Iterate the layers in increasing order of precedence, adding the *.rules
// from each layer, so that higher-precedence layers can override
// rules defined in lower-precedence ones.
codex-rs/core/src/exec_policy_tests.rs
+184
View File
@@ -11,6 +11,7 @@ use crate::config_loader::RequirementSource;
use crate::config_loader::Sourced;
use codex_app_server_protocol::ConfigLayerSource;
use codex_config::RequirementsExecPolicy;
use codex_protocol::config_types::TrustLevel;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
@@ -68,6 +69,33 @@ fn starlark_string(value: &str) -> String {
value.replace('\\', "\\\\").replace('"', "\\\"")
}
async fn write_project_trust_config(
codex_home: &Path,
trusted_projects: &[(&Path, TrustLevel)],
) -> std::io::Result<()> {
tokio::fs::write(
codex_home.join(codex_config::CONFIG_TOML_FILE),
toml::to_string(&crate::config::ConfigToml {
projects: Some(
trusted_projects
.iter()
.map(|(project, trust_level)| {
(
project.to_string_lossy().to_string(),
crate::config::ProjectConfig {
trust_level: Some(*trust_level),
},
)
})
.collect::<std::collections::HashMap<_, _>>(),
),
..Default::default()
})
.expect("serialize config"),
)
.await
}
fn read_only_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
@@ -1694,3 +1722,159 @@ async fn assert_exec_approval_requirement_for_command(
assert_eq!(requirement, expected_requirement);
}
#[tokio::test]
async fn exec_policies_only_load_from_trusted_project_layers() -> std::io::Result<()> {
let temp = tempfile::tempdir()?;
let codex_home = temp.path().join("home_execpolicy_nested");
let project_root = temp.path().join("project_execpolicy_nested");
let nested = project_root.join("nested");
let root_rules = project_root.join(".codex").join(RULES_DIR_NAME);
let nested_rules = nested.join(".codex").join(RULES_DIR_NAME);
fs::create_dir_all(&codex_home)?;
fs::create_dir_all(&nested_rules)?;
fs::write(project_root.join(".git"), "gitdir: here")?;
fs::create_dir_all(&root_rules)?;
fs::write(
root_rules.join("deny-rm.rules"),
r#"prefix_rule(pattern=["rm"], decision="forbidden")"#,
)?;
fs::write(
nested_rules.join("deny-mv.rules"),
r#"prefix_rule(pattern=["mv"], decision="forbidden")"#,
)?;
write_project_trust_config(&codex_home, &[(&nested, TrustLevel::Trusted)]).await?;
let config = ConfigBuilder::default()
.codex_home(codex_home)
.fallback_cwd(Some(nested))
.build()
.await?;
let policy = load_exec_policy(&config.config_layer_stack)
.await
.map_err(std::io::Error::other)?;
assert_eq!(
policy
.check_multiple([vec!["rm".to_string()]].iter(), &|_| Decision::Allow)
.decision,
Decision::Allow,
);
assert_eq!(
policy
.check_multiple([vec!["mv".to_string()]].iter(), &|_| Decision::Allow)
.decision,
Decision::Forbidden,
);
Ok(())
}
#[tokio::test]
async fn exec_policies_require_project_trust_without_config_toml() -> std::io::Result<()> {
let temp = tempfile::tempdir()?;
let project_root = temp.path().join("project_execpolicy");
let nested = project_root.join("nested");
let rules_dir = project_root.join(".codex").join(RULES_DIR_NAME);
fs::create_dir_all(&nested)?;
fs::write(project_root.join(".git"), "gitdir: here")?;
fs::create_dir_all(&rules_dir)?;
fs::write(
rules_dir.join("deny-rm.rules"),
r#"prefix_rule(pattern=["rm"], decision="forbidden")"#,
)?;
let cases = [
(
"unknown",
Vec::<(&Path, TrustLevel)>::new(),
Decision::Allow,
),
(
"untrusted",
vec![(&project_root as &Path, TrustLevel::Untrusted)],
Decision::Allow,
),
(
"trusted",
vec![(&project_root as &Path, TrustLevel::Trusted)],
Decision::Forbidden,
),
];
for (name, trust_entries, expected_decision) in cases {
let codex_home = temp.path().join(format!("home_execpolicy_{name}"));
fs::create_dir_all(&codex_home)?;
write_project_trust_config(&codex_home, &trust_entries).await?;
let config = ConfigBuilder::default()
.codex_home(codex_home)
.fallback_cwd(Some(nested.clone()))
.build()
.await?;
let policy = load_exec_policy(&config.config_layer_stack)
.await
.map_err(std::io::Error::other)?;
assert_eq!(
policy
.check_multiple([vec!["rm".to_string()]].iter(), &|_| Decision::Allow)
.decision,
expected_decision,
"unexpected execpolicy decision for {name}",
);
}
Ok(())
}
#[tokio::test]
async fn exec_policy_warnings_ignore_untrusted_project_rules_without_config_toml()
-> std::io::Result<()> {
let temp = tempfile::tempdir()?;
let project_root = temp.path().join("project_execpolicy_warning");
let nested = project_root.join("nested");
let rules_dir = project_root.join(".codex").join(RULES_DIR_NAME);
fs::create_dir_all(&nested)?;
fs::write(project_root.join(".git"), "gitdir: here")?;
fs::create_dir_all(&rules_dir)?;
fs::write(rules_dir.join("broken.rules"), "prefix_rule(")?;
let cases = [
("unknown", Vec::<(&Path, TrustLevel)>::new(), false),
(
"untrusted",
vec![(&project_root as &Path, TrustLevel::Untrusted)],
false,
),
(
"trusted",
vec![(&project_root as &Path, TrustLevel::Trusted)],
true,
),
];
for (name, trust_entries, expect_warning) in cases {
let codex_home = temp.path().join(format!("home_execpolicy_warning_{name}"));
fs::create_dir_all(&codex_home)?;
write_project_trust_config(&codex_home, &trust_entries).await?;
let config = ConfigBuilder::default()
.codex_home(codex_home)
.fallback_cwd(Some(nested.clone()))
.build()
.await?;
let warning = check_execpolicy_for_warnings(&config.config_layer_stack)
.await
.map_err(std::io::Error::other)?;
assert_eq!(
matches!(warning, Some(ExecPolicyError::ParsePolicy { .. })),
expect_warning,
"unexpected execpolicy warning state for {name}",
);
}
Ok(())
}
codex-rs/tui/src/app.rs
+5 -9
View File
@@ -439,16 +439,12 @@ fn emit_project_config_warnings(app_event_tx: &AppEventSender, config: &Config)
let ConfigLayerSource::Project { dot_codex_folder } = &layer.name else {
continue;
};
if layer.disabled_reason.is_none() {
let Some(disabled_reason) = &layer.disabled_reason else {
continue;
}
};
disabled_folders.push((
dot_codex_folder.as_path().display().to_string(),
layer
.disabled_reason
.as_ref()
.map(ToString::to_string)
.unwrap_or_else(|| "config.toml is disabled.".to_string()),
disabled_reason.clone(),
));
}
@@ -457,8 +453,8 @@ fn emit_project_config_warnings(app_event_tx: &AppEventSender, config: &Config)
}
let mut message = concat!(
"Project config.toml files are disabled in the following folders. ",
"Settings in those files are ignored, but skills and exec policies still load.\n",
"Project-local config, hooks, and exec policies are disabled in the following folders ",
"until the project is trusted, but skills still load.\n",
)
.to_string();
for (index, (folder, reason)) in disabled_folders.iter().enumerate() {
codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_snapshot_for_git_repo.snap
+3 -2
View File
@@ -1,12 +1,13 @@
---
source: tui/src/onboarding/trust_directory.rs
assertion_line: 218
expression: terminal.backend()
---
> You are in /workspace/project
Do you trust the contents of this directory? Working with untrusted
contents comes with higher risk of prompt injection.
contents comes with higher risk of prompt injection. Trusting the
directory allows project-local config, hooks, and exec policies to
load.
› 1. Yes, continue
2. No, quit
codex-rs/tui/src/onboarding/trust_directory.rs
+6 -3
View File
@@ -53,10 +53,13 @@ impl WidgetRef for &TrustDirectoryWidget {
column.push(
Paragraph::new(
"Do you trust the contents of this directory? Working with untrusted contents comes with higher risk of prompt injection.".to_string(),
"Do you trust the contents of this directory? Working with untrusted \
contents comes with higher risk of prompt injection. Trusting the \
directory allows project-local config, hooks, and exec policies to load."
.to_string(),
)
.wrap(Wrap { trim: true })
.inset(Insets::tlbr(/*top*/ 0, /*left*/ 2, /*bottom*/ 0, /*right*/ 0)),
.wrap(Wrap { trim: true })
.inset(Insets::tlbr(/*top*/ 0, /*left*/ 2, /*bottom*/ 0, /*right*/ 0)),
);
column.push("");