diff --git a/tests/tools/test_file_tools.py b/tests/tools/test_file_tools.py index b009c009f25dc..329247ec313dc 100644 --- a/tests/tools/test_file_tools.py +++ b/tests/tools/test_file_tools.py @@ -459,6 +459,20 @@ class TestSensitivePathCheck: assert "error" in result assert "sensitive system path" in result["error"] + def test_macos_private_var_carveouts(self): + """macOS temp dirs under /private/var must not be blanket-blocked, + while the genuinely-sensitive /private/var subtrees still are.""" + from tools.file_tools import _check_sensitive_path + + # $TMPDIR / /tmp / /var/folders realpath into these on macOS. + assert _check_sensitive_path("/private/var/folders/xy/T/tmp.txt") is None + assert _check_sensitive_path("/private/var/tmp/build.log") is None + # Sensitive subtrees remain blocked. + assert _check_sensitive_path("/private/var/db/secret") is not None + assert _check_sensitive_path("/private/var/root/x") is not None + # /etc (and its macOS /private/etc mirror) stay blocked. + assert _check_sensitive_path("/private/etc/hosts") is not None + @patch("tools.file_tools._get_file_ops") def test_normal_file_not_blocked(self, mock_get, monkeypatch): monkeypatch.setattr("tools.file_tools._hermes_config_resolved", "/home/user/.hermes/config.yaml") diff --git a/tests/tools/test_fuzzy_match.py b/tests/tools/test_fuzzy_match.py index e8d0f2a4808ae..f602aa5ad710a 100644 --- a/tests/tools/test_fuzzy_match.py +++ b/tests/tools/test_fuzzy_match.py @@ -11,6 +11,20 @@ class TestExactMatch: assert count == 1 assert new == "hi world" + def test_whitespace_only_old_string_rejected(self): + """A whitespace-only old_string is not a meaningful anchor.""" + content = "alpha\n \nbeta\n" + new, count, _, err = fuzzy_find_and_replace(content, " ", "XXX") + assert count == 0 + assert err is not None + assert "whitespace" in err + assert new == content # untouched + + def test_empty_old_string_rejected(self): + new, count, _, err = fuzzy_find_and_replace("abc", "", "x") + assert count == 0 + assert err is not None + def test_multiline_exact(self): content = "line1\nline2\nline3" diff --git a/tests/tools/test_terminal_foreground_timeout_cap.py b/tests/tools/test_terminal_foreground_timeout_cap.py index f18807bbc1a76..0081de88a50d0 100644 --- a/tests/tools/test_terminal_foreground_timeout_cap.py +++ b/tests/tools/test_terminal_foreground_timeout_cap.py @@ -47,6 +47,28 @@ class TestForegroundTimeoutCap: assert str(FOREGROUND_MAX_TIMEOUT) in result["error"] assert "background=true" in result["error"] + def test_zero_timeout_rejected(self): + """timeout=0 must be rejected, not silently coerced to the default.""" + from tools.terminal_tool import terminal_tool + + with patch("tools.terminal_tool._get_env_config", return_value=_make_env_config()), \ + patch("tools.terminal_tool._start_cleanup_thread"): + result = json.loads(terminal_tool(command="echo hi", timeout=0)) + + assert result.get("error") + assert "positive" in result["error"] + + def test_negative_timeout_rejected(self): + """timeout=-1 must be rejected, not fire an immediate '-1s' timeout.""" + from tools.terminal_tool import terminal_tool + + with patch("tools.terminal_tool._get_env_config", return_value=_make_env_config()), \ + patch("tools.terminal_tool._start_cleanup_thread"): + result = json.loads(terminal_tool(command="echo hi", timeout=-1)) + + assert result.get("error") + assert "positive" in result["error"] + def test_foreground_allows_help_variant_for_server_command(self): """Informational variants like '--help' should not be blocked.""" diff --git a/tools/file_tools.py b/tools/file_tools.py index a31ec7432e0a9..9a9590c9f7f5c 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -641,7 +641,13 @@ def _filter_read_blocked_search_results(result, task_id: str = "default") -> int # terminal tool's approval system. These match prefixes after os.path.realpath. _SENSITIVE_PATH_PREFIXES = ( "/etc/", "/boot/", "/usr/lib/systemd/", - "/private/etc/", "/private/var/", + "/private/etc/", + # macOS: /private/var mirrors /var. Block the sensitive subtrees, NOT the + # whole thing — a blanket "/private/var/" refused every legitimate temp-file + # write, because $TMPDIR, /tmp, and /var/folders all realpath() into + # /private/var/folders/... on macOS (and _resolve_path_for_task resolves + # symlinks), and /private/var/tmp is a normal temp dir. + "/private/var/db/", "/private/var/root/", ) _SENSITIVE_EXACT_PATHS = {"/var/run/docker.sock", "/run/docker.sock"} diff --git a/tools/fuzzy_match.py b/tools/fuzzy_match.py index 3f599e7b2ac05..44a7885855a39 100644 --- a/tools/fuzzy_match.py +++ b/tools/fuzzy_match.py @@ -83,6 +83,13 @@ def fuzzy_find_and_replace(content: str, old_string: str, new_string: str, if not old_string: return content, 0, None, "old_string cannot be empty" + if not old_string.strip(): + # A whitespace-only old_string matches trivially (a blank line, run of + # spaces, etc.) and, when it recurs, either mass-replaces under + # replace_all or raises a hard-to-diagnose ambiguity error. It's never + # a meaningful anchor — reject it so the caller provides real context. + return content, 0, None, "old_string is only whitespace — provide non-blank text to match" + if old_string == new_string: return content, 0, None, "old_string and new_string are identical" diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index 5ebf349ae8a19..7f9141ff8a9ef 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -2307,6 +2307,16 @@ def terminal_tool( ) cwd = config["cwd"] default_timeout = config["timeout"] + + # Validate an explicit timeout before it flows into deadline math. + # ``timeout or default`` silently turns 0 into the default (0 can't mean + # "no timeout" here), and a negative value is truthy so it would sail + # through to ``deadline = now + timeout`` and fire an immediate, + # nonsensical "-Ns" timeout. Reject non-positive values outright. + if timeout is not None and timeout <= 0: + return tool_error( + f"timeout must be a positive number of seconds (got {timeout})." + ) effective_timeout = timeout or default_timeout # Reject foreground commands where the model explicitly requests