From 06febed533c00f8c922d6f02b23f01cb3be772e0 Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Thu, 30 Jul 2026 00:35:31 +0200 Subject: [PATCH 1/4] feat(validate): require Codex workspace trust --- README.md | 5 +- examples/native/README.md | 4 +- examples/native/agent-codex.kdl | 2 +- src/compile_agent.rs | 43 ++++- src/validate.rs | 271 ++++++++++++++++++++++++++++++++ tests/compile_agent.rs | 101 ++++++++++++ tests/validate.rs | 222 ++++++++++++++++++++++++++ 7 files changed, 638 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 71401109..f212f8e5 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,8 @@ ${EDITOR:-vi} "$CATALOG/agents///agent.kdl" ``` Replace ``, ``, ``, and ``. Add every file referenced by -`copy` under `$CATALOG/_templates`. +`copy` under `$CATALOG/_templates`. The Codex declaration repeats the exact decoded `` +bytes in its command-local `projects` trust table; keep both values byte-identical. The compact declaration shape is: @@ -110,7 +111,7 @@ agent "" { // role "worker" // supervisor "" env { ST_AGENT "." } - command #"exec codex --dangerously-bypass-approvals-and-sandbox --dangerously-bypass-hook-trust ''"# + command #"exec codex -c 'projects={""={trust_level="trusted"}}' --dangerously-bypass-approvals-and-sandbox --dangerously-bypass-hook-trust ''"# ding render { diff --git a/examples/native/README.md b/examples/native/README.md index eb467910..2b2d0fba 100644 --- a/examples/native/README.md +++ b/examples/native/README.md @@ -13,7 +13,9 @@ The examples use ``, ``, and `` placeholders. st2 pro machine-specific install paths. Copy the appropriate file into `/agents///agent.kdl`, replace every placeholder, and add the referenced catalog-owned templates. `role` is optional metadata; `supervisor` is optional runtime routing. -Uncomment them when the seat has an assigned role or reports to another bus identity. +Uncomment them when the seat has an assigned role or reports to another bus identity. In the Codex +declaration, replacing `` in both places keeps its command-local project trust key +byte-identical to the declared workspace. ## Lifecycle diff --git a/examples/native/agent-codex.kdl b/examples/native/agent-codex.kdl index 4432824e..57bd9a59 100644 --- a/examples/native/agent-codex.kdl +++ b/examples/native/agent-codex.kdl @@ -14,7 +14,7 @@ agent "" { // st2 owns the hook declaration and installed scripts for this unattended seat. The rendered bus // contract requires agent-declared status: busy while executing work, available only when // yielding/ready, and dnd only for an explicit hold. - command #"exec codex --dangerously-bypass-approvals-and-sandbox --dangerously-bypass-hook-trust ''"# + command #"exec codex -c 'projects={""={trust_level="trusted"}}' --dangerously-bypass-approvals-and-sandbox --dangerously-bypass-hook-trust ''"# ding render { diff --git a/src/compile_agent.rs b/src/compile_agent.rs index 5644024c..ccfcee83 100644 --- a/src/compile_agent.rs +++ b/src/compile_agent.rs @@ -46,11 +46,23 @@ impl AgentInput { "exec claude --permission-mode bypassPermissions{model}{extra} {}", shell_single_quote(boot) )), - "codex" => Ok(format!( - "exec codex --dangerously-bypass-approvals-and-sandbox \ - --dangerously-bypass-hook-trust{model}{extra} {}", - shell_single_quote(boot) - )), + "codex" => { + let project_trust = shell_single_quote(&codex_project_trust(&self.workspace)); + let command = format!( + "exec codex -c {project_trust} \ + --dangerously-bypass-approvals-and-sandbox \ + --dangerously-bypass-hook-trust{model}{extra} {}", + shell_single_quote(boot) + ); + if let Some(reason) = + crate::validate::codex_project_trust_error(&command, &self.workspace) + { + anyhow::bail!( + "compile-agent: generated Codex command has invalid project trust: {reason}" + ); + } + Ok(command) + } other => anyhow::bail!( "compile-agent: harness '{other}' is not supported (expected claude or codex)" ), @@ -58,6 +70,19 @@ impl AgentInput { } } +/// One command-local Codex config override. TOML's table serializer owns key escaping so the +/// decoded key is byte-identical to the declared workspace even when it contains quotes or slashes. +fn codex_project_trust(workspace: &str) -> String { + let mut project = toml::Table::new(); + project.insert( + "trust_level".to_string(), + toml::Value::String("trusted".to_string()), + ); + let mut projects = toml::Table::new(); + projects.insert(workspace.to_string(), toml::Value::Table(project)); + format!("projects={}", toml::Value::Table(projects)) +} + fn shell_single_quote(value: &str) -> String { format!("'{}'", value.replace('\'', "'\\''")) } @@ -67,7 +92,13 @@ fn kdl_str(value: &str) -> String { } fn kdl_raw(value: &str) -> String { - format!("#\"{value}\"#") + for count in 1.. { + let hashes = "#".repeat(count); + if !value.contains(&format!("\"{hashes}")) { + return format!("{hashes}\"{value}\"{hashes}"); + } + } + unreachable!("a finite string always has an unused raw-string delimiter") } fn kdl_multiline_raw(value: &str) -> String { diff --git a/src/validate.rs b/src/validate.rs index 6862432e..359d3797 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -252,6 +252,28 @@ fn validate_scoped(root: &Path, this_host: Option<&str>) -> Report { )); } + // Codex decides workspace trust from the selected account's command-line configuration. + // Keep this structural and fleet-wide: a remote agent's declaration can drift just as + // readily as a local one's, and no host filesystem fact is needed to compare decoded bytes. + if !s.retired + && let Some(workspace) = s.workspace.as_deref() + { + for task in &s.tasks { + if task.name == "agent" + && let Some(command) = task.command.as_deref() + && crate::hooks::command_invokes_codex(command) + && let Some(reason) = codex_project_trust_error(command, workspace) + { + issues.push(Issue::error( + "codex-project-trust", + rp.clone(), + ag.clone(), + format!("Codex task 'agent' project trust is invalid: {reason}"), + )); + } + } + } + // Path fields must be absolute or $CATALOG-rooted, and must exist. for (field, raw) in path_fields(s) { if let Some(issue) = check_path(root, &rp, &ag, &field, &raw, runs_on_selected_host) { @@ -368,6 +390,255 @@ fn check_path( None } +/// Require one self-contained Codex `projects` override for the declared workspace. +/// +/// The command is still an opaque `sh -c` line to the runner. Validation only tokenizes the first +/// simple command well enough to inspect Codex's actual `-c`/`--config` arguments: it never executes +/// or expands anything, and stops at shell control operators. Quotes are removed because Codex sees +/// their decoded contents. TOML then decodes the project key for a byte-for-byte comparison with the +/// already-decoded workspace declaration. +pub(crate) fn codex_project_trust_error(command: &str, workspace: &str) -> Option { + let words = match first_simple_command_words(command) { + Ok(words) => words, + Err(reason) => return Some(reason.to_string()), + }; + let overrides = match codex_config_overrides(&words) { + Ok(overrides) => overrides, + Err(reason) => return Some(reason.to_string()), + }; + + let mut projects = Vec::new(); + for config in &overrides { + match projects_override(config, workspace) { + ProjectsOverride::Unrelated => {} + relevant => projects.push(relevant), + } + } + + match projects.as_slice() { + [] => Some(format!( + "missing a -c/--config projects override for workspace '{workspace}'" + )), + [ProjectsOverride::Valid] => None, + [ProjectsOverride::Invalid(reason)] => Some(reason.clone()), + [_] => unreachable!("unrelated overrides are not collected"), + many => Some(format!( + "found {} projects config assignments; expected exactly one", + many.len() + )), + } +} + +/// Decode shell words from the positively classified Codex command, stopping before a second shell +/// command. This deliberately implements only quoting/escaping boundaries, not expansion or general +/// shell evaluation. +fn first_simple_command_words(command: &str) -> Result, &'static str> { + let command = command.trim(); + let command = command + .strip_prefix("exec ") + .unwrap_or(command) + .trim_start(); + let Some(program_end) = command.find(char::is_whitespace) else { + return Ok(Vec::new()); + }; + shell_words(&command[program_end..]) +} + +fn shell_words(input: &str) -> Result, &'static str> { + #[derive(Clone, Copy)] + enum Quote { + Unquoted, + Single, + Double, + } + + let mut words = Vec::new(); + let mut word = String::new(); + let mut started = false; + let mut quote = Quote::Unquoted; + let mut chars = input.chars(); + + while let Some(ch) = chars.next() { + match quote { + Quote::Single => { + if ch == '\'' { + quote = Quote::Unquoted; + } else { + word.push(ch); + } + } + Quote::Double => match ch { + '"' => quote = Quote::Unquoted, + '\\' => { + let Some(escaped) = chars.next() else { + return Err("command ends with an incomplete escape"); + }; + if escaped != '\n' { + word.push(escaped); + } + } + _ => word.push(ch), + }, + Quote::Unquoted => match ch { + '\'' => { + quote = Quote::Single; + started = true; + } + '"' => { + quote = Quote::Double; + started = true; + } + '\\' => { + let Some(escaped) = chars.next() else { + return Err("command ends with an incomplete escape"); + }; + if escaped != '\n' { + word.push(escaped); + started = true; + } + } + '\n' | '\r' => { + if started { + words.push(word); + } + return Ok(words); + } + c if c.is_whitespace() => { + if started { + words.push(std::mem::take(&mut word)); + started = false; + } + } + // The classifier only recognizes a direct Codex invocation. Do not inspect a later + // pipeline/command or mistake its prose for Codex arguments. + ';' | '|' | '&' | '<' | '>' | '(' | ')' => { + if started { + words.push(word); + } + return Ok(words); + } + '#' if !started => return Ok(words), + _ => { + word.push(ch); + started = true; + } + }, + } + } + + match quote { + Quote::Unquoted => { + if started { + words.push(word); + } + Ok(words) + } + Quote::Single | Quote::Double => Err("command has an unclosed shell quote"), + } +} + +/// Collect the four config spellings accepted by Codex/Clap: +/// `-c value`, `-cvalue`, `--config value`, and `--config=value`. +fn codex_config_overrides(words: &[String]) -> Result, &'static str> { + let mut overrides = Vec::new(); + let mut i = 0; + while i < words.len() { + let word = &words[i]; + if word == "--" { + break; + } + if word == "-c" || word == "--config" { + let Some(value) = words.get(i + 1) else { + return Err("-c/--config is missing its key=value argument"); + }; + overrides.push(value.clone()); + i += 2; + continue; + } + if let Some(value) = word.strip_prefix("--config=") { + if value.is_empty() { + return Err("--config= is missing its key=value argument"); + } + overrides.push(value.to_string()); + } else if let Some(value) = word.strip_prefix("-c") + && !word.starts_with("--") + && !value.is_empty() + { + overrides.push(value.to_string()); + } + i += 1; + } + Ok(overrides) +} + +enum ProjectsOverride { + Unrelated, + Valid, + Invalid(String), +} + +fn projects_override(config: &str, workspace: &str) -> ProjectsOverride { + let parsed = config.parse::(); + let targets_projects = parsed + .as_ref() + .is_ok_and(|table| table.contains_key("projects")) + || toml_lhs_targets_projects(config); + if !targets_projects { + return ProjectsOverride::Unrelated; + } + + let table = match parsed { + Ok(table) => table, + Err(error) => { + return ProjectsOverride::Invalid(format!( + "projects config is not valid TOML: {error}" + )); + } + }; + if table.len() != 1 { + return ProjectsOverride::Invalid( + "projects config must contain exactly one top-level assignment".to_string(), + ); + } + let Some(projects) = table.get("projects").and_then(toml::Value::as_table) else { + return ProjectsOverride::Invalid("projects config must be a table".to_string()); + }; + if projects.len() != 1 { + return ProjectsOverride::Invalid(format!( + "projects table has {} keys; expected exactly one", + projects.len() + )); + } + let (project, value) = projects.iter().next().expect("length checked"); + if project.as_bytes() != workspace.as_bytes() { + return ProjectsOverride::Invalid(format!( + "project key '{project}' does not byte-match workspace '{workspace}'" + )); + } + let Some(project) = value.as_table() else { + return ProjectsOverride::Invalid( + "the workspace project value must be a table".to_string(), + ); + }; + if project.get("trust_level").and_then(toml::Value::as_str) != Some("trusted") { + return ProjectsOverride::Invalid( + "the workspace project must set trust_level exactly to \"trusted\"".to_string(), + ); + } + ProjectsOverride::Valid +} + +/// Recognize a malformed assignment whose TOML key is nevertheless `projects`, so it cannot be +/// ignored beside an otherwise-valid override. Appending a dummy value lets TOML decode quoted and +/// dotted keys without interpreting the original value. +fn toml_lhs_targets_projects(config: &str) -> bool { + let lhs = config.split_once('=').map_or(config, |(lhs, _)| lhs).trim(); + let probe = format!("{lhs}=0"); + probe + .parse::() + .is_ok_and(|table| table.contains_key("projects")) +} + /// Catch runner-significant KDL shapes that the permissive lowerer cannot accept silently. TOML/JSON /// tasks are keyed maps and cannot be nameless; `schedule` is a reserved future KDL surface. fn kdl_shape_check(root: &Path, path: &Path) -> Vec { diff --git a/tests/compile_agent.rs b/tests/compile_agent.rs index 45e5b1af..b9c6e85c 100644 --- a/tests/compile_agent.rs +++ b/tests/compile_agent.rs @@ -2,6 +2,7 @@ //! declarations, leaves workspaces untouched until materialization, and has no removed CLI aliases. use std::fs; +use std::os::unix::fs::PermissionsExt; use std::process::Command; use st2::discover; @@ -243,6 +244,106 @@ fn compile_agent_generates_codex_then_materializes_composed_agents_md() { assert!(kdl.contains("json-upsert \".codex/hooks.json\"")); } +#[test] +fn compile_agent_codex_trust_roundtrips_workspace_bytes_through_toml_shell_and_kdl() { + let tmp = tempfile::tempdir().unwrap(); + let catalog = tmp.path().join("catalog"); + let workspace = tmp + .path() + .join("workspace with 'single \"# double and \\backslash"); + let hooks_root = tmp.path().join("hooks"); + let persona = tmp.path().join("worker.md"); + fs::create_dir_all(&workspace).unwrap(); + fs::write(&persona, "# Worker\n").unwrap(); + + let output = Command::new(env!("CARGO_BIN_EXE_st2")) + .arg("compile-agent") + .arg(&catalog) + .args([ + "--role", + "worker", + "--identity", + "worker", + "--host", + "h", + "--harness", + "codex", + ]) + .arg("--dir") + .arg(&workspace) + .arg("--persona") + .arg(&persona) + .env("ST_HOOKS", &hooks_root) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + + let found = discover(&catalog); + assert!(found.errors.is_empty(), "{:?}", found.errors); + let spec = found + .specs + .iter() + .find(|spec| spec.identity == "worker") + .unwrap(); + let workspace_text = workspace.to_str().unwrap(); + assert_eq!(spec.workspace.as_deref(), Some(workspace_text)); + let command = spec + .tasks + .iter() + .find(|task| task.name == "agent") + .and_then(|task| task.command.as_deref()) + .unwrap(); + let report = st2::validate::validate(&catalog); + assert_eq!( + report.errors(), + 0, + "generated output must validate: {:?}", + report.issues + ); + + // Exercise the emitted shell line against a real shell and capture the exact argv Codex gets. + let bin = tmp.path().join("bin"); + fs::create_dir_all(&bin).unwrap(); + let fake_codex = bin.join("codex"); + fs::write(&fake_codex, "#!/bin/sh\nprintf '%s\\0' \"$@\"\n").unwrap(); + fs::set_permissions(&fake_codex, fs::Permissions::from_mode(0o755)).unwrap(); + let shell_bin = std::env::split_paths(&std::env::var_os("PATH").unwrap()) + .map(|dir| dir.join("sh")) + .find(|path| path.is_file()) + .expect("sh is available on PATH"); + let shell = Command::new(shell_bin) + .args(["-c", command]) + .env("PATH", &bin) + .output() + .unwrap(); + assert!( + shell.status.success(), + "{}", + String::from_utf8_lossy(&shell.stderr) + ); + let args: Vec<&[u8]> = shell + .stdout + .split(|byte| *byte == 0) + .filter(|arg| !arg.is_empty()) + .collect(); + assert_eq!(args.first().copied(), Some(b"-c".as_slice())); + let config = std::str::from_utf8(args.get(1).unwrap()).unwrap(); + let config = config.parse::().unwrap(); + let projects = config["projects"].as_table().unwrap(); + let (key, project) = projects.iter().next().unwrap(); + assert_eq!(projects.len(), 1); + assert_eq!(key.as_bytes(), workspace_text.as_bytes()); + assert_eq!( + project["trust_level"].as_str(), + Some("trusted"), + "argv: {args:?}" + ); +} + #[test] fn removed_generator_aliases_are_unknown_commands() { let tmp = tempfile::tempdir().unwrap(); diff --git a/tests/validate.rs b/tests/validate.rs index 0e0783ac..42737c09 100644 --- a/tests/validate.rs +++ b/tests/validate.rs @@ -2,6 +2,8 @@ //! it hit the spec. Each test builds a minimal catalog exercising one failure mode and asserts the //! exact issue code + severity; a clean catalog (and our shipped `examples/`) must validate spotless. +use std::path::Path; + use st2::validate::{Report, Severity, validate, validate_for_host}; /// Write a set of `(relative-path, body)` files into a fresh temp catalog. @@ -19,6 +21,25 @@ fn has(r: &Report, code: &str, sev: Severity) -> bool { r.issues.iter().any(|i| i.code == code && i.severity == sev) } +fn service(workspace: Option<&str>, command: &str, retired: bool, env: &str) -> String { + let workspace = workspace + .map(|path| format!("workspace {path:?};")) + .unwrap_or_default(); + let retired = if retired { "retired true;" } else { "" }; + format!( + r#"agent "w" {{ + host "hetz" + {workspace} + {retired} + pty "agent" {{ command {command:?}; env {{ {env} }} }} +}}"# + ) +} + +fn projects_config(workspace: &str, trust: &str, extra: &str) -> String { + format!("projects={{{workspace:?}={{trust_level={trust:?}{extra}}}}}") +} + // ---- clean cases ----------------------------------------------------------------------------- #[test] @@ -159,6 +180,189 @@ fn a_catalog_rooted_path_that_exists_is_clean() { assert!(!has(&validate(c.path()), "bad-path", Severity::Error)); } +#[test] +fn codex_project_trust_accepts_every_supported_config_argument_form() { + let workspace = tempfile::tempdir().unwrap(); + let workspace = workspace.path().to_str().unwrap(); + let config = projects_config(workspace, "trusted", ", note=\"allowed\""); + let commands = [ + format!("exec codex -c '{config}'"), + format!("exec codex -c'{config}'"), + format!("exec codex --config '{config}'"), + format!("exec codex --config='{config}'"), + ]; + + for command in commands { + let declaration = service(Some(workspace), &command, false, ""); + let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); + let report = validate(c.path()); + assert!( + !has(&report, "codex-project-trust", Severity::Error), + "command {command:?} should be valid: {:?}", + report.issues + ); + } +} + +#[test] +fn active_codex_requires_project_trust_even_with_an_explicit_codex_home() { + let workspace = tempfile::tempdir().unwrap(); + let workspace = workspace.path().to_str().unwrap(); + let declaration = service( + Some(workspace), + "exec codex --model gpt-5", + false, + r#"CODEX_HOME "/selected/account""#, + ); + let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); + assert!(has( + &validate(c.path()), + "codex-project-trust", + Severity::Error + )); +} + +#[test] +fn codex_project_trust_is_structural_and_remains_fleet_wide_under_host_scope() { + let declaration = service( + Some("/workspace/declared/on/another/host"), + "exec codex", + false, + "", + ); + let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); + let report = validate_for_host(c.path(), "Silber"); + assert!(has( + &report, + "codex-project-trust", + Severity::Error + )); + assert_eq!( + report.warnings(), + 0, + "remote filesystem presence must remain host-scoped: {:?}", + report.issues + ); +} + +#[test] +fn codex_project_key_is_compared_without_normalizing_either_path() { + let workspace = tempfile::tempdir().unwrap(); + let workspace = workspace.path().to_str().unwrap(); + let lookalike = format!( + "{workspace}/../{}", + Path::new(workspace).file_name().unwrap().to_string_lossy() + ); + let config = projects_config(&lookalike, "trusted", ""); + let declaration = service( + Some(workspace), + &format!("exec codex -c '{config}'"), + false, + "", + ); + let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); + assert!(has( + &validate(c.path()), + "codex-project-trust", + Severity::Error + )); +} + +#[test] +fn codex_projects_table_must_have_exactly_one_workspace_key() { + let workspace = tempfile::tempdir().unwrap(); + let workspace = workspace.path().to_str().unwrap(); + let cases = [ + "projects={}".to_string(), + format!( + "projects={{{workspace:?}={{trust_level=\"trusted\"}}, \"/other\"={{trust_level=\"trusted\"}}}}" + ), + ]; + for config in cases { + let declaration = service( + Some(workspace), + &format!("exec codex -c '{config}'"), + false, + "", + ); + let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); + assert!(has( + &validate(c.path()), + "codex-project-trust", + Severity::Error + )); + } +} + +#[test] +fn codex_project_trust_fails_closed_on_value_toml_and_duplicate_errors() { + let workspace = tempfile::tempdir().unwrap(); + let workspace = workspace.path().to_str().unwrap(); + let untrusted = projects_config(workspace, "untrusted", ""); + let malformed = "projects={"; + let valid = projects_config(workspace, "trusted", ""); + let cases = [ + format!("exec codex -c '{untrusted}'"), + format!("exec codex -c '{malformed}'"), + format!("exec codex -c '{valid}' --config '{valid}'"), + "exec codex -c 'projects={".to_string(), + ]; + for command in cases { + let declaration = service(Some(workspace), &command, false, ""); + let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); + assert!( + has(&validate(c.path()), "codex-project-trust", Severity::Error), + "command {command:?} should fail closed" + ); + } +} + +#[test] +fn codex_only_reads_projects_from_real_config_arguments() { + let workspace = tempfile::tempdir().unwrap(); + let workspace = workspace.path().to_str().unwrap(); + let config = projects_config(workspace, "trusted", ""); + let cases = [ + format!("exec codex '{config}'"), + format!("exec codex -- --config '{config}'"), + format!("exec codex -c 'model=\"projects={{}}\"' ; echo '{config}'"), + format!("exec codex\nprintf '%s' --config '{config}'"), + ]; + for command in cases { + let declaration = service(Some(workspace), &command, false, ""); + let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); + assert!( + has(&validate(c.path()), "codex-project-trust", Severity::Error), + "incidental projects text in {command:?} must not count" + ); + } +} + +#[test] +fn codex_project_trust_ignores_retired_non_codex_and_workspace_less_agents() { + let workspace = tempfile::tempdir().unwrap(); + let workspace = workspace.path().to_str().unwrap(); + let declarations = [ + service(Some(workspace), "exec codex", true, ""), + service(Some(workspace), "exec claude", false, ""), + service(None, "exec codex", false, ""), + format!( + r#"agent "w" {{ + host "hetz" + workspace {workspace:?} + pty "helper" {{ command "exec codex" }} +}}"# + ), + ]; + for declaration in declarations { + let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); + assert!( + !has(&validate(c.path()), "codex-project-trust", Severity::Error), + "control should be unaffected" + ); + } +} + #[test] fn a_duplicate_bus_id_is_an_error() { let c = catalog(&[ @@ -414,6 +618,24 @@ fn cli_json_is_well_formed() { assert_eq!(v["issues"][0]["severity"], "error"); } +#[test] +fn cli_json_exposes_the_stable_codex_project_trust_code() { + let workspace = tempfile::tempdir().unwrap(); + let declaration = service( + Some(workspace.path().to_str().unwrap()), + "exec codex", + false, + "", + ); + let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); + let out = run_validate(&[c.path().as_os_str(), std::ffi::OsStr::new("--json")]); + assert!(!out.status.success()); + let v: serde_json::Value = serde_json::from_slice(&out.stdout).expect("valid JSON"); + assert_eq!(v["errors"], 1); + assert_eq!(v["issues"][0]["code"], "codex-project-trust"); + assert_eq!(v["issues"][0]["severity"], "error"); +} + #[test] fn a_hand_authored_native_catalog_validates_without_errors() { let workspace = tempfile::tempdir().unwrap(); From db12c4814d2a578cddd17f0470c78f80a12c62ef Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Thu, 30 Jul 2026 00:45:55 +0200 Subject: [PATCH 2/4] fix(validate): match POSIX double-quote escapes --- examples/native/README.md | 2 +- src/validate.rs | 48 +++++++++++++++++++++++++++++++++++++-- tests/validate.rs | 6 +---- 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/examples/native/README.md b/examples/native/README.md index 2b2d0fba..3f05e5bc 100644 --- a/examples/native/README.md +++ b/examples/native/README.md @@ -13,7 +13,7 @@ The examples use ``, ``, and `` placeholders. st2 pro machine-specific install paths. Copy the appropriate file into `/agents///agent.kdl`, replace every placeholder, and add the referenced catalog-owned templates. `role` is optional metadata; `supervisor` is optional runtime routing. -Uncomment them when the seat has an assigned role or reports to another bus identity. In the Codex +Uncomment them when the agent has an assigned role or reports to another bus identity. In the Codex declaration, replacing `` in both places keeps its command-local project trust key byte-identical to the declared workspace. diff --git a/src/validate.rs b/src/validate.rs index 359d3797..03f90825 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -473,8 +473,13 @@ fn shell_words(input: &str) -> Result, &'static str> { let Some(escaped) = chars.next() else { return Err("command ends with an incomplete escape"); }; - if escaped != '\n' { - word.push(escaped); + match escaped { + '\n' => {} + '$' | '`' | '"' | '\\' => word.push(escaped), + _ => { + word.push('\\'); + word.push(escaped); + } } } _ => word.push(ch), @@ -729,3 +734,42 @@ fn overlay_lint(rp: &str, ag: &Option, s: &AgentSpec) -> Vec { } out } + +#[cfg(test)] +mod tests { + use super::*; + use std::process::Command; + + #[test] + fn double_quote_decoding_matches_a_real_posix_shell() { + let source = r#""special:\$:\`:\":\\:" "non-special:\q:\a" -c "projects={\"/tmp/a\\\\b\"={trust_level=\"trusted\"}}""#; + let decoded = shell_words(source).unwrap(); + + let script = format!("set -- {source}; printf '%s\\0' \"$@\""); + let output = Command::new("sh") + .args(["-c", &script]) + .output() + .expect("sh is available"); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let actual: Vec = output + .stdout + .split(|byte| *byte == 0) + .filter(|word| !word.is_empty()) + .map(|word| String::from_utf8(word.to_vec()).unwrap()) + .collect(); + assert_eq!(decoded, actual); + assert_eq!(decoded[0], "special:$:`:\":\\:"); + assert_eq!(decoded[1], r"non-special:\q:\a"); + + let command = format!("exec codex {source}"); + assert_eq!( + codex_project_trust_error(&command, "/tmp/a\\b"), + None, + "the shell-decoded TOML key must match the declared workspace" + ); + } +} diff --git a/tests/validate.rs b/tests/validate.rs index 42737c09..b9c05549 100644 --- a/tests/validate.rs +++ b/tests/validate.rs @@ -232,11 +232,7 @@ fn codex_project_trust_is_structural_and_remains_fleet_wide_under_host_scope() { ); let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); let report = validate_for_host(c.path(), "Silber"); - assert!(has( - &report, - "codex-project-trust", - Severity::Error - )); + assert!(has(&report, "codex-project-trust", Severity::Error)); assert_eq!( report.warnings(), 0, From 89d924a1d1eb3e359926fed913b00a6ae2c53d37 Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Thu, 30 Jul 2026 09:49:45 +0200 Subject: [PATCH 3/4] refactor(validate): keep commands opaque --- README.md | 3 +- examples/native/README.md | 3 +- examples/native/agent-codex.kdl | 1 + src/compile_agent.rs | 12 +- src/validate.rs | 315 -------------------------------- tests/validate.rs | 218 ---------------------- 6 files changed, 8 insertions(+), 544 deletions(-) diff --git a/README.md b/README.md index f212f8e5..a65b19fe 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,8 @@ ${EDITOR:-vi} "$CATALOG/agents///agent.kdl" Replace ``, ``, ``, and ``. Add every file referenced by `copy` under `$CATALOG/_templates`. The Codex declaration repeats the exact decoded `` -bytes in its command-local `projects` trust table; keep both values byte-identical. +bytes in its command-local `projects` trust table; keep both values byte-identical. This is a +harness launch convention inside the opaque command, not agent-spec grammar enforced by st2. The compact declaration shape is: diff --git a/examples/native/README.md b/examples/native/README.md index 3f05e5bc..dafc1694 100644 --- a/examples/native/README.md +++ b/examples/native/README.md @@ -15,7 +15,8 @@ machine-specific install paths. Copy the appropriate file into catalog-owned templates. `role` is optional metadata; `supervisor` is optional runtime routing. Uncomment them when the agent has an assigned role or reports to another bus identity. In the Codex declaration, replacing `` in both places keeps its command-local project trust key -byte-identical to the declared workspace. +byte-identical to the declared workspace. st2 treats that command as opaque; the trust flag is a +Codex launch convention, not part of generic catalog validation. ## Lifecycle diff --git a/examples/native/agent-codex.kdl b/examples/native/agent-codex.kdl index 57bd9a59..9b3b4259 100644 --- a/examples/native/agent-codex.kdl +++ b/examples/native/agent-codex.kdl @@ -14,6 +14,7 @@ agent "" { // st2 owns the hook declaration and installed scripts for this unattended seat. The rendered bus // contract requires agent-declared status: busy while executing work, available only when // yielding/ready, and dnd only for an explicit hold. + // Codex-specific launch policy stays inside this opaque command; st2 does not parse its flags. command #"exec codex -c 'projects={""={trust_level="trusted"}}' --dangerously-bypass-approvals-and-sandbox --dangerously-bypass-hook-trust ''"# ding diff --git a/src/compile_agent.rs b/src/compile_agent.rs index ccfcee83..1911aa04 100644 --- a/src/compile_agent.rs +++ b/src/compile_agent.rs @@ -54,13 +54,6 @@ impl AgentInput { --dangerously-bypass-hook-trust{model}{extra} {}", shell_single_quote(boot) ); - if let Some(reason) = - crate::validate::codex_project_trust_error(&command, &self.workspace) - { - anyhow::bail!( - "compile-agent: generated Codex command has invalid project trust: {reason}" - ); - } Ok(command) } other => anyhow::bail!( @@ -70,8 +63,9 @@ impl AgentInput { } } -/// One command-local Codex config override. TOML's table serializer owns key escaping so the -/// decoded key is byte-identical to the declared workspace even when it contains quotes or slashes. +/// One generator-owned, command-local Codex config override. TOML's table serializer owns key +/// escaping so the decoded key is byte-identical to the declared workspace even when it contains +/// quotes or slashes. Generic catalog validation continues to treat the resulting command as opaque. fn codex_project_trust(workspace: &str) -> String { let mut project = toml::Table::new(); project.insert( diff --git a/src/validate.rs b/src/validate.rs index 03f90825..6862432e 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -252,28 +252,6 @@ fn validate_scoped(root: &Path, this_host: Option<&str>) -> Report { )); } - // Codex decides workspace trust from the selected account's command-line configuration. - // Keep this structural and fleet-wide: a remote agent's declaration can drift just as - // readily as a local one's, and no host filesystem fact is needed to compare decoded bytes. - if !s.retired - && let Some(workspace) = s.workspace.as_deref() - { - for task in &s.tasks { - if task.name == "agent" - && let Some(command) = task.command.as_deref() - && crate::hooks::command_invokes_codex(command) - && let Some(reason) = codex_project_trust_error(command, workspace) - { - issues.push(Issue::error( - "codex-project-trust", - rp.clone(), - ag.clone(), - format!("Codex task 'agent' project trust is invalid: {reason}"), - )); - } - } - } - // Path fields must be absolute or $CATALOG-rooted, and must exist. for (field, raw) in path_fields(s) { if let Some(issue) = check_path(root, &rp, &ag, &field, &raw, runs_on_selected_host) { @@ -390,260 +368,6 @@ fn check_path( None } -/// Require one self-contained Codex `projects` override for the declared workspace. -/// -/// The command is still an opaque `sh -c` line to the runner. Validation only tokenizes the first -/// simple command well enough to inspect Codex's actual `-c`/`--config` arguments: it never executes -/// or expands anything, and stops at shell control operators. Quotes are removed because Codex sees -/// their decoded contents. TOML then decodes the project key for a byte-for-byte comparison with the -/// already-decoded workspace declaration. -pub(crate) fn codex_project_trust_error(command: &str, workspace: &str) -> Option { - let words = match first_simple_command_words(command) { - Ok(words) => words, - Err(reason) => return Some(reason.to_string()), - }; - let overrides = match codex_config_overrides(&words) { - Ok(overrides) => overrides, - Err(reason) => return Some(reason.to_string()), - }; - - let mut projects = Vec::new(); - for config in &overrides { - match projects_override(config, workspace) { - ProjectsOverride::Unrelated => {} - relevant => projects.push(relevant), - } - } - - match projects.as_slice() { - [] => Some(format!( - "missing a -c/--config projects override for workspace '{workspace}'" - )), - [ProjectsOverride::Valid] => None, - [ProjectsOverride::Invalid(reason)] => Some(reason.clone()), - [_] => unreachable!("unrelated overrides are not collected"), - many => Some(format!( - "found {} projects config assignments; expected exactly one", - many.len() - )), - } -} - -/// Decode shell words from the positively classified Codex command, stopping before a second shell -/// command. This deliberately implements only quoting/escaping boundaries, not expansion or general -/// shell evaluation. -fn first_simple_command_words(command: &str) -> Result, &'static str> { - let command = command.trim(); - let command = command - .strip_prefix("exec ") - .unwrap_or(command) - .trim_start(); - let Some(program_end) = command.find(char::is_whitespace) else { - return Ok(Vec::new()); - }; - shell_words(&command[program_end..]) -} - -fn shell_words(input: &str) -> Result, &'static str> { - #[derive(Clone, Copy)] - enum Quote { - Unquoted, - Single, - Double, - } - - let mut words = Vec::new(); - let mut word = String::new(); - let mut started = false; - let mut quote = Quote::Unquoted; - let mut chars = input.chars(); - - while let Some(ch) = chars.next() { - match quote { - Quote::Single => { - if ch == '\'' { - quote = Quote::Unquoted; - } else { - word.push(ch); - } - } - Quote::Double => match ch { - '"' => quote = Quote::Unquoted, - '\\' => { - let Some(escaped) = chars.next() else { - return Err("command ends with an incomplete escape"); - }; - match escaped { - '\n' => {} - '$' | '`' | '"' | '\\' => word.push(escaped), - _ => { - word.push('\\'); - word.push(escaped); - } - } - } - _ => word.push(ch), - }, - Quote::Unquoted => match ch { - '\'' => { - quote = Quote::Single; - started = true; - } - '"' => { - quote = Quote::Double; - started = true; - } - '\\' => { - let Some(escaped) = chars.next() else { - return Err("command ends with an incomplete escape"); - }; - if escaped != '\n' { - word.push(escaped); - started = true; - } - } - '\n' | '\r' => { - if started { - words.push(word); - } - return Ok(words); - } - c if c.is_whitespace() => { - if started { - words.push(std::mem::take(&mut word)); - started = false; - } - } - // The classifier only recognizes a direct Codex invocation. Do not inspect a later - // pipeline/command or mistake its prose for Codex arguments. - ';' | '|' | '&' | '<' | '>' | '(' | ')' => { - if started { - words.push(word); - } - return Ok(words); - } - '#' if !started => return Ok(words), - _ => { - word.push(ch); - started = true; - } - }, - } - } - - match quote { - Quote::Unquoted => { - if started { - words.push(word); - } - Ok(words) - } - Quote::Single | Quote::Double => Err("command has an unclosed shell quote"), - } -} - -/// Collect the four config spellings accepted by Codex/Clap: -/// `-c value`, `-cvalue`, `--config value`, and `--config=value`. -fn codex_config_overrides(words: &[String]) -> Result, &'static str> { - let mut overrides = Vec::new(); - let mut i = 0; - while i < words.len() { - let word = &words[i]; - if word == "--" { - break; - } - if word == "-c" || word == "--config" { - let Some(value) = words.get(i + 1) else { - return Err("-c/--config is missing its key=value argument"); - }; - overrides.push(value.clone()); - i += 2; - continue; - } - if let Some(value) = word.strip_prefix("--config=") { - if value.is_empty() { - return Err("--config= is missing its key=value argument"); - } - overrides.push(value.to_string()); - } else if let Some(value) = word.strip_prefix("-c") - && !word.starts_with("--") - && !value.is_empty() - { - overrides.push(value.to_string()); - } - i += 1; - } - Ok(overrides) -} - -enum ProjectsOverride { - Unrelated, - Valid, - Invalid(String), -} - -fn projects_override(config: &str, workspace: &str) -> ProjectsOverride { - let parsed = config.parse::(); - let targets_projects = parsed - .as_ref() - .is_ok_and(|table| table.contains_key("projects")) - || toml_lhs_targets_projects(config); - if !targets_projects { - return ProjectsOverride::Unrelated; - } - - let table = match parsed { - Ok(table) => table, - Err(error) => { - return ProjectsOverride::Invalid(format!( - "projects config is not valid TOML: {error}" - )); - } - }; - if table.len() != 1 { - return ProjectsOverride::Invalid( - "projects config must contain exactly one top-level assignment".to_string(), - ); - } - let Some(projects) = table.get("projects").and_then(toml::Value::as_table) else { - return ProjectsOverride::Invalid("projects config must be a table".to_string()); - }; - if projects.len() != 1 { - return ProjectsOverride::Invalid(format!( - "projects table has {} keys; expected exactly one", - projects.len() - )); - } - let (project, value) = projects.iter().next().expect("length checked"); - if project.as_bytes() != workspace.as_bytes() { - return ProjectsOverride::Invalid(format!( - "project key '{project}' does not byte-match workspace '{workspace}'" - )); - } - let Some(project) = value.as_table() else { - return ProjectsOverride::Invalid( - "the workspace project value must be a table".to_string(), - ); - }; - if project.get("trust_level").and_then(toml::Value::as_str) != Some("trusted") { - return ProjectsOverride::Invalid( - "the workspace project must set trust_level exactly to \"trusted\"".to_string(), - ); - } - ProjectsOverride::Valid -} - -/// Recognize a malformed assignment whose TOML key is nevertheless `projects`, so it cannot be -/// ignored beside an otherwise-valid override. Appending a dummy value lets TOML decode quoted and -/// dotted keys without interpreting the original value. -fn toml_lhs_targets_projects(config: &str) -> bool { - let lhs = config.split_once('=').map_or(config, |(lhs, _)| lhs).trim(); - let probe = format!("{lhs}=0"); - probe - .parse::() - .is_ok_and(|table| table.contains_key("projects")) -} - /// Catch runner-significant KDL shapes that the permissive lowerer cannot accept silently. TOML/JSON /// tasks are keyed maps and cannot be nameless; `schedule` is a reserved future KDL surface. fn kdl_shape_check(root: &Path, path: &Path) -> Vec { @@ -734,42 +458,3 @@ fn overlay_lint(rp: &str, ag: &Option, s: &AgentSpec) -> Vec { } out } - -#[cfg(test)] -mod tests { - use super::*; - use std::process::Command; - - #[test] - fn double_quote_decoding_matches_a_real_posix_shell() { - let source = r#""special:\$:\`:\":\\:" "non-special:\q:\a" -c "projects={\"/tmp/a\\\\b\"={trust_level=\"trusted\"}}""#; - let decoded = shell_words(source).unwrap(); - - let script = format!("set -- {source}; printf '%s\\0' \"$@\""); - let output = Command::new("sh") - .args(["-c", &script]) - .output() - .expect("sh is available"); - assert!( - output.status.success(), - "{}", - String::from_utf8_lossy(&output.stderr) - ); - let actual: Vec = output - .stdout - .split(|byte| *byte == 0) - .filter(|word| !word.is_empty()) - .map(|word| String::from_utf8(word.to_vec()).unwrap()) - .collect(); - assert_eq!(decoded, actual); - assert_eq!(decoded[0], "special:$:`:\":\\:"); - assert_eq!(decoded[1], r"non-special:\q:\a"); - - let command = format!("exec codex {source}"); - assert_eq!( - codex_project_trust_error(&command, "/tmp/a\\b"), - None, - "the shell-decoded TOML key must match the declared workspace" - ); - } -} diff --git a/tests/validate.rs b/tests/validate.rs index b9c05549..0e0783ac 100644 --- a/tests/validate.rs +++ b/tests/validate.rs @@ -2,8 +2,6 @@ //! it hit the spec. Each test builds a minimal catalog exercising one failure mode and asserts the //! exact issue code + severity; a clean catalog (and our shipped `examples/`) must validate spotless. -use std::path::Path; - use st2::validate::{Report, Severity, validate, validate_for_host}; /// Write a set of `(relative-path, body)` files into a fresh temp catalog. @@ -21,25 +19,6 @@ fn has(r: &Report, code: &str, sev: Severity) -> bool { r.issues.iter().any(|i| i.code == code && i.severity == sev) } -fn service(workspace: Option<&str>, command: &str, retired: bool, env: &str) -> String { - let workspace = workspace - .map(|path| format!("workspace {path:?};")) - .unwrap_or_default(); - let retired = if retired { "retired true;" } else { "" }; - format!( - r#"agent "w" {{ - host "hetz" - {workspace} - {retired} - pty "agent" {{ command {command:?}; env {{ {env} }} }} -}}"# - ) -} - -fn projects_config(workspace: &str, trust: &str, extra: &str) -> String { - format!("projects={{{workspace:?}={{trust_level={trust:?}{extra}}}}}") -} - // ---- clean cases ----------------------------------------------------------------------------- #[test] @@ -180,185 +159,6 @@ fn a_catalog_rooted_path_that_exists_is_clean() { assert!(!has(&validate(c.path()), "bad-path", Severity::Error)); } -#[test] -fn codex_project_trust_accepts_every_supported_config_argument_form() { - let workspace = tempfile::tempdir().unwrap(); - let workspace = workspace.path().to_str().unwrap(); - let config = projects_config(workspace, "trusted", ", note=\"allowed\""); - let commands = [ - format!("exec codex -c '{config}'"), - format!("exec codex -c'{config}'"), - format!("exec codex --config '{config}'"), - format!("exec codex --config='{config}'"), - ]; - - for command in commands { - let declaration = service(Some(workspace), &command, false, ""); - let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); - let report = validate(c.path()); - assert!( - !has(&report, "codex-project-trust", Severity::Error), - "command {command:?} should be valid: {:?}", - report.issues - ); - } -} - -#[test] -fn active_codex_requires_project_trust_even_with_an_explicit_codex_home() { - let workspace = tempfile::tempdir().unwrap(); - let workspace = workspace.path().to_str().unwrap(); - let declaration = service( - Some(workspace), - "exec codex --model gpt-5", - false, - r#"CODEX_HOME "/selected/account""#, - ); - let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); - assert!(has( - &validate(c.path()), - "codex-project-trust", - Severity::Error - )); -} - -#[test] -fn codex_project_trust_is_structural_and_remains_fleet_wide_under_host_scope() { - let declaration = service( - Some("/workspace/declared/on/another/host"), - "exec codex", - false, - "", - ); - let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); - let report = validate_for_host(c.path(), "Silber"); - assert!(has(&report, "codex-project-trust", Severity::Error)); - assert_eq!( - report.warnings(), - 0, - "remote filesystem presence must remain host-scoped: {:?}", - report.issues - ); -} - -#[test] -fn codex_project_key_is_compared_without_normalizing_either_path() { - let workspace = tempfile::tempdir().unwrap(); - let workspace = workspace.path().to_str().unwrap(); - let lookalike = format!( - "{workspace}/../{}", - Path::new(workspace).file_name().unwrap().to_string_lossy() - ); - let config = projects_config(&lookalike, "trusted", ""); - let declaration = service( - Some(workspace), - &format!("exec codex -c '{config}'"), - false, - "", - ); - let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); - assert!(has( - &validate(c.path()), - "codex-project-trust", - Severity::Error - )); -} - -#[test] -fn codex_projects_table_must_have_exactly_one_workspace_key() { - let workspace = tempfile::tempdir().unwrap(); - let workspace = workspace.path().to_str().unwrap(); - let cases = [ - "projects={}".to_string(), - format!( - "projects={{{workspace:?}={{trust_level=\"trusted\"}}, \"/other\"={{trust_level=\"trusted\"}}}}" - ), - ]; - for config in cases { - let declaration = service( - Some(workspace), - &format!("exec codex -c '{config}'"), - false, - "", - ); - let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); - assert!(has( - &validate(c.path()), - "codex-project-trust", - Severity::Error - )); - } -} - -#[test] -fn codex_project_trust_fails_closed_on_value_toml_and_duplicate_errors() { - let workspace = tempfile::tempdir().unwrap(); - let workspace = workspace.path().to_str().unwrap(); - let untrusted = projects_config(workspace, "untrusted", ""); - let malformed = "projects={"; - let valid = projects_config(workspace, "trusted", ""); - let cases = [ - format!("exec codex -c '{untrusted}'"), - format!("exec codex -c '{malformed}'"), - format!("exec codex -c '{valid}' --config '{valid}'"), - "exec codex -c 'projects={".to_string(), - ]; - for command in cases { - let declaration = service(Some(workspace), &command, false, ""); - let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); - assert!( - has(&validate(c.path()), "codex-project-trust", Severity::Error), - "command {command:?} should fail closed" - ); - } -} - -#[test] -fn codex_only_reads_projects_from_real_config_arguments() { - let workspace = tempfile::tempdir().unwrap(); - let workspace = workspace.path().to_str().unwrap(); - let config = projects_config(workspace, "trusted", ""); - let cases = [ - format!("exec codex '{config}'"), - format!("exec codex -- --config '{config}'"), - format!("exec codex -c 'model=\"projects={{}}\"' ; echo '{config}'"), - format!("exec codex\nprintf '%s' --config '{config}'"), - ]; - for command in cases { - let declaration = service(Some(workspace), &command, false, ""); - let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); - assert!( - has(&validate(c.path()), "codex-project-trust", Severity::Error), - "incidental projects text in {command:?} must not count" - ); - } -} - -#[test] -fn codex_project_trust_ignores_retired_non_codex_and_workspace_less_agents() { - let workspace = tempfile::tempdir().unwrap(); - let workspace = workspace.path().to_str().unwrap(); - let declarations = [ - service(Some(workspace), "exec codex", true, ""), - service(Some(workspace), "exec claude", false, ""), - service(None, "exec codex", false, ""), - format!( - r#"agent "w" {{ - host "hetz" - workspace {workspace:?} - pty "helper" {{ command "exec codex" }} -}}"# - ), - ]; - for declaration in declarations { - let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); - assert!( - !has(&validate(c.path()), "codex-project-trust", Severity::Error), - "control should be unaffected" - ); - } -} - #[test] fn a_duplicate_bus_id_is_an_error() { let c = catalog(&[ @@ -614,24 +414,6 @@ fn cli_json_is_well_formed() { assert_eq!(v["issues"][0]["severity"], "error"); } -#[test] -fn cli_json_exposes_the_stable_codex_project_trust_code() { - let workspace = tempfile::tempdir().unwrap(); - let declaration = service( - Some(workspace.path().to_str().unwrap()), - "exec codex", - false, - "", - ); - let c = catalog(&[("hetz/w/agent.kdl", &declaration)]); - let out = run_validate(&[c.path().as_os_str(), std::ffi::OsStr::new("--json")]); - assert!(!out.status.success()); - let v: serde_json::Value = serde_json::from_slice(&out.stdout).expect("valid JSON"); - assert_eq!(v["errors"], 1); - assert_eq!(v["issues"][0]["code"], "codex-project-trust"); - assert_eq!(v["issues"][0]["severity"], "error"); -} - #[test] fn a_hand_authored_native_catalog_validates_without_errors() { let workspace = tempfile::tempdir().unwrap(); From abecebe48abb99df9a87abca18bca74d3d27ec3c Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Fri, 31 Jul 2026 16:08:35 +0200 Subject: [PATCH 4/4] fix(compile-agent): make Codex workspace trust opt-in --- README.md | 10 ++++--- examples/native/README.md | 7 ++--- examples/native/agent-codex.kdl | 4 +-- src/compile_agent.rs | 22 +++++++++++----- src/main.rs | 18 ++++++++++++- tests/compile_agent.rs | 46 ++++++++++++++++++++++++++++++++- 6 files changed, 89 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 5008dd16..48811878 100644 --- a/README.md +++ b/README.md @@ -98,9 +98,11 @@ ${EDITOR:-vi} "$CATALOG/agents///agent.kdl" ``` Replace ``, ``, ``, and ``. Add every file referenced by -`copy` under `$CATALOG/_templates`. The Codex declaration repeats the exact decoded `` -bytes in its argv-local `projects` trust table; keep both values byte-identical. This is a harness -launch convention inside opaque argv, not agent-spec grammar enforced by st2. +`copy` under `$CATALOG/_templates`. The maintained declaration does not add workspace trust. +`compile-agent` also omits trust by default. Pass `--harness codex --trust-workspace` to opt in to an +argv-local Codex `projects` trust override. The generator serializes the declared workspace as the +exact decoded key; other harnesses reject the flag. This is a launch convention inside opaque argv, +not agent-spec grammar enforced by st2. The compact declaration shape is: @@ -113,7 +115,7 @@ agent "" { // role "worker" // supervisor "" env { ST_AGENT "." } - argv "codex" "-c" "projects={\"\"={trust_level=\"trusted\"}}" "--dangerously-bypass-approvals-and-sandbox" "--dangerously-bypass-hook-trust" "" + argv "codex" "--dangerously-bypass-approvals-and-sandbox" "--dangerously-bypass-hook-trust" "" ding render { diff --git a/examples/native/README.md b/examples/native/README.md index 4f3249d1..d07e6608 100644 --- a/examples/native/README.md +++ b/examples/native/README.md @@ -14,9 +14,10 @@ machine-specific install paths. Copy the appropriate file into `/agents///agent.kdl`, replace every placeholder, and add the referenced catalog-owned templates. `role` is optional metadata; `supervisor` is optional runtime routing. Uncomment them when the agent has an assigned role or reports to another bus identity. In the Codex -declaration, replacing `` in both places keeps its argv-local project trust key -byte-identical to the declared workspace. st2 treats that argv as opaque; the trust flag is a -Codex launch convention, not part of generic catalog validation. +declaration, workspace trust is absent by default. The experimental generator adds an argv-local +Codex project trust override only with `compile-agent --harness codex --trust-workspace`. It preserves +the exact decoded workspace bytes as the key. st2 treats that argv as opaque; the trust flag is not +part of generic catalog validation. ## Lifecycle diff --git a/examples/native/agent-codex.kdl b/examples/native/agent-codex.kdl index bf93b299..893013fd 100644 --- a/examples/native/agent-codex.kdl +++ b/examples/native/agent-codex.kdl @@ -14,8 +14,8 @@ agent "" { // st2 owns the hook declaration and installed scripts for this unattended seat. The rendered bus // contract requires agent-declared status: busy while executing work, available only when // yielding/ready, and dnd only for an explicit hold. - // Codex-specific launch policy stays inside this opaque argv; st2 does not parse its flags. - argv "codex" "-c" "projects={\"\"={trust_level=\"trusted\"}}" "--dangerously-bypass-approvals-and-sandbox" "--dangerously-bypass-hook-trust" "" + // Harness-specific launch policy stays inside this opaque argv; st2 does not parse its flags. + argv "codex" "--dangerously-bypass-approvals-and-sandbox" "--dangerously-bypass-hook-trust" "" ding render { diff --git a/src/compile_agent.rs b/src/compile_agent.rs index 396ea69f..9f36373e 100644 --- a/src/compile_agent.rs +++ b/src/compile_agent.rs @@ -14,6 +14,7 @@ pub struct AgentInput { pub host: String, pub role: String, pub harness: String, + pub trust_workspace: bool, pub model: Option, pub workspace: String, pub supervisor: Option, @@ -37,13 +38,17 @@ impl AgentInput { "--permission-mode".to_string(), "bypassPermissions".to_string(), ], - "codex" => vec![ - "codex".to_string(), - "-c".to_string(), - codex_project_trust(&self.workspace), - "--dangerously-bypass-approvals-and-sandbox".to_string(), - "--dangerously-bypass-hook-trust".to_string(), - ], + "codex" => { + let mut argv = vec!["codex".to_string()]; + if self.trust_workspace { + argv.extend(["-c".to_string(), codex_project_trust(&self.workspace)]); + } + argv.extend([ + "--dangerously-bypass-approvals-and-sandbox".to_string(), + "--dangerously-bypass-hook-trust".to_string(), + ]); + argv + } other => anyhow::bail!( "compile-agent: harness '{other}' is not supported (expected claude or codex)" ), @@ -149,6 +154,9 @@ pub fn compile_agent( input.harness ); } + if input.trust_workspace && input.harness != "codex" { + anyhow::bail!("compile-agent: --trust-workspace requires --harness codex"); + } let persona = fs::read_to_string(persona_file).map_err(|error| { anyhow::anyhow!( "reading persona {} for '{}': {error}", diff --git a/src/main.rs b/src/main.rs index 6237892b..ff6c5ecb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -141,6 +141,10 @@ enum Command { persona: PathBuf, #[arg(long, default_value = "claude")] harness: String, + /// Add an argv-local Codex project trust override for the exact workspace. This is opt-in + /// and requires `--harness codex`. + #[arg(long)] + trust_workspace: bool, /// Host (defaults to the local hostname). #[arg(long)] host: Option, @@ -595,6 +599,7 @@ fn main() -> Result<()> { dir, persona, harness, + trust_workspace, host, model, supervisor, @@ -602,7 +607,16 @@ fn main() -> Result<()> { } => { let catalog = catalog_arg(catalog)?; compile_agent_cmd( - &catalog, &identity, &role, &dir, &persona, &harness, host, model, supervisor, + &catalog, + &identity, + &role, + &dir, + &persona, + &harness, + trust_workspace, + host, + model, + supervisor, extra_arg, ) } @@ -663,6 +677,7 @@ fn compile_agent_cmd( dir: &str, persona: &Path, harness: &str, + trust_workspace: bool, host: Option, model: Option, supervisor: Option, @@ -674,6 +689,7 @@ fn compile_agent_cmd( host: host.clone(), role: role.to_string(), harness: harness.to_string(), + trust_workspace, model, workspace: dir.to_string(), supervisor, diff --git a/tests/compile_agent.rs b/tests/compile_agent.rs index 7ff7ba5b..f6f7e68b 100644 --- a/tests/compile_agent.rs +++ b/tests/compile_agent.rs @@ -251,13 +251,15 @@ fn compile_agent_generates_codex_then_materializes_composed_agents_md() { let kdl = fs::read_to_string(catalog.join("agents/h/worker/agent.kdl")).unwrap(); assert!(kdl.contains("argv \"codex\"")); assert!(!kdl.contains("exec codex")); + assert!(!kdl.contains("\"-c\"")); + assert!(!kdl.contains("projects=")); assert!(kdl.contains("set status busy")); assert!(kdl.contains("--dangerously-bypass-hook-trust")); assert!(kdl.contains("json-upsert \".codex/hooks.json\"")); } #[test] -fn compile_agent_codex_trust_roundtrips_workspace_bytes_through_toml_and_kdl() { +fn compile_agent_opt_in_codex_trust_roundtrips_workspace_bytes_through_toml_and_kdl() { let tmp = tempfile::tempdir().unwrap(); let catalog = tmp.path().join("catalog"); let workspace = tmp @@ -280,6 +282,7 @@ fn compile_agent_codex_trust_roundtrips_workspace_bytes_through_toml_and_kdl() { "h", "--harness", "codex", + "--trust-workspace", ]) .arg("--dir") .arg(&workspace) @@ -332,6 +335,47 @@ fn compile_agent_codex_trust_roundtrips_workspace_bytes_through_toml_and_kdl() { ); } +#[test] +fn compile_agent_rejects_workspace_trust_for_non_codex_before_writing() { + let tmp = tempfile::tempdir().unwrap(); + let catalog = tmp.path().join("catalog"); + let workspace = tmp.path().join("workspace"); + let persona = tmp.path().join("worker.md"); + fs::create_dir_all(&workspace).unwrap(); + fs::write(&persona, "# Worker\n").unwrap(); + + let output = Command::new(env!("CARGO_BIN_EXE_st2")) + .arg("compile-agent") + .arg(&catalog) + .args([ + "--identity", + "worker", + "--host", + "h", + "--harness", + "claude", + "--trust-workspace", + ]) + .arg("--dir") + .arg(&workspace) + .arg("--persona") + .arg(&persona) + .output() + .unwrap(); + + assert!(!output.status.success()); + assert!( + String::from_utf8_lossy(&output.stderr) + .contains("--trust-workspace requires --harness codex"), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert!( + !catalog.exists(), + "a rejected trust/harness combination wrote catalog output" + ); +} + #[test] fn removed_generator_aliases_are_unknown_commands() { let tmp = tempfile::tempdir().unwrap();