diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index b74c314bb..1a8042fed 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -374,6 +374,8 @@ class AgentLoop: sandbox=self.exec_config.sandbox, path_append=self.exec_config.path_append, allowed_env_keys=self.exec_config.allowed_env_keys, + allow_patterns=self.exec_config.allow_patterns, + deny_patterns=self.exec_config.deny_patterns, ) ) if self.web_config.enable: diff --git a/nanobot/agent/runner.py b/nanobot/agent/runner.py index d002d8989..3d941f382 100644 --- a/nanobot/agent/runner.py +++ b/nanobot/agent/runner.py @@ -833,13 +833,13 @@ class AgentRunner: # Markers identifying tool results that represent a workspace / safety boundary rejection. _WORKSPACE_BLOCK_MARKERS: tuple[str, ...] = ( - "blocked by safety guard", "outside the configured workspace", "outside allowed directory", "working_dir is outside", "working_dir could not be resolved", "path traversal detected", "path outside working dir", + "internal/private url detected", ) @classmethod diff --git a/nanobot/agent/subagent.py b/nanobot/agent/subagent.py index 0dd549fe8..18f0bd53b 100644 --- a/nanobot/agent/subagent.py +++ b/nanobot/agent/subagent.py @@ -188,6 +188,8 @@ class SubagentManager: sandbox=self.exec_config.sandbox, path_append=self.exec_config.path_append, allowed_env_keys=self.exec_config.allowed_env_keys, + allow_patterns=self.exec_config.allow_patterns, + deny_patterns=self.exec_config.deny_patterns, )) if self.web_config.enable: tools.register( diff --git a/nanobot/agent/tools/shell.py b/nanobot/agent/tools/shell.py index 059428aa1..b7f841a5c 100644 --- a/nanobot/agent/tools/shell.py +++ b/nanobot/agent/tools/shell.py @@ -52,7 +52,7 @@ class ExecTool(Tool): self.timeout = timeout self.working_dir = working_dir self.sandbox = sandbox - self.deny_patterns = deny_patterns or [ + self.deny_patterns = (deny_patterns or []) + [ r"\brm\s+-[rf]{1,2}\b", # rm -r, rm -rf, rm -fr r"\bdel\s+/[fq]\b", # del /f, del /q r"\brmdir\s+/s\b", # rmdir /s @@ -273,13 +273,19 @@ class ExecTool(Tool): cmd = command.strip() lower = cmd.lower() - for pattern in self.deny_patterns: - if re.search(pattern, lower): - return "Error: Command blocked by safety guard (dangerous pattern detected)" + # allow_patterns take priority over deny_patterns so that users can + # exempt specific commands (e.g. "rm -rf" inside a build directory) + # from the hardcoded deny list via configuration. + explicitly_allowed = bool(self.allow_patterns) and any( + re.search(p, lower) for p in self.allow_patterns + ) + if not explicitly_allowed: + for pattern in self.deny_patterns: + if re.search(pattern, lower): + return "Error: Command blocked by deny pattern filter" - if self.allow_patterns: - if not any(re.search(p, lower) for p in self.allow_patterns): - return "Error: Command blocked by safety guard (not in allowlist)" + if self.allow_patterns: + return "Error: Command blocked by allowlist filter (not in allowlist)" from nanobot.security.network import contains_internal_url if contains_internal_url(cmd): diff --git a/nanobot/config/schema.py b/nanobot/config/schema.py index bb7ed0731..be4eb7202 100644 --- a/nanobot/config/schema.py +++ b/nanobot/config/schema.py @@ -223,6 +223,8 @@ class ExecToolConfig(Base): path_append: str = "" sandbox: str = "" # sandbox backend: "" (none) or "bwrap" allowed_env_keys: list[str] = Field(default_factory=list) # Env var names to pass through to subprocess (e.g. ["GOPATH", "JAVA_HOME"]) + allow_patterns: list[str] = Field(default_factory=list) # Regex patterns that bypass deny_patterns (e.g. [r"rm\s+-rf\s+/tmp/"]) + deny_patterns: list[str] = Field(default_factory=list) # Extra regex patterns to block (appended to built-in list) class MCPServerConfig(Base): """MCP server connection configuration (stdio or HTTP).""" diff --git a/tests/agent/test_runner.py b/tests/agent/test_runner.py index 10d25348a..aa558b4ff 100644 --- a/tests/agent/test_runner.py +++ b/tests/agent/test_runner.py @@ -352,6 +352,27 @@ async def test_runner_stops_on_workspace_violation_without_fail_on_tool_error(): ] +def test_is_workspace_violation_recognizes_ssrf_block(): + """Internal/private URL block must be classified as a fatal workspace violation. + + Regression guard: the deny/allowlist filter messages were intentionally split + out of `_WORKSPACE_BLOCK_MARKERS` so the LLM can retry, but SSRF rejections + are a hard security boundary and must remain fatal. + """ + from nanobot.agent.runner import AgentRunner + + ssrf_msg = "Error: Command blocked by safety guard (internal/private URL detected)" + assert AgentRunner._is_workspace_violation(ssrf_msg) is True + + # Sanity: deny/allowlist filter messages are deliberately *not* fatal. + assert AgentRunner._is_workspace_violation( + "Error: Command blocked by deny pattern filter" + ) is False + assert AgentRunner._is_workspace_violation( + "Error: Command blocked by allowlist filter (not in allowlist)" + ) is False + + @pytest.mark.asyncio async def test_runner_persists_large_tool_results_for_follow_up_calls(tmp_path): from nanobot.agent.runner import AgentRunSpec, AgentRunner diff --git a/tests/tools/test_exec_allow_patterns.py b/tests/tools/test_exec_allow_patterns.py new file mode 100644 index 000000000..1a2a90521 --- /dev/null +++ b/tests/tools/test_exec_allow_patterns.py @@ -0,0 +1,58 @@ +"""Tests for allow_patterns priority over deny_patterns.""" + +from __future__ import annotations + +from nanobot.agent.tools.shell import ExecTool + + +def test_deny_patterns_block_rm_rf(): + """Baseline: rm -rf is blocked by default deny list.""" + tool = ExecTool() + result = tool._guard_command("rm -rf /tmp/build", "/tmp") + assert result is not None + assert "deny pattern filter" in result.lower() + + +def test_allow_patterns_bypass_deny(): + """allow_patterns take priority: matching command skips deny check.""" + tool = ExecTool(allow_patterns=[r"rm\s+-rf\s+/tmp/"]) + result = tool._guard_command("rm -rf /tmp/build", "/tmp") + assert result is None + + +def test_allow_patterns_must_match_to_bypass(): + """Non-matching allow_patterns do NOT bypass deny.""" + tool = ExecTool(allow_patterns=[r"rm\s+-rf\s+/opt/"]) + result = tool._guard_command("rm -rf /tmp/build", "/tmp") + assert result is not None + assert "deny pattern filter" in result.lower() + + +def test_extra_deny_patterns_from_config(): + """User-supplied deny patterns are appended to built-in list.""" + tool = ExecTool(deny_patterns=[r"\bping\b"]) + # ping is blocked by extra deny + assert tool._guard_command("ping example.com", "/tmp") is not None + # rm -rf still blocked by built-in deny + assert tool._guard_command("rm -rf /tmp/x", "/tmp") is not None + + +def test_allow_patterns_bypass_extra_deny(): + """allow_patterns also bypasses user-supplied deny patterns.""" + tool = ExecTool( + deny_patterns=[r"\bping\b"], + allow_patterns=[r"\bping\s+example\.com\b"], + ) + result = tool._guard_command("ping example.com", "/tmp") + assert result is None + + +def test_allow_patterns_is_whitelist_only(): + """When allow_patterns is set, non-matching non-denied commands are blocked.""" + tool = ExecTool(allow_patterns=[r"\becho\b"]) + # echo matches allow → ok + assert tool._guard_command("echo hello", "/tmp") is None + # ls does not match allow and is not in deny → blocked by allowlist + result = tool._guard_command("ls /tmp", "/tmp") + assert result is not None + assert "allowlist" in result.lower() diff --git a/tests/tools/test_exec_security.py b/tests/tools/test_exec_security.py index 20687dcbf..64dc49563 100644 --- a/tests/tools/test_exec_security.py +++ b/tests/tools/test_exec_security.py @@ -94,7 +94,7 @@ def test_exec_blocks_writes_to_history_jsonl(command): tool = ExecTool() result = tool._guard_command(command, "/tmp") assert result is not None - assert "dangerous pattern" in result.lower() + assert "deny pattern filter" in result.lower() @pytest.mark.parametrize(