diff --git a/Cargo.toml b/Cargo.toml index 81ec299..1f14ec2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,9 +38,9 @@ clap = { version = "4.6.1", features = ["derive"] } # schemas are embedded (include_str!) and use only internal #/definitions refs, # so no resolver is needed at all. jsonschema = { version = "0.46.5", default-features = false } -# Bash write-mutation patterns (sandbox policy). default-features off drops the -# Unicode tables: the shell-command patterns are ASCII, and ASCII `\b`/`\s` is -# the intended semantics, so `perf` (matching speed) is all we keep beyond `std`. +# Eval validation, transcript checks, dispatch parsing, and staging patterns. +# Default features are disabled to drop Unicode tables: these authored patterns +# use ASCII syntax, so `perf` (matching speed) is all we keep beyond `std`. regex = { version = "1.12.3", default-features = false, features = ["std", "perf"] } serde = { version = "1.0.228", features = ["derive"] } serde_json = { version = "1.0.150", features = ["preserve_order"] } diff --git a/README.md b/README.md index 53868ec..d3a647d 100644 --- a/README.md +++ b/README.md @@ -136,6 +136,9 @@ eval-magic harness show codex - `eval-magic docs isolation` explains how live or installed skill sources can contaminate a comparison and how to verify isolation. Its source is [docs/guides/isolation.md](docs/guides/isolation.md). +- `eval-magic docs guard` explains eval-authored command allowances, packaged defaults, and the + containment checks those allowances cannot bypass. Its source is + [docs/guides/guard.md](docs/guides/guard.md). - [docs/developer_overview.md](docs/developer_overview.md) maps the codebase, sources of truth, verification workflow, and internal documentation. diff --git a/build.rs b/build.rs index 3765cb2..d3316ae 100644 --- a/build.rs +++ b/build.rs @@ -22,6 +22,38 @@ fn main() { let out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap()); fs::write(out_dir.join("guide_topics.rs"), generated) .expect("failed to write generated guide topic table"); + + let profile_dir = manifest_dir.join("guard-profiles"); + println!("cargo:rerun-if-changed={}", profile_dir.display()); + let generated = render_guard_profiles(&profile_dir); + fs::write(out_dir.join("guard_profiles.rs"), generated) + .expect("failed to write generated guard profile table"); +} + +fn render_guard_profiles(profile_dir: &Path) -> String { + let mut profiles: Vec = fs::read_dir(profile_dir) + .unwrap_or_else(|err| panic!("failed to read {}: {err}", profile_dir.display())) + .map(|entry| entry.expect("failed to read guard profile entry").path()) + .filter(|path| path.extension().and_then(|value| value.to_str()) == Some("toml")) + .collect(); + profiles.sort(); + assert!( + !profiles.is_empty(), + "guard-profiles must contain a TOML profile" + ); + + let mut generated = String::from("const PACKAGED_GUARD_PROFILES: &[(&str, &str)] = &[\n"); + for path in profiles { + let name = path + .file_name() + .and_then(|value| value.to_str()) + .unwrap_or_else(|| panic!("guard profile path is not UTF-8: {}", path.display())); + let body = fs::read_to_string(&path) + .unwrap_or_else(|err| panic!("failed to read {}: {err}", path.display())); + generated.push_str(&format!(" ({name:?}, {body:?}),\n")); + } + generated.push_str("];\n"); + generated } fn discover_guides(guide_dir: &Path) -> Vec { diff --git a/docs/claude-notes.md b/docs/claude-notes.md index 3bb0728..42b8ab4 100644 --- a/docs/claude-notes.md +++ b/docs/claude-notes.md @@ -169,3 +169,6 @@ only write boundary a dispatch has, since the session itself runs under `bypassP invokes the hidden `guard` subcommand (**stable on-disk contract — never rename**), which denies via Claude Code's `hookSpecificOutput` JSON shape and stays silent to allow. Both layers fail open. A deny aborts the offending dispatch; `detect-stray-writes` remains the after-the-fact backstop. +The shared cwd-aware policy allows ordinary installs, builds, tests, and in-place edits inside the +task env, while explicit outside destinations, output escapes, repository-routing escapes, and +remote Git mutations remain blocked. diff --git a/docs/cline-notes.md b/docs/cline-notes.md index f42224c..b9a1525 100644 --- a/docs/cline-notes.md +++ b/docs/cline-notes.md @@ -151,6 +151,9 @@ the descriptor references. "Probe capture" refers to the observed dispatches des `.cline/plugins/slow-powers-eval-guard/index.js` whose `beforeTool` hook forwards every tool call to `eval-magic guard-hook --harness cline` (`run_commands`' `commands` array joined into one `command` string for the shared arbiter) and returns `{skip: true, reason}` on deny. + The shared cwd-aware policy allows ordinary installs, builds, tests, and in-place edits inside + the task env while denying recognized explicit destinations outside it, output escapes, + repository-routing escapes, and remote Git mutations. Spike-verified on 3.0.53 (all in a throwaway dir, hand-staged plugin): project plugin dirs auto-load in headless one-shot dispatches (a bare `index.js` needs no package.json; a loose `.js` file at the plugins root is IGNORED); the hook context is `{snapshot, tool, toolCall, diff --git a/docs/codex-notes.md b/docs/codex-notes.md index 50c7be4..1aaa79d 100644 --- a/docs/codex-notes.md +++ b/docs/codex-notes.md @@ -165,7 +165,10 @@ update/delete source, and move destination from that body and resolves relative hook payload's `cwd`. Bash output validation uses a quote-aware lexical scan for `>`, `>>`, `>|`, file-descriptor-prefixed redirects, and `tee`: every literal target must resolve under an allowed root, while dynamic, malformed, or outside targets are blocked. Merely mentioning an allowed root -elsewhere in the command does not scope an unrelated redirect. +elsewhere in the command does not scope an unrelated redirect. The same cwd-aware policy allows +ordinary installs, builds, tests, and in-place edits inside the task env while denying recognized +explicit destinations outside it. Repository-routing escapes and remote Git mutations remain +blocked. Guard installation initializes `.eval-magic-outputs/guard-denials.jsonl` and records its absolute path in the optional marker field `denialLogPath`. Each block appends only timestamp, harness, diff --git a/docs/developer_overview.md b/docs/developer_overview.md index 7d048f6..a59f812 100644 --- a/docs/developer_overview.md +++ b/docs/developer_overview.md @@ -51,6 +51,7 @@ preconditions, handoffs, and recovery commands. a task environment is built from and the skills under test resolve through it. - `schema/` contains the JSON schemas for user input and generated artifacts. - `harnesses/` contains built-in descriptors, descriptor scaffolding, and embedded harness assets. +- `guard-profiles/` contains packaged command-policy defaults discovered and embedded by `build.rs`. - `profiles/` contains shared prompt profiles. - `tests/cli/` covers CLI and packaging contracts; `tests/run/` covers campaign behavior across the run boundary. Focused unit tests normally live beside the implementation. @@ -148,5 +149,7 @@ implementation evidence in an internal note. `eval-magic docs isolation`. - [Shipped codebase guide](guides/codebase.md) is the repository source for `eval-magic docs codebase`. +- [Shipped guard guide](guides/guard.md) is the repository source for + `eval-magic docs guard`. - [Shipped conversations guide](guides/conversations.md) is the repository source for `eval-magic docs conversations`. diff --git a/docs/guides/guard.md b/docs/guides/guard.md new file mode 100644 index 0000000..25d51cd --- /dev/null +++ b/docs/guides/guard.md @@ -0,0 +1,148 @@ +# Configuring guarded commands + +> **Audience:** eval authors deciding which development commands an agent may run inside a task +> environment. + +The write guard combines a fixed containment boundary with an eval-authored command policy. Use +the `guard` field in the `evals.json` file to grant the tools or command prefixes a task needs. +Without an explicit `guard` field, eval-magic detects packaged profiles from the staged task tree. + +## Understand the two policy layers + +Containment checks run before command allowances. A command policy cannot override these checks: + +- Direct write and patch tools must target the task environment. +- Shell redirects and `tee` targets must resolve inside the task environment. +- Remote Git mutations and repository-routing escapes are blocked. +- Recognized package, build, and edit destinations must stay inside the task environment. Global + and user installation modes are blocked unless the classifier can prove an in-task destination. + +After those checks, the command policy handles recognized development mutations. A recognized +command that has no matching allowance is blocked. A tool claimed by `allow_commands` also has its +other subcommands blocked. Commands that neither the containment classifier nor the command policy +recognizes remain best-effort allowed and are inspected by `detect-stray-writes` after the run. + +## Choose the configuration scope + +A config-level `guard` field is the default for every eval: + +```json +{ + "skill_name": "rust-maintainer", + "guard": { + "allow_tools": ["cargo"] + }, + "evals": [ + { + "id": "repair-workspace", + "prompt": "Repair the workspace and run its tests.", + "expected_output": "The workspace tests pass." + } + ] +} +``` + +A per-eval `guard` field completely replaces the config-level field. It does not merge with it. +Even an empty object disables the config default and automatic profile detection for that eval: + +```json +{ + "skill_name": "web-maintainer", + "guard": { + "profiles": ["language/javascript"] + }, + "evals": [ + { + "id": "serve-next-app", + "prompt": "Start the development server.", + "expected_output": "The server starts.", + "guard": { + "allow_commands": ["npm run dev"] + } + } + ] +} +``` + +Any explicit `guard` field disables automatic detection. When it names `profiles`, only those +profiles are expanded. + +## Choose allowance granularity + +The `guard` object accepts three arrays: + +- `allow_tools` contains executable basenames. An entry such as `cargo` permits all Cargo + subcommands after containment checks. +- `allow_commands` contains one literal shell command per entry. Each entry is a token prefix, so + `cargo test` permits `cargo test --workspace` but not `cargo build`. +- `profiles` contains packaged profile IDs. Profile commands are added to `allow_commands`. + +Command rules cannot contain pipes, separators, redirects, variable expansions, or command +substitutions. Eval validation rejects those shapes. Matching normalizes an executable path to its +basename, skips leading environment assignments, and understands the `env`, `command`, `exec`, +`nice`, and `timeout` wrappers. A literal command passed through `sh -c`, `bash -c`, or `zsh -c` is +matched recursively. Every segment of a compound command must be allowed independently. + +For example, this policy permits the listed Next.js lifecycle scripts but claims no other npm +subcommands: + +```json +{ + "guard": { + "allow_commands": [ + "npm run dev", + "npm run build", + "npm run start" + ] + } +} +``` + +With that policy, `npm run dev -- --hostname 127.0.0.1` is allowed and `npm install` is blocked. +Use `allow_tools` only when every subcommand of the tool is appropriate for the eval. + +## Use packaged profiles + +Packaged profiles provide lightweight defaults. Detection is recursive, so a frontend and backend +in the same task environment can activate multiple profiles. The detector skips `.git`, +`.eval-magic-outputs`, harness configuration and staged-skill directories, `target`, +`node_modules`, and `.venv`. + +The packaged profiles are: + +- `language/rust` is detected from `Cargo.toml`. It allows `cargo build`, `check`, `test`, `run`, + `fmt`, and `clippy`. +- `language/javascript` is detected from `package.json`. It allows npm install, CI, test, build, + lint, and typecheck commands, plus corresponding pnpm, Yarn, and Bun install, add, test, build, + lint, and typecheck commands. +- `framework/nextjs` is detected when `package.json` declares a `next` dependency. It allows npm, + pnpm, Yarn, and Bun dev, build, and start scripts, plus direct `next` invocations through `npx`, + `pnpm exec`, `yarn`, and `bunx`. +- `language/python` is detected from `pyproject.toml`, `setup.py`, or a + `requirements*.txt` file. It allows pip installs, Python module invocations for pip, build, + pytest, and unittest, and direct `pytest`. + +Name profiles explicitly when the files in the task tree are not the policy you want: + +```json +{ + "guard": { + "profiles": ["language/javascript", "framework/nextjs"], + "allow_commands": ["npm run integration"] + } +} +``` + +Explicit commands and expanded profile commands are deduplicated in the effective policy. + +## Audit the effective policy + +Each task in `dispatch.json` records its fully expanded `guard_policy`. The armed marker records the +same policy as `guardPolicy`, so the live hook and the campaign plan cannot resolve defaults +differently. `detect-stray-writes` reads the frozen task policy from `dispatch.json` and applies the +same classifier after the run. A legacy dispatch without `guard_policy` uses an empty command +policy rather than guessing which defaults applied. + +The command policy is not a complete shell sandbox. Keep task environments isolated, inspect guard +denials and stray-write findings during ingest, and use narrow `allow_commands` entries when the +eval does not need every operation a tool exposes. diff --git a/docs/opencode-notes.md b/docs/opencode-notes.md index ec1aaa0..26109d4 100644 --- a/docs/opencode-notes.md +++ b/docs/opencode-notes.md @@ -104,9 +104,11 @@ Two boundary notes, shared with the other harnesses' guards: - The marker's sole allowed root is the private task env on every host. Host temp locations such as `/tmp` and `$TMPDIR` remain out of bounds; dispatch prompts direct scratch work to `/tmp/` instead without rewriting `TMPDIR`, `TMP`, or `TEMP`. -- Bash coverage is the shared heuristic denylist (installs, git mutations, redirects, config-dir - tampering): a bare `touch /abs/outside/path` matches no pattern and is allowed — after-the-fact - detection of those is `detect-stray-writes`' job, same as claude/codex. +- Bash coverage is the shared target-aware heuristic. Ordinary installs, builds, tests, and + in-place edits run from the task env; recognized explicit project/output destinations must also + remain there. Output redirects, repository-routing escapes, and remote Git mutations remain + blocked. A bare `touch /abs/outside/path` matches no pattern and is allowed — after-the-fact + detection of those is `detect-stray-writes`' job, same as the other harnesses. One hook-shape caveat: `tool.execute.before` fires for *every* tool (OpenCode has no matcher surface), so each tool call spawns one `eval-magic guard-hook`. Classification stays in the diff --git a/guard-profiles/framework-nextjs.toml b/guard-profiles/framework-nextjs.toml new file mode 100644 index 0000000..1350c2f --- /dev/null +++ b/guard-profiles/framework-nextjs.toml @@ -0,0 +1,28 @@ +id = "framework/nextjs" +package_json_dependencies = ["next"] +allow_commands = [ + "npm run dev", + "npm run build", + "npm run start", + "pnpm run dev", + "pnpm run build", + "pnpm run start", + "yarn run dev", + "yarn run build", + "yarn run start", + "bun run dev", + "bun run build", + "bun run start", + "npx next dev", + "npx next build", + "npx next start", + "pnpm exec next dev", + "pnpm exec next build", + "pnpm exec next start", + "yarn next dev", + "yarn next build", + "yarn next start", + "bunx next dev", + "bunx next build", + "bunx next start", +] diff --git a/guard-profiles/language-javascript.toml b/guard-profiles/language-javascript.toml new file mode 100644 index 0000000..a05f013 --- /dev/null +++ b/guard-profiles/language-javascript.toml @@ -0,0 +1,28 @@ +id = "language/javascript" +markers = ["package.json"] +allow_commands = [ + "npm install", + "npm ci", + "npm test", + "npm run build", + "npm run lint", + "npm run typecheck", + "pnpm install", + "pnpm add", + "pnpm test", + "pnpm run build", + "pnpm run lint", + "pnpm run typecheck", + "yarn install", + "yarn add", + "yarn test", + "yarn run build", + "yarn run lint", + "yarn run typecheck", + "bun install", + "bun add", + "bun test", + "bun run build", + "bun run lint", + "bun run typecheck", +] diff --git a/guard-profiles/language-python.toml b/guard-profiles/language-python.toml new file mode 100644 index 0000000..e51e6cb --- /dev/null +++ b/guard-profiles/language-python.toml @@ -0,0 +1,16 @@ +id = "language/python" +markers = ["pyproject.toml", "setup.py"] +marker_patterns = ["requirements*.txt"] +allow_commands = [ + "pip install", + "pip3 install", + "python -m pip install", + "python3 -m pip install", + "python -m build", + "python3 -m build", + "python -m pytest", + "python3 -m pytest", + "python -m unittest", + "python3 -m unittest", + "pytest", +] diff --git a/guard-profiles/language-rust.toml b/guard-profiles/language-rust.toml new file mode 100644 index 0000000..bc4de90 --- /dev/null +++ b/guard-profiles/language-rust.toml @@ -0,0 +1,10 @@ +id = "language/rust" +markers = ["Cargo.toml"] +allow_commands = [ + "cargo build", + "cargo check", + "cargo test", + "cargo run", + "cargo fmt", + "cargo clippy", +] diff --git a/harnesses/template.toml b/harnesses/template.toml index b117556..201e185 100644 --- a/harnesses/template.toml +++ b/harnesses/template.toml @@ -22,8 +22,7 @@ label = "{label}" ## Where the harness discovers project-local skills. Declaring skills_dir unlocks native ## staging; without it every run is forced to --no-stage. The first path segment of skills_dir -## must appear in config_dirs (it feeds the staging sibling filter, guard tamper rules, and -## stray-write lookbehind). +## must appear in config_dirs (it feeds the staging sibling filter and task-repository baseline). ## VERIFY: which directory does the harness actually scan for skills? Quote the doc or the ## observed behavior in the notes file. # skills_dir = ".{label}/skills" diff --git a/schema/evals.schema.json b/schema/evals.schema.json index 88f1f48..1bb1e07 100644 --- a/schema/evals.schema.json +++ b/schema/evals.schema.json @@ -15,6 +15,10 @@ "$ref": "#/definitions/codebase", "description": "Default codebase every eval's task environment is built from. A per-eval codebase overrides it." }, + "guard": { + "$ref": "#/definitions/guardPolicy", + "description": "Default shell-command policy for guarded runs. A per-eval guard block replaces it. When neither is present, eval-magic detects packaged profiles from the task tree." + }, "evals": { "type": "array", "minItems": 1, @@ -106,6 +110,10 @@ "$ref": "#/definitions/codebase", "description": "Codebase this eval's task environment is built from, overriding the config-level default." }, + "guard": { + "$ref": "#/definitions/guardPolicy", + "description": "Shell-command policy for this eval. Presence replaces the config-level guard policy and disables automatic profile detection." + }, "isolation": { "type": "string", "enum": ["shared", "isolated"], @@ -129,6 +137,30 @@ } ] }, + "guardPolicy": { + "type": "object", + "additionalProperties": false, + "properties": { + "profiles": { + "type": "array", + "uniqueItems": true, + "items": { "type": "string", "minLength": 1 }, + "description": "Packaged guard profiles to include. Naming profiles is explicit; automatic detection does not add any others." + }, + "allow_tools": { + "type": "array", + "uniqueItems": true, + "items": { "type": "string", "minLength": 1 }, + "description": "Shell executable basenames allowed with any arguments, after non-overridable containment checks." + }, + "allow_commands": { + "type": "array", + "uniqueItems": true, + "items": { "type": "string", "minLength": 1 }, + "description": "Literal shell-token prefixes allowed for their claimed executable, with trailing arguments permitted." + } + } + }, "responder": { "type": "object", "required": ["type"], diff --git a/schema/harness-descriptor.schema.json b/schema/harness-descriptor.schema.json index 6de0a99..f1865b3 100644 --- a/schema/harness-descriptor.schema.json +++ b/schema/harness-descriptor.schema.json @@ -20,7 +20,7 @@ "config_dirs": { "type": "array", "items": { "type": "string", "minLength": 1 }, - "description": "Harness-owned config directory names (e.g. \".claude\"). Feeds the staging sibling filter, guard tamper rules, and stray-write lookbehind." + "description": "Harness-owned config directory names (e.g. \".claude\"). Feeds the staging sibling filter and task-repository baseline inclusion." }, "run": { "type": "object", diff --git a/schema/stray-writes.schema.json b/schema/stray-writes.schema.json index d466b36..45cfd4c 100644 --- a/schema/stray-writes.schema.json +++ b/schema/stray-writes.schema.json @@ -47,7 +47,7 @@ }, "warnings": { "type": "array", - "description": "Heuristic: a Bash command matched a mutating pattern (install, git, sed -i), or a literal redirection/tee target resolved outside the task environment from the invocation cwd.", + "description": "Heuristic: a recognized development mutation had an invocation cwd or explicit destination outside the task environment, an output redirection/tee target could not be proven in bounds, or a Git operation escaped the local task repository.", "items": { "$ref": "#/definitions/finding" } }, "live_source_reads": { diff --git a/src/adapters/descriptor/validation.rs b/src/adapters/descriptor/validation.rs index 148e961..fd97dbe 100644 --- a/src/adapters/descriptor/validation.rs +++ b/src/adapters/descriptor/validation.rs @@ -165,7 +165,7 @@ fn check_config_dirs_cover_skills_dir(d: &HarnessDescriptor) -> Result<(), Strin if !d.config_dirs.iter().any(|dir| dir == top) { return Err(format!( "config_dirs {:?} misses \"{top}\", the parent of skills_dir — staging's \ - sibling-asset filter and the guard tamper rules key off config_dirs", + sibling-asset filter keys off config_dirs", d.config_dirs )); } diff --git a/src/adapters/descriptor_adapter.rs b/src/adapters/descriptor_adapter.rs index 858099b..bfd0ce5 100644 --- a/src/adapters/descriptor_adapter.rs +++ b/src/adapters/descriptor_adapter.rs @@ -297,13 +297,21 @@ impl HarnessAdapter for DescriptorAdapter { stage_root: &Path, guard_exe: &Path, ttl: Option, + guard_policy: &crate::core::GuardPolicyConfig, ) -> io::Result { match &self.descriptor.guard { Some(guard) => { let skills_dir = self .skills_dir(stage_root) .expect("descriptor validation pairs [guard] with skills_dir"); - super::guard::install_guard(guard, &skills_dir, stage_root, guard_exe, ttl) + super::guard::install_guard( + guard, + &skills_dir, + stage_root, + guard_exe, + ttl, + guard_policy, + ) } None => Err(io::Error::new( io::ErrorKind::Unsupported, diff --git a/src/adapters/guard.rs b/src/adapters/guard.rs index c03596e..b0097c0 100644 --- a/src/adapters/guard.rs +++ b/src/adapters/guard.rs @@ -52,11 +52,12 @@ pub(crate) fn install_guard( stage_root: &Path, guard_exe: &Path, ttl: Option, + guard_policy: &crate::core::GuardPolicyConfig, ) -> io::Result { fs::create_dir_all(skills_dir)?; let marker_path = skills_dir.join(GUARD_MARKER); - write_marker(&marker_path, stage_root, ttl)?; + write_marker(&marker_path, stage_root, ttl, guard_policy)?; match guard.engine { GuardEngine::JsonHooks => { @@ -412,7 +413,6 @@ mod cline_plugin_tests; #[cfg(test)] mod guard_denial_tests; - #[cfg(test)] mod tests { use super::*; @@ -457,6 +457,7 @@ mod tests { stage_root, Path::new("/g/eval-magic"), None, + &Default::default(), ) .unwrap() } @@ -491,8 +492,7 @@ mod tests { GuardMarker { active: Some(true), allowed_roots: Some(vec!["/work/.eval-magic".to_string()]), - expires_at: None, - denial_log_path: None, + ..Default::default() } } @@ -639,8 +639,7 @@ mod tests { /work/.eval-magic/tmp.\"}}" ); - let payload = - r#"{ "tool_name": "Bash", "tool_input": { "command": "npm install left-pad" } }"#; + let payload = r#"{ "tool_name": "Bash", "cwd": "/work/.eval-magic", "tool_input": { "command": "npm install --prefix /outside left-pad" } }"#; assert_eq!( verdict("codex", payload, Some(marker())).expect("should block"), "{\"decision\":\"block\",\"reason\":\"eval guard: blocked Bash \ @@ -737,7 +736,7 @@ mod tests { #[test] fn codex_deny_returns_decision_block_json() { - let payload = r#"{ "hook_event_name": "PreToolUse", "tool_name": "Bash", "tool_input": { "command": "npm install left-pad" } }"#; + let payload = r#"{ "hook_event_name": "PreToolUse", "tool_name": "Bash", "cwd": "/work/.eval-magic", "tool_input": { "command": "npm install --prefix /outside left-pad" } }"#; let out = verdict("codex", payload, Some(marker())).expect("should block"); let v: Value = serde_json::from_str(&out).unwrap(); assert_eq!(v["decision"], "block"); diff --git a/src/adapters/guard/cline_plugin_tests.rs b/src/adapters/guard/cline_plugin_tests.rs index f3fb923..dbf166e 100644 --- a/src/adapters/guard/cline_plugin_tests.rs +++ b/src/adapters/guard/cline_plugin_tests.rs @@ -43,6 +43,7 @@ fn install(label: &str, stage_root: &Path) -> PathBuf { stage_root, Path::new("/g/eval-magic"), None, + &Default::default(), ) .unwrap() } @@ -59,6 +60,7 @@ fn marker() -> GuardMarker { allowed_roots: Some(vec!["/work/.eval-magic".to_string()]), expires_at: None, denial_log_path: None, + guard_policy: None, } } @@ -257,8 +259,7 @@ fn cline_deny_verdict_bytes_match_the_on_disk_contract() { /// arbiter's shell patterns must classify it. #[test] fn cline_deny_verdict_classifies_a_joined_shell_command() { - let payload = - r#"{ "tool_name": "run_commands", "tool_input": { "command": "npm install left-pad" } }"#; + let payload = r#"{ "tool_name": "run_commands", "cwd": "/work/.eval-magic", "tool_input": { "command": "npm install --prefix /outside left-pad" } }"#; let verdict = verdict("cline", payload, Some(marker())).expect("should block"); assert!(verdict.contains("package install/add"), "{verdict}"); } diff --git a/src/adapters/guard/guard_denial_tests.rs b/src/adapters/guard/guard_denial_tests.rs index 07063a6..06d0e9a 100644 --- a/src/adapters/guard/guard_denial_tests.rs +++ b/src/adapters/guard/guard_denial_tests.rs @@ -36,6 +36,7 @@ fn install(label: &str, stage_root: &Path) -> PathBuf { stage_root, Path::new("/g/eval-magic"), None, + &Default::default(), ) .unwrap() } @@ -51,6 +52,7 @@ fn marker() -> GuardMarker { allowed_roots: Some(vec!["/work/.eval-magic".to_string()]), expires_at: None, denial_log_path: None, + guard_policy: None, } } diff --git a/src/adapters/harness.rs b/src/adapters/harness.rs index dbe1dd0..1516b33 100644 --- a/src/adapters/harness.rs +++ b/src/adapters/harness.rs @@ -95,13 +95,10 @@ pub trait HarnessAdapter { /// The project-local config dir names this harness reads or the adapter /// writes (e.g. `.claude`). Staging excludes every harness's config dirs /// when copying a skill's sibling assets, so a stray checked-in config dir - /// never rides into a staged env. Via - /// [`all_config_dir_names`](super::registry::all_config_dir_names) this list - /// also feeds the guard's Bash tamper rule and detect-stray-writes' - /// staging-dir lookbehind, so adding a dir here automatically grows the - /// write-guard's deny surface. List the parent of - /// [`skills_dir`](Self::skills_dir) plus any hook/config dirs the adapter - /// writes. + /// never rides into a staged env. The task-repository baseline also force-adds + /// existing config dirs when a sourced codebase's `.gitignore` covers them. + /// List the parent of [`skills_dir`](Self::skills_dir) plus any hook/config + /// dirs the adapter writes. fn config_dir_names(&self) -> Vec { Vec::new() } @@ -302,6 +299,7 @@ pub trait HarnessAdapter { _stage_root: &Path, _guard_exe: &Path, _ttl: Option, + _guard_policy: &crate::core::GuardPolicyConfig, ) -> io::Result { Err(io::Error::new( io::ErrorKind::Unsupported, diff --git a/src/adapters/registry.rs b/src/adapters/registry.rs index 38fb99c..73bb4f7 100644 --- a/src/adapters/registry.rs +++ b/src/adapters/registry.rs @@ -380,9 +380,8 @@ pub fn default_harness_name() -> &'static str { } /// The union of every harness's project-local config dir names (sorted, -/// deduplicated): the dirs harness-agnostic code must treat as protected — -/// staging's sibling-asset filter, the guard's Bash tamper rule, and -/// detect-stray-writes' staging-dir lookbehind. +/// deduplicated): staging excludes them from sibling assets, and task-repository +/// setup force-adds the runner-owned copies to the baseline. pub fn all_config_dir_names() -> Vec { let mut names: Vec = registry() .iter() diff --git a/src/cli/args.rs b/src/cli/args.rs index 0930c32..b622ee7 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -431,14 +431,29 @@ pub struct RunArgs { /// not rewrite `TMPDIR`, `TMP`, or `TEMP`. /// Because the harness already cwd-bounds the agent's direct file tools to the /// env, the guard's main remaining value is blocking Bash-subprocess escapes the - /// cwd boundary doesn't cover — `npm install`, `git worktree add`, `sed -i`, - /// redirects that resolve outside the env — and acting as a backstop when the - /// isolated session runs with relaxed permissions. Local Git operations such - /// as status, diff, add, commit, and branching are allowed inside the task - /// repository. Repository-routing escapes and remote Git operations are - /// blocked; `--no-guard` opts out of those blocks, though task repositories - /// still begin with no remotes. Literal relative redirect and `tee` targets - /// resolve from the tool invocation cwd; dynamic, malformed, or outside + /// cwd boundary doesn't cover and acting as a backstop when the isolated session + /// runs with relaxed permissions. Recognized development mutations require an + /// allowance from the eval's `guard` configuration. `allow_commands` grants + /// literal shell-token prefixes; `allow_tools` grants every invocation of an + /// executable basename. A per-eval block replaces the config-level default. With + /// no explicit block, eval-magic composes packaged profiles detected from the + /// staged task tree. See `eval-magic docs guard` for configuration, matching, + /// packaged profiles, and examples. + /// + /// Command allowances never bypass containment checks. Known destination options + /// with dynamic, missing, or outside values are blocked, as are global/user + /// install modes that do not have a supported in-env destination. Recognized + /// destinations include npm `--prefix`, pnpm `-C`/`--dir`, Yarn/Bun `--cwd`, + /// pip `--target`/`--prefix`/`--root`/`--src`, and Cargo `-C`/`--target-dir` + /// plus its target-dir environment variables. Generic shell commands are not a + /// complete parser: for example, a bare `touch /outside` remains an + /// after-the-fact `detect-stray-writes` concern. + /// + /// Local Git operations such as status, diff, add, commit, and branching are + /// allowed inside the task repository. Repository-routing escapes and remote Git + /// operations are blocked; `--no-guard` opts out of those blocks, though task + /// repositories still begin with no remotes. Literal relative redirect and `tee` + /// targets resolve from the tool invocation cwd; dynamic, malformed, or outside /// targets are blocked. Every denial appends privacy-safe metadata /// (never the full command or patch) to the task's /// `.eval-magic-outputs/guard-denials.jsonl`; `ingest` joins those logs into diff --git a/src/cli/run/dispatch.rs b/src/cli/run/dispatch.rs index 086861c..db8cef3 100644 --- a/src/cli/run/dispatch.rs +++ b/src/cli/run/dispatch.rs @@ -15,8 +15,8 @@ use serde::{Deserialize, Serialize}; use crate::adapters::{CliManifestContext, adapter_for}; use crate::core::fs::artifact_path; use crate::core::{ - AvailableSkill, Eval, Harness, POSIX_TOOLING_REQUIREMENT, ResponderPolicy, ScriptedTurn, - SkillSource, SourceRecord, + AvailableSkill, Eval, GuardPolicyConfig, Harness, POSIX_TOOLING_REQUIREMENT, ResponderPolicy, + ScriptedTurn, SkillSource, SourceRecord, }; use super::RunError; @@ -75,6 +75,9 @@ pub struct DispatchTask { /// without one serializes exactly as it did before the field existed. #[serde(default, skip_serializing_if = "Option::is_none")] pub responder_dir: Option, + /// Fully expanded command policy used by the live guard and post-run audit. + #[serde(default)] + pub guard_policy: GuardPolicyConfig, #[serde(default, skip_serializing)] pub dispatch_prompt: String, } @@ -309,6 +312,7 @@ pub fn build_dispatch_task(opts: &DispatchTaskOpts) -> Result Vec { ids.iter() @@ -552,6 +557,7 @@ mod tests { turns: None, codebase: None, responder: None, + guard: None, }) .collect() } diff --git a/src/cli/run/dispatch/tests/guard_policy.rs b/src/cli/run/dispatch/tests/guard_policy.rs new file mode 100644 index 0000000..dff90eb --- /dev/null +++ b/src/cli/run/dispatch/tests/guard_policy.rs @@ -0,0 +1,14 @@ +use super::*; + +#[test] +fn dispatch_task_serializes_its_frozen_guard_policy() { + let mut task = build_dispatch_task(&base_opts()).unwrap(); + task.guard_policy.allow_commands = vec!["cargo test".to_string()]; + + let value = serde_json::to_value(task).unwrap(); + + assert_eq!( + value["guard_policy"]["allow_commands"], + serde_json::json!(["cargo test"]) + ); +} diff --git a/src/cli/run/fixtures.rs b/src/cli/run/fixtures.rs index 24f31f7..8d369c1 100644 --- a/src/cli/run/fixtures.rs +++ b/src/cli/run/fixtures.rs @@ -201,6 +201,7 @@ mod tests { turns: None, codebase: None, responder: None, + guard: None, } } diff --git a/src/cli/run/orchestrate/build.rs b/src/cli/run/orchestrate/build.rs index ee499e3..9433abf 100644 --- a/src/cli/run/orchestrate/build.rs +++ b/src/cli/run/orchestrate/build.rs @@ -211,7 +211,7 @@ pub(super) fn write_dispatch( let outputs_dir_str = outputs_dir.to_string_lossy().into_owned(); let run_dir_str = run_dir.to_string_lossy().into_owned(); - tasks.push(build_dispatch_task(&DispatchTaskOpts { + let mut task = build_dispatch_task(&DispatchTaskOpts { eval_id: &ev.id, condition: cond_name, skill_path: cond_skill_path, @@ -237,7 +237,13 @@ pub(super) fn write_dispatch( eval_root: Some(env_root_str.as_str()), codebase: codebase_record.as_ref(), skill_source: Some(&skill_source_record), - })?); + })?; + task.guard_policy = staged + .guard_policies + .get(&env_root) + .cloned() + .expect("every staged task environment has a guard policy"); + tasks.push(task); } } } @@ -399,7 +405,11 @@ pub(super) fn post_build( let adapter = adapter_for(ctx.harness); let exe = std::env::current_exe()?; for target in &targets { - adapter.install_guard(&target.root, &exe, None)?; + let policy = staged + .guard_policies + .get(&target.root) + .expect("every staged task environment has a guard policy"); + adapter.install_guard(&target.root, &exe, None, policy)?; } if let Some(msg) = adapter.guard_armed_message() { println!("{msg}"); diff --git a/src/cli/run/orchestrate/mod.rs b/src/cli/run/orchestrate/mod.rs index d5c0918..4a93b51 100644 --- a/src/cli/run/orchestrate/mod.rs +++ b/src/cli/run/orchestrate/mod.rs @@ -18,7 +18,8 @@ use crate::adapters::{CliDispatchContext, adapter_for}; use crate::cli::command_target_args; use crate::core::fs::artifact_path; use crate::core::{ - CodebaseSource, CodebaseUse, Eval, Mode, RunContext, SkillSource, SourceKind, SourceRecord, + CodebaseSource, CodebaseUse, Eval, GuardPolicyConfig, Mode, RunContext, SkillSource, + SourceKind, SourceRecord, }; use crate::source::ResolvedSource; @@ -224,6 +225,7 @@ struct Staged { sibling_meta: Vec<(String, String)>, bootstrap_content: Option, plan_mode_content: Option, + guard_policies: std::collections::HashMap, } /// Build the iteration workspace and dispatch plan for a run. diff --git a/src/cli/run/orchestrate/resolve.rs b/src/cli/run/orchestrate/resolve.rs index f19a773..c100e8a 100644 --- a/src/cli/run/orchestrate/resolve.rs +++ b/src/cli/run/orchestrate/resolve.rs @@ -131,7 +131,13 @@ pub(super) fn resolve_request(ctx: &RunContext, opts: &RunOptions) -> Result>(); let total_evals = config.evals.len(); // Resolve declared codebases here, while the run has still created nothing: diff --git a/src/cli/run/orchestrate/stage.rs b/src/cli/run/orchestrate/stage.rs index fc9b13e..3af2203 100644 --- a/src/cli/run/orchestrate/stage.rs +++ b/src/cli/run/orchestrate/stage.rs @@ -7,6 +7,7 @@ use std::path::{Path, PathBuf}; use crate::core::RunContext; use crate::core::fs::copy_entry_materialized; +use crate::sandbox::guard_profiles::{detect_profiles, expand_policy}; use crate::sandbox::teardown_guard; use super::super::RunError; @@ -96,6 +97,7 @@ pub(super) fn stage_conditions( // Distinct codebases materialized so far this iteration, by key. Every // environment sharing a codebase is provisioned from one materialization. let mut materialized: HashMap = HashMap::new(); + let mut guard_policies = HashMap::new(); for target in &targets { // Disarm a prior run's guard before re-staging, so a crashed run can't leave @@ -181,6 +183,24 @@ pub(super) fn stage_conditions( copy_fixtures(ev, &skills.join(&ctx.skill_name), &target.root, &mut claims)?; } } + + let eval = target + .eval_ids + .first() + .and_then(|eval_id| r.selected_evals.iter().find(|eval| &eval.id == eval_id)) + .expect("canonical task environments contain one selected eval"); + let policy = match eval.guard.as_ref() { + Some(policy) => expand_policy(policy), + None => { + let profiles = detect_profiles(&target.root)?; + expand_policy(&crate::core::GuardPolicyConfig { + profiles, + ..crate::core::GuardPolicyConfig::default() + }) + } + } + .map_err(|message| RunError::msg(format!("eval '{}': {message}", eval.id)))?; + guard_policies.insert(target.root.clone(), policy); } Ok(Staged { @@ -189,6 +209,7 @@ pub(super) fn stage_conditions( sibling_meta, bootstrap_content, plan_mode_content, + guard_policies, }) } diff --git a/src/cli/run/util.rs b/src/cli/run/util.rs index 7b3e57d..b7ce0f9 100644 --- a/src/cli/run/util.rs +++ b/src/cli/run/util.rs @@ -477,6 +477,7 @@ mod tests { turns: None, codebase: None, responder: None, + guard: None, } } diff --git a/src/core/types.rs b/src/core/types.rs index 7990b46..3ed8cb5 100644 --- a/src/core/types.rs +++ b/src/core/types.rs @@ -127,6 +127,21 @@ pub struct Eval { /// serializes exactly as it did before the field existed. #[serde(default, skip_serializing_if = "Option::is_none")] pub responder: Option, + /// Shell-command policy for this eval. When present it replaces the + /// config-level policy rather than extending it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub guard: Option, +} + +/// Authored shell-command allowances for a guarded eval run. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct GuardPolicyConfig { + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub profiles: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub allow_tools: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub allow_commands: Vec, } /// One scripted user follow-up delivered after an assistant response. @@ -279,6 +294,17 @@ pub struct EvalsConfig { #[serde(default, skip_serializing_if = "Option::is_none")] pub codebase: Option, pub evals: Vec, + /// Default shell-command policy for evals that do not replace it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub guard: Option, +} + +impl EvalsConfig { + /// Return the authored policy effective for `eval`. A per-eval block is a + /// complete replacement, including when it is empty. + pub fn guard_for<'a>(&'a self, eval: &'a Eval) -> Option<&'a GuardPolicyConfig> { + eval.guard.as_ref().or(self.guard.as_ref()) + } } /// A skill staged and discoverable for an eval — its natural name, on-disk @@ -722,6 +748,7 @@ mod tests { turns: None, codebase: None, responder: None, + guard: None, }; let out = serde_json::to_value(&eval).unwrap(); assert!(out.get("files").is_none()); @@ -747,6 +774,7 @@ mod tests { turns: None, codebase: None, responder: None, + guard: None, }; let out = serde_json::to_value(&eval).unwrap(); assert_eq!( diff --git a/src/pipeline/detect_stray_writes.rs b/src/pipeline/detect_stray_writes.rs index 4fdb653..68fe1b6 100644 --- a/src/pipeline/detect_stray_writes.rs +++ b/src/pipeline/detect_stray_writes.rs @@ -5,9 +5,10 @@ //! //! - **violations**: file-write tools (per the adapters' cross-harness //! vocabulary union) whose target path resolves outside the task's eval root. -//! - **warnings**: shell commands matching a mutating pattern that don't -//! reference the eval root, or literal redirect/`tee` targets resolving -//! outside it from the invocation cwd. +//! - **warnings**: recognized development mutations whose invocation cwd or +//! explicit destination escapes the eval root, output redirect/`tee` targets +//! that cannot be proven in bounds, and Git operations that escape the local +//! task repository. //! - **live_source_reads**: read tools / shell commands that touched the live //! skill-under-test directory instead of its staged copy. //! - **guard denials**: raw per-task JSONL is joined through `dispatch.json` @@ -20,12 +21,12 @@ use serde::{Deserialize, Serialize}; use crate::adapters::all_tool_vocabulary; use crate::core::fs::{normalize_separators, write_json}; -use crate::core::{ConditionsRecord, RunRecord, ToolInvocation}; +use crate::core::{ConditionsRecord, GuardPolicyConfig, RunRecord, ToolInvocation}; use crate::pipeline::error::PipelineError; use crate::pipeline::guard_denials::collect_guard_denials; use crate::pipeline::io::now_iso8601; use crate::pipeline::slots::{run_key, run_slots}; -use crate::sandbox::policy::classify_bash_with_cwd; +use crate::sandbox::policy::classify_bash_with_policy; use crate::sandbox::{is_shell_tool, is_under, is_write_tool, lexically_absolute, path_arg}; use crate::validation::{SchemaName, validate_against_schema}; @@ -73,6 +74,21 @@ pub fn detect_stray_writes( invocations: &[ToolInvocation], eval_root: &str, invocation_cwd: &Path, +) -> RunFindings { + detect_stray_writes_with_policy( + invocations, + eval_root, + invocation_cwd, + &GuardPolicyConfig::default(), + ) +} + +/// Classify invocations with the command policy frozen for this task. +pub fn detect_stray_writes_with_policy( + invocations: &[ToolInvocation], + eval_root: &str, + invocation_cwd: &Path, + guard_policy: &GuardPolicyConfig, ) -> RunFindings { let mut findings = RunFindings::default(); @@ -94,10 +110,11 @@ pub fn detect_stray_writes( if is_shell_tool(&inv.name) { let command = command_of(inv); - if let Some(classification) = classify_bash_with_cwd( + if let Some(classification) = classify_bash_with_policy( command, std::slice::from_ref(&eval_root.to_string()), invocation_cwd, + guard_policy, ) { findings.warnings.push(StrayFinding { tool: inv.name.clone(), @@ -226,6 +243,13 @@ struct DispatchRef { run_index: Option, #[serde(default)] eval_root: Option, + #[serde(default)] + guard_policy: GuardPolicyConfig, +} + +struct TaskBoundary { + eval_root: String, + guard_policy: GuardPolicyConfig, } /// Build, validate, and write `/stray-writes.json` for every @@ -252,7 +276,7 @@ pub fn detect_stray_writes_report( .map(|c| c.name.clone()) .collect(); - let allowed_roots_by_key = eval_roots_by_key(iteration_dir); + let boundaries_by_key = task_boundaries_by_key(iteration_dir); let guard_denials = collect_guard_denials(iteration_dir, iteration, repo_root)?; let mut runs = Vec::new(); @@ -289,15 +313,20 @@ pub fn detect_stray_writes_report( &source, )?; - let eval_root = allowed_roots_by_key.get(&run_key(eval_id, cond, slot.run_index)); + let boundary = boundaries_by_key.get(&run_key(eval_id, cond, slot.run_index)); invocations_inspected += run.tool_invocations.len(); // `dispatch.json` is the authoritative source of the private task // environment boundary. Without it we skip out-of-bounds write // classification rather than guess. Live-source-read detection is // independent of this boundary and still runs. - let findings = match eval_root { - Some(dir) => detect_stray_writes(&run.tool_invocations, dir, Path::new(dir)), + let findings = match boundary { + Some(boundary) => detect_stray_writes_with_policy( + &run.tool_invocations, + &boundary.eval_root, + Path::new(&boundary.eval_root), + &boundary.guard_policy, + ), None => { let run_label = slot .run_index @@ -355,22 +384,31 @@ pub fn detect_stray_writes_report( Ok(report) } -/// Map `":[:r]"` → the task's `eval_root` from -/// `dispatch.json`. Empty when the file is absent or malformed. -fn eval_roots_by_key(iteration_dir: &Path) -> std::collections::HashMap { +/// Map `":[:r]"` to the task boundary and frozen guard +/// policy from `dispatch.json`. Empty when the file is absent or malformed. +fn task_boundaries_by_key(iteration_dir: &Path) -> std::collections::HashMap { let mut out = std::collections::HashMap::new(); if let Ok(raw) = std::fs::read_to_string(iteration_dir.join("dispatch.json")) && let Ok(env) = serde_json::from_str::(&raw) { for t in env.tasks.unwrap_or_default() { - if let Some(dir) = t.eval_root { - out.insert(run_key(&t.eval_id, &t.condition, t.run_index), dir); + if let Some(eval_root) = t.eval_root { + out.insert( + run_key(&t.eval_id, &t.condition, t.run_index), + TaskBoundary { + eval_root, + guard_policy: t.guard_policy, + }, + ); } } } out } +#[cfg(test)] +mod realistic_development_tests; + #[cfg(test)] mod tests { use super::*; @@ -472,6 +510,23 @@ mod tests { assert!(f.warnings[0].reason.to_lowercase().contains("install")); } + #[test] + fn configured_command_policy_is_shared_with_the_stray_write_audit() { + let policy = crate::core::GuardPolicyConfig { + allow_commands: vec!["cargo test".to_string()], + ..crate::core::GuardPolicyConfig::default() + }; + + let findings = detect_stray_writes_with_policy( + &[inv("Bash", json!({"command": "cargo test --workspace"}), 0)], + ALLOWED_ROOT, + Path::new(ALLOWED_ROOT), + &policy, + ); + + assert!(findings.warnings.is_empty()); + } + #[test] fn a_codex_command_execution_install_is_a_warning() { let f = detect_stray_writes( @@ -551,32 +606,6 @@ mod tests { assert!(f.warnings[0].reason.to_lowercase().contains("worktree")); } - #[test] - fn creating_a_path_under_dot_claude_is_a_warning() { - let f = detect_stray_writes( - &[inv("Bash", json!({"command": "mkdir -p .claude/foo"}), 0)], - ALLOWED_ROOT, - repo(), - ); - assert_eq!(f.warnings.len(), 1); - assert!(f.warnings[0].reason.to_lowercase().contains("config dir")); - } - - #[test] - fn creating_a_path_under_dot_codex_is_a_warning() { - let f = detect_stray_writes( - &[inv( - "Bash", - json!({"command": "cp evil.json .codex/hooks.json"}), - 0, - )], - ALLOWED_ROOT, - repo(), - ); - assert_eq!(f.warnings.len(), 1); - assert!(f.warnings[0].reason.to_lowercase().contains("config dir")); - } - #[test] fn read_only_tools_are_never_flagged() { let f = detect_stray_writes( diff --git a/src/pipeline/detect_stray_writes/realistic_development_tests.rs b/src/pipeline/detect_stray_writes/realistic_development_tests.rs new file mode 100644 index 0000000..255e33b --- /dev/null +++ b/src/pipeline/detect_stray_writes/realistic_development_tests.rs @@ -0,0 +1,176 @@ +use std::path::Path; + +use serde_json::json; + +use super::detect_stray_writes_with_policy; +use crate::adapters::all_tool_vocabulary; +use crate::core::{GuardPolicyConfig, ToolInvocation}; +use crate::sandbox::decide::{GuardMarker, decide_with_cwd}; + +const ALLOWED_ROOT: &str = "/work/iteration-1/env-g1-with_skill"; + +struct Case { + command: &'static str, + cwd: &'static str, + allow: bool, +} + +fn invocation(name: &str, command: &str) -> ToolInvocation { + ToolInvocation { + name: name.to_string(), + args: Some(json!({"command": command})), + result: None, + ordinal: 0, + } +} + +#[test] +fn realistic_development_commands_match_between_guard_and_stray_write_audit() { + let allowed = [ + "npm install", + "npm --prefix ./web install", + "npm --prefix './web app' install", + "npm --global --prefix ./tools install left-pad", + "npm install global", + "npm install --location=project left-pad", + "npm install -- --global", + "npm install -- --prefix /outside/package-name", + "pnpm install", + "pnpm --dir ./web add left-pad", + "yarn install", + "yarn --cwd ./web add left-pad", + "bun install", + "bun --cwd ./web add left-pad", + "pip install -r requirements.txt", + "python -m pip install -e .", + "pip install --target .venv/lib left-pad", + "pip install --target '.venv/site packages' left-pad", + "cargo build", + "cargo test", + "CARGO_TARGET_DIR=target cargo test", + "cargo test -- --target-dir /outside/fixture", + "npm test", + "pytest", + "sed -i 's/old/new/' src/lib.rs", + "sed -i.bak 's/old/new/' src/lib.rs", + "sed --in-place=.bak -e 's/old/new/' src/lib.rs", + "mkdir -p .claude/skills/local-skill", + "touch skills/local-skill/SKILL.md", + ] + .map(|command| Case { + command, + cwd: ALLOWED_ROOT, + allow: true, + }); + let denied = [ + ("npm install", "/outside"), + ("pip install left-pad", "/outside"), + ("cargo build", "/outside"), + ("cargo test", "/outside"), + ("sed -i 's/old/new/' src/lib.rs", "/outside"), + ("npm install --prefix /outside/project", ALLOWED_ROOT), + ("npm --prefix=\"$PROJECT\" install", ALLOWED_ROOT), + ("npm install --prefix", ALLOWED_ROOT), + ("npm install --prefix --global", ALLOWED_ROOT), + ("npm install --prefix='unterminated", ALLOWED_ROOT), + ("npm --global install left-pad", ALLOWED_ROOT), + ("npm install --global=true left-pad", ALLOWED_ROOT), + ("npm install --location=global left-pad", ALLOWED_ROOT), + ("npm install --location global left-pad", ALLOWED_ROOT), + ( + "npm install --location=\"$LOCATION\" left-pad", + ALLOWED_ROOT, + ), + ("pnpm --dir /outside/project install", ALLOWED_ROOT), + ("pnpm -C \"$PROJECT\" install", ALLOWED_ROOT), + ("pnpm --global add left-pad", ALLOWED_ROOT), + ("yarn --cwd /outside/project install", ALLOWED_ROOT), + ("yarn global add left-pad", ALLOWED_ROOT), + ("bun --cwd=/outside/project install", ALLOWED_ROOT), + ("bun install --global left-pad", ALLOWED_ROOT), + ("pip install --target /outside/site left-pad", ALLOWED_ROOT), + ("pip install --target", ALLOWED_ROOT), + ( + "pip install --prefix=/outside/prefix left-pad", + ALLOWED_ROOT, + ), + ("pip install --root /outside/root left-pad", ALLOWED_ROOT), + ("pip install --src /outside/src -e example", ALLOWED_ROOT), + ("python -m pip install --user left-pad", ALLOWED_ROOT), + ("cargo -C /outside/project build", ALLOWED_ROOT), + ("cargo build --target-dir /outside/target", ALLOWED_ROOT), + ("cargo build --target-dir", ALLOWED_ROOT), + ("cargo build --target-dir --release", ALLOWED_ROOT), + ("CARGO_TARGET_DIR=/outside/target cargo test", ALLOWED_ROOT), + ( + "CARGO_BUILD_TARGET_DIR=\"$TARGET\" cargo build", + ALLOWED_ROOT, + ), + ("sed -i 's/old/new/' /outside/src/lib.rs", ALLOWED_ROOT), + ("sed -i 's/old/new/' \"$FILE\"", ALLOWED_ROOT), + ("printf done > /outside/result.txt", ALLOWED_ROOT), + ("git push origin main", ALLOWED_ROOT), + ] + .map(|(command, cwd)| Case { + command, + cwd, + allow: false, + }); + let policy = GuardPolicyConfig { + allow_tools: [ + "npm", "pnpm", "yarn", "bun", "pip", "python", "cargo", "pytest", "sed", "mkdir", + "touch", + ] + .map(str::to_string) + .to_vec(), + ..GuardPolicyConfig::default() + }; + let marker = GuardMarker { + active: Some(true), + allowed_roots: Some(vec![ALLOWED_ROOT.to_string()]), + expires_at: None, + denial_log_path: None, + guard_policy: Some(policy.clone()), + }; + + for case in allowed.into_iter().chain(denied) { + for tool in &all_tool_vocabulary().shell_tools { + let evaluation = decide_with_cwd( + tool, + &json!({"command": case.command}), + Some(&marker), + 0, + Path::new(case.cwd), + ); + let findings = detect_stray_writes_with_policy( + &[invocation(tool, case.command)], + ALLOWED_ROOT, + Path::new(case.cwd), + &policy, + ); + + assert_eq!( + evaluation.decision.allow, case.allow, + "guard mismatch for {tool}: {}", + case.command + ); + assert_eq!( + findings.warnings.is_empty(), + case.allow, + "stray-write mismatch for {tool}: {}", + case.command + ); + if let Some(finding) = findings.warnings.first() { + assert!( + evaluation + .decision + .reason + .as_deref() + .is_some_and(|reason| reason.contains(&finding.reason)), + "guard and audit reasons diverged for {tool}: {}", + case.command + ); + } + } + } +} diff --git a/src/pipeline/permission_denials.rs b/src/pipeline/permission_denials.rs index c5affa7..c2706cf 100644 --- a/src/pipeline/permission_denials.rs +++ b/src/pipeline/permission_denials.rs @@ -223,6 +223,7 @@ mod tests { allowed_roots: Some(vec!["/env".to_string()]), expires_at: None, denial_log_path: None, + guard_policy: None, }; let verdict = crate::sandbox::decide( "Write", diff --git a/src/sandbox/command_policy.rs b/src/sandbox/command_policy.rs new file mode 100644 index 0000000..6bf64b3 --- /dev/null +++ b/src/sandbox/command_policy.rs @@ -0,0 +1,297 @@ +//! Configured shell-command allowances for the write guard. + +use std::path::Path; + +use crate::core::GuardPolicyConfig; + +use super::policy::BashClassification; +use super::shell_targets::{ShellToken, ShellWord, lex_shell}; + +pub(super) const COMMAND_POLICY_REASON: &str = "command not allowed by eval guard policy"; + +pub(crate) fn validate_policy_syntax(policy: &GuardPolicyConfig) -> Result<(), String> { + for tool in &policy.allow_tools { + if tool.is_empty() + || tool + != Path::new(tool) + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or("") + || !tool + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || b"-+_.".contains(&byte)) + { + return Err(format!( + "allow_tools entry {tool:?} must be a literal executable basename" + )); + } + } + for rule in &policy.allow_commands { + let lexed = lex_shell(rule); + if lexed.malformed + || lexed.tokens.is_empty() + || lexed + .tokens + .iter() + .any(|token| !matches!(token, ShellToken::Word(word) if !word.dynamic)) + { + return Err(format!( + "allow_commands entry {rule:?} must be one literal shell command without operators, redirects, or expansions" + )); + } + let words: Vec = lexed + .tokens + .into_iter() + .filter_map(|token| match token { + ShellToken::Word(word) => Some(word), + _ => None, + }) + .collect(); + if words.first().is_some_and(is_assignment) || normalized_words(&words).is_none() { + return Err(format!( + "allow_commands entry {rule:?} must begin with a literal executable" + )); + } + } + Ok(()) +} + +fn executable_name(word: &ShellWord) -> Option<&str> { + (!word.dynamic) + .then(|| Path::new(&word.value).file_name()?.to_str()) + .flatten() +} + +fn is_assignment(word: &ShellWord) -> bool { + !word.dynamic + && word + .value + .split_once('=') + .is_some_and(|(name, _)| !name.is_empty() && !name.contains('/')) +} + +fn command_words(command: &str) -> Option> { + let lexed = lex_shell(command); + if lexed.malformed { + return None; + } + let mut words = Vec::new(); + for token in lexed.tokens { + match token { + ShellToken::Word(word) => words.push(word), + ShellToken::InputRedirect | ShellToken::FdDuplicate => {} + ShellToken::OutputRedirect | ShellToken::Pipe | ShellToken::Separator => return None, + } + } + Some(words) +} + +enum NormalizedCommand { + Words(Vec), + Script(String), +} + +fn normalized_command(words: &[ShellWord]) -> Option { + let command_index = words.iter().position(|word| !is_assignment(word))?; + let mut literal = Vec::with_capacity(words.len() - command_index); + literal.push(executable_name(&words[command_index])?.to_string()); + for word in &words[command_index + 1..] { + if word.dynamic { + literal.push("\0dynamic".to_string()); + continue; + } + literal.push(word.value.clone()); + } + let mut command = 0; + + loop { + match literal.get(command)?.as_str() { + "env" => { + command += 1; + while let Some(word) = literal.get(command) { + if word == "--" { + command += 1; + break; + } + if word == "-u" || word == "--unset" || word == "-C" || word == "--chdir" { + command += 2; + } else if word.starts_with('-') || assignment_value(word) { + command += 1; + } else { + break; + } + } + } + "command" => { + command += 1; + while literal + .get(command) + .is_some_and(|word| word.starts_with('-')) + { + command += 1; + } + } + "exec" => { + command += 1; + while let Some(word) = literal.get(command) { + if word == "-a" { + command += 2; + } else if word.starts_with('-') { + command += 1; + } else { + break; + } + } + } + "nice" => { + command += 1; + while let Some(word) = literal.get(command) { + if word == "-n" || word == "--adjustment" { + command += 2; + } else if word.starts_with('-') { + command += 1; + } else { + break; + } + } + } + "timeout" => { + command += 1; + while let Some(word) = literal.get(command) { + if matches!(word.as_str(), "-s" | "--signal" | "-k" | "--kill-after") { + command += 2; + } else if word.starts_with('-') { + command += 1; + } else { + break; + } + } + command += 1; // duration + } + "sh" | "bash" | "zsh" if literal.get(command + 1).is_some_and(|word| word == "-c") => { + return Some(NormalizedCommand::Script(literal.get(command + 2)?.clone())); + } + _ => break, + } + } + + let executable = Path::new(literal.get(command)?) + .file_name()? + .to_str()? + .to_string(); + let mut out = Vec::with_capacity(literal.len() - command); + out.push(executable); + out.extend(literal[command + 1..].iter().cloned()); + Some(NormalizedCommand::Words(out)) +} + +fn normalized_words(words: &[ShellWord]) -> Option> { + match normalized_command(words)? { + NormalizedCommand::Words(words) => Some(words), + NormalizedCommand::Script(script) => parsed_rule(&script), + } +} + +fn assignment_value(word: &str) -> bool { + word.split_once('=') + .is_some_and(|(name, _)| !name.is_empty() && !name.contains('/')) +} + +fn parsed_rule(rule: &str) -> Option> { + normalized_words(&command_words(rule)?) +} + +fn segment_denied(words: &[ShellWord], policy: &GuardPolicyConfig, malformed: bool) -> bool { + if words + .iter() + .find(|word| !is_assignment(word)) + .and_then(executable_name) + .is_some_and(|tool| policy.allow_tools.iter().any(|allowed| allowed == tool)) + { + return false; + } + let Some(normalized) = normalized_command(words) else { + return false; + }; + let actual = match normalized { + NormalizedCommand::Words(words) => words, + NormalizedCommand::Script(script) => { + return malformed || classify_command_policy(&script, policy).is_some(); + } + }; + let Some(tool) = actual.first() else { + return false; + }; + + if policy.allow_tools.iter().any(|allowed| allowed == tool) { + return false; + } + + let mut claimed = false; + for rule in &policy.allow_commands { + let Some(rule) = parsed_rule(rule) else { + continue; + }; + if rule.first() == Some(tool) { + claimed = true; + if !malformed && actual.starts_with(&rule) { + return false; + } + } + } + claimed + || super::guard_profiles::claims_command(&actual) + || super::mutation_targets::segment_is_recognized(words) +} + +pub(super) fn literal_shell_scripts(command: &str) -> Vec { + let lexed = lex_shell(command); + let mut scripts = Vec::new(); + let mut segment = Vec::new(); + let collect = |segment: &[ShellWord], scripts: &mut Vec| { + if let Some(NormalizedCommand::Script(script)) = normalized_command(segment) + && !script.contains('\0') + { + scripts.push(script); + } + }; + for token in lexed.tokens { + match token { + ShellToken::Word(word) => segment.push(word), + ShellToken::Pipe | ShellToken::Separator => { + collect(&segment, &mut scripts); + segment.clear(); + } + ShellToken::OutputRedirect | ShellToken::FdDuplicate | ShellToken::InputRedirect => {} + } + } + collect(&segment, &mut scripts); + scripts +} + +pub(super) fn classify_command_policy( + command: &str, + policy: &GuardPolicyConfig, +) -> Option { + let lexed = lex_shell(command); + let mut segment = Vec::new(); + for token in lexed.tokens { + match token { + ShellToken::Word(word) => segment.push(word), + ShellToken::Pipe | ShellToken::Separator => { + if segment_denied(&segment, policy, lexed.malformed) { + return Some(BashClassification { + reason: COMMAND_POLICY_REASON, + resolved_targets: Vec::new(), + }); + } + segment.clear(); + } + ShellToken::OutputRedirect | ShellToken::FdDuplicate | ShellToken::InputRedirect => {} + } + } + segment_denied(&segment, policy, lexed.malformed).then(|| BashClassification { + reason: COMMAND_POLICY_REASON, + resolved_targets: Vec::new(), + }) +} diff --git a/src/sandbox/decide.rs b/src/sandbox/decide.rs index fc483a8..59b9f52 100644 --- a/src/sandbox/decide.rs +++ b/src/sandbox/decide.rs @@ -2,19 +2,22 @@ //! //! [`decide`] is the single decision point the armed PreToolUse hook consults: //! given a tool call and the on-disk guard marker, it allows or denies. Writes -//! outside every allowed root and un-scoped Bash mutations are denied; everything -//! else — all read tools, and the orchestrator's own in-sandbox writes — is -//! allowed. When the guard is not armed, every call is allowed. +//! outside every allowed root and recognized Bash targets that escape those roots +//! are denied; everything else — all read tools, and the orchestrator's own +//! in-sandbox writes — is allowed. When the guard is not armed, every call is +//! allowed. use chrono::DateTime; use serde::Deserialize; use serde_json::Value; use std::path::Path; +use crate::core::GuardPolicyConfig; use crate::core::fs::artifact_path; +use super::command_policy::COMMAND_POLICY_REASON; use super::policy::{ - OUTPUT_REDIRECTION_REASON, apply_patch_paths, classify_bash_with_cwd, is_patch_tool, + OUTPUT_REDIRECTION_REASON, apply_patch_paths, classify_bash_with_policy, is_patch_tool, is_shell_tool, is_under_any, is_write_tool, path_arg, resolve_path, }; @@ -39,6 +42,8 @@ pub struct GuardMarker { pub expires_at: Option, #[serde(default)] pub denial_log_path: Option, + #[serde(default)] + pub guard_policy: Option, } /// The outcome of [`decide`]: allow, or deny with a human-readable reason. @@ -157,6 +162,10 @@ pub(crate) fn decide_with_cwd( let roots = marker .and_then(|m| m.allowed_roots.clone()) .unwrap_or_default(); + let default_policy = GuardPolicyConfig::default(); + let guard_policy = marker + .and_then(|m| m.guard_policy.as_ref()) + .unwrap_or(&default_policy); if is_write_tool(tool_name) { if let Some(p) = path_arg(tool_input) @@ -211,15 +220,22 @@ pub(crate) fn decide_with_cwd( .get("command") .and_then(Value::as_str) .unwrap_or(""); - if let Some(classification) = classify_bash_with_cwd(command, &roots, invocation_cwd) { + if let Some(classification) = + classify_bash_with_policy(command, &roots, invocation_cwd, guard_policy) + { let hint = if classification.reason == OUTPUT_REDIRECTION_REASON { scratch_hint(&roots) } else { String::new() }; + let boundary = if classification.reason == COMMAND_POLICY_REASON { + "" + } else { + " — runs outside the eval sandbox" + }; return GuardEvaluation::deny( format!( - "{GUARD_REASON_PREFIX}blocked {tool_name} ({}) — runs outside the eval sandbox{hint}", + "{GUARD_REASON_PREFIX}blocked {tool_name} ({}){boundary}{hint}", classification.reason, ), classification.resolved_targets, @@ -261,6 +277,7 @@ mod tests { allowed_roots: Some(ROOTS.iter().map(|s| s.to_string()).collect()), expires_at: Some(future()), denial_log_path: None, + guard_policy: None, } } @@ -341,12 +358,15 @@ mod tests { } #[test] - fn denies_an_install_command() { - let d = decide_now( + fn denies_an_install_command_from_outside_the_guarded_environment() { + let d = decide_with_cwd( "Bash", - json!({ "command": "npm install left-pad" }), + &json!({ "command": "npm install left-pad" }), Some(&marker()), - ); + now_ms(), + Path::new("/outside/project"), + ) + .decision; assert!(!d.allow); let reason = d.reason.unwrap(); assert!(reason.to_lowercase().contains("install")); @@ -354,7 +374,41 @@ mod tests { } #[test] - fn allows_a_bash_command_scoped_to_an_allowed_root() { + fn marker_command_policy_allows_a_configured_tool() { + let marker: GuardMarker = serde_json::from_value(json!({ + "active": true, + "allowedRoots": ["/work/.eval-magic/task"], + "guardPolicy": { "allow_tools": ["cargo"] } + })) + .unwrap(); + + let d = decide_with_cwd( + "Bash", + &json!({ "command": "cargo build --release" }), + Some(&marker), + now_ms(), + Path::new("/work/.eval-magic/task"), + ) + .decision; + + assert!(d.allow, "{:?}", d.reason); + + let denied = decide_with_cwd( + "Bash", + &json!({ "command": "npm install" }), + Some(&marker), + now_ms(), + Path::new("/work/.eval-magic/task"), + ) + .decision; + assert_eq!( + denied.reason.as_deref(), + Some("eval guard: blocked Bash (command not allowed by eval guard policy)") + ); + } + + #[test] + fn allows_bash_with_an_in_bounds_redirect() { let d = decide_now( "Bash", json!({ "command": "echo hi > /work/.eval-magic/x/outputs/log" }), @@ -499,43 +553,32 @@ mod tests { } #[test] - fn denies_bash_that_creates_a_path_under_dot_claude_via_non_redirect_verb() { - assert!( - !decide_now( - "Bash", - json!({ "command": "mkdir -p .claude/foo" }), - Some(&marker()) - ) - .allow - ); - assert!( - !decide_now( - "Bash", - json!({ "command": "cp out.txt .claude/bar" }), - Some(&marker()) - ) - .allow - ); - } - - #[test] - fn denies_bash_that_creates_a_bare_skills_dir() { - assert!( - !decide_now( - "Bash", - json!({ "command": "mkdir skills" }), - Some(&marker()) - ) - .allow - ); - assert!( - !decide_now( + fn allows_ordinary_filesystem_commands_inside_the_guarded_environment() { + let marker = marker(); + let cwd = Path::new("/work/.eval-magic/task"); + for command in [ + "mkdir -p .claude/foo", + "cp out.txt .claude/bar", + "mkdir skills", + "cp -r src ./skills", + "mkdir -p .codex/foo", + "cp hooks.json .codex/hooks.json", + "mkdir -p .agents/foo", + "touch .opencode/opencode.json", + ] { + let result = decide_with_cwd( "Bash", - json!({ "command": "cp -r src ./skills" }), - Some(&marker()) - ) - .allow - ); + &json!({ "command": command }), + Some(&marker), + now_ms(), + cwd, + ); + assert!( + result.decision.allow, + "{command} should be allowed: {:?}", + result.decision.reason + ); + } } #[test] @@ -561,50 +604,6 @@ mod tests { assert!(d.allow); } - #[test] - fn denies_bash_that_creates_a_path_under_dot_codex_via_non_redirect_verb() { - assert!( - !decide_now( - "Bash", - json!({ "command": "mkdir -p .codex/foo" }), - Some(&marker()) - ) - .allow - ); - assert!( - !decide_now( - "Bash", - json!({ "command": "cp evil.json .codex/hooks.json" }), - Some(&marker()) - ) - .allow - ); - } - - #[test] - fn denies_bash_that_creates_a_path_under_dot_agents_via_non_redirect_verb() { - assert!( - !decide_now( - "Bash", - json!({ "command": "mkdir -p .agents/foo" }), - Some(&marker()) - ) - .allow - ); - } - - #[test] - fn denies_bash_that_creates_a_path_under_dot_opencode_via_non_redirect_verb() { - assert!( - !decide_now( - "Bash", - json!({ "command": "touch .opencode/opencode.json" }), - Some(&marker()) - ) - .allow - ); - } - #[test] fn still_allows_reads_of_other_harness_config_dirs_with_no_create_verb() { for command in [ @@ -635,17 +634,4 @@ mod tests { ); assert!(d.allow); } - - #[test] - fn does_not_flag_a_skills_prefixed_dir_as_a_bare_skills_write() { - // A `skills`-prefixed path that is NOT an allowed root: the bare-`skills/` - // heuristic only fires on a bare `skills` at a path boundary, so a - // `skills-`-prefixed dir must not be flagged and the write is allowed. - let d = decide_now( - "Bash", - json!({ "command": "mkdir -p /work/skills-data/x/outputs" }), - Some(&marker()), - ); - assert!(d.allow); - } } diff --git a/src/sandbox/guard_profiles.rs b/src/sandbox/guard_profiles.rs new file mode 100644 index 0000000..c498d5b --- /dev/null +++ b/src/sandbox/guard_profiles.rs @@ -0,0 +1,222 @@ +//! Packaged command-policy profiles and task-tree auto-detection. + +use std::collections::{BTreeSet, HashMap}; +use std::fs; +use std::io; +use std::path::Path; +use std::sync::LazyLock; + +use serde::Deserialize; + +use crate::core::GuardPolicyConfig; + +include!(concat!(env!("OUT_DIR"), "/guard_profiles.rs")); + +#[derive(Debug, Deserialize)] +struct GuardProfile { + id: String, + #[serde(default)] + markers: Vec, + #[serde(default)] + marker_patterns: Vec, + #[serde(default)] + package_json_dependencies: Vec, + #[serde(default)] + allow_commands: Vec, +} + +static PROFILES: LazyLock> = LazyLock::new(|| { + let mut profiles = HashMap::new(); + for (path, body) in PACKAGED_GUARD_PROFILES { + let profile: GuardProfile = toml::from_str(body) + .unwrap_or_else(|error| panic!("invalid guard profile {path}: {error}")); + let id = profile.id.clone(); + assert!( + profiles.insert(id.clone(), profile).is_none(), + "duplicate guard profile {id}" + ); + } + profiles +}); + +pub(crate) fn has_profile(id: &str) -> bool { + PROFILES.contains_key(id) +} + +pub(crate) fn claims_command(actual: &[String]) -> bool { + PROFILES.values().any(|profile| { + profile.allow_commands.iter().any(|command| { + let rule: Vec<&str> = command.split_whitespace().collect(); + actual.len() >= rule.len() + && actual + .iter() + .zip(rule) + .all(|(word, expected)| word == expected) + }) + }) +} + +/// Expand authored profile references into the exact policy frozen into run artifacts. +pub(crate) fn expand_policy(policy: &GuardPolicyConfig) -> Result { + let mut expanded = policy.clone(); + for id in &policy.profiles { + let profile = PROFILES + .get(id) + .ok_or_else(|| format!("unknown guard profile {id:?}"))?; + expanded + .allow_commands + .extend(profile.allow_commands.clone()); + } + expanded.allow_tools.sort(); + expanded.allow_tools.dedup(); + expanded.allow_commands.sort(); + expanded.allow_commands.dedup(); + Ok(expanded) +} + +/// Detect every applicable packaged profile in a staged task tree. +pub(crate) fn detect_profiles(root: &Path) -> io::Result> { + let mut detected = BTreeSet::new(); + visit(root, &mut detected)?; + Ok(detected.into_iter().collect()) +} + +fn visit(path: &Path, detected: &mut BTreeSet) -> io::Result<()> { + for entry in fs::read_dir(path)? { + let entry = entry?; + let file_type = entry.file_type()?; + let name = entry.file_name(); + let name = name.to_string_lossy(); + if file_type.is_dir() { + if !excluded_directory(&name) { + visit(&entry.path(), detected)?; + } + continue; + } + if !file_type.is_file() { + continue; + } + + for profile in PROFILES.values() { + if profile.markers.iter().any(|marker| marker == &name) + || profile + .marker_patterns + .iter() + .any(|pattern| marker_matches(pattern, &name)) + { + detected.insert(profile.id.clone()); + } + } + if name == "package.json" { + detect_package_json_profiles(&entry.path(), detected); + } + } + Ok(()) +} + +fn marker_matches(pattern: &str, name: &str) -> bool { + let Some((prefix, suffix)) = pattern.split_once('*') else { + return pattern == name; + }; + !suffix.contains('*') && name.starts_with(prefix) && name.ends_with(suffix) +} + +fn excluded_directory(name: &str) -> bool { + matches!( + name, + ".git" + | ".eval-magic-outputs" + | ".claude" + | ".codex" + | ".agents" + | ".opencode" + | ".cline" + | "target" + | "node_modules" + | ".venv" + ) +} + +fn detect_package_json_profiles(path: &Path, detected: &mut BTreeSet) { + let Ok(body) = fs::read_to_string(path) else { + return; + }; + let Ok(value) = serde_json::from_str::(&body) else { + return; + }; + for profile in PROFILES.values() { + if profile.package_json_dependencies.iter().any(|dependency| { + [ + "dependencies", + "devDependencies", + "peerDependencies", + "optionalDependencies", + ] + .iter() + .any(|field| { + value + .get(field) + .and_then(|deps| deps.get(dependency)) + .is_some() + }) + }) { + detected.insert(profile.id.clone()); + } + } +} + +#[cfg(test)] +mod tests { + use std::fs; + + use tempfile::tempdir; + + use super::{PROFILES, detect_profiles, expand_policy}; + use crate::core::GuardPolicyConfig; + use crate::sandbox::command_policy::validate_policy_syntax; + + #[test] + fn packaged_profiles_contain_valid_command_rules() { + for profile in PROFILES.values() { + let policy = GuardPolicyConfig { + allow_commands: profile.allow_commands.clone(), + ..GuardPolicyConfig::default() + }; + validate_policy_syntax(&policy) + .unwrap_or_else(|error| panic!("profile {}: {error}", profile.id)); + } + } + + #[test] + fn explicit_profiles_expand_without_adding_detected_profiles() { + let policy = GuardPolicyConfig { + profiles: vec!["language/rust".to_string()], + ..GuardPolicyConfig::default() + }; + + let expanded = expand_policy(&policy).unwrap(); + + assert!(expanded.allow_commands.contains(&"cargo test".to_string())); + assert!(!expanded.allow_commands.contains(&"npm test".to_string())); + } + + #[test] + fn detection_joins_language_and_framework_profiles_recursively() { + let root = tempdir().unwrap(); + fs::create_dir_all(root.path().join("frontend")).unwrap(); + fs::write( + root.path().join("frontend/package.json"), + r#"{"dependencies":{"next":"15.0.0"}}"#, + ) + .unwrap(); + fs::create_dir_all(root.path().join("backend")).unwrap(); + fs::write(root.path().join("backend/requirements-dev.txt"), "").unwrap(); + + let detected = detect_profiles(root.path()).unwrap(); + + assert_eq!( + detected, + ["framework/nextjs", "language/javascript", "language/python"] + ); + } +} diff --git a/src/sandbox/install.rs b/src/sandbox/install.rs index cb93a20..87fc3c6 100644 --- a/src/sandbox/install.rs +++ b/src/sandbox/install.rs @@ -20,7 +20,7 @@ use chrono::{DateTime, SecondsFormat}; use serde::{Deserialize, Serialize}; use serde_json::json; -use crate::core::Harness; +use crate::core::{GuardPolicyConfig, Harness}; use super::now_ms; use super::{guard::read_marker, marker_is_armed}; @@ -83,6 +83,7 @@ pub(crate) fn write_marker( marker_path: &Path, stage_root: &Path, ttl: Option, + guard_policy: &GuardPolicyConfig, ) -> io::Result<()> { let expires_ms = now_ms() + ttl.unwrap_or(GUARD_TTL).as_millis() as i64; let denial_log_path = absolutize(&stage_root.join(GUARD_DENIALS_DIR).join(GUARD_DENIALS_LOG)); @@ -99,6 +100,7 @@ pub(crate) fn write_marker( "allowedRoots": marker_allowed_roots(stage_root), "expiresAt": iso_millis(expires_ms), "denialLogPath": denial_log_path, + "guardPolicy": guard_policy, }), ) } @@ -253,7 +255,12 @@ mod tests { let c = setup(); let adapter = crate::adapters::adapter_for(Harness::resolve("claude-code").unwrap()); let marker = adapter - .install_guard(&c.stage_root, Path::new("/g/eval-magic"), None) + .install_guard( + &c.stage_root, + Path::new("/g/eval-magic"), + None, + &Default::default(), + ) .unwrap(); let settings = @@ -287,7 +294,12 @@ mod tests { let c = setup(); let adapter = crate::adapters::adapter_for(Harness::resolve("codex").unwrap()); let marker = adapter - .install_guard(&c.stage_root, Path::new("/g/eval-magic"), None) + .install_guard( + &c.stage_root, + Path::new("/g/eval-magic"), + None, + &Default::default(), + ) .unwrap(); let hooks = fs::read_to_string(c.stage_root.join(".codex").join("hooks.json")).unwrap(); @@ -320,13 +332,17 @@ mod tests { let c = setup(); let exe = Path::new("/g/eval-magic"); let claude = crate::adapters::adapter_for(Harness::resolve("claude-code").unwrap()); - claude.install_guard(&c.stage_root, exe, None).unwrap(); + claude + .install_guard(&c.stage_root, exe, None, &Default::default()) + .unwrap(); assert!(guard_is_armed(&c.stage_root)); teardown_guard(&c.stage_root); assert!(!guard_is_armed(&c.stage_root)); let codex = crate::adapters::adapter_for(Harness::resolve("codex").unwrap()); - codex.install_guard(&c.stage_root, exe, None).unwrap(); + codex + .install_guard(&c.stage_root, exe, None, &Default::default()) + .unwrap(); assert!(guard_is_armed(&c.stage_root)); } } diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs index a3d46a3..9679704 100644 --- a/src/sandbox/mod.rs +++ b/src/sandbox/mod.rs @@ -17,10 +17,13 @@ //! enforces the shared boundary; put harness integration and descriptor //! rendering in `adapters`. +pub(crate) mod command_policy; pub mod decide; mod git_command; pub mod guard; +pub(crate) mod guard_profiles; pub mod install; +mod mutation_targets; pub mod policy; mod shell_targets; diff --git a/src/sandbox/mutation_targets.rs b/src/sandbox/mutation_targets.rs new file mode 100644 index 0000000..a2596ba --- /dev/null +++ b/src/sandbox/mutation_targets.rs @@ -0,0 +1,484 @@ +//! Target-aware classification for common development commands that mutate the filesystem. +//! +//! Package installs, pip installs, Cargo builds/tests, and in-place `sed` edits use the invocation +//! cwd as their implicit destination and validate the path options they own. This stays narrower +//! than a general shell parser; unrecognized commands remain the post-hoc audit's responsibility. + +use std::path::Path; + +use crate::core::fs::artifact_path; + +use super::policy::{BashClassification, is_under_any, resolve_path}; +use super::shell_targets::{ShellToken, ShellWord, lex_shell}; + +const PACKAGE_REASON: &str = "package install/add"; +const PIP_REASON: &str = "pip install"; +const SED_REASON: &str = "in-place file edit (sed -i)"; +const CARGO_REASON: &str = "cargo build/test output"; + +#[derive(Clone, Copy)] +struct PathOption { + long: &'static str, + short: Option<&'static str>, + attached_short: bool, +} + +fn is_command(word: &ShellWord, name: &str) -> bool { + Path::new(&word.value) + .file_name() + .is_some_and(|file_name| file_name == name) + && !word.dynamic +} + +fn command_position(words: &[&ShellWord], names: &[&str]) -> Option { + words + .iter() + .position(|word| names.iter().any(|name| is_command(word, name))) +} + +fn has_word(words: &[&ShellWord], start: usize, values: &[&str]) -> bool { + words + .iter() + .skip(start) + .take_while(|word| word.value != "--") + .any(|word| values.contains(&word.value.as_str())) +} + +fn package_global_mode(words: &[&ShellWord], manager: &str, command: usize, action: usize) -> bool { + for (index, word) in words.iter().enumerate().skip(command + 1) { + if word.value == "--" { + break; + } + match word.value.as_str() { + "--global" | "-g" => return true, + "global" if manager == "yarn" && index < action => return true, + "--location" if manager == "npm" => { + let Some(location) = words.get(index + 1) else { + return true; + }; + if location.dynamic || location.value != "project" { + return true; + } + } + value if value.starts_with("--global=") || value.starts_with("-g=") => { + let enabled = value.split_once('=').map(|(_, value)| value).unwrap_or(""); + if word.dynamic || !matches!(enabled, "false" | "0") { + return true; + } + } + value if manager == "npm" && value.starts_with("--location=") => { + let location = value.strip_prefix("--location=").unwrap_or_default(); + if word.dynamic || location != "project" { + return true; + } + } + _ => {} + } + } + false +} + +fn denial(reason: &'static str, resolved_targets: Vec) -> BashClassification { + BashClassification { + reason, + resolved_targets, + } +} + +fn target_denial( + reason: &'static str, + target: &ShellWord, + allowed_roots: &[String], + invocation_cwd: &Path, +) -> Option { + if target.dynamic || target.value.is_empty() || target.value.contains('\0') { + return Some(denial(reason, Vec::new())); + } + if is_under_any(&target.value, allowed_roots, invocation_cwd) { + return None; + } + Some(denial( + reason, + vec![artifact_path(&resolve_path(&target.value, invocation_cwd))], + )) +} + +fn cwd_denial( + reason: &'static str, + allowed_roots: &[String], + invocation_cwd: &Path, +) -> Option { + let cwd = ShellWord { + value: ".".to_string(), + dynamic: false, + }; + target_denial(reason, &cwd, allowed_roots, invocation_cwd) +} + +/// Validate every recognized path option. The boolean says whether the command +/// supplied at least one explicit destination. +fn validate_path_options( + words: &[&ShellWord], + start: usize, + options: &[PathOption], + reason: &'static str, + allowed_roots: &[String], + invocation_cwd: &Path, +) -> Result { + let mut saw_target = false; + let mut index = start; + while index < words.len() { + let word = words[index]; + if word.value == "--" { + break; + } + let mut matched = false; + for option in options { + let separate = word.value == option.long + || option + .short + .is_some_and(|short| word.value.as_str() == short); + if separate { + saw_target = true; + let Some(target) = words.get(index + 1) else { + return Err(denial(reason, Vec::new())); + }; + if target.value.starts_with('-') { + return Err(denial(reason, Vec::new())); + } + if let Some(denial) = target_denial(reason, target, allowed_roots, invocation_cwd) { + return Err(denial); + } + index += 2; + matched = true; + break; + } + + let long_prefix = format!("{}=", option.long); + let inline = word.value.strip_prefix(&long_prefix).or_else(|| { + option.short.and_then(|short| { + option + .attached_short + .then(|| word.value.strip_prefix(short)) + .flatten() + .filter(|value| !value.is_empty()) + }) + }); + if let Some(value) = inline { + saw_target = true; + let target = ShellWord { + value: value.to_string(), + dynamic: word.dynamic, + }; + if let Some(denial) = target_denial(reason, &target, allowed_roots, invocation_cwd) + { + return Err(denial); + } + index += 1; + matched = true; + break; + } + } + if !matched { + index += 1; + } + } + Ok(saw_target) +} + +fn classify_package_manager( + words: &[&ShellWord], + manager: &str, + path_options: &[PathOption], + allowed_roots: &[String], + invocation_cwd: &Path, +) -> Option { + let command = command_position(words, &[manager])?; + let action = words + .iter() + .enumerate() + .skip(command + 1) + .take_while(|(_, word)| word.value != "--") + .find(|(_, word)| ["install", "add", "ci", "i"].contains(&word.value.as_str())) + .map(|(index, _)| index)?; + let saw_destination = match validate_path_options( + words, + command + 1, + path_options, + PACKAGE_REASON, + allowed_roots, + invocation_cwd, + ) { + Ok(saw_destination) => saw_destination, + Err(denial) => return Some(denial), + }; + let global = package_global_mode(words, manager, command, action); + if global && !(manager == "npm" && saw_destination) { + return Some(denial(PACKAGE_REASON, Vec::new())); + } + cwd_denial(PACKAGE_REASON, allowed_roots, invocation_cwd) +} + +fn classify_package_install( + words: &[&ShellWord], + allowed_roots: &[String], + invocation_cwd: &Path, +) -> Option { + const NPM: &[PathOption] = &[PathOption { + long: "--prefix", + short: None, + attached_short: false, + }]; + const PNPM: &[PathOption] = &[PathOption { + long: "--dir", + short: Some("-C"), + attached_short: true, + }]; + const YARN_BUN: &[PathOption] = &[PathOption { + long: "--cwd", + short: None, + attached_short: false, + }]; + + [ + ("npm", NPM), + ("pnpm", PNPM), + ("yarn", YARN_BUN), + ("bun", YARN_BUN), + ] + .into_iter() + .find_map(|(manager, options)| { + classify_package_manager(words, manager, options, allowed_roots, invocation_cwd) + }) +} + +fn classify_pip_install( + words: &[&ShellWord], + allowed_roots: &[String], + invocation_cwd: &Path, +) -> Option { + const OPTIONS: &[PathOption] = &[ + PathOption { + long: "--target", + short: Some("-t"), + attached_short: false, + }, + PathOption { + long: "--prefix", + short: None, + attached_short: false, + }, + PathOption { + long: "--root", + short: None, + attached_short: false, + }, + PathOption { + long: "--src", + short: None, + attached_short: false, + }, + ]; + + let command = command_position(words, &["pip", "pip3"])?; + if !has_word(words, command + 1, &["install"]) { + return None; + } + if let Err(denial) = validate_path_options( + words, + command + 1, + OPTIONS, + PIP_REASON, + allowed_roots, + invocation_cwd, + ) { + return Some(denial); + } + if has_word(words, command + 1, &["--user"]) { + return Some(denial(PIP_REASON, Vec::new())); + } + cwd_denial(PIP_REASON, allowed_roots, invocation_cwd) +} + +fn assignment_target(word: &ShellWord, name: &str) -> Option { + word.value + .strip_prefix(&format!("{name}=")) + .map(|value| ShellWord { + value: value.to_string(), + dynamic: word.dynamic, + }) +} + +fn classify_cargo( + words: &[&ShellWord], + allowed_roots: &[String], + invocation_cwd: &Path, +) -> Option { + const OPTIONS: &[PathOption] = &[ + PathOption { + long: "--target-dir", + short: None, + attached_short: false, + }, + PathOption { + long: "-C", + short: None, + attached_short: false, + }, + ]; + + let command = command_position(words, &["cargo"])?; + if !has_word(words, command + 1, &["build", "test"]) { + return None; + } + if let Some(denial) = cwd_denial(CARGO_REASON, allowed_roots, invocation_cwd) { + return Some(denial); + } + for word in &words[..command] { + for name in ["CARGO_TARGET_DIR", "CARGO_BUILD_TARGET_DIR"] { + if let Some(target) = assignment_target(word, name) + && let Some(denial) = + target_denial(CARGO_REASON, &target, allowed_roots, invocation_cwd) + { + return Some(denial); + } + } + } + validate_path_options( + words, + command + 1, + OPTIONS, + CARGO_REASON, + allowed_roots, + invocation_cwd, + ) + .err() +} + +fn classify_sed( + words: &[&ShellWord], + allowed_roots: &[String], + invocation_cwd: &Path, +) -> Option { + let command = command_position(words, &["sed"])?; + let mut in_place = false; + let mut expression_option = false; + let mut positionals = Vec::new(); + let mut index = command + 1; + + while index < words.len() { + let word = words[index]; + match word.value.as_str() { + "-i" | "--in-place" => { + in_place = true; + if words + .get(index + 1) + .is_some_and(|next| next.value.is_empty()) + { + index += 1; + } + } + "-e" | "--expression" | "-f" | "--file" => { + expression_option = true; + index += usize::from(words.get(index + 1).is_some()); + } + value + if value.starts_with("-i") && value.len() > 2 + || value.starts_with("--in-place=") => + { + in_place = true; + } + value if (value.starts_with("-e") || value.starts_with("-f")) && value.len() > 2 => { + expression_option = true; + } + value if value.starts_with('-') => {} + _ => positionals.push(word), + } + index += 1; + } + if !in_place { + return None; + } + + let targets = if expression_option { + positionals.as_slice() + } else { + positionals.get(1..).unwrap_or_default() + }; + targets + .iter() + .find_map(|target| target_denial(SED_REASON, target, allowed_roots, invocation_cwd)) +} + +fn classify_segment( + words: &[&ShellWord], + allowed_roots: &[String], + invocation_cwd: &Path, +) -> Option { + classify_package_install(words, allowed_roots, invocation_cwd) + .or_else(|| classify_pip_install(words, allowed_roots, invocation_cwd)) + .or_else(|| classify_cargo(words, allowed_roots, invocation_cwd)) + .or_else(|| classify_sed(words, allowed_roots, invocation_cwd)) +} + +/// Whether this segment is one of the development mutations whose in-bounds +/// execution still requires an explicit command-policy allowance. +pub(super) fn segment_is_recognized(words: &[ShellWord]) -> bool { + let words: Vec<&ShellWord> = words.iter().collect(); + + let package = ["npm", "pnpm", "yarn", "bun"].iter().any(|manager| { + command_position(&words, &[*manager]).is_some_and(|command| { + words + .iter() + .skip(command + 1) + .take_while(|word| word.value != "--") + .any(|word| ["install", "add", "ci", "i"].contains(&word.value.as_str())) + }) + }); + let pip = command_position(&words, &["pip", "pip3"]) + .is_some_and(|command| has_word(&words, command + 1, &["install"])); + let cargo = command_position(&words, &["cargo"]) + .is_some_and(|command| has_word(&words, command + 1, &["build", "test"])); + let sed = command_position(&words, &["sed"]).is_some_and(|command| { + words.iter().skip(command + 1).any(|word| { + matches!(word.value.as_str(), "-i" | "--in-place") + || word.value.starts_with("--in-place=") + || word.value.starts_with("-i") && word.value.len() > 2 + }) + }); + + package || pip || cargo || sed +} + +pub(super) fn classify_mutation_targets( + command: &str, + allowed_roots: &[String], + invocation_cwd: &Path, +) -> Option { + let lexed = lex_shell(command); + let mut segment = Vec::new(); + for token in &lexed.tokens { + match token { + ShellToken::Word(word) => segment.push(word), + ShellToken::Pipe | ShellToken::Separator => { + if let Some(denial) = classify_segment(&segment, allowed_roots, invocation_cwd) { + return Some(denial); + } + segment.clear(); + } + ShellToken::OutputRedirect | ShellToken::FdDuplicate | ShellToken::InputRedirect => {} + } + } + classify_segment(&segment, allowed_roots, invocation_cwd).or_else(|| { + // A broken quote or escape can turn an explicit destination into a + // misleading partial literal. Only fail closed when the malformed + // segment is otherwise recognizable as one of the mutations this + // module owns; unrelated malformed shell remains outside this + // intentionally narrow heuristic. + lexed + .malformed + .then(|| classify_segment(&segment, &[], invocation_cwd)) + .flatten() + .map(|classification| denial(classification.reason, Vec::new())) + }) +} diff --git a/src/sandbox/policy.rs b/src/sandbox/policy.rs index b0ba04e..4799c67 100644 --- a/src/sandbox/policy.rs +++ b/src/sandbox/policy.rs @@ -7,9 +7,7 @@ //! ([`all_tool_vocabulary`]), so no harness's tool naming is hardcoded here. use std::path::{Component, Path, PathBuf}; -use std::sync::LazyLock; -use regex::Regex; use serde_json::Value; use crate::adapters::all_tool_vocabulary; @@ -41,60 +39,6 @@ pub fn is_shell_tool(tool_name: &str) -> bool { .any(|t| t == tool_name) } -/// Bash command patterns that mutate state outside an eval's sandbox. Heuristics -/// — Bash is too flexible to parse exactly. `detect-stray-writes` surfaces these -/// as warnings; the opt-in guard denies them. Each is meaningful only when the -/// command does not reference an allowed root (see [`classify_bash`]). -/// -/// Output redirects and `tee` are intentionally absent. The quote-aware target -/// scanner resolves relative paths from the tool invocation cwd instead of -/// relying on command-text containment. -/// -/// Compiled once. The patterns are known-valid, so a compile failure here is a -/// programmer error and panics. -static BASH_MUTATION_PATTERNS: LazyLock> = LazyLock::new(|| { - let config_dirs = crate::adapters::all_config_dir_names() - .iter() - .map(|d| regex::escape(d)) - .collect::>() - .join("|"); - [ - ( - r"\b(npm|pnpm|yarn|bun)\s+(install|add|ci|i)\b".to_string(), - "package install/add", - ), - (r"\bpip3?\s+install\b".to_string(), "pip install"), - (r"\bsed\s+-i\b".to_string(), "in-place file edit (sed -i)"), - // A create/copy/move/link verb whose operand is a path under any - // harness config dir (`adapters::all_config_dir_names`) — catches - // stray writes to a config dir that aren't a `>` redirect (caught - // below). Read-only verbs (`cat`, `ls`) aren't listed, so inspecting - // the dirs stays allowed. - ( - format!(r"\b(cp|mv|mkdir|touch|ln|rsync|install)\b[^|;&\n]*({config_dirs})(/|\b)"), - "path under a harness config dir", - ), - // The same create verbs whose operand is a top-level `skills/` directory — - // catches a bare `skills/` left in the cwd. `skills-data` and other - // `skills`-prefixed names are excluded by the trailing `/`, whitespace, or - // end-of-string boundary. - ( - r#"\b(cp|mv|mkdir|touch|ln|rsync)\b[^|;&\n]*[\s'"=/]\.{0,2}/?skills(/|\s|$)"# - .to_string(), - "creates a bare skills/ dir", - ), - ] - .into_iter() - .map(|(re, reason)| { - ( - Regex::new(&re) - .unwrap_or_else(|e| panic!("bundled bash pattern {re:?} is invalid: {e}")), - reason, - ) - }) - .collect() -}); - /// Pull the target path from a write tool's arguments (`file_path` → /// `notebook_path` → `path` → `filePath`, the last being OpenCode's camelCase /// spelling). Returns `None` when the input is not an object or carries no @@ -245,10 +189,34 @@ pub(crate) fn classify_bash_with_cwd( command: &str, allowed_roots: &[String], invocation_cwd: &Path, +) -> Option { + classify_bash_with_policy( + command, + allowed_roots, + invocation_cwd, + &crate::core::GuardPolicyConfig::default(), + ) +} + +/// Classify one shell tool call under its resolved eval command policy. +pub(crate) fn classify_bash_with_policy( + command: &str, + allowed_roots: &[String], + invocation_cwd: &Path, + policy: &crate::core::GuardPolicyConfig, ) -> Option { if command.is_empty() { return None; } + classify_fixed_containment(command, allowed_roots, invocation_cwd) + .or_else(|| super::command_policy::classify_command_policy(command, policy)) +} + +fn classify_fixed_containment( + command: &str, + allowed_roots: &[String], + invocation_cwd: &Path, +) -> Option { if let Some(denial) = super::shell_targets::classify_output_targets(command, allowed_roots, invocation_cwd) { @@ -259,23 +227,24 @@ pub(crate) fn classify_bash_with_cwd( { return Some(denial); } - if allowed_roots.iter().any(|r| command.contains(r)) { - return None; + if let Some(denial) = + super::mutation_targets::classify_mutation_targets(command, allowed_roots, invocation_cwd) + { + return Some(denial); } - BASH_MUTATION_PATTERNS - .iter() - .find(|(re, _)| re.is_match(command)) - .map(|(_, reason)| BashClassification { - reason, - resolved_targets: Vec::new(), - }) + for script in super::command_policy::literal_shell_scripts(command) { + if let Some(denial) = classify_fixed_containment(&script, allowed_roots, invocation_cwd) { + return Some(denial); + } + } + None } -/// If a Bash command matches a mutation pattern and is not scoped to one of -/// `allowed_roots`, return the human reason; otherwise `None`. A command is -/// treated as scoped when it textually references an allowed root. Output-file -/// targets are the exception: they are resolved lexically from the process cwd -/// and every target must fall under an allowed root. +/// Return the human reason when a Bash command has a recognized output, +/// repository, project, or mutation target that cannot be proven inside +/// `allowed_roots`; otherwise return `None`. Relative targets and commands with +/// an implicit destination resolve from the process cwd. Hook and audit callers +/// use [`classify_bash_with_cwd`] with the invocation cwd instead. pub fn classify_bash(command: &str, allowed_roots: &[String]) -> Option<&'static str> { let cwd = std::env::current_dir().unwrap_or_default(); classify_bash_with_cwd(command, allowed_roots, &cwd).map(|result| result.reason) @@ -286,6 +255,8 @@ mod tests { use super::*; use serde_json::json; + mod command_policy; + const ROOTS: [&str; 2] = ["/work/.eval-magic", "/work/.claude/skills"]; fn roots() -> Vec { @@ -459,18 +430,50 @@ mod tests { } #[test] - fn classify_bash_flags_installs_and_git_worktree_escape() { + fn classify_bash_flags_targets_outside_allowed_roots() { + let cwd = Path::new("/outside/project"); assert_eq!( - classify_bash("npm install left-pad", &roots()), - Some("package install/add") + classify_bash_with_cwd("npm install left-pad", &roots(), cwd).map(|d| d.reason), + Some("package install/add"), ); assert_eq!( - classify_bash("git worktree add ../wt -b scratch", &roots()), - Some("git worktree add (working tree outside the sandbox)") + classify_bash_with_cwd("git worktree add ../wt -b scratch", &roots(), cwd) + .map(|d| d.reason), + Some("git worktree add (working tree outside the sandbox)"), ); assert_eq!( - classify_bash("echo hi > out.log", &roots()), - Some("output redirection to a file") + classify_bash_with_cwd("echo hi > out.log", &roots(), cwd).map(|d| d.reason), + Some("output redirection to a file"), + ); + } + + #[test] + fn classify_bash_denies_package_install_with_an_outside_destination() { + let roots = vec!["/work/env".to_string()]; + + let denial = classify_bash_with_cwd( + "npm install left-pad --prefix /outside/project", + &roots, + Path::new("/work/env"), + ) + .expect("outside package destination should be denied"); + + assert_eq!(denial.reason, "package install/add"); + assert_eq!(denial.resolved_targets, vec!["/outside/project"]); + + let broad_policy = crate::core::GuardPolicyConfig { + allow_tools: vec!["npm".to_string()], + ..crate::core::GuardPolicyConfig::default() + }; + assert_eq!( + classify_bash_with_policy( + "npm install left-pad --prefix /outside/project", + &roots, + Path::new("/work/env"), + &broad_policy, + ) + .map(|classification| classification.reason), + Some("package install/add") ); } @@ -569,30 +572,34 @@ mod tests { } #[test] - fn classify_bash_flags_creates_under_every_harness_config_dir_but_allows_reads() { + fn classify_bash_does_not_special_case_harness_config_dirs() { + let cwd = Path::new("/work/.eval-magic/task"); for dir in crate::adapters::all_config_dir_names() { assert_eq!( - classify_bash(&format!("mkdir -p {dir}/x"), &[]), - Some("path under a harness config dir"), - "mkdir under {dir} should be flagged" + classify_bash_with_cwd(&format!("mkdir -p {dir}/x"), &roots(), cwd), + None, + "mkdir under {dir} should be allowed" ); assert_eq!( - classify_bash(&format!("cp evil.json {dir}/hooks.json"), &[]), - Some("path under a harness config dir"), - "cp into {dir} should be flagged" + classify_bash_with_cwd(&format!("cp hooks.json {dir}/hooks.json"), &roots(), cwd), + None, + "cp into {dir} should be allowed" ); assert_eq!( - classify_bash(&format!("cat {dir}/settings.json"), &[]), + classify_bash_with_cwd(&format!("cat {dir}/settings.json"), &roots(), cwd), None, "read of {dir} should stay allowed" ); - assert_eq!(classify_bash(&format!("ls {dir}"), &[]), None); + assert_eq!( + classify_bash_with_cwd(&format!("ls {dir}"), &roots(), cwd), + None + ); } } #[test] - fn classify_bash_allows_scoped_and_readonly_commands() { - // Textually references an allowed root → scoped → allowed. + fn classify_bash_allows_in_bounds_outputs_and_readonly_commands() { + // The redirect target resolves under an allowed root. assert_eq!( classify_bash("echo hi > /work/.eval-magic/x/log", &roots()), None diff --git a/src/sandbox/policy/tests/command_policy.rs b/src/sandbox/policy/tests/command_policy.rs new file mode 100644 index 0000000..9ece762 --- /dev/null +++ b/src/sandbox/policy/tests/command_policy.rs @@ -0,0 +1,156 @@ +use super::*; + +#[test] +fn allows_configured_package_install_from_an_allowed_cwd() { + let roots = vec!["/work/env".to_string()]; + let policy = crate::core::GuardPolicyConfig { + allow_commands: vec!["npm install".to_string()], + ..crate::core::GuardPolicyConfig::default() + }; + + assert_eq!( + classify_bash_with_policy( + "npm install left-pad", + &roots, + Path::new("/work/env"), + &policy, + ), + None + ); +} + +#[test] +fn allows_only_matching_prefixes_for_a_claimed_tool() { + let roots = vec!["/work/env".to_string()]; + let policy = crate::core::GuardPolicyConfig { + profiles: Vec::new(), + allow_tools: Vec::new(), + allow_commands: vec!["npm run dev".to_string()], + }; + + assert_eq!( + classify_bash_with_policy( + "npm run dev -- --host 127.0.0.1", + &roots, + Path::new("/work/env"), + &policy, + ), + None + ); + for command in [ + "npm install", + "npm $SUBCOMMAND", + "npm run dev 'unterminated", + ] { + assert_eq!( + classify_bash_with_policy(command, &roots, Path::new("/work/env"), &policy) + .map(|denial| denial.reason), + Some("command not allowed by eval guard policy"), + "{command}", + ); + } + assert_eq!( + classify_bash_with_policy("cargo metadata", &roots, Path::new("/work/env"), &policy,), + None + ); +} + +#[test] +fn recognized_development_mutations_require_an_allowance() { + let roots = vec!["/work/env".to_string()]; + let policy = crate::core::GuardPolicyConfig::default(); + + for command in [ + "npm install", + "pip install -r requirements.txt", + "cargo build", + "npx next dev", + "python -m pytest", + "sed -i 's/old/new/' src/lib.rs", + ] { + assert_eq!( + classify_bash_with_policy(command, &roots, Path::new("/work/env"), &policy) + .map(|denial| denial.reason), + Some("command not allowed by eval guard policy"), + "{command}", + ); + } +} + +#[test] +fn matches_wrapped_commands_and_each_compound_segment() { + let roots = vec!["/work/env".to_string()]; + let policy = crate::core::GuardPolicyConfig { + allow_commands: vec!["cargo test".to_string(), "npm run dev".to_string()], + ..crate::core::GuardPolicyConfig::default() + }; + + for command in [ + "MODE=ci /usr/bin/cargo test --workspace", + "env MODE=ci cargo test", + "command cargo test", + "exec cargo test", + "exec -a worker cargo test", + "nice -n 5 cargo test", + "timeout 30s cargo test", + "sh -c 'cargo test --workspace'", + ] { + assert_eq!( + classify_bash_with_policy(command, &roots, Path::new("/work/env"), &policy), + None, + "{command}", + ); + } + + for command in [ + "npm run dev && npm install", + "sh -c 'npm run dev && npm install'", + "exec -a worker npm install", + ] { + assert_eq!( + classify_bash_with_policy(command, &roots, Path::new("/work/env"), &policy) + .map(|denial| denial.reason), + Some("command not allowed by eval guard policy"), + "{command}", + ); + } +} + +#[test] +fn allow_tools_applies_to_a_shell_wrapper_itself() { + let roots = vec!["/work/env".to_string()]; + let policy = crate::core::GuardPolicyConfig { + allow_tools: vec!["sh".to_string()], + ..crate::core::GuardPolicyConfig::default() + }; + + assert_eq!( + classify_bash_with_policy( + "sh -c 'npm install'", + &roots, + Path::new("/work/env"), + &policy, + ), + None + ); +} + +#[test] +fn shell_wrapper_allowance_cannot_bypass_fixed_containment() { + let roots = vec!["/work/env".to_string()]; + let policy = crate::core::GuardPolicyConfig { + allow_tools: vec!["sh".to_string()], + ..crate::core::GuardPolicyConfig::default() + }; + + assert_eq!( + classify_bash_with_policy( + "sh -c 'cargo build --target-dir /outside/target'", + &roots, + Path::new("/work/env"), + &policy, + ) + .map(|classification| classification.reason), + Some("cargo build/test output") + ); +} diff --git a/src/validation/evals.rs b/src/validation/evals.rs index ad5abc9..b24cce6 100644 --- a/src/validation/evals.rs +++ b/src/validation/evals.rs @@ -8,6 +8,8 @@ use regex::Regex; use serde_json::Value; use crate::core::{Assertion, DeliverWhen, EvalsConfig}; +use crate::sandbox::command_policy::validate_policy_syntax; +use crate::sandbox::guard_profiles::has_profile; use crate::validation::error::ValidationError; use crate::validation::schema::{SchemaName, validate_against_schema}; @@ -19,6 +21,10 @@ pub fn validate_evals_config(config: &Value, source: &str) -> Result Result Result Result<(), ValidationError> { + for profile in &policy.profiles { + if !has_profile(profile) { + return Err(ValidationError::InvalidConfig { + path: source.to_string(), + message: format!("{label}: unknown guard profile {profile:?}"), + }); + } + } + validate_policy_syntax(policy).map_err(|message| ValidationError::InvalidConfig { + path: source.to_string(), + message: format!("{label}: {message}"), + }) +} + /// Name what is wrong with a `codebase` block before the schema reports only /// that it matched neither `oneOf` branch. /// diff --git a/src/validation/evals_guard_tests.rs b/src/validation/evals_guard_tests.rs new file mode 100644 index 0000000..5f94bd9 --- /dev/null +++ b/src/validation/evals_guard_tests.rs @@ -0,0 +1,63 @@ +use serde_json::{Value, json}; + +use super::evals::validate_evals_config; + +fn base() -> Value { + json!({ + "skill_name": "demo", + "evals": [{ + "id": "e1", + "prompt": "do the thing", + "expected_output": "the thing is done" + }] + }) +} + +#[test] +fn eval_guard_policy_replaces_the_config_default() { + let mut config = base(); + config["guard"] = json!({ + "profiles": ["language/rust"], + "allow_commands": ["cargo test"] + }); + config["evals"][0]["guard"] = json!({ + "allow_tools": ["cargo"], + "allow_commands": ["npm run dev"] + }); + + let parsed = validate_evals_config(&config, "evals.json").unwrap(); + let policy = parsed.guard_for(&parsed.evals[0]).unwrap(); + + assert!(policy.profiles.is_empty()); + assert_eq!(policy.allow_tools, ["cargo"]); + assert_eq!(policy.allow_commands, ["npm run dev"]); +} + +#[test] +fn rejects_unknown_guard_profiles() { + let mut config = base(); + config["guard"] = json!({ "profiles": ["framework/imaginary"] }); + + let error = validate_evals_config(&config, "evals.json") + .unwrap_err() + .to_string(); + + assert!(error.contains("framework/imaginary"), "error was: {error}"); +} + +#[test] +fn rejects_dynamic_or_compound_guard_command_rules() { + for rule in ["cargo $ACTION", "npm test && curl example.com"] { + let mut config = base(); + config["evals"][0]["guard"] = json!({ "allow_commands": [rule] }); + + let error = validate_evals_config(&config, "evals.json") + .unwrap_err() + .to_string(); + + assert!( + error.contains("allow_commands"), + "error for {rule:?}: {error}" + ); + } +} diff --git a/src/validation/mod.rs b/src/validation/mod.rs index b4238f4..24ae285 100644 --- a/src/validation/mod.rs +++ b/src/validation/mod.rs @@ -6,6 +6,8 @@ pub mod batch; pub mod error; pub mod evals; +#[cfg(test)] +mod evals_guard_tests; pub mod schema; pub use batch::{FileOutcome, ValidationReport, validate_all, validate_one}; diff --git a/tests/cli/docs.rs b/tests/cli/docs.rs index 099a3e5..9474d5c 100644 --- a/tests/cli/docs.rs +++ b/tests/cli/docs.rs @@ -189,6 +189,28 @@ fn docs_codebase_keeps_the_declaration_rules_caveat_and_provisioning_contract() .stdout(contains(".gitignore")); } +#[test] +fn docs_guard_keeps_configuration_defaults_and_boundary_contracts() { + skill_eval() + .args(["docs", "guard"]) + .assert() + .success() + .stdout(contains("# Configuring guarded commands")) + .stdout(contains("allow_tools")) + .stdout(contains("allow_commands")) + .stdout(contains("language/rust")) + .stdout(contains("framework/nextjs")) + .stdout(contains("replaces")) + .stdout(contains("dispatch.json")) + .stdout(contains("cannot override")); + + skill_eval() + .args(["run", "--help"]) + .assert() + .success() + .stdout(contains("eval-magic docs guard")); +} + #[test] fn shipped_guides_do_not_depend_on_repository_relative_links() { for (topic, _, body, path) in guide_sources() { diff --git a/tests/cli/guard.rs b/tests/cli/guard.rs index a09f0a8..b03fce0 100644 --- a/tests/cli/guard.rs +++ b/tests/cli/guard.rs @@ -6,6 +6,8 @@ use predicates::str::contains; use std::fs; use tempfile::TempDir; +mod development_tests; + /// The internal `guard` hook entry point is hidden from `--help` (its unique /// description never appears) yet remains callable. #[test] @@ -132,13 +134,20 @@ fn guard_allows_fd_duplication_and_the_null_device() { #[test] fn guard_codex_subcommand_blocks_with_codex_verdict_shape() { let tmp = TempDir::new().unwrap(); - let marker = write_codex_armed_marker(tmp.path(), &tmp.path().join(".eval-magic")); + let workspace = tmp.path().join(".eval-magic"); + fs::create_dir_all(&workspace).unwrap(); + let marker = write_codex_armed_marker(tmp.path(), &workspace); skill_eval() .arg("guard-codex") .arg(&marker) .write_stdin( - r#"{ "tool_name": "Bash", "tool_input": { "command": "npm install left-pad" } }"#, + serde_json::json!({ + "tool_name": "Bash", + "cwd": workspace, + "tool_input": { "command": "npm install --prefix /outside left-pad" }, + }) + .to_string(), ) .assert() .success() @@ -189,7 +198,7 @@ fn guard_codex_block_verdict_bytes_are_stable() { .arg("guard-codex") .arg(&marker) .write_stdin( - r#"{ "tool_name": "Bash", "tool_input": { "command": "npm install left-pad" } }"#, + r#"{ "tool_name": "Bash", "cwd": "/work/env", "tool_input": { "command": "npm install --prefix /outside left-pad" } }"#, ) .assert() .success() @@ -233,7 +242,12 @@ fn guard_hook_resolves_the_harness_verdict_shape() { .args(["guard-hook", "--harness", "codex"]) .arg(&marker) .write_stdin( - r#"{ "tool_name": "Bash", "tool_input": { "command": "npm install left-pad" } }"#, + serde_json::json!({ + "tool_name": "Bash", + "cwd": tmp.path().join(".eval-magic"), + "tool_input": { "command": "npm install --prefix /outside left-pad" }, + }) + .to_string(), ) .assert() .success() @@ -293,22 +307,31 @@ fn guard_hook_opencode_round_trips_write_verdicts() { .stdout(""); } -/// The plugin file itself is protected: a bash call mutating anything under -/// `.opencode` trips the config-dir tamper rule. +/// Harness config directories are ordinary paths inside the isolated env; the +/// guard does not special-case a Bash command that works there. #[test] -fn guard_hook_opencode_blocks_bash_tampering_with_the_plugin() { +fn guard_hook_opencode_allows_bash_work_inside_the_environment() { let tmp = TempDir::new().unwrap(); - let marker = write_opencode_armed_marker(tmp.path(), &tmp.path().join(".eval-magic")); + let workspace = tmp.path().join(".eval-magic"); + fs::create_dir_all(&workspace).unwrap(); + let marker = write_opencode_armed_marker(tmp.path(), &workspace); skill_eval() .args(["guard-hook", "--harness", "opencode"]) .arg(&marker) .write_stdin( - r#"{ "tool_name": "bash", "tool_input": { "command": "touch .opencode/plugins/slow-powers-eval-guard.js" } }"#, + serde_json::json!({ + "tool_name": "bash", + "cwd": workspace, + "tool_input": { + "command": "touch .opencode/plugins/slow-powers-eval-guard.js" + }, + }) + .to_string(), ) .assert() .success() - .stdout(contains(r#""decision":"block""#)); + .stdout(""); } /// Byte-pin of the OpenCode block verdict — same compatibility contract as diff --git a/tests/cli/guard/development_tests.rs b/tests/cli/guard/development_tests.rs new file mode 100644 index 0000000..369d49a --- /dev/null +++ b/tests/cli/guard/development_tests.rs @@ -0,0 +1,38 @@ +use super::*; + +#[test] +fn guard_allows_configured_development_tools_from_the_environment() { + let tmp = TempDir::new().unwrap(); + let workspace = tmp.path().join(".eval-magic"); + fs::create_dir_all(&workspace).unwrap(); + let marker = write_armed_marker(tmp.path(), &workspace); + let mut marker_value: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&marker).unwrap()).unwrap(); + marker_value["guardPolicy"] = serde_json::json!({ + "allow_tools": ["npm", "pip", "cargo", "sed"] + }); + fs::write(&marker, serde_json::to_string(&marker_value).unwrap()).unwrap(); + + for command in [ + "npm install", + "pip install -r requirements.txt", + "cargo build", + "npm test", + "sed -i 's/old/new/' src/lib.rs", + ] { + skill_eval() + .arg("guard") + .arg(&marker) + .write_stdin( + serde_json::json!({ + "tool_name": "Bash", + "cwd": workspace, + "tool_input": { "command": command }, + }) + .to_string(), + ) + .assert() + .success() + .stdout(""); + } +} diff --git a/tests/cli/stray_writes.rs b/tests/cli/stray_writes.rs index 5486556..55353a0 100644 --- a/tests/cli/stray_writes.rs +++ b/tests/cli/stray_writes.rs @@ -331,6 +331,7 @@ fn detect_stray_writes_uses_eval_root_boundary_from_dispatch() { "condition": "old_skill", "outputs_dir": outputs_dir.to_string_lossy(), "eval_root": eval_root.to_string_lossy(), + "guard_policy": { "allow_commands": ["cargo test"] }, } ], })) @@ -351,6 +352,7 @@ fn detect_stray_writes_uses_eval_root_boundary_from_dispatch() { {"name": "Write", "args": {"file_path": output_artifact}, "ordinal": 0}, {"name": "Edit", "args": {"file_path": source_edit}, "ordinal": 1}, {"name": "Write", "args": {"file_path": stray}, "ordinal": 2}, + {"name": "Bash", "args": {"command": "cargo test --workspace"}, "ordinal": 3}, ], "total_tokens": null, "duration_ms": null, @@ -375,6 +377,7 @@ fn detect_stray_writes_uses_eval_root_boundary_from_dispatch() { serde_json::from_str(&fs::read_to_string(iteration_dir.join("stray-writes.json")).unwrap()) .unwrap(); assert_eq!(report["totals"]["violations"], json!(1)); + assert_eq!(report["totals"]["warnings"], json!(0)); assert_eq!(report["runs"].as_array().unwrap().len(), 1); assert_eq!(report["runs"][0]["violations"][0]["path"], json!(stray)); } diff --git a/tests/run/guard_policy.rs b/tests/run/guard_policy.rs new file mode 100644 index 0000000..e49e7b7 --- /dev/null +++ b/tests/run/guard_policy.rs @@ -0,0 +1,107 @@ +//! End-to-end guard-policy resolution and artifact freezing. + +use std::fs; + +use crate::helpers::*; + +#[test] +fn automatic_profiles_compose_and_are_frozen_into_dispatch_and_marker() { + let tmp = tempfile::TempDir::new().unwrap(); + let evals = r#"{ + "skill_name": "mr-review", + "evals": [{ + "id": "e1", + "prompt": "build the app", + "expected_output": "built", + "files": ["frontend/package.json", "backend/pyproject.toml"] + }] + }"#; + let (skill_dir, cwd) = setup(tmp.path(), evals); + let eval_dir = skill_dir.join("mr-review/evals"); + fs::create_dir_all(eval_dir.join("frontend")).unwrap(); + fs::write( + eval_dir.join("frontend/package.json"), + r#"{"dependencies":{"next":"15.0.0"}}"#, + ) + .unwrap(); + fs::create_dir_all(eval_dir.join("backend")).unwrap(); + fs::write(eval_dir.join("backend/pyproject.toml"), "").unwrap(); + + skill_eval() + .current_dir(&cwd) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args(["--skill", "mr-review", "--mode", "new-skill", "--guard"]) + .assert() + .success(); + + let dispatch = read_json(&iteration_dir(&cwd).join("dispatch.json")); + let policy = &dispatch["tasks"][0]["guard_policy"]; + assert_eq!( + policy["profiles"], + serde_json::json!(["framework/nextjs", "language/javascript", "language/python"]) + ); + assert!( + policy["allow_commands"] + .as_array() + .unwrap() + .iter() + .any(|command| command == "npm run dev") + ); + assert!( + policy["allow_commands"] + .as_array() + .unwrap() + .iter() + .any(|command| command == "python -m pytest") + ); + + let marker = read_json( + &cli_env_dir(&cwd, "g1", "with_skill").join(".claude/skills/.slow-powers-eval-guard.json"), + ); + assert_eq!(marker["guardPolicy"], *policy); +} + +#[test] +fn per_eval_guard_replaces_the_default_and_disables_detection() { + let tmp = tempfile::TempDir::new().unwrap(); + let evals = r#"{ + "skill_name": "mr-review", + "guard": { "profiles": ["language/rust"] }, + "evals": [{ + "id": "e1", + "prompt": "serve the app", + "expected_output": "served", + "files": ["package.json"], + "guard": { "allow_commands": ["npm run dev"] } + }] + }"#; + let (skill_dir, cwd) = setup(tmp.path(), evals); + fs::write( + skill_dir.join("mr-review/evals/package.json"), + r#"{"dependencies":{"next":"15.0.0"}}"#, + ) + .unwrap(); + + skill_eval() + .current_dir(&cwd) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args([ + "--skill", + "mr-review", + "--mode", + "new-skill", + "--dry-run", + "--no-guard", + ]) + .assert() + .success(); + + let dispatch = read_json(&iteration_dir(&cwd).join("dispatch.json")); + let policy = &dispatch["tasks"][0]["guard_policy"]; + assert_eq!( + policy, + &serde_json::json!({ "allow_commands": ["npm run dev"] }) + ); +} diff --git a/tests/run/main.rs b/tests/run/main.rs index 0f8ed08..b960739 100644 --- a/tests/run/main.rs +++ b/tests/run/main.rs @@ -24,6 +24,7 @@ mod diff_scope; mod env_layout; mod git_isolation; mod grouping; +mod guard_policy; mod judges; mod lifecycle; mod opencode;