diff --git a/.env.example b/.env.example index 0f65804f..655e0bf3 100644 --- a/.env.example +++ b/.env.example @@ -101,13 +101,16 @@ ROBOCO_DEFAULT_EMBEDDING_MODEL=qwen3-embedding:0.6b # ROBOCO_GROK_AGENT_IMAGE=roboco-agent-grok:latest # ROBOCO_GROK_CLI_MODEL=grok-build -# Per-role tool permissions are computed as native grok flags (subagents off; -# edit/shell removed for non-coding roles; raw git mutation + rm -rf denied for -# coding roles) — there is nothing to set here. +# Per-role tool permissions are computed as native grok flags (subagents off +# except intake; edit/shell removed for non-coding roles; git network/branch/ +# history mutation + rm -rf denied for coding roles; web search off for all — +# gated web is via the roboco-search MCP). Credential-exfil / identity-forgery / +# internal-API shell patterns are blocked by the same bash-guard the Claude path +# runs, wired as a grok PreToolUse hook. Nothing to set here. -# Force one reasoning effort for ALL Grok agents: low | medium | high | xhigh | -# max (or empty for the per-role default — coordination/docs/board roles request -# "low" to cut reasoning cost, code roles keep full reasoning). +# Reasoning effort for ALL Grok agents: low | medium | high | xhigh | max. Empty +# keeps grok's model default for every role (parity with Claude — no per-role +# cut); set this to trade quality for cost across the whole fleet. # ROBOCO_GROK_REASONING_EFFORT= # Hard ceiling on agentic turns per run (loop guard). diff --git a/docker/scripts/bash-guard-hook.sh b/docker/scripts/bash-guard-hook.sh index b47489d7..8cee9bb0 100755 --- a/docker/scripts/bash-guard-hook.sh +++ b/docker/scripts/bash-guard-hook.sh @@ -10,7 +10,16 @@ # # Claude Code passes the PreToolUse event on stdin as JSON: # { "tool_name": "Bash", "tool_input": { "command": "...", "description": "..." } } -# Exit 0 = allow. Exit 2 = deny with message on stdout. +# The grok CLI passes the same event with camelCase keys (toolName / toolInput); +# the extractor accepts either, so the one tested script guards both runtimes. +# Exit 0 = allow. Exit 2 = deny. +# +# ROBOCO_GUARD_SKIP_GIT=1 skips the git-ops category. The grok path sets it because +# grok handles git via NATIVE --deny rules, which deny GRACEFULLY (the agent gets +# a permission error and recovers) — whereas a grok hook deny CANCELS the whole +# run. So grok keeps git on --deny (operational reflex → recoverable) and uses +# this hook only for the exfil categories (no legit use → a hard cancel is the +# right response). Claude has no such --deny, so it keeps the git block here. # # Deny categories: # - Network git ops (require token injection only done by the MCP layer) @@ -27,7 +36,7 @@ cmd=$(printf '%s' "$input" | python3 -c ' import json, sys try: d = json.loads(sys.stdin.read()) - ti = d.get("tool_input", {}) or {} + ti = d.get("tool_input") or d.get("toolInput") or {} print(ti.get("command", "")) except Exception: print("") @@ -36,7 +45,7 @@ except Exception: low=$(printf '%s' "$cmd" | tr "[:upper:]" "[:lower:]") -# Skeletonize the command for the git-ops check ONLY (#165): strip heredoc +# Skeletonize the command for the git-ops check ONLY: strip heredoc # bodies and echo/printf literal arguments. Those are data the shell writes # to a file, never commands the shell executes — so a README/heredoc that # merely documents `git commit` must not be mistaken for invoking git. @@ -82,7 +91,10 @@ fi git_skel_low=$(printf '%s' "$git_skel" | tr "[:upper:]" "[:lower:]") # --- git network / auth ops --------------------------------------------------- -if echo "$git_skel_low" | grep -qE '(^|[[:space:];&|])git[[:space:]]+(fetch|pull|push|clone|remote|ls-remote|checkout|commit|merge|rebase|reset|cherry-pick|revert|tag[[:space:]]+-d|update-ref|reflog[[:space:]]+delete)'; then +# Skipped on grok (handled by native --deny so a blocked git op is recoverable, +# not a run-cancelling hook deny). See the header. +if [[ "${ROBOCO_GUARD_SKIP_GIT:-}" != "1" ]] && \ + echo "$git_skel_low" | grep -qE '(^|[[:space:];&|])git[[:space:]]+(fetch|pull|push|clone|remote|ls-remote|checkout|commit|merge|rebase|reset|cherry-pick|revert|tag[[:space:]]+-d|update-ref|reflog[[:space:]]+delete)'; then echo "Denied: shell git for network / auth / branch-mutating ops is blocked." >&2 echo "Use the verb listed in your role's State→Verb table (e.g. commit, complete, i_am_done)." >&2 exit 2 @@ -119,20 +131,20 @@ fi # - scheme-less: `curl roboco-orchestrator:8000/api` # - protocol-relative: `curl //roboco-orchestrator:8000/api` # - any flag ordering: `curl -s -X POST http://localhost:8000/x -d ...` -# Interpreter / library-driven HTTP is handled by the #175 rule below. +# Interpreter / library-driven HTTP is handled by the rule below. # KNOWN GAP (still out of scope here): # - Variable expansion: `URL=http://orchestrator/x; curl $URL` — the guard # sees `curl $URL`, not the expanded URL, so this slips through. The -# X-Agent-Role check (task 4) is the second gate. +# server-side X-Agent-Role check is the second gate. if echo "$low" | grep -qE '(^|[[:space:];&|])(curl|wget|http|https|httpie)[[:space:]]' && \ echo "$low" | grep -qE '((http|https)://)?/?(roboco-[a-z0-9_-]+|localhost|127\.0\.0\.1|0\.0\.0\.0)[:/]'; then echo "Denied: internal API calls bypass the gateway. Use the MCP verbs (roboco-flow / roboco-do / roboco-git-readonly / roboco-optimal / roboco-docs) — they route through the orchestrator with the right auth and tracing." >&2 exit 2 fi -# --- interpreter/library HTTP to an internal host (task #175) ----------------- +# --- interpreter/library HTTP to an internal host ---------------------------- # The curl/wget rule above only fires when the FIRST token is an HTTP CLI. -# smoke-17 showed an agent reach the orchestrator with forged X-Agent-* +# A live run showed an agent reach the orchestrator with forged X-Agent-* # identity headers via: # python3 << 'EOF' # import httpx @@ -140,7 +152,7 @@ fi # headers={"X-Agent-ID": "", "X-Agent-Role": "developer"}) # EOF # The binary is python3 (slips the CLI check) and it imports httpx, not -# roboco.* (slips the #164 import check). Close it language-agnostically: +# roboco.* (slips the roboco-internals import check). Close it language-agnostically: # deny when the command pairs an HTTP-client token with a forbidden # internal host. The whole command (heredoc body included) is in $low, # consistent with the curl/wget sibling above. Legitimate shell work does @@ -200,7 +212,7 @@ if echo "$low" | grep -qE '(^|[[:space:];&|])(python3?|perl|node|ruby|awk|sed)[[ exit 2 fi -# --- gateway-internals import bypass (task #164) ------------------------------ +# --- gateway-internals import bypass ------------------------------------------ # An agent must reach the orchestrator ONLY through its manifest-bound MCP # verbs. Importing the server package directly # uv run python3 -c "from roboco.mcp.flow_server import open_pr; open_pr(...)" @@ -217,7 +229,7 @@ if echo "$low" | grep -qE '(python3?|uv[[:space:]]+run|poetry[[:space:]]+run|pip exit 2 fi -# --- agent-identity forgery (task #164) -------------------------------------- +# --- agent-identity forgery -------------------------------------------------- # ROBOCO_AGENT_ID is the agent's identity. It is injected by the orchestrator # at spawn and the agent process must never rewrite it — doing so lets one # agent act as another (forged audit trail, bypassed ownership checks). No diff --git a/docker/scripts/tests/bash-guard-tests.sh b/docker/scripts/tests/bash-guard-tests.sh index 62179db5..6581e485 100755 --- a/docker/scripts/tests/bash-guard-tests.sh +++ b/docker/scripts/tests/bash-guard-tests.sh @@ -106,6 +106,29 @@ run_case "allow declare -a arr" 0 "declare -a arr=(a b c)" run_case "allow cat README" 0 "cat README.md" run_case "allow curl non-github" 0 "curl https://example.com/info" +# ---------- grok variant: camelCase input + ROBOCO_GUARD_SKIP_GIT ---------- +# The grok CLI sends `toolInput` (camelCase) and the grok hook runs with +# ROBOCO_GUARD_SKIP_GIT=1 (git is handled by graceful native --deny). Exfil +# categories must STILL deny; git must now pass through to --deny. +run_case_grok() { + local label="$1" expected="$2" cmd="$3" json actual + # shellcheck disable=SC2016 + json=$(python3 -c 'import json, sys; print(json.dumps({"toolName":"run_terminal_command","toolInput":{"command":sys.argv[1]}}))' "$cmd") + echo "$json" | ROBOCO_GUARD_SKIP_GIT=1 $HOOK >/dev/null 2>&1 + actual=$? + if [[ "$actual" == "$expected" ]]; then + PASS=$((PASS + 1)) + else + FAIL=$((FAIL + 1)) + FAILS+=("[$label] expected $expected, got $actual | cmd: $cmd") + fi +} +run_case_grok "grok skip-git: git push allowed" 0 "git push origin main" +run_case_grok "grok camelCase: netrc denied" 2 "cat ~/.netrc" +run_case_grok "grok camelCase: env denied" 2 "env" +run_case_grok "grok camelCase: identity forgery" 2 "export ROBOCO_AGENT_ID=other" +run_case_grok "grok camelCase: allow ls" 0 "ls -la /workspace" + # ---------- Report ---------- echo echo "===== bash-guard-hook tests =====" diff --git a/roboco/llm/providers/grok_cli_config.py b/roboco/llm/providers/grok_cli_config.py index 91251511..621457d0 100644 --- a/roboco/llm/providers/grok_cli_config.py +++ b/roboco/llm/providers/grok_cli_config.py @@ -51,39 +51,53 @@ GROK_AGENTS_PATH = Path.home() / ".grok" / "AGENTS.md" SYSTEM_PROMPT_PATH = Path( os.environ.get("ROBOCO_SYSTEM_PROMPT", "/app/system-prompt.md") ) +# grok loads blocking ``PreToolUse`` hooks from ``$HOME/.grok/hooks/*.json`` +# (always trusted). We install the SAME bash-guard the Claude path runs as a +# PreToolUse hook to get its full exfil-pattern analysis (credential files, +# /proc/environ, internal-API forgery, identity forgery, …) — far beyond the +# glob ``--deny`` rules. It runs with ``ROBOCO_GUARD_SKIP_GIT=1``: a grok hook +# deny CANCELS the run, which is the right response for an exfil attempt (no +# legit use) but wrong for a routine git op, so git stays on the graceful +# ``--deny`` rules. The script is baked into the agent base image. +GROK_HOOKS_DIR = Path.home() / ".grok" / "hooks" +BASH_GUARD_HOOK = os.environ.get( + "ROBOCO_BASH_GUARD_HOOK", "/app/scripts/bash-guard-hook.sh" +) # The entrypoint reads the computed flags (one token per line) from this file. GROK_ARGS_PATH = Path(os.environ.get("ROBOCO_GROK_ARGS_FILE", "/tmp/roboco-grok-args")) # Hard ceiling on agentic turns (loop guard). Operator-tunable. _DEFAULT_MAX_TURNS = 200 -# Roles that request reduced reasoning (grok bills reasoning at the output rate, -# so it dominates cost). Code-quality roles keep full reasoning; coordination / -# docs / board roles ask for ``low``. -_MINIMAL_REASONING_ROLES = frozenset( - { - "cell_pm", - "main_pm", - "documenter", - "product_owner", - "head_marketing", - "auditor", - "prompter", - "secretary", - } -) +# Reasoning effort is left at grok's model default for every role (parity with +# the Claude path, which sets no per-role thinking budget). An operator can still +# trade quality for cost across the fleet with ``ROBOCO_GROK_REASONING_EFFORT``. _FULL_REASONING_OVERRIDES = frozenset({"default", "full", "none", ""}) # Roles that legitimately run a shell. Review / board roles never do. _BASH_ROLES = frozenset({"developer", "documenter", "cell_pm", "main_pm"}) +# The intake interviewer reads the codebase to draft a task and may fan out +# exploration to subagents (parity with the Claude intake's ``Task`` allowance); +# every other role drives work through the gateway verbs, never CLI subagents. +_SUBAGENT_ALLOWED_ROLES = frozenset({"prompter"}) + # Grok CLI tool IDs (from the CLI's --tools/--disallowed-tools reference). _TOOL_SHELL = "run_terminal_cmd" _TOOL_EDIT = "search_replace" _TOOL_SUBAGENT = "Agent" -# Raw git mutation is gateway-mediated (the commit / open_pr verbs); agents never -# push or commit via raw bash. Denied for every bash-capable role. +# Raw git network / branch / history mutation is gateway-mediated (the commit / +# open_pr verbs); agents never run these via raw bash. Denied for every +# bash-capable role — the same set the Claude bash-guard blocks. +# +# Git ops are GRACEFUL native ``--deny`` denials (a blocked command returns a +# permission error to the model, which adapts to the gateway verb — the run +# continues), deliberately NOT routed through the bash-guard hook: verified live +# that a grok hook deny CANCELS the whole run, which would turn one reflexive git +# op into a dropped task. The exfil categories (credential reads, /proc/environ, +# internal-API forgery, …) DO run through the hook (see GROK_HOOKS_DIR) — there a +# hard cancel is the right response, since no legitimate agent triggers them. _GIT_MUTATE_DENY = ( "Bash(git push*)", "Bash(git fetch*)", @@ -92,6 +106,13 @@ _GIT_MUTATE_DENY = ( "Bash(git commit*)", "Bash(git remote*)", "Bash(git reset*)", + "Bash(git ls-remote*)", + "Bash(git checkout*)", + "Bash(git merge*)", + "Bash(git rebase*)", + "Bash(git cherry-pick*)", + "Bash(git revert*)", + "Bash(git update-ref*)", ) _DESTRUCTIVE_DENY = ("Bash(rm -rf*)",) @@ -126,7 +147,9 @@ def _allows_write(role: str) -> bool: def _disallowed_tools(role: str) -> str: """Comma-separated ``--disallowed-tools`` value for a role.""" - tools = [_TOOL_SUBAGENT] + tools: list[str] = [] + if role not in _SUBAGENT_ALLOWED_ROLES: + tools.append(_TOOL_SUBAGENT) if role not in _BASH_ROLES: tools.append(_TOOL_SHELL) if not _allows_write(role): @@ -141,18 +164,17 @@ def _deny_rules(role: str) -> list[str]: return [*_DESTRUCTIVE_DENY, *_GIT_MUTATE_DENY] -def _effort_for(role: str) -> str | None: - """Resolve ``--effort`` for a role; ``None`` keeps grok's default reasoning. +def _effort() -> str | None: + """Resolve ``--effort`` from the fleet override; ``None`` = grok's default. - A global ``ROBOCO_GROK_REASONING_EFFORT`` override wins over the per-role - default (``default`` / ``full`` / empty disables the reduction). + No per-role reduction (parity with Claude). A global + ``ROBOCO_GROK_REASONING_EFFORT`` lets an operator dial cost vs quality; + ``default`` / ``full`` / empty keeps the model default. """ override = os.environ.get("ROBOCO_GROK_REASONING_EFFORT", "").strip() - if override: - return ( - None if override.lower() in _FULL_REASONING_OVERRIDES else override.lower() - ) - return "low" if role in _MINIMAL_REASONING_ROLES else None + if override and override.lower() not in _FULL_REASONING_OVERRIDES: + return override.lower() + return None def grok_cli_args_for_role( @@ -160,14 +182,17 @@ def grok_cli_args_for_role( ) -> list[str]: """The per-role ``grok -p`` flag tokens (excludes ``-p``/model/cwd). - Order: tool removal, turn cap, deny rules, then effort. Each token is a - separate list element so callers can splice them without shell quoting. + Order: tool removal, web off, turn cap, deny rules, then effort. Each token is + a separate list element so callers can splice them without shell quoting. """ args: list[str] = ["--disallowed-tools", _disallowed_tools(role)] + # No direct web for any role (parity with the Claude path's tool set); the + # roles that get web reach it through the gated roboco-search MCP server. + args += ["--disable-web-search"] args += ["--max-turns", str(max_turns)] for rule in _deny_rules(role): args += ["--deny", rule] - effort = _effort_for(role) + effort = _effort() if effort: args += ["--effort", effort] return args @@ -207,8 +232,50 @@ def write_agents_md( return True +def bash_guard_hook_config(hook_path: str = BASH_GUARD_HOOK) -> dict[str, Any]: + """The grok hooks JSON installing the bash-guard as a blocking PreToolUse hook. + + Matcher ``Bash`` covers grok's ``run_terminal_command`` alias too. The hook + runs with ``ROBOCO_GUARD_SKIP_GIT=1`` so it only blocks the exfil categories + (git ops stay on the graceful ``--deny`` rules); it denies via exit 2. + """ + return { + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": hook_path, + "env": {"ROBOCO_GUARD_SKIP_GIT": "1"}, + } + ], + } + ] + } + } + + +def write_grok_hooks( + *, hooks_dir: Path = GROK_HOOKS_DIR, hook_path: str = BASH_GUARD_HOOK +) -> bool: + """Install the bash-guard PreToolUse hook into ``~/.grok/hooks/`` (best-effort). + + Skips writing (returns False) when the guard script is absent, so a missing + hook never fails the render. + """ + if not Path(hook_path).is_file(): + return False + hooks_dir.mkdir(parents=True, exist_ok=True) + (hooks_dir / "roboco-bash-guard.json").write_text( + json.dumps(bash_guard_hook_config(hook_path), indent=2), encoding="utf-8" + ) + return True + + def main() -> int: - """Entrypoint: write ``~/.grok/config.toml`` + AGENTS.md + the per-role args.""" + """Entrypoint: write ``~/.grok/config.toml`` + AGENTS.md + hooks + per-role args.""" agent_id = os.environ.get("ROBOCO_AGENT_ID", "") mcp_path = os.environ.get("ROBOCO_MCP_CONFIG", "/app/mcp-config.json") try: @@ -223,6 +290,7 @@ def main() -> int: render_config_toml(_load_mcp_config(mcp_path)), encoding="utf-8" ) write_agents_md() + write_grok_hooks() GROK_ARGS_PATH.write_text( "\n".join(grok_cli_args(agent_id, max_turns=max_turns)) + "\n", encoding="utf-8" ) diff --git a/tests/unit/llm/providers/test_grok_cli_config.py b/tests/unit/llm/providers/test_grok_cli_config.py index 9833468f..3e8bbe44 100644 --- a/tests/unit/llm/providers/test_grok_cli_config.py +++ b/tests/unit/llm/providers/test_grok_cli_config.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json import tomllib from typing import TYPE_CHECKING @@ -91,20 +92,76 @@ def test_main_pm_keeps_shell_but_denies_git(monkeypatch: pytest.MonkeyPatch) -> assert "run_terminal_cmd" not in dis # PM keeps a shell assert "search_replace" in dis # but does not write code assert "Bash(git push*)" in args # raw git mutation denied - assert args[args.index("--effort") + 1] == "low" + # Parity with Claude: model-default reasoning for every role, no per-role cut. + assert "--effort" not in args -def test_effort_override(monkeypatch: pytest.MonkeyPatch) -> None: +def test_prompter_allows_subagents_but_no_shell_or_edit( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delenv("ROBOCO_GROK_REASONING_EFFORT", raising=False) + dis = _disallowed(gc.grok_cli_args_for_role("prompter")) + # The intake interviewer may fan out to subagents (parity with Claude's Task)… + assert "Agent" not in dis + # …but it is still a read-only conversational role: no shell, no editing. + assert "run_terminal_cmd" in dis + assert "search_replace" in dis + + +def test_web_search_disabled_for_every_role() -> None: + for role in ("developer", "prompter", "secretary", "main_pm", "pr_reviewer"): + assert "--disable-web-search" in gc.grok_cli_args_for_role(role) + + +def test_effort_is_fleet_override_only(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("ROBOCO_GROK_REASONING_EFFORT", "high") - assert ( - gc.grok_cli_args("be-dev-1")[gc.grok_cli_args("be-dev-1").index("--effort") + 1] - == "high" - ) - # "full" disables the per-role reduction entirely. + args = gc.grok_cli_args("be-dev-1") + assert args[args.index("--effort") + 1] == "high" + # "full" / "default" / empty keep grok's model default (no --effort). monkeypatch.setenv("ROBOCO_GROK_REASONING_EFFORT", "full") assert "--effort" not in gc.grok_cli_args("main-pm") + monkeypatch.delenv("ROBOCO_GROK_REASONING_EFFORT", raising=False) + assert "--effort" not in gc.grok_cli_args("documenter") def test_max_turns_is_emitted() -> None: args = gc.grok_cli_args("be-dev-1", max_turns=7) assert args[args.index("--max-turns") + 1] == "7" + + +def test_bash_roles_deny_the_full_git_mutation_set() -> None: + # Graceful native --deny rules (the agent recovers) covering the same git + # network / branch / history ops the Claude bash-guard blocks. + args = gc.grok_cli_args_for_role("developer") + for op in ("push", "fetch", "clone", "checkout", "merge", "rebase", "revert"): + assert f"Bash(git {op}*)" in args + assert "Bash(rm -rf*)" in args + + +def test_bash_guard_hook_config_skips_git() -> None: + handler = gc.bash_guard_hook_config("/app/scripts/bash-guard-hook.sh")[ + "hooks" + ]["PreToolUse"][0] + assert handler["matcher"] == "Bash" + inner = handler["hooks"][0] + assert inner["command"] == "/app/scripts/bash-guard-hook.sh" + # Git is handled by graceful --deny, so the hook skips it (exfil only). + assert inner["env"]["ROBOCO_GUARD_SKIP_GIT"] == "1" + + +def test_write_grok_hooks_installs_when_script_present(tmp_path: Path) -> None: + script = tmp_path / "bash-guard-hook.sh" + script.write_text("#!/bin/bash\nexit 0\n", encoding="utf-8") + hooks_dir = tmp_path / ".grok" / "hooks" + assert gc.write_grok_hooks(hooks_dir=hooks_dir, hook_path=str(script)) is True + written = json.loads((hooks_dir / "roboco-bash-guard.json").read_text()) + assert written["hooks"]["PreToolUse"][0]["matcher"] == "Bash" + + +def test_write_grok_hooks_noops_when_script_absent(tmp_path: Path) -> None: + hooks_dir = tmp_path / ".grok" / "hooks" + assert ( + gc.write_grok_hooks(hooks_dir=hooks_dir, hook_path=str(tmp_path / "nope.sh")) + is False + ) + assert not hooks_dir.exists()