diff --git a/agent-shell/src/git_state.rs b/agent-shell/src/git_state.rs new file mode 100644 index 0000000..815de35 --- /dev/null +++ b/agent-shell/src/git_state.rs @@ -0,0 +1,209 @@ +//! HGW-2: git state that would run a program later, outside the sandbox. +//! +//! The sandbox lets a run write `.git/` inside its write roots, so `git init`, `git config` and +//! `git commit` work. A run can then leave git set up to start a program the next time the +//! member uses git in a terminal, where no sandbox applies: a hook, `core.hooksPath`, +//! `core.fsmonitor`, an alias that starts a shell, a diff or filter driver, an include. The +//! Seatbelt profile keeps hook files out of `.git/hooks`, but configuration cannot be filtered by +//! path. So the runner looks at the repositories a run could have touched before and after it, +//! and reports what is new ([`crate::RunReport::git_warnings`]) for the member to see. It reports, +//! it does not change anything. +//! +//! Honest scope: the repositories checked are the run's folder, its parents up to the write root +//! that holds it, and the folders directly inside the run's folder. A repository created deeper +//! than that is not checked. + +use std::collections::BTreeSet; +use std::path::{Path, PathBuf}; + +/// Most entries of the run's folder looked at for nested repositories. +const MAX_CHILDREN: usize = 256; +/// Largest `.git/config` read. +const MAX_CONFIG_BYTES: u64 = 256 * 1024; + +/// The repositories (their `.git` folders) a run in `cwd` with `write_roots` could touch. +pub(crate) fn repo_dirs(cwd: &Path, write_roots: &[PathBuf]) -> Vec { + let mut out: Vec = Vec::new(); + let mut push = |p: PathBuf| { + if !out.contains(&p) { + out.push(p); + } + }; + if let Some(root) = write_roots + .iter() + .filter(|r| cwd.starts_with(r)) + .max_by_key(|r| r.components().count()) + { + for dir in cwd.ancestors() { + push(dir.join(".git")); + if dir == root.as_path() { + break; + } + } + } + if let Ok(rd) = std::fs::read_dir(cwd) { + for e in rd.flatten().take(MAX_CHILDREN) { + if e.file_type().map(|t| t.is_dir()).unwrap_or(false) { + push(e.path().join(".git")); + } + } + } + out +} + +/// One line per risky item across `git_dirs` (sorted, without repeats). +pub(crate) fn risks(git_dirs: &[PathBuf]) -> BTreeSet { + let mut out = BTreeSet::new(); + for g in git_dirs { + let Ok(meta) = std::fs::symlink_metadata(g) else { + continue; + }; + if !meta.is_dir() { + continue; + } + let hooks = g.join("hooks"); + match std::fs::symlink_metadata(&hooks) { + Ok(m) if m.file_type().is_symlink() => { + out.insert(format!("{} is a symbolic link", hooks.display())); + } + Ok(m) if m.is_dir() => { + if let Ok(rd) = std::fs::read_dir(&hooks) { + for e in rd.flatten() { + let name = e.file_name().to_string_lossy().into_owned(); + if !name.ends_with(".sample") { + out.insert(format!( + "git hook {name} in {} (git runs it outside the sandbox)", + hooks.display() + )); + } + } + } + } + _ => {} + } + let config = g.join("config"); + if let Ok(m) = std::fs::symlink_metadata(&config) { + if m.file_type().is_symlink() { + out.insert(format!("{} is a symbolic link", config.display())); + } else if m.len() <= MAX_CONFIG_BYTES { + if let Ok(text) = std::fs::read_to_string(&config) { + for (key, value) in risky_config(&text) { + out.insert(format!( + "git config {key} = {value} in {} (git may run a program from it)", + config.display() + )); + } + } + } else { + out.insert(format!("{} is too large to check", config.display())); + } + } + } + out +} + +/// `(section.key, value)` pairs in a git config text that make git start a program. Keys are +/// lowercased (git's own rule for section and key names; a subsection keeps its case). +pub(crate) fn risky_config(text: &str) -> Vec<(String, String)> { + let mut out = Vec::new(); + let mut section = String::new(); + for raw in text.lines() { + let line = raw.trim(); + if line.is_empty() || line.starts_with('#') || line.starts_with(';') { + continue; + } + if let Some(rest) = line.strip_prefix('[') { + let inner = rest.split(']').next().unwrap_or("").trim(); + section = match inner.split_once(char::is_whitespace) { + Some((name, sub)) => { + format!( + "{}.{}", + name.to_ascii_lowercase(), + sub.trim().trim_matches('"') + ) + } + None => inner.to_ascii_lowercase(), + }; + continue; + } + let (k, v) = match line.split_once('=') { + Some((k, v)) => (k.trim(), v.trim().trim_matches('"')), + None => (line, "true"), + }; + let key = format!("{section}.{}", k.to_ascii_lowercase()); + if is_risky(&key, v) { + out.push((key, v.to_string())); + } + } + out +} + +fn is_risky(key: &str, value: &str) -> bool { + let lower = key.to_ascii_lowercase(); + let (first, rest) = lower.split_once('.').unwrap_or((lower.as_str(), "")); + let last = rest.rsplit('.').next().unwrap_or(rest); + let off = matches!( + value.to_ascii_lowercase().as_str(), + "" | "false" | "0" | "no" | "off" + ); + match first { + "core" => match last { + "fsmonitor" => !off, + "hookspath" | "sshcommand" | "gitproxy" | "editor" | "pager" | "askpass" => true, + _ => false, + }, + "alias" => value.trim_start().starts_with('!'), + "sequence" => last == "editor", + "credential" => last == "helper", + "gpg" => last == "program", + "diff" => matches!(last, "external" | "command" | "textconv"), + "filter" => matches!(last, "clean" | "smudge" | "process"), + "merge" => last == "driver", + "include" | "includeif" => last == "path", + "uploadpack" => last == "packobjectshook", + "remote" => matches!(last, "uploadpack" | "receivepack"), + "pager" | "browser" | "man" | "difftool" | "mergetool" => { + matches!(last, "cmd" | "path") || first == "pager" + } + _ => false, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn risky_keys_are_found_and_harmless_ones_are_not() { + let text = "[core]\n\trepositoryformatversion = 0\n\tfsmonitor = false\n\thooksPath = .h\n[alias]\n\tst = status\n\tx = !sh -c id\n[filter \"lfs\"]\n\tsmudge = git-lfs smudge %f\n[remote \"origin\"]\n\turl = https://example.invalid/r.git\n[includeIf \"gitdir:~/\"]\n\tpath = ~/.evil\n"; + let keys: Vec = risky_config(text).into_iter().map(|(k, _)| k).collect(); + assert_eq!( + keys, + vec![ + "core.hookspath", + "alias.x", + "filter.lfs.smudge", + "includeif.gitdir:~/.path" + ] + ); + assert!(risky_config("[core]\n\tfsmonitor = true\n") + .iter() + .any(|(k, _)| k == "core.fsmonitor")); + } + + #[test] + fn repo_dirs_cover_the_cwd_its_parents_to_the_root_and_its_children() { + let d = tempfile::tempdir().expect("tmp"); + let root = d.path().join("root"); + std::fs::create_dir_all(root.join("a/b/c")).expect("dirs"); + let got = repo_dirs(&root.join("a/b"), std::slice::from_ref(&root)); + assert!(got.contains(&root.join("a/b/.git"))); + assert!(got.contains(&root.join("a/.git"))); + assert!(got.contains(&root.join(".git"))); + assert!(got.contains(&root.join("a/b/c/.git"))); + assert!( + !got.contains(&d.path().join(".git")), + "never above the write root" + ); + } +} diff --git a/agent-shell/src/lib.rs b/agent-shell/src/lib.rs index db61bf7..0ef78d6 100644 --- a/agent-shell/src/lib.rs +++ b/agent-shell/src/lib.rs @@ -75,6 +75,7 @@ //! That crate is async and tied to the legacy tool trait, so the logic is re-homed here as a //! synchronous, dependency-light library. +mod git_state; pub mod sandbox; #[cfg(windows)] mod win; @@ -847,6 +848,12 @@ pub struct RunReport { pub cleanup_error: Option, /// Whether (and how) the run was wrapped in the OS sandbox. pub sandbox: SandboxSummary, + /// HGW-2: git state the run left behind that would start a program the next time the member + /// uses git outside the sandbox (a hook, `core.hooksPath`, an alias that runs a shell, …), and + /// that was not there before the run. Reported, never changed. Empty (and not serialized) + /// when there is nothing new. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub git_warnings: Vec, } impl RunReport { @@ -1141,6 +1148,9 @@ impl ShellRunner { cmd.env(k, v); } + // HGW-2: what git would run later, before the run (only what is new is reported). + let repos = git_state::repo_dirs(&plan.cwd, &plan.write_roots); + let git_before = git_state::risks(&repos); let tree = ProcessTree::prepare().map_err(|e| ShellError::Spawn { program: req.program.clone(), reason: format!("cannot create the process tree container: {e}"), @@ -1199,6 +1209,10 @@ impl ShellRunner { ); } + let git_warnings: Vec = git_state::risks(&repos) + .difference(&git_before) + .cloned() + .collect(); Ok(RunReport { program: req.program.clone(), resolved_path: plan.resolved_path, @@ -1218,6 +1232,7 @@ impl ShellRunner { output_incomplete, cleanup_error, sandbox: plan.sandbox, + git_warnings, }) } } @@ -1794,6 +1809,7 @@ mod tests { output_incomplete: false, cleanup_error: Some("scratch removal failed".into()), sandbox: SandboxSummary::not_enforced("test", &[]), + git_warnings: Vec::new(), }; assert!(!report.passed()); diff --git a/agent-shell/src/sandbox.rs b/agent-shell/src/sandbox.rs index 72337fb..0232613 100644 --- a/agent-shell/src/sandbox.rs +++ b/agent-shell/src/sandbox.rs @@ -7,14 +7,19 @@ //! - **macOS: Seatbelt** through `/usr/bin/sandbox-exec`. The profile is generated per run //! ([`seatbelt_profile`]) and starts from `(deny default)`. It never allows a network //! operation. Every path is passed as a `-D` parameter, never spliced into the profile text, -//! so a folder name cannot change the policy. Writes to `.git/hooks`, `.git/config` and -//! `.env*` files are denied even inside a granted folder (a hook planted there would run later -//! outside the sandbox). +//! so a folder name cannot change the policy. Inside a granted folder `.git/` is writable +//! (HGW-2: `git init`, `git config` and `git commit` work), but a hook file other than git's own +//! `*.sample` files cannot be written, the hooks folder cannot be removed or moved, and a +//! symbolic link cannot be put in its place (a hook planted there would run later outside the +//! sandbox). `.env*` files are neither readable nor writable, except `.env.example` and +//! `.env.sample`. Configuration that makes git run +//! a program cannot be filtered by path, so the runner reports any it finds new after a run +//! (`RunReport::git_warnings`). //! - **Linux: bubblewrap** (`bwrap`, found only in fixed system directories). The run gets fresh //! namespaces with networking unshared (`--unshare-all`), read-only binds of the system and //! toolchain directories, writable binds of the granted folders and the scratch HOME, and -//! read-only masks over `.git/hooks`, `.git/config` and the `.env*` files that exist at the top -//! of a granted folder ([`bwrap_command`]). A Landlock fallback is not implemented: with no +//! read-only masks over `.git/hooks` and the `.env*` files that exist at the top of a granted +//! folder ([`bwrap_command`]). `.git/config` is writable (HGW-2), as on macOS. A Landlock fallback is not implemented: with no //! working `bwrap` the backend is unavailable. //! - **Windows**: the runner builds and uses a Job Object for bounded process cleanup, but has //! no OS sandbox backend. A Job Object is cleanup containment, not a filesystem, network, or @@ -364,9 +369,21 @@ pub fn seatbelt_profile(n_read: usize, n_write: usize) -> String { "(allow file-write-data (literal \"/dev/null\") (literal \"/dev/zero\") (literal \"/dev/dtracehelper\"))\n", ); p.push_str("(allow file-ioctl (literal \"/dev/dtracehelper\"))\n"); - // Later rules win: these hold even inside a granted folder. - p.push_str("(deny file-write* (regex #\"/\\.git/hooks(/|$)\") (regex #\"/\\.git/config$\"))\n"); - p.push_str("(deny file-read* file-write* (regex #\"/\\.env[^/]*$\"))\n"); + // Later rules win: these hold even inside a granted folder. HGW-2: `.git/` is writable (git + // init/config/commit), except a hook that is not one of git's `*.sample` files, removing or + // moving the hooks folder, and a symbolic link in its place. + p.push_str( + "(deny file-write* (require-all (regex #\"/\\.git/hooks/[^/]+$\") (require-not (regex #\"\\.sample$\"))))\n", + ); + p.push_str("(deny file-write-unlink (regex #\"/\\.git/hooks$\"))\n"); + p.push_str( + "(deny file-write-create (require-all (regex #\"/\\.git/hooks$\") (vnode-type SYMLINK)))\n", + ); + // `.env*` files are neither readable nor writable, except the templates' `.env.example` and + // `.env.sample` (HGW-W2: not secrets; agent-guard exempts them the same way). + p.push_str( + "(deny file-read* file-write* (require-all (regex #\"/\\.env[^/]*$\") (require-not (regex #\"/\\.env\\.(example|sample)$\"))))\n", + ); p } @@ -436,10 +453,8 @@ pub fn bwrap_command(spec: &SandboxSpec, program: &Path, args: &[String]) -> Vec ]); // Masks go after the writable binds, so they sit on top of them. for w in &spec.write_roots { - for sub in [".git/hooks", ".git/config"] { - let p = w.join(sub); - push(&["--ro-bind-try".as_ref(), p.as_os_str(), p.as_os_str()]); - } + let p = w.join(".git/hooks"); + push(&["--ro-bind-try".as_ref(), p.as_os_str(), p.as_os_str()]); } for f in &spec.masked_files { push(&["--ro-bind".as_ref(), "/dev/null".as_ref(), f.as_os_str()]); @@ -450,7 +465,8 @@ pub fn bwrap_command(spec: &SandboxSpec, program: &Path, args: &[String]) -> Vec a } -/// `.env*` files at the top of each write root (bwrap masks them; Seatbelt denies by pattern). +/// `.env*` files at the top of each write root (bwrap masks them; Seatbelt denies by pattern), +/// except the templates' `.env.example` and `.env.sample`. pub(crate) fn env_files(write_roots: &[PathBuf]) -> Vec { let mut out = Vec::new(); for w in write_roots { @@ -460,7 +476,8 @@ pub(crate) fn env_files(write_roots: &[PathBuf]) -> Vec { for e in rd.flatten() { let name = e.file_name().to_string_lossy().to_ascii_lowercase(); let is_file = e.file_type().map(|t| t.is_file()).unwrap_or(false); - if is_file && (name == ".env" || name.starts_with(".env.")) { + let template = name == ".env.example" || name == ".env.sample"; + if is_file && !template && (name == ".env" || name.starts_with(".env.")) { out.push(e.path()); } } @@ -557,6 +574,9 @@ mod tests { std::fs::write(root.join(".env"), "x").expect("w"); std::fs::write(root.join(".env.local"), "x").expect("w"); std::fs::write(root.join("env.txt"), "x").expect("w"); + // HGW-W2 (#75): templates' example files are not secrets (agent-guard exempts them too). + std::fs::write(root.join(".env.example"), "x").expect("w"); + std::fs::write(root.join(".env.sample"), "x").expect("w"); std::fs::create_dir(root.join(".env.d")).expect("d"); let got = env_files(std::slice::from_ref(&root)); assert_eq!(got, vec![root.join(".env"), root.join(".env.local")]); diff --git a/agent-shell/tests/sandbox.rs b/agent-shell/tests/sandbox.rs index 245201e..84f02cb 100644 --- a/agent-shell/tests/sandbox.rs +++ b/agent-shell/tests/sandbox.rs @@ -23,6 +23,7 @@ fn allow() -> Allowlist { .allow("sh", ArgPolicy::Any) .allow("curl", ArgPolicy::Any) .allow("echo", ArgPolicy::Any) + .allow("git", ArgPolicy::Any) } /// `base/grant` (the granted folder, with `sub/`), `base/other` (not granted). @@ -199,6 +200,12 @@ fn seatbelt_profile_denies_by_default_and_never_allows_network() { let deny_hooks = p.find(".git/hooks").expect("hooks deny"); assert!(deny_hooks > allow_w); assert!(p.contains("\\.env")); + // HGW-2: `.git/config` is writable inside a write root (git init and git config work); the + // hooks stay closed: no non-sample hook file, no removing or symlinking the hooks folder. + assert!(!p.contains(".git/config"), "{p}"); + assert!(p.contains("require-not"), "{p}"); + assert!(p.contains("\\.sample$"), "{p}"); + assert!(p.contains("vnode-type SYMLINK"), "{p}"); } #[test] @@ -237,6 +244,43 @@ fn bwrap_command_isolates_network_and_binds_only_the_grant_writable() { let bind_at = a.iter().position(|x| x == &g).expect("bind"); let mask_at = a.iter().position(|x| x == &env).expect("mask"); assert!(mask_at > bind_at); + // HGW-2: an existing hooks folder stays read-only; `.git/config` is writable (git config). + let hooks = fx.grant().join(".git/hooks").to_string_lossy().into_owned(); + assert!(has(&["--ro-bind-try", &hooks, &hooks]), "{a:?}"); + assert!(!a.iter().any(|x| x.ends_with(".git/config")), "{a:?}"); +} + +/// HGW-2: a run that leaves git in a state that would run a program the next time the member uses +/// git in a terminal (a hook, `core.hooksPath`, `core.fsmonitor`, an alias that runs a shell…) is +/// reported in `git_warnings`. What was already there before the run is not reported again. +#[test] +fn new_risky_git_state_is_reported_after_a_run() { + let fx = Fx::new(); + let repo = fx.grant().join("repo"); + std::fs::create_dir_all(repo.join(".git/hooks")).expect("repo"); + // Already there before the run (a member's own hook manager): not reported. + std::fs::write(repo.join(".git/hooks/pre-push"), "#!/bin/sh\n").expect("hook"); + std::fs::write(repo.join(".git/config"), "[core]\n\tbare = false\n").expect("config"); + let r = scoped(fx.grant(), SandboxPolicy::new(SandboxMode::Off)); + let cmd = "printf '[core]\\n\\thooksPath = .h\\n[alias]\\n\\tx = !sh -c id\\n' >> .git/config; echo '#!/bin/sh' > .git/hooks/post-checkout; echo ok"; + let rep = r.run(&sh(cmd, &repo)).expect("run"); + assert_eq!(rep.exit_code, Some(0), "{}", rep.stderr); + let w = rep.git_warnings.join("\n"); + assert!(w.contains("core.hookspath"), "{w}"); + assert!(w.contains("alias.x"), "{w}"); + assert!(w.contains("post-checkout"), "{w}"); + assert!( + !w.contains("pre-push"), + "pre-existing state is not reported: {w}" + ); + // A run that changes nothing about git reports nothing. + let rep = r.run(&sh("echo quiet", &repo)).expect("run"); + assert!(rep.git_warnings.is_empty(), "{:?}", rep.git_warnings); + let v = rep.to_json(); + assert!( + v.get("git_warnings").is_none(), + "empty warnings are not serialized: {v}" + ); } #[test] @@ -396,6 +440,87 @@ mod macos { } } + /// HGW-2: `git init`, `git add` and `git commit` work inside the grant under Seatbelt (they + /// write `.git/config` and the sample hooks), and nothing lands outside it. + #[test] + fn git_init_add_and_commit_work_inside_the_grant() { + let fx = Fx::new(); + let r = scoped(fx.grant(), required()); + let cmd = "git init -q . && echo hi > a.txt && git add a.txt && git -c user.name=Hermes -c user.email=hermes@example.invalid commit -q -m init && git config user.name Hermes && git log --oneline | wc -l"; + let rep = r.run(&sh(cmd, &fx.grant())).expect("run"); + assert!(rep.sandbox.enforced); + assert_eq!(rep.exit_code, Some(0), "stderr: {}", rep.stderr); + assert_eq!(rep.stdout.trim(), "1"); + assert!(fx.grant().join(".git/config").is_file()); + assert!(rep.git_warnings.is_empty(), "{:?}", rep.git_warnings); + } + + /// HGW-2: with `.git` writable, hooks stay closed: no hook file but samples, no renaming a + /// sample into a hook, no removing the hooks folder, no symlinking a new one in. + #[test] + fn git_hooks_stay_closed_when_git_is_writable() { + let fx = Fx::new(); + let r = scoped(fx.grant(), required()); + let rep = r.run(&sh("git init -q .", &fx.grant())).expect("init"); + assert_eq!(rep.exit_code, Some(0), "stderr: {}", rep.stderr); + let hooks = fx.grant().join(".git/hooks"); + for cmd in [ + "echo x > .git/hooks/pre-commit", + "mv .git/hooks/pre-commit.sample .git/hooks/pre-commit", + "cp .git/hooks/pre-commit.sample .git/hooks/post-merge", + ] { + let rep = r.run(&sh(cmd, &fx.grant())).expect("run"); + assert_ne!(rep.exit_code, Some(0), "{cmd}"); + } + assert!(!hooks.join("pre-commit").exists()); + assert!(!hooks.join("post-merge").exists()); + let _ = r.run(&sh("rm -rf .git/hooks", &fx.grant())).expect("rm"); + assert!(hooks.is_dir(), "the hooks folder cannot be removed"); + let _ = r + .run(&sh("mv .git/hooks .git/h2", &fx.grant())) + .expect("mv"); + assert!(hooks.is_dir(), "the hooks folder cannot be moved away"); + std::fs::create_dir_all(fx.grant().join("sub/fresh/.git")).expect("fresh"); + let rep = r + .run(&sh( + "mkdir -p ../../h && ln -s ../../h .git/hooks", + &fx.grant().join("sub/fresh"), + )) + .expect("ln"); + assert_ne!(rep.exit_code, Some(0)); + assert!(!fx.grant().join("sub/fresh/.git/hooks").exists()); + // The sample a fresh `git init` writes is fine. + assert!(hooks.join("pre-commit.sample").exists() || hooks.is_dir()); + } + + /// HGW-W2 (#75): `.env.example` and `.env.sample` (template files, not secrets) are readable; + /// `.env` and `.env.local` stay unreadable and unwritable. + #[test] + fn env_example_files_are_readable_and_real_env_files_are_not() { + let fx = Fx::new(); + for (name, body) in [ + (".env.example", "RPC_URL=\n"), + (".env.sample", "KEY=\n"), + (".env", "SECRET=1\n"), + (".env.local", "SECRET=2\n"), + ] { + std::fs::write(fx.grant().join(name), body).expect("seed"); + } + let r = scoped(fx.grant(), required()); + let rep = r + .run(&sh("cat .env.example .env.sample", &fx.grant())) + .expect("run"); + assert_eq!(rep.exit_code, Some(0), "{}", rep.stderr); + assert_eq!(rep.stdout, "RPC_URL=\nKEY=\n"); + for secret in [".env", ".env.local"] { + let rep = r + .run(&sh(&format!("cat {secret}"), &fx.grant())) + .expect("run"); + assert_ne!(rep.exit_code, Some(0), "{secret}"); + assert!(!rep.stdout.contains("SECRET"), "{secret}"); + } + } + #[test] fn plan_shows_the_sandbox_before_anything_runs() { let fx = Fx::new(); @@ -414,3 +539,79 @@ mod macos { ); } } + +// ------------------------------------------------------------------------------------------ +// Linux: the real thing (bubblewrap). Runs where bwrap can create a sandbox; the Linux CI lane +// sets CITRATE_REQUIRE_BWRAP=1 so a missing or broken bwrap fails instead of skipping. +// ------------------------------------------------------------------------------------------ + +#[cfg(target_os = "linux")] +mod linux { + use super::*; + + fn required() -> Option { + match citrate_agent_shell::sandbox::detect() { + Ok(_) => Some(SandboxPolicy::new(SandboxMode::Required)), + Err(why) => { + assert!( + std::env::var("CITRATE_REQUIRE_BWRAP").as_deref() != Ok("1"), + "CITRATE_REQUIRE_BWRAP=1 but bwrap is unusable here: {why}" + ); + eprintln!("skipping the bwrap run test: {why}"); + None + } + } + } + + #[test] + fn writes_outside_the_grant_are_denied_under_bwrap() { + let Some(p) = required() else { return }; + let fx = Fx::new(); + let r = scoped(fx.grant(), p); + let f = fx.other().join("escape.txt"); + let rep = r + .run(&sh(&format!("echo x > '{}'", f.display()), &fx.grant())) + .expect("run"); + assert!(rep.sandbox.enforced); + assert_ne!(rep.exit_code, Some(0)); + assert!(!f.exists()); + let ok = fx.grant().join("in.txt"); + let rep = r + .run(&sh(&format!("echo x > '{}'", ok.display()), &fx.grant())) + .expect("run"); + assert_eq!(rep.exit_code, Some(0), "{}", rep.stderr); + assert!(ok.exists()); + } + + #[test] + fn git_init_add_and_commit_work_inside_the_grant_under_bwrap() { + let Some(p) = required() else { return }; + if !Path::new("/usr/bin/git").exists() { + eprintln!("skipping: no /usr/bin/git"); + return; + } + let fx = Fx::new(); + let r = scoped(fx.grant(), p); + let cmd = "git init -q . && echo hi > a.txt && git add a.txt && git -c user.name=Hermes -c user.email=hermes@example.invalid commit -q -m init && git config user.name Hermes && git log --oneline | wc -l"; + let rep = r.run(&sh(cmd, &fx.grant())).expect("run"); + assert_eq!(rep.exit_code, Some(0), "stderr: {}", rep.stderr); + assert_eq!(rep.stdout.trim(), "1"); + } + + #[test] + fn network_is_unshared_under_bwrap() { + let Some(p) = required() else { return }; + let fx = Fx::new(); + let r = scoped(fx.grant(), p); + // Only a loopback interface exists in a fresh network namespace. + let rep = r + .run(&sh( + "cat /proc/net/dev | tail -n +3 | cut -d: -f1 | tr -d ' '", + &fx.grant(), + )) + .expect("run"); + assert_eq!(rep.exit_code, Some(0), "{}", rep.stderr); + let ifaces: Vec<&str> = rep.stdout.lines().filter(|l| !l.is_empty()).collect(); + assert_eq!(ifaces, vec!["lo"], "{ifaces:?}"); + } +} diff --git a/agent-shell/tests/windows.rs b/agent-shell/tests/windows.rs index 2969f0b..723ae13 100644 --- a/agent-shell/tests/windows.rs +++ b/agent-shell/tests/windows.rs @@ -287,6 +287,7 @@ fn cleanup_error_makes_serialized_report_fail() { readable_extra: Vec::new(), summary: "test".into(), }, + git_warnings: Vec::new(), }; assert!(!report.passed()); diff --git a/agent-sidecar/src/files.rs b/agent-sidecar/src/files.rs index 384c407..33b57e4 100644 --- a/agent-sidecar/src/files.rs +++ b/agent-sidecar/src/files.rs @@ -13,7 +13,10 @@ //! Every call runs in this order: //! //! 1. **Grant and deny list.** Build configuration (`foundry.toml`, env files and the rest of -//! `toolchain_config::build_config_file`) is refused first: the member edits it. Each path +//! `toolchain_config::build_config_file`) is refused first: the member edits it. HGW-2: +//! inside a session's project root (`SessionGrants::project_root_of`) build configuration +//! other than env files may be written; env files and anything inside a `.git` folder are +//! refused everywhere (git writes there, through the sandboxed `shell_run`). Each path //! then goes through [`FolderGrants::check`] for a write, which //! asks the agent-guard default-deny list first (credentials, keychains, browser profiles, //! wallet storage, app data, shell history) and then needs a live write grant covering the @@ -195,16 +198,14 @@ pub(crate) fn checked_whole_file_write( if !path.is_absolute() { return Err(Refusal::Failed("path must be absolute".into())); } - if let Some(name) = crate::toolchain_config::build_config_file(path) { - return Err(Refusal::Policy( - crate::toolchain_config::build_config_refusal(&name), - )); + // HGW-2: env files and `.git` internals are refused on the path as given, before any I/O; + // other build configuration waits for the resolved path (it is allowed in a project root). + if let Some(r) = config_refusal_as_given(path) { + return Err(Refusal::Policy(r)); } let (file, root) = grants.check_write(path).map_err(Refusal::Policy)?; - if let Some(name) = crate::toolchain_config::build_config_file(&file) { - return Err(Refusal::Policy( - crate::toolchain_config::build_config_refusal(&name), - )); + if let Some(r) = config_refusal_resolved(path, &file, grants.project_root_of(&file)) { + return Err(Refusal::Policy(r)); } if let Some(r) = leaf_refusal(&[path, &file]) { return Err(r); @@ -229,6 +230,27 @@ pub(crate) fn checked_whole_file_write( }) } +/// HGW-2: the refusal for a path as given (no I/O yet): `.git` internals and env files always; +/// other build configuration is decided on the resolved path. +fn config_refusal_as_given(path: &Path) -> Option { + crate::toolchain_config::git_internal_refusal(path) + .or_else(|| crate::toolchain_config::env_file_refusal(path)) +} + +/// HGW-2: the refusal once the path is resolved: `.git` internals, env files, and other build +/// configuration outside a project root, checked on the path as given and as resolved (the +/// project is the one holding the resolved path). +fn config_refusal_resolved( + given: &Path, + resolved: &Path, + project: Option, +) -> Option { + let project = project.as_deref(); + crate::toolchain_config::git_internal_refusal(resolved) + .or_else(|| crate::toolchain_config::build_config_write_refusal(given, project)) + .or_else(|| crate::toolchain_config::build_config_write_refusal(resolved, project)) +} + /// A refusal when a leaf in `paths` is a symbolic link, is not a regular file, or has other hard /// links (writing it could change a file outside the grant). A missing leaf is fine (a new file). /// The grant-session write tools pass the path as given and as resolved (they never follow a @@ -415,6 +437,15 @@ impl FileTools { ] } + /// HGW-2: the project root holding `canonical`, when the grants are a session's (only a + /// session has project roots). + fn project_of(&self, canonical: &Path) -> Option { + match &self.grants { + GrantSource::Session(g) => g.project_root_of(canonical), + GrantSource::File(_) | GrantSource::Fixed(_) => None, + } + } + /// The grant set to check against now, and the time to check it at. fn load_grants(&self) -> Result<(FolderGrants, u64), Refusal> { match &self.grants { @@ -445,23 +476,23 @@ impl FileTools { "{raw:?} is not an absolute path; use an absolute path inside a folder the member granted" ))); } - // Build configuration is the member's to edit: no fs tool creates, changes, moves or - // removes it (checked on the path as given and as resolved). - if let Some(name) = crate::toolchain_config::build_config_file(p) { - return Err(Refusal::Policy( - crate::toolchain_config::build_config_refusal(&name), - )); + // Build configuration is the member's to edit outside a project root (HGW-2): no fs tool + // creates, changes, moves or removes it there (checked on the path as given and as + // resolved). Env files and `.git` internals are refused everywhere. + if let Some(r) = config_refusal_as_given(p) { + return Err(Refusal::Policy(r)); } match grants.check(p, Op::Write, now) { Decision::Allowed { canonical, grant_id, } => { - if let Some(name) = crate::toolchain_config::build_config_file(canonical.as_path()) - { - return Err(Refusal::Policy( - crate::toolchain_config::build_config_refusal(&name), - )); + if let Some(r) = config_refusal_resolved( + p, + canonical.as_path(), + self.project_of(canonical.as_path()), + ) { + return Err(Refusal::Policy(r)); } // A hard link can name a file kept outside the grant (the deny list is // path-based): no fs tool reads, snapshots, edits, moves or removes one. diff --git a/agent-sidecar/src/grants.rs b/agent-sidecar/src/grants.rs index b92401e..5e123d0 100644 --- a/agent-sidecar/src/grants.rs +++ b/agent-sidecar/src/grants.rs @@ -79,6 +79,10 @@ pub struct SessionGrants { clock: fn() -> u64, inner: RwLock, ignored: std::sync::atomic::AtomicUsize, + /// HGW-2: the session's project roots (canonical): folders where the agent's file tools may + /// create and change build configuration. Named by citrate-core (`projectRoots`) or created by + /// this session's `project_create`. A marker file alone never adds one. + projects: RwLock>, } impl std::fmt::Debug for SessionGrants { @@ -100,9 +104,59 @@ impl SessionGrants { home, clock: now_secs, ignored: std::sync::atomic::AtomicUsize::new(0), + projects: RwLock::new(Vec::new()), } } + /// HGW-2: add a project root. It must be an absolute, existing folder covered by live read + /// and write folder grants over its whole subtree (the same rule as `shell_run`'s folder). + /// Returns its canonical path. The grants are checked again at every write. + pub fn add_project_root(&self, root: &Path) -> Result { + if !root.is_absolute() { + return Err(format!( + "the project root {} is not an absolute path", + root.display() + )); + } + if !root.is_dir() { + return Err(format!( + "the project root {} is not an existing folder", + root.display() + )); + } + let (canonical, _) = self.shell_scope(root).map_err(|e| { + format!( + "the project root {} is not inside folders granted for reading and writing: {e}", + root.display() + ) + })?; + let mut p = self + .projects + .write() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if !p.contains(&canonical) { + p.push(canonical.clone()); + } + Ok(canonical) + } + + /// HGW-2: the session's project roots, in the order they were added. + pub fn project_roots(&self) -> Vec { + self.projects.read().map(|p| p.clone()).unwrap_or_default() + } + + /// HGW-2: the project root that contains `canonical` (an already-resolved path), if any. A + /// poisoned lock answers none (build configuration then stays refused). + pub fn project_root_of(&self, canonical: &Path) -> Option { + self.projects + .read() + .ok()? + .iter() + .filter(|r| canonical.starts_with(r)) + .max_by_key(|r| r.components().count()) + .cloned() + } + /// Use a different clock (tests). pub fn with_clock(mut self, clock: fn() -> u64) -> Self { self.clock = clock; diff --git a/agent-sidecar/src/hgw_support.rs b/agent-sidecar/src/hgw_support.rs index ad6ec9f..29dbab5 100644 --- a/agent-sidecar/src/hgw_support.rs +++ b/agent-sidecar/src/hgw_support.rs @@ -129,6 +129,29 @@ pub(crate) fn manager_with( mgr } +/// `POST /sessions` body with `grants`, `hicAware` and every key of `extra` (an object). +pub(crate) fn create_req_with( + grants: serde_json::Value, + hic_aware: bool, + extra: serde_json::Value, +) -> serde_json::Result { + let mut b = serde_json::json!({ + "model": "gemma-4", + "systemPrompt": "You are Hermes.", + "llm": {"baseUrl": "http://127.0.0.1:18080/v1", "bearer": "k"}, + "tools": [], + "maxToolsPerRequest": 8, + "hicAware": hic_aware, + "grants": grants, + }); + if let (Some(o), Some(e)) = (b.as_object_mut(), extra.as_object()) { + for (k, v) in e { + o.insert(k.clone(), v.clone()); + } + } + serde_json::from_value(b) +} + /// `POST /sessions` body with `grants` and `hicAware`. pub(crate) fn create_req(grants: serde_json::Value, hic_aware: bool) -> sessions::CreateSessionReq { serde_json::from_value(serde_json::json!({ diff --git a/agent-sidecar/src/lib.rs b/agent-sidecar/src/lib.rs index 6251a21..ddecf0a 100644 --- a/agent-sidecar/src/lib.rs +++ b/agent-sidecar/src/lib.rs @@ -2362,6 +2362,8 @@ mod decide_route_tests; mod deploy_guard_session_tests; #[cfg(all(test, unix))] mod hgw_support; +#[cfg(all(test, unix))] +mod project_policy_tests; #[cfg(test)] mod search_session_tests; #[cfg(test)] diff --git a/agent-sidecar/src/project_policy_tests.rs b/agent-sidecar/src/project_policy_tests.rs new file mode 100644 index 0000000..c8fbc31 --- /dev/null +++ b/agent-sidecar/src/project_policy_tests.rs @@ -0,0 +1,230 @@ +//! HGW-2 — project write policy: inside a project root, Hermes's file tools may create and change +//! build configuration (`package.json`, `foundry.toml`, `remappings.txt`, `tsconfig.json`, …); +//! everywhere else the member still edits it. `.env*` files stay the member's everywhere, and the +//! file tools never write inside `.git/` (git itself does, through the sandboxed `shell_run`). +//! +//! A project root is a folder citrate-core names in `projectRoots` when it opens the session (a +//! project Hermes created earlier, or a folder the member marked as a project) or one this +//! session's `project_create` made. A marker file alone never makes a folder a project. +//! +//! BDD map: +//! - `build_config_is_writable_inside_a_project_root_and_nowhere_else` +//! - `env_files_stay_the_members_even_inside_a_project` +//! - `a_marker_file_alone_does_not_make_a_project` +//! - `the_file_tools_never_write_inside_git` +//! - `project_roots_must_be_absolute_existing_folders_inside_the_grants` + +#![cfg(unix)] + +use super::hgw_support::*; +use super::*; +use citrate_agent_loop::AssistantTurn; + +fn file_write(id: &str, path: &std::path::Path, content: &str) -> citrate_agent_loop::ToolCall { + tool_call( + id, + grants::FILE_WRITE_TOOL, + serde_json::json!({ "path": path, "content": content }), + ) +} + +async fn run_calls( + fx: &Fx, + calls: Vec, + extra: serde_json::Value, + roots: &[std::path::PathBuf], +) -> Vec { + let ids: Vec = calls.iter().map(|c| c.id.clone()).collect(); + let mut turns: Vec = calls + .into_iter() + .map(|c| AssistantTurn::tools(vec![c])) + .collect(); + turns.push(AssistantTurn::text("ok")); + let mgr = Arc::new(manager_with(fx, turns, None)); + let id = mgr + .create(create_req_with(fx.rw_doc_for(roots), true, extra).expect("req")) + .expect("session"); + let s = mgr.get(&id).expect("session"); + mgr.send(&id, "go".into(), None).expect("send"); + let mut out = Vec::new(); + for c in ids { + out.push(result_of(&s, &c).await); + } + wait_done(&s).await; + out +} + +fn content(r: &serde_json::Value) -> String { + r["content"].as_str().unwrap_or_default().to_string() +} + +#[tokio::test(flavor = "multi_thread")] +async fn build_config_is_writable_inside_a_project_root_and_nowhere_else() { + let fx = Fx::new("proj-config"); + let proj = fx.proj(); + let other = fx.other(); + let res = run_calls( + &fx, + vec![ + fs_write("c1", &proj.join("package.json"), "{\"name\":\"demo\"}\n"), + file_write("c2", &proj.join("foundry.toml"), "[profile.default]\n"), + fs_write( + "c3", + &proj.join("remappings.txt"), + "forge-std/=lib/forge-std/src/\n", + ), + fs_write("c4", &proj.join("tsconfig.json"), "{}\n"), + fs_write("c5", &proj.join("src/Makefile"), "all:\n"), + fs_write("c6", &other.join("package.json"), "{}\n"), + file_write("c7", &other.join("foundry.toml"), "x\n"), + ], + serde_json::json!({ "projectRoots": [proj] }), + &[proj.clone(), other.clone()], + ) + .await; + for (i, r) in res.iter().take(5).enumerate() { + assert_eq!(r["status"], "ok", "call {}: {r}", i + 1); + } + assert_eq!( + std::fs::read_to_string(proj.join("package.json")).expect("package.json"), + "{\"name\":\"demo\"}\n" + ); + assert!(proj.join("foundry.toml").exists()); + assert!(proj.join("remappings.txt").exists()); + for r in &res[5..] { + assert_ne!(r["status"], "ok", "{r}"); + let c = content(r); + assert!(c.contains("build configuration"), "{c}"); + assert!(c.contains("project"), "the refusal points at projects: {c}"); + } + assert!(!other.join("package.json").exists()); + assert!(!other.join("foundry.toml").exists()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn without_project_roots_build_config_is_refused_as_before() { + let fx = Fx::new("proj-none"); + let proj = fx.proj(); + let res = run_calls( + &fx, + vec![fs_write("c1", &proj.join("package.json"), "{}\n")], + serde_json::json!({}), + std::slice::from_ref(&proj), + ) + .await; + assert_ne!(res[0]["status"], "ok"); + assert!(content(&res[0]).contains("build configuration")); + assert!(!proj.join("package.json").exists()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn env_files_stay_the_members_even_inside_a_project() { + let fx = Fx::new("proj-env"); + let proj = fx.proj(); + let res = run_calls( + &fx, + vec![ + fs_write("c1", &proj.join(".env"), "KEY=1\n"), + file_write("c2", &proj.join(".env.local"), "KEY=1\n"), + ], + serde_json::json!({ "projectRoots": [proj] }), + std::slice::from_ref(&proj), + ) + .await; + for r in &res { + assert_ne!(r["status"], "ok", "{r}"); + } + assert!(!proj.join(".env").exists()); + assert!(!proj.join(".env.local").exists()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_marker_file_alone_does_not_make_a_project() { + let fx = Fx::new("proj-marker"); + let other = fx.other(); + std::fs::create_dir_all(other.join(".citrate")).expect("marker dir"); + std::fs::write( + other.join(".citrate/project.json"), + serde_json::json!({"version": 1, "name": "other", "root": other}).to_string(), + ) + .expect("marker"); + let res = run_calls( + &fx, + vec![fs_write("c1", &other.join("package.json"), "{}\n")], + serde_json::json!({ "projectRoots": [fx.proj()] }), + &[fx.proj(), other.clone()], + ) + .await; + assert_ne!(res[0]["status"], "ok", "{}", res[0]); + assert!(!other.join("package.json").exists()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn the_file_tools_never_write_inside_git() { + let fx = Fx::new("proj-git"); + let proj = fx.proj(); + std::fs::create_dir_all(proj.join(".git/hooks")).expect("git dir"); + let res = run_calls( + &fx, + vec![ + fs_write("c1", &proj.join(".git/hooks/pre-commit"), "#!/bin/sh\n"), + file_write("c2", &proj.join(".git/config"), "[core]\n"), + fs_write("c3", &fx.other().join(".git/HEAD"), "ref: x\n"), + ], + serde_json::json!({ "projectRoots": [proj] }), + &[proj.clone(), fx.other()], + ) + .await; + for r in &res { + assert_ne!(r["status"], "ok", "{r}"); + assert!(content(r).contains(".git"), "{r}"); + } + assert!(!proj.join(".git/hooks/pre-commit").exists()); + assert!(!proj.join(".git/config").exists()); +} + +#[test] +fn project_roots_must_be_absolute_existing_folders_inside_the_grants() { + let fx = Fx::new("proj-roots"); + let mgr = manager_with(&fx, vec![], None); + let doc = fx.rw_doc(); + for bad in [ + serde_json::json!(["relative/dir"]), + serde_json::json!([fx.proj().join("missing")]), + serde_json::json!([fx.other()]), + ] { + let req = create_req_with( + doc.clone(), + true, + serde_json::json!({ "projectRoots": bad }), + ) + .expect("req"); + assert!( + matches!(mgr.create(req), Err(sessions::SessionError::Invalid(_))), + "{bad}" + ); + } + // Without a grant document there are no project roots. + let mut req = create_req_with( + serde_json::Value::Null, + true, + serde_json::json!({ "projectRoots": [fx.proj()] }), + ) + .expect("req"); + req.grants = None; + assert!(matches!( + mgr.create(req), + Err(sessions::SessionError::Invalid(_)) + )); + let ok = create_req_with( + doc, + true, + serde_json::json!({ "projectRoots": [fx.proj()] }), + ) + .expect("req"); + let id = mgr + .create(ok) + .expect("a granted, existing project root is accepted"); + let s = mgr.get(&id).expect("session"); + assert_eq!(s.grants().expect("grants").project_roots(), vec![fx.proj()]); +} diff --git a/agent-sidecar/src/sessions.rs b/agent-sidecar/src/sessions.rs index 37cc9f9..bdbb552 100644 --- a/agent-sidecar/src/sessions.rs +++ b/agent-sidecar/src/sessions.rs @@ -358,6 +358,13 @@ pub struct CreateSessionReq { /// the same rules as `POST /personas/check`). Never both. #[serde(default)] pub custom_persona: Option, + /// HGW-2: the session's project roots: absolute, existing folders covered by live read and + /// write folder grants (subtree), where the file tools may create and change build + /// configuration. citrate-core sends the projects Hermes created before (`project_create` + /// results it kept) and folders the member marked as projects. Needs `grants`. Absent = + /// none (build configuration stays the member's, as before). + #[serde(default)] + pub project_roots: Vec, } /// HUP-S3.3: what a session did with its persona (`POST /sessions` answers it as `persona`). @@ -1623,6 +1630,11 @@ impl SessionManager { "the tool name '{SHELL_RUN_TOOL}' is reserved by the sidecar while shell_run is on" ))); } + if req.grants.is_none() && !req.project_roots.is_empty() { + return Err(SessionError::Invalid( + "projectRoots needs folder grants (a grant document) to check them against".into(), + )); + } let grants = match req.grants { None => None, Some(doc) => { @@ -1655,6 +1667,13 @@ impl SessionManager { g.replace(&doc).map_err(|e| { SessionError::Invalid(format!("the grant document was refused: {e}")) })?; + // HGW-2: the project roots core names, each checked against these grants now. + for root in &req.project_roots { + g.add_project_root(std::path::Path::new(root)) + .map_err(|e| { + SessionError::Invalid(format!("projectRoots was refused: {e}")) + })?; + } specs.extend(crate::grants::file_tool_specs()); specs.extend(crate::sheets::sheet_tool_specs()); if fs_on_grants { diff --git a/agent-sidecar/src/shell_run.rs b/agent-sidecar/src/shell_run.rs index ebccc65..e1c61cb 100644 --- a/agent-sidecar/src/shell_run.rs +++ b/agent-sidecar/src/shell_run.rs @@ -462,7 +462,17 @@ fn report(argv: &[String], r: &RunReport) -> ToolOutcome { "output_incomplete": r.output_incomplete, "cleanup_error": r.cleanup_error, "sandbox": r.sandbox, + "git_warnings": r.git_warnings, }); + // HGW-2: git state the command left that would run a program outside the sandbox later. + let git_note = if r.git_warnings.is_empty() { + String::new() + } else { + format!( + ". Warning: the command left git set up to run a program outside the sandbox the next time git is used ({}); the member should review it", + r.git_warnings.join("; ") + ) + }; let isolation = if r.sandbox.enforced { format!("in the {} sandbox, no network", r.sandbox.backend) } else { @@ -504,7 +514,7 @@ fn report(argv: &[String], r: &RunReport) -> ToolOutcome { SHELL_RUN_TOOL, RunStatus::Completed, format!( - "{} {ended} after {} ms ({isolation})", + "{} {ended} after {} ms ({isolation}){git_note}", r.program, r.duration_ms ), ) @@ -644,6 +654,7 @@ mod tests { readable_extra: Vec::new(), summary: "test".into(), }, + git_warnings: Vec::new(), } } diff --git a/agent-sidecar/src/shell_run_tests.rs b/agent-sidecar/src/shell_run_tests.rs index 22d38af..b0e7a31 100644 --- a/agent-sidecar/src/shell_run_tests.rs +++ b/agent-sidecar/src/shell_run_tests.rs @@ -553,6 +553,42 @@ fn a_tainted_hic_aware_session_asks_with_the_reason_and_runs_only_on_allow() { drop(host); } +/// HGW-2: a command that leaves git set up to run a program later (outside the sandbox) is +/// reported to the model and the member: the run facts carry `git_warnings` and the summary says +/// so. +#[test] +fn risky_git_state_left_by_a_command_is_reported() { + let fx = Fx::new(); + std::fs::create_dir_all(fx.proj().join(".git/hooks")).unwrap(); + let s = session(&fx, &cfg_open()); + let approvals = s.approvals().clone(); + let h = spawn( + s.host(TaintState::default(), StopFlag::default(), false), + call( + "c1", + &[ + "sh", + "-c", + "printf '[core]\\n\\thooksPath = .h\\n' > .git/config", + ], + &fx.proj(), + ), + ); + let p = wait_pending(&approvals); + approvals.decide(&p.id, true, &p.argv, &p.cwd).unwrap(); + let out = h.join().unwrap(); + let env = envelope(&out); + let facts = env.run.expect("run facts"); + let w = facts["git_warnings"].to_string(); + assert!(w.contains("core.hookspath"), "{w}"); + assert!(env.summary.contains("git"), "{}", env.summary); + assert!( + env.summary.contains("outside the sandbox"), + "{}", + env.summary + ); +} + #[test] fn an_untainted_card_has_no_taint_reason() { let fx = Fx::new(); diff --git a/agent-sidecar/src/toolchain.rs b/agent-sidecar/src/toolchain.rs index 78a9020..6b139ad 100644 --- a/agent-sidecar/src/toolchain.rs +++ b/agent-sidecar/src/toolchain.rs @@ -777,6 +777,7 @@ mod tests { readable_extra: Vec::new(), summary: "test".into(), }, + git_warnings: Vec::new(), } } diff --git a/agent-sidecar/src/toolchain_config.rs b/agent-sidecar/src/toolchain_config.rs index c950326..68f5d4a 100644 --- a/agent-sidecar/src/toolchain_config.rs +++ b/agent-sidecar/src/toolchain_config.rs @@ -29,7 +29,11 @@ //! [`build_config_file`] is the matching rule for the agent's file tools: build configuration //! (the files above, plus `foundry.toml`, `remappings.txt`, `package.json`, `aderyn.toml`, //! make/just files and `.cargo/config*`) is edited by the member, never created, changed, moved -//! or removed by the agent. +//! or removed by the agent, except (HGW-2, owner decision 2026-10-15) inside a project root +//! ([`build_config_write_refusal`]): a project Hermes created or a folder the member marked as a +//! project, where the agent sets up its own build. Env files stay the member's everywhere. This +//! does not loosen the toolchain check above: forge still refuses to run with `ffi` or wide +//! `fs_permissions`, whoever wrote the file. use std::path::{Component, Path, PathBuf}; @@ -86,10 +90,49 @@ pub(crate) fn build_config_file(path: &Path) -> Option { /// The refusal the file tools give for [`build_config_file`]. pub(crate) fn build_config_refusal(name: &str) -> String { format!( - "{name} is build configuration, which the member edits; the agent's file tools do not create, change, move or remove it" + "{name} is build configuration, which the member edits; the agent's file tools do not create, change, move or remove it outside a project (a folder made with project_create, or one the member marked as a project)" ) } +/// HGW-2: the file tools' rule for one write, given the project root that holds the resolved +/// path (if any). Env files are always the member's; other build configuration is the agent's +/// to write only inside a project root. `None` = allowed. +pub(crate) fn build_config_write_refusal(path: &Path, project: Option<&Path>) -> Option { + if let Some(r) = env_file_refusal(path) { + return Some(r); + } + let name = build_config_file(path)?; + match project { + Some(_) => None, + None => Some(build_config_refusal(&name)), + } +} + +/// HGW-2: env files are only ever the member's: `Some(refusal)` for one (matching on the last +/// component, any case). +pub(crate) fn env_file_refusal(path: &Path) -> Option { + let name = path.file_name()?.to_string_lossy().into_owned(); + env_file(&name).then(|| { + format!( + "{name} is an env file, build configuration that may hold secrets, which only the member creates or changes; the agent's file tools never write it, in a project or not" + ) + }) +} + +/// HGW-2: the file tools never write inside a `.git` folder: git does that through the sandboxed +/// `shell_run`, whose sandbox keeps hooks out. `Some(refusal)` for such a path. +pub(crate) fn git_internal_refusal(path: &Path) -> Option { + let inside = path + .components() + .any(|c| matches!(c, Component::Normal(n) if n.eq_ignore_ascii_case(".git"))); + inside.then(|| { + format!( + "{} is inside a .git folder; the agent's file tools do not write there (git commands run through shell_run do)", + path.display() + ) + }) +} + /// Refuse (with the reason) when the project's build configuration asks for more than the agent /// toolchain allows. `project` is the canonical project folder. pub(crate) fn check_project_config(project: &Path) -> Result<(), String> {