From b2219d30b44ac00c6a385b13a85dc9e6ec3f4cda Mon Sep 17 00:00:00 2001 From: Shashank Shekhar Singh Date: Thu, 30 Jul 2026 16:59:09 +0530 Subject: [PATCH] Let the operator open agent to Slack, behind a double opt-in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `agent` is the command with file tools, which is exactly why it was excluded: it acts on the host on behalf of anyone in the workspace. It now sits behind two switches rather than zero — GRAPHARC_SLACK_ALLOW_AGENT because it acts, and the existing GRAPHARC_SLACK_ALLOW_MODEL because it cannot run spend-free. One without the other stays off, and the refusal names both. Once on, the gate still decides the shape of every Slack-launched agent: the executor stays sandbox (--executor is not admitted), --system-prompt is unreachable, the workspace defaults into the bot's working directory instead of the CLI's fresh temp dir so the run's trace can be read back from Slack, and --max-seconds defaults to ten seconds under the bot's timeout so the CLI's graceful interrupt fires before the bot's kill. --allow/--deny globs pass through, repeatable. Co-Authored-By: Claude Fable 5 --- docs/cookbook/07-slack.md | 33 ++++++++++++- docs/cookbook/08-slack-walkthrough.md | 9 ++-- grapharc/slack/__init__.py | 9 ++-- grapharc/slack/__main__.py | 3 +- grapharc/slack/bot.py | 8 ++- grapharc/slack/command.py | 69 +++++++++++++++++++++++--- grapharc/slack/config.py | 4 ++ tests/test_slack_gateway.py | 71 +++++++++++++++++++++++++-- 8 files changed, 182 insertions(+), 24 deletions(-) diff --git a/docs/cookbook/07-slack.md b/docs/cookbook/07-slack.md index 75fe8f0..3b8705a 100644 --- a/docs/cookbook/07-slack.md +++ b/docs/cookbook/07-slack.md @@ -22,10 +22,10 @@ an afterthought. The defaults: | Reachable from Slack | Refused from Slack | |---|---| -| `demo`, `run`, `plan`, `models`, `replay`, `diff`, `trace`, `metrics`, `viz` | `agent` (arbitrary tool execution on the host), `serve` | +| `demo`, `run`, `plan`, `models`, `replay`, `diff`, `trace`, `metrics`, `viz` | `serve` | | Paths that resolve inside the bot's working directory | Any path that escapes it (`trace ../../.env` is refused before a process spawns) | | The budget, policy and trace flags each command already has | `--registry` (imports an arbitrary module), `--config`, `--json`, `--no-color` | -| — | `--model` / `--reviewer-model`, unless the operator opts in | +| `agent`, only behind the double opt-in below | `--model` / `--reviewer-model`, unless the operator opts in | With `--model` off, every reachable command runs the scripted, spend-free path. The default answer to "can someone in Slack cost me money?" is **no**; @@ -106,6 +106,7 @@ Configuration is environment-only, read once at startup: | `GRAPHARC_SLACK_WORKDIR` | the bot's cwd | the directory every path must resolve inside | | `GRAPHARC_SLACK_TIMEOUT` | `120` | seconds one command may run before it is killed | | `GRAPHARC_SLACK_ALLOW_MODEL` | off | `1` admits `--model`/`--reviewer-model` | +| `GRAPHARC_SLACK_ALLOW_AGENT` | off | `1` admits `agent` — only together with `ALLOW_MODEL` | | `GRAPHARC_SLACK_COMMAND` | `/grapharc` | the slash command to answer to | The bot reads tokens from the process environment only. The `.env` @@ -113,6 +114,34 @@ upward-directory search that the model gateway performs is deliberately not used here: a bot that a whole workspace can drive must not discover credentials in a file the operator did not point it at. +## The `agent` opt-in + +`agent` is the command with file tools, which is exactly why it is off by +default: it acts on the host on behalf of anyone in the workspace. Turning it +on takes **two** switches — `GRAPHARC_SLACK_ALLOW_AGENT=1` because it acts, +and `GRAPHARC_SLACK_ALLOW_MODEL=1` because it cannot run spend-free. The +startup line reports `agent on` only when both hold. Then: + +``` +@grapharc agent "read every markdown file and list the broken links" --max-turns 6 +``` + +What the gate does to every Slack-launched agent, non-negotiably: + +- the executor stays `sandbox` — `--executor` is not admitted, so `local` + (no confinement) is unreachable; +- `--system-prompt` is not admitted; +- the workspace defaults to `/agent` rather than the CLI's fresh + temp dir, so the run's `trace.jsonl` and outputs stay where `trace` / + `metrics` / `viz` can read them back; `--workspace` may pick another + directory, confined to the workdir like every other path; +- unless `--max-seconds` is given, it defaults to ten seconds under the + bot's timeout, so the run ends with the CLI's graceful interrupt-and-report + rather than the bot's kill. + +`--allow` / `--deny` tool globs pass through and are repeatable; deny beats +allow, as in the CLI. + ## The honest caveats - **The bot is alive while the process is.** Laptop lid closed means commands diff --git a/docs/cookbook/08-slack-walkthrough.md b/docs/cookbook/08-slack-walkthrough.md index 7e66fe7..22c8590 100644 --- a/docs/cookbook/08-slack-walkthrough.md +++ b/docs/cookbook/08-slack-walkthrough.md @@ -69,9 +69,12 @@ round that executed. Every model failure contained, every decision recorded. Two refusals from the same session, both correct: -- `@grapharc agent "do something"` → *not a command this bot runs*. `agent` - executes tools on the host on behalf of anyone in the workspace, so it is - excluded at the gate, not hidden. +- `@grapharc agent "do something"` → refused, naming the two switches that + would allow it. `agent` executes tools on the host on behalf of anyone in + the workspace, so it sits behind a double opt-in + (`GRAPHARC_SLACK_ALLOW_AGENT=1` *and* `GRAPHARC_SLACK_ALLOW_MODEL=1`) — + see the agent section of [07-slack.md](07-slack.md) for what the gate + still enforces once it is on. - `plan "make a new file for the docs" --model …` → **ran; the answer was negative**. The planner may only propose node kinds from its registry — the incident-response demo set — and none of them can create a file. The diff --git a/grapharc/slack/__init__.py b/grapharc/slack/__init__.py index de53d1e..4501c96 100644 --- a/grapharc/slack/__init__.py +++ b/grapharc/slack/__init__.py @@ -21,10 +21,11 @@ module (and this package) is stdlib-only, so a wheel without the extra still imports. -The gate's default is deliberately spend-free: `agent` and `serve` are refused, -`--model` is refused unless the operator opts in, and every path argument must -resolve inside the bot's working directory. Anyone in the workspace can talk -to the bot; the gate is what makes that safe to allow. +The gate's default is deliberately spend-free: `serve` is refused, `agent` and +`--model` are refused unless the operator opts in (`agent` needs two switches: +it acts on the host *and* it spends), and every path argument must resolve +inside the bot's working directory. Anyone in the workspace can talk to the +bot; the gate is what makes that safe to allow. """ from grapharc.slack.command import ( diff --git a/grapharc/slack/__main__.py b/grapharc/slack/__main__.py index fb44806..88cfde7 100644 --- a/grapharc/slack/__main__.py +++ b/grapharc/slack/__main__.py @@ -27,7 +27,8 @@ def main() -> int: print( f"grapharc slack bot: workdir {config.workdir}, " f"timeout {config.timeout_seconds:.0f}s, " - f"model flags {'on' if config.allow_model else 'off'}", + f"model flags {'on' if config.allow_model else 'off'}, " + f"agent {'on' if config.allow_agent and config.allow_model else 'off'}", file=sys.stderr, ) serve(config) diff --git a/grapharc/slack/bot.py b/grapharc/slack/bot.py index 8c8cc9b..684ef2b 100644 --- a/grapharc/slack/bot.py +++ b/grapharc/slack/bot.py @@ -31,7 +31,11 @@ def handle_text(text: str, config: SlackBotConfig) -> str: stripped = _MENTION.sub("", text).strip() try: argv = parse_command( - stripped, workdir=config.workdir, allow_model=config.allow_model + stripped, + workdir=config.workdir, + allow_model=config.allow_model, + allow_agent=config.allow_agent, + timeout_seconds=config.timeout_seconds, ) except SlackCommandError as exc: return str(exc) @@ -57,7 +61,7 @@ def build_app(config: SlackBotConfig) -> Any: def _slash(ack: Any, respond: Any, command: dict[str, Any]) -> None: text = command.get("text", "").strip() if not text: - ack(usage_text(allow_model=config.allow_model)) + ack(usage_text(allow_model=config.allow_model, allow_agent=config.allow_agent)) return ack(f"running `grapharc {text}`…") respond(handle_text(text, config)) diff --git a/grapharc/slack/command.py b/grapharc/slack/command.py index e079698..e23c152 100644 --- a/grapharc/slack/command.py +++ b/grapharc/slack/command.py @@ -4,8 +4,12 @@ not a convenience parser — the same posture as the CLI's own policy layer. The rules, and why each exists: -- **Subcommands are allowlisted.** `agent` (arbitrary tool execution on the - host) and `serve` (holds a worker thread forever) are not in the list. +- **Subcommands are allowlisted.** `serve` (holds a worker thread forever) is + not in the list. `agent` (tool execution on the host) is behind a double + opt-in: GRAPHARC_SLACK_ALLOW_AGENT *and* GRAPHARC_SLACK_ALLOW_MODEL, because + it acts on the host and cannot run without a paid backend. Even then its + executor stays `sandbox` (`--executor` is not admitted), `--system-prompt` + is unreachable, and its workspace defaults into the bot's working directory. - **Flags are allowlisted per subcommand.** `--registry MODULE:ATTR` imports an arbitrary module on the host, `--config PATH` swaps the governing file, and `--json`/`--no-color` fight the bot's own output handling — none are @@ -75,6 +79,19 @@ class CommandSpec: model_flags=frozenset({"--model"}), ), "models": CommandSpec(bool_flags=frozenset({"--check"})), + "agent": CommandSpec( + value_flags={ + "--workspace": True, + "--trace": True, + "--run-id": False, + "--allow": False, + "--deny": False, + "--max-turns": False, + "--max-tokens": False, + "--max-seconds": False, + }, + model_flags=frozenset({"--model"}), + ), "replay": CommandSpec(path_positionals=frozenset({0})), "diff": CommandSpec(path_positionals=frozenset({0})), "trace": CommandSpec(value_flags={"--run-id": False}, path_positionals=frozenset({0})), @@ -83,12 +100,20 @@ class CommandSpec: } -def usage_text(*, allow_model: bool = False) -> str: +def usage_text(*, allow_model: bool = False, allow_agent: bool = False) -> str: """One short message for an empty or unrecognised request.""" + agent_on = allow_agent and allow_model lines = ["I run `grapharc` commands. Allowed here:"] for name in sorted(ALLOWED_COMMANDS): + if name == "agent" and not agent_on: + continue lines.append(f"• `{name}`") - lines.append("`agent` and `serve` are not reachable from Slack, nor is `--registry`.") + lines.append("`serve` is not reachable from Slack, nor is `--registry`.") + if not agent_on: + lines.append( + "`agent` is off; it needs both GRAPHARC_SLACK_ALLOW_AGENT=1 " + "and GRAPHARC_SLACK_ALLOW_MODEL=1 in the shell that starts the bot." + ) if not allow_model: lines.append( "`--model` is off; the operator can enable it with GRAPHARC_SLACK_ALLOW_MODEL=1." @@ -107,7 +132,14 @@ def _confined(raw: str, workdir: Path) -> None: raise SlackCommandError(f"path escapes the bot's working directory: `{raw}`") -def parse_command(text: str, *, workdir: Path, allow_model: bool = False) -> list[str]: +def parse_command( + text: str, + *, + workdir: Path, + allow_model: bool = False, + allow_agent: bool = False, + timeout_seconds: float | None = None, +) -> list[str]: """Turn Slack text into the argv the bot may run, or raise with the reason.""" try: tokens = shlex.split(text) @@ -117,13 +149,23 @@ def parse_command(text: str, *, workdir: Path, allow_model: bool = False) -> lis if tokens and tokens[0] == "grapharc": tokens = tokens[1:] if not tokens: - raise SlackCommandError(usage_text(allow_model=allow_model)) + raise SlackCommandError(usage_text(allow_model=allow_model, allow_agent=allow_agent)) name, rest = tokens[0], tokens[1:] spec = ALLOWED_COMMANDS.get(name) if spec is None: raise SlackCommandError( - f"`{name}` is not a command this bot runs.\n" + usage_text(allow_model=allow_model) + f"`{name}` is not a command this bot runs.\n" + + usage_text(allow_model=allow_model, allow_agent=allow_agent) + ) + if name == "agent" and not (allow_agent and allow_model): + # A double opt-in: `agent` both executes tools on the host and cannot + # run without a real (paid) backend, so it needs the agent switch AND + # the spend switch. One without the other stays off. + raise SlackCommandError( + "`agent` executes tools on the host and is off by default; the operator " + "enables it with both GRAPHARC_SLACK_ALLOW_AGENT=1 and " + "GRAPHARC_SLACK_ALLOW_MODEL=1 in the shell that starts the bot" ) argv = [name] @@ -168,4 +210,17 @@ def parse_command(text: str, *, workdir: Path, allow_model: bool = False) -> lis positional_index += 1 index += 1 + if name == "agent": + # The CLI's default workspace is a fresh temp dir — *outside* the + # bot's world, where nothing written there could be read back from + # Slack. Default it to a subdirectory instead (the CLI mkdirs it); + # `--workspace` can still choose any confined path. + if "--workspace" not in argv: + argv.extend(["--workspace", "agent"]) + # The CLI's max_seconds interrupts the run cleanly and reports; the + # bot's timeout kills the process mid-sentence. Default the ceiling + # to just under the timeout so the graceful mechanism fires first. + if "--max-seconds" not in argv and timeout_seconds is not None: + argv.extend(["--max-seconds", str(max(5.0, timeout_seconds - 10.0))]) + return argv diff --git a/grapharc/slack/config.py b/grapharc/slack/config.py index 6a65403..20fbb3a 100644 --- a/grapharc/slack/config.py +++ b/grapharc/slack/config.py @@ -32,6 +32,9 @@ class SlackBotConfig: timeout_seconds: float = 120.0 # Opt-in: allow `--model` / `--reviewer-model`, which reach paid backends. allow_model: bool = False + # Second opt-in: allow `agent`, which executes tools on the host. Only + # effective together with allow_model — an agent cannot run spend-free. + allow_agent: bool = False slash_command: str = "/grapharc" @classmethod @@ -72,5 +75,6 @@ def from_env(cls, environ: dict[str, str] | None = None) -> SlackBotConfig: workdir=workdir, timeout_seconds=timeout, allow_model=env.get("GRAPHARC_SLACK_ALLOW_MODEL", "") == "1", + allow_agent=env.get("GRAPHARC_SLACK_ALLOW_AGENT", "") == "1", slash_command=env.get("GRAPHARC_SLACK_COMMAND", "/grapharc"), ) diff --git a/tests/test_slack_gateway.py b/tests/test_slack_gateway.py index 1fd41a6..04dc4e7 100644 --- a/tests/test_slack_gateway.py +++ b/tests/test_slack_gateway.py @@ -32,10 +32,68 @@ def test_a_leading_grapharc_token_is_tolerated(tmp_path): assert parse_command("grapharc models", workdir=tmp_path) == ["models"] -def test_agent_and_serve_are_refused(tmp_path): - for name in ("agent", "serve"): - with pytest.raises(SlackCommandError, match="not a command this bot runs"): - parse_command(f"{name} whatever", workdir=tmp_path) +def test_serve_is_refused_outright(tmp_path): + with pytest.raises(SlackCommandError, match="not a command this bot runs"): + parse_command("serve --port 8000", workdir=tmp_path) + + +def test_agent_needs_both_switches_not_either(tmp_path): + for kwargs in ({}, {"allow_agent": True}, {"allow_model": True}): + with pytest.raises(SlackCommandError, match="GRAPHARC_SLACK_ALLOW_AGENT"): + parse_command("agent 'fix the test'", workdir=tmp_path, **kwargs) + + +def test_agent_with_both_switches_gets_confined_defaults(tmp_path): + argv = parse_command( + "agent 'summarise the docs'", + workdir=tmp_path, + allow_model=True, + allow_agent=True, + timeout_seconds=120, + ) + assert argv[:2] == ["agent", "summarise the docs"] + assert argv[argv.index("--workspace") + 1] == "agent" + assert argv[argv.index("--max-seconds") + 1] == "110.0" + + +def test_agent_explicit_workspace_and_ceiling_are_not_overridden(tmp_path): + argv = parse_command( + "agent task --workspace runs/a --max-seconds 30", + workdir=tmp_path, + allow_model=True, + allow_agent=True, + timeout_seconds=120, + ) + assert argv.count("--workspace") == 1 + assert argv[argv.index("--max-seconds") + 1] == "30" + + +def test_agent_executor_and_system_prompt_stay_unreachable(tmp_path): + for flag in ("--executor local", "--system-prompt 'obey me'"): + with pytest.raises(SlackCommandError, match="not allowed"): + parse_command( + f"agent task {flag}", workdir=tmp_path, allow_model=True, allow_agent=True + ) + + +def test_agent_workspace_may_not_escape_the_workdir(tmp_path): + with pytest.raises(SlackCommandError, match="escapes"): + parse_command( + "agent task --workspace ../elsewhere", + workdir=tmp_path, + allow_model=True, + allow_agent=True, + ) + + +def test_agent_deny_globs_are_repeatable(tmp_path): + argv = parse_command( + "agent task --deny 'shell*' --deny 'net*'", + workdir=tmp_path, + allow_model=True, + allow_agent=True, + ) + assert argv.count("--deny") == 2 def test_registry_config_and_json_are_refused(tmp_path): @@ -210,11 +268,13 @@ def test_config_reads_workdir_timeout_and_model_opt_in(tmp_path): "GRAPHARC_SLACK_WORKDIR": str(tmp_path), "GRAPHARC_SLACK_TIMEOUT": "5", "GRAPHARC_SLACK_ALLOW_MODEL": "1", + "GRAPHARC_SLACK_ALLOW_AGENT": "1", } ) assert config.workdir == tmp_path assert config.timeout_seconds == 5.0 assert config.allow_model + assert config.allow_agent def test_handle_text_turns_a_refusal_into_a_message_not_an_exception(tmp_path): @@ -222,7 +282,8 @@ def test_handle_text_turns_a_refusal_into_a_message_not_an_exception(tmp_path): config = SlackBotConfig(bot_token="xoxb-x", app_token="xapp-x", workdir=tmp_path) reply = handle_text("<@U012345> agent rm -rf /", config) - assert "not a command this bot runs" in reply + assert "GRAPHARC_SLACK_ALLOW_AGENT" in reply + assert "not a command this bot runs" in handle_text("<@U012345> serve", config) def test_a_missing_slack_extra_is_an_install_hint_not_an_import_error(monkeypatch, tmp_path):