mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(grok): scope opencode edit/bash/external-dir permissions per role
Grok wrote ONE global permission block, so a Grok pr_reviewer (or qa / PM / auditor) ran with edit=allow + bash=allow on untrusted PR content. Now the permissions are derived per role, mirroring orchestrator._get_role_permissions on the Claude path: - edit — allow only roles that write code (role_config.allows_write: developer / documenter); everyone else edit=deny. - bash — allow only roles that legitimately run a shell (developer / documenter / cell_pm / main_pm); the read-only reviewers (qa / pr_reviewer / auditor) and the board get bash=deny. secret-scrub still guards the rest. - external_directory — only the pr_reviewer reads scratch outside its cwd (the /tmp diff); delivery roles get deny. One-shot roles resolve these in GrokProvider._append_grok_env; the interactive intake/secretary set edit=deny + bash=deny in the orchestrator (intake keeps external-dir reads for sibling product repos, the secretary does not). The Claude path is untouched — the permission env is a GROK-only contract. Targeted gate green (ruff/mypy/xenon + provider + interactive-spawn tests).
This commit is contained in:
@@ -44,6 +44,7 @@ from typing import TYPE_CHECKING, Protocol
|
|||||||
from roboco.agents_config import get_agent_role
|
from roboco.agents_config import get_agent_role
|
||||||
from roboco.llm.providers._docker import container_running, stop_container
|
from roboco.llm.providers._docker import container_running, stop_container
|
||||||
from roboco.llm.providers.base import AgentProvider, ProviderError, SpawnResult
|
from roboco.llm.providers.base import AgentProvider, ProviderError, SpawnResult
|
||||||
|
from roboco.services.gateway.role_config import get_role_config
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
@@ -90,6 +91,42 @@ _MINIMAL_REASONING_ROLES = frozenset(
|
|||||||
)
|
)
|
||||||
_FULL_REASONING_OVERRIDES = frozenset({"default", "full", "none", ""})
|
_FULL_REASONING_OVERRIDES = frozenset({"default", "full", "none", ""})
|
||||||
|
|
||||||
|
# Per-role opencode permission policy (Claude-parity with
|
||||||
|
# orchestrator._get_role_permissions). Claude denies Write/Edit for the read-only
|
||||||
|
# roles and Bash(git commit/push) for PMs; opencode's permission is coarser
|
||||||
|
# (allow/deny per tool class), so:
|
||||||
|
# * edit — allow only roles that write code (role_config.allows_write:
|
||||||
|
# developer / documenter). Everyone else edit=deny.
|
||||||
|
# * bash — allow only roles that legitimately run a shell; the read-only
|
||||||
|
# reviewers (qa / pr_reviewer / auditor) and the board never do. secret-scrub
|
||||||
|
# still guards bash (git-mutate / cred files) for the roles that keep it.
|
||||||
|
# * external_directory — only the pr_reviewer reads scratch outside its cwd
|
||||||
|
# (a diff it writes to /tmp). Delivery roles work inside their workspace, so
|
||||||
|
# external_directory=deny (the headless-ask auto-deny that blocked the
|
||||||
|
# pr-reviewer is moot once that one role is explicitly allowed).
|
||||||
|
_BASH_ROLES = frozenset({"developer", "documenter", "cell_pm", "main_pm"})
|
||||||
|
_EXTERNAL_DIR_ROLES = frozenset({"pr_reviewer"})
|
||||||
|
|
||||||
|
|
||||||
|
def _edit_permission_for(agent_id: str) -> str:
|
||||||
|
"""opencode ``edit`` permission for an agent's role (allow iff it writes code)."""
|
||||||
|
role = get_agent_role(agent_id) or ""
|
||||||
|
try:
|
||||||
|
return "allow" if get_role_config(role).allows_write else "deny"
|
||||||
|
except KeyError:
|
||||||
|
return "deny" # unknown role → safest
|
||||||
|
|
||||||
|
|
||||||
|
def _bash_permission_for(agent_id: str) -> str:
|
||||||
|
"""opencode ``bash`` permission for an agent's role."""
|
||||||
|
return "allow" if (get_agent_role(agent_id) or "") in _BASH_ROLES else "deny"
|
||||||
|
|
||||||
|
|
||||||
|
def _external_dir_permission_for(agent_id: str) -> str:
|
||||||
|
"""opencode ``external_directory`` permission for an agent's role."""
|
||||||
|
role = get_agent_role(agent_id) or ""
|
||||||
|
return "allow" if role in _EXTERNAL_DIR_ROLES else "deny"
|
||||||
|
|
||||||
|
|
||||||
def _reasoning_effort_for(agent_id: str) -> str | None:
|
def _reasoning_effort_for(agent_id: str) -> str | None:
|
||||||
"""Resolve the opencode --variant reasoning effort for an agent.
|
"""Resolve the opencode --variant reasoning effort for an agent.
|
||||||
@@ -243,6 +280,20 @@ class GrokProvider(AgentProvider):
|
|||||||
# Reused as the generic agent session id so the transcript stays
|
# Reused as the generic agent session id so the transcript stays
|
||||||
# locatable at finalize, exactly as on the Claude Code path.
|
# locatable at finalize, exactly as on the Claude Code path.
|
||||||
cmd.extend(["-e", f"ROBOCO_AGENT_SESSION_ID={config.claude_session_id}"])
|
cmd.extend(["-e", f"ROBOCO_AGENT_SESSION_ID={config.claude_session_id}"])
|
||||||
|
# Per-role opencode permissions (Claude-parity): read-only roles get
|
||||||
|
# edit=deny, only delivery roles get bash, only the pr-reviewer gets
|
||||||
|
# external-directory reads. opencode_config.main() reads these.
|
||||||
|
cmd.extend(
|
||||||
|
[
|
||||||
|
"-e",
|
||||||
|
f"ROBOCO_GROK_EDIT_PERMISSION={_edit_permission_for(config.agent_id)}",
|
||||||
|
"-e",
|
||||||
|
f"ROBOCO_GROK_BASH_PERMISSION={_bash_permission_for(config.agent_id)}",
|
||||||
|
"-e",
|
||||||
|
"ROBOCO_GROK_EXTERNAL_DIR_PERMISSION="
|
||||||
|
f"{_external_dir_permission_for(config.agent_id)}",
|
||||||
|
]
|
||||||
|
)
|
||||||
# Reasoning effort (opencode --variant) by role; omitted = full reasoning.
|
# Reasoning effort (opencode --variant) by role; omitted = full reasoning.
|
||||||
variant = _reasoning_effort_for(config.agent_id)
|
variant = _reasoning_effort_for(config.agent_id)
|
||||||
if variant:
|
if variant:
|
||||||
|
|||||||
@@ -3415,6 +3415,23 @@ class AgentOrchestrator:
|
|||||||
"ROBOCO_SYSTEM_PROMPT=/app/system-prompt.md",
|
"ROBOCO_SYSTEM_PROMPT=/app/system-prompt.md",
|
||||||
]
|
]
|
||||||
)
|
)
|
||||||
|
# Both interactive roles are read-only conversational agents: no code
|
||||||
|
# edits, no shell (Claude-parity — the SDK gates deny everything but
|
||||||
|
# Read/Grep/Glob + their tools). Intake reads sibling product repos
|
||||||
|
# that sit OUTSIDE its cwd, so it keeps external-directory reads; the
|
||||||
|
# Secretary only reads /app + the API, so it doesn't.
|
||||||
|
is_intake = isinstance(spec, _IntakeRunSpec)
|
||||||
|
cmd.extend(
|
||||||
|
[
|
||||||
|
"-e",
|
||||||
|
"ROBOCO_GROK_EDIT_PERMISSION=deny",
|
||||||
|
"-e",
|
||||||
|
"ROBOCO_GROK_BASH_PERMISSION=deny",
|
||||||
|
"-e",
|
||||||
|
"ROBOCO_GROK_EXTERNAL_DIR_PERMISSION="
|
||||||
|
f"{'allow' if is_intake else 'deny'}",
|
||||||
|
]
|
||||||
|
)
|
||||||
# Per-role reasoning effort: the opencode-serve driver passes this as
|
# Per-role reasoning effort: the opencode-serve driver passes this as
|
||||||
# the message `variant` (same lever as the one-shot --variant).
|
# the message `variant` (same lever as the one-shot --variant).
|
||||||
if spec.grok_variant:
|
if spec.grok_variant:
|
||||||
|
|||||||
@@ -23,7 +23,12 @@ from roboco.llm.providers import (
|
|||||||
ProviderRegistry,
|
ProviderRegistry,
|
||||||
SpawnResult,
|
SpawnResult,
|
||||||
)
|
)
|
||||||
from roboco.llm.providers.grok import _reasoning_effort_for
|
from roboco.llm.providers.grok import (
|
||||||
|
_bash_permission_for,
|
||||||
|
_edit_permission_for,
|
||||||
|
_external_dir_permission_for,
|
||||||
|
_reasoning_effort_for,
|
||||||
|
)
|
||||||
from roboco.models.base import ModelProvider
|
from roboco.models.base import ModelProvider
|
||||||
from roboco.models.runtime import OrchestratorAgentConfig
|
from roboco.models.runtime import OrchestratorAgentConfig
|
||||||
|
|
||||||
@@ -279,6 +284,59 @@ def test_reasoning_effort_override(monkeypatch: pytest.MonkeyPatch) -> None:
|
|||||||
assert _reasoning_effort_for("be-pm") is None # "default" => full reasoning
|
assert _reasoning_effort_for("be-pm") is None # "default" => full reasoning
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Per-role opencode permissions (Claude-parity)
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_edit_permission_allows_only_writer_roles() -> None:
|
||||||
|
assert _edit_permission_for("be-dev-1") == "allow"
|
||||||
|
assert _edit_permission_for("be-doc") == "allow"
|
||||||
|
for slug in ("be-qa", "pr-reviewer-1", "be-pm", "main-pm", "auditor"):
|
||||||
|
assert _edit_permission_for(slug) == "deny", slug
|
||||||
|
|
||||||
|
|
||||||
|
def test_bash_permission_allows_only_shell_roles() -> None:
|
||||||
|
for slug in ("be-dev-1", "be-doc", "be-pm", "main-pm"):
|
||||||
|
assert _bash_permission_for(slug) == "allow", slug
|
||||||
|
for slug in ("be-qa", "pr-reviewer-1", "auditor", "product-owner"):
|
||||||
|
assert _bash_permission_for(slug) == "deny", slug
|
||||||
|
|
||||||
|
|
||||||
|
def test_external_dir_permission_only_pr_reviewer() -> None:
|
||||||
|
assert _external_dir_permission_for("pr-reviewer-1") == "allow"
|
||||||
|
for slug in ("be-dev-1", "be-qa", "be-pm", "auditor"):
|
||||||
|
assert _external_dir_permission_for(slug) == "deny", slug
|
||||||
|
|
||||||
|
|
||||||
|
async def test_grok_spawn_sets_readonly_permissions_for_reviewer() -> None:
|
||||||
|
# A read-only reviewer (qa) gets edit=deny + bash=deny + external_dir=deny.
|
||||||
|
host = _FakeHost()
|
||||||
|
provider = GrokProvider(host)
|
||||||
|
with patch(
|
||||||
|
"asyncio.create_subprocess_exec", AsyncMock(return_value=_proc())
|
||||||
|
) as exec_mock:
|
||||||
|
await provider.spawn(_config(agent_id="be-qa"))
|
||||||
|
cmd = list(exec_mock.call_args.args)
|
||||||
|
assert "ROBOCO_GROK_EDIT_PERMISSION=deny" in cmd
|
||||||
|
assert "ROBOCO_GROK_BASH_PERMISSION=deny" in cmd
|
||||||
|
assert "ROBOCO_GROK_EXTERNAL_DIR_PERMISSION=deny" in cmd
|
||||||
|
|
||||||
|
|
||||||
|
async def test_grok_spawn_pr_reviewer_is_read_only_but_reads_scratch() -> None:
|
||||||
|
# The pr-reviewer never writes code (edit=deny) but reads its /tmp diff
|
||||||
|
# (external_directory=allow) — the one role that needs it.
|
||||||
|
host = _FakeHost()
|
||||||
|
provider = GrokProvider(host)
|
||||||
|
with patch(
|
||||||
|
"asyncio.create_subprocess_exec", AsyncMock(return_value=_proc())
|
||||||
|
) as exec_mock:
|
||||||
|
await provider.spawn(_config(agent_id="pr-reviewer-1"))
|
||||||
|
cmd = list(exec_mock.call_args.args)
|
||||||
|
assert "ROBOCO_GROK_EDIT_PERMISSION=deny" in cmd
|
||||||
|
assert "ROBOCO_GROK_EXTERNAL_DIR_PERMISSION=allow" in cmd
|
||||||
|
|
||||||
|
|
||||||
async def test_grok_spawn_sets_variant_for_minimal_role() -> None:
|
async def test_grok_spawn_sets_variant_for_minimal_role() -> None:
|
||||||
host = _FakeHost()
|
host = _FakeHost()
|
||||||
provider = GrokProvider(host)
|
provider = GrokProvider(host)
|
||||||
|
|||||||
@@ -67,6 +67,21 @@ def test_intake_grok_uses_openai_env_and_opencode_mount() -> None:
|
|||||||
assert cmd[-1] == GROK_PROMPTER_IMAGE
|
assert cmd[-1] == GROK_PROMPTER_IMAGE
|
||||||
# The xAI endpoint is never mislabelled as Anthropic.
|
# The xAI endpoint is never mislabelled as Anthropic.
|
||||||
assert not any(c.startswith("ANTHROPIC_") for c in cmd)
|
assert not any(c.startswith("ANTHROPIC_") for c in cmd)
|
||||||
|
# Intake is read-only (no code edits, no shell) but reads sibling product
|
||||||
|
# repos OUTSIDE its cwd, so it keeps external-directory reads.
|
||||||
|
assert "ROBOCO_GROK_EDIT_PERMISSION=deny" in cmd
|
||||||
|
assert "ROBOCO_GROK_BASH_PERMISSION=deny" in cmd
|
||||||
|
assert "ROBOCO_GROK_EXTERNAL_DIR_PERMISSION=allow" in cmd
|
||||||
|
|
||||||
|
|
||||||
|
def test_intake_anthropic_omits_grok_permission_env() -> None:
|
||||||
|
# The opencode permission env is a GROK-only contract; the Claude path never
|
||||||
|
# sets it (it gates tools via the SDK can_use_tool allowlist instead).
|
||||||
|
cmd = AgentOrchestrator._build_intake_run_cmd(
|
||||||
|
_intake_spec("anthropic", base_url="https://api.anthropic.com", token="sk-ant")
|
||||||
|
)
|
||||||
|
assert not any(c.startswith("ROBOCO_GROK_EDIT_PERMISSION=") for c in cmd)
|
||||||
|
assert not any(c.startswith("ROBOCO_GROK_BASH_PERMISSION=") for c in cmd)
|
||||||
|
|
||||||
|
|
||||||
def test_intake_grok_omits_variant_when_unset() -> None:
|
def test_intake_grok_omits_variant_when_unset() -> None:
|
||||||
@@ -109,3 +124,8 @@ def test_secretary_grok_uses_openai_env_and_grok_image() -> None:
|
|||||||
assert "ROBOCO_AGENT_TOKEN=hmac-secretary" in cmd
|
assert "ROBOCO_AGENT_TOKEN=hmac-secretary" in cmd
|
||||||
assert cmd[-1] == GROK_SECRETARY_IMAGE
|
assert cmd[-1] == GROK_SECRETARY_IMAGE
|
||||||
assert not any(c.startswith("ANTHROPIC_") for c in cmd)
|
assert not any(c.startswith("ANTHROPIC_") for c in cmd)
|
||||||
|
# The Secretary is read-only and reads only /app + the API, so edit/bash
|
||||||
|
# are denied and it gets NO external-directory reads (unlike intake).
|
||||||
|
assert "ROBOCO_GROK_EDIT_PERMISSION=deny" in cmd
|
||||||
|
assert "ROBOCO_GROK_BASH_PERMISSION=deny" in cmd
|
||||||
|
assert "ROBOCO_GROK_EXTERNAL_DIR_PERMISSION=deny" in cmd
|
||||||
|
|||||||
Reference in New Issue
Block a user