diff --git a/lib/crates/fabro-server/src/run_files_security.rs b/lib/crates/fabro-server/src/run_files_security.rs index 287570b93..b4e6fa948 100644 --- a/lib/crates/fabro-server/src/run_files_security.rs +++ b/lib/crates/fabro-server/src/run_files_security.rs @@ -102,15 +102,21 @@ pub fn is_sensitive(path: &str) -> bool { fn normalize_for_match(path: &str) -> String { // Replace backslashes with forward slashes so Windows-style paths (if - // they ever leak through git output) match the same way; lowercase so - // the patterns are effectively case-insensitive. `to_lowercase()` can - // expand certain Unicode codepoints to multiple chars — the strip loop - // below runs against the fully-lowercased string so prefix matching is - // consistent regardless of the input's case. + // they ever leak through git output) match the same way. Case-folding + // is ASCII-only on purpose: all denylist globs are ASCII, and full + // Unicode `to_lowercase()` can expand a single codepoint into multiple + // chars (e.g. `İ` -> `i\u{307}`), which then silently can't match an + // ASCII glob. ASCII-only fold makes the failure mode explicit (a + // homoglyphic path just doesn't match) rather than disguised. let mut out = String::with_capacity(path.len()); for ch in path.chars() { let c = if ch == '\\' { '/' } else { ch }; - out.extend(c.to_lowercase()); + let lowered = if c.is_ascii() { + c.to_ascii_lowercase() + } else { + c + }; + out.push(lowered); } // Drop leading `./`, `../`, and bare-leading `/` prefixes until the // path has a meaningful first segment. Inner `..` components are left diff --git a/lib/crates/fabro-workflow/src/sandbox_git.rs b/lib/crates/fabro-workflow/src/sandbox_git.rs index 7d525c58f..ce8c9e9e4 100644 --- a/lib/crates/fabro-workflow/src/sandbox_git.rs +++ b/lib/crates/fabro-workflow/src/sandbox_git.rs @@ -200,7 +200,13 @@ pub(crate) async fn git_diff_with_timeout( base: &str, timeout_ms: u64, ) -> std::result::Result { - let cmd = format!("{GIT_REMOTE} diff {base} HEAD"); + // `-c core.quotePath=false` forces paths with non-ASCII, tabs, quotes, + // or backslashes to emit unquoted. The Run Files Changed endpoint's + // `strip_denylisted_sections` parser only recognizes unquoted + // `diff --git a/ b/` headers; without this flag git would + // wrap such paths in `"a/…"` / `"b/…"` and evade the denylist (see + // docs/agent/reviews/2026-04-19-run-files-security-review.md). + let cmd = format!("{GIT_REMOTE} -c core.quotePath=false diff {base} HEAD"); match sandbox .exec_command(&cmd, timeout_ms, None, None, None) .await @@ -264,7 +270,7 @@ pub async fn git_replace_worktree(sandbox: &dyn Sandbox, path: &str, branch: &st /// /// These invocations use [`sandbox_git_hardening_env`] via `exec_command` to /// disable terminal prompts and external diff drivers. -const GIT_HARDENED: &str = "git -c maintenance.auto=0 -c gc.auto=0 -c core.hooksPath=/dev/null -c core.fsmonitor=false -c protocol.file.allow=never"; +const GIT_HARDENED: &str = "git -c maintenance.auto=0 -c gc.auto=0 -c core.hooksPath=/dev/null -c core.fsmonitor=false -c protocol.file.allow=never -c core.quotePath=false"; /// Environment additions applied to every hardened sandbox-side git invocation. ///