Safe-mode shell blocklist scans the entire command text (including here-doc payloads), causing false-positive blocks and non-uniform enforcement across safe_mode #114

New issue

Open

opened 2026-08-05 17:53:43 +00:00 by CoreRasurae · 0 comments

CoreRasurae commented

2026-08-05 17:53:43 +00:00

Member

Metadata

Commit Message: feat(agents): scope safe-mode shell blocklist to invocation regions
Branch: feature/m3-shell-safe-mode-blocklist-matching

Background and context

Actor Configuration Standard v1.1.0 (docs/index.md) defines the safe-mode shell
blocklist twice:

§4.5.4.1: "The shell tool MUST refuse to execute any command containing any of
the literal substrings: rm, del, format, shutdown, reboot, kill
(case-insensitive comparison)."
§13.5: "The safe-mode blocklist defined in §4.5.4 ... MUST always be enforced
regardless of mode."

cleveractors.agents.tool.ToolAgent._execute_shell_command (commit 5520daf)
implements this as a single check:

if self.safe_mode:
    dangerous_commands = ["rm", "del", "format", "shutdown", "reboot", "kill"]
    if any(cmd in command.lower() for cmd in dangerous_commands):
        raise ExecutionError(f"Dangerous command '{command}' blocked in safe mode")

command is the entire shell string passed to the shell tool, including any
here-doc payload the command writes to disk. Since the check is a raw substring
scan over that whole string, it fires on ordinary English words and Python
identifiers that happen to contain a blocked token, with no relation to what the
shell will actually execute — "del" ⊂ "deliberately", "format" ⊂
"format_value"/format(...), etc.

This was hit running packages/calculator-app-actor.yaml (a calculator_builder
LLM agent with allow_shell: true, tools: [file_read, file_write, shell],
writing a Python calculator app via cat > file.py <<'PYEOF' ... PYEOF heredocs).
Every attempt to write commands.py or engine.py was rejected — not because the
agent tried to run rm/kill/etc., but because the file content being written
contained the words "deliberately" and "format_value"/format(...).

Separately, the current if self.safe_mode: gate means the blocklist is not
enforced when safe_mode: false, which contradicts §13.5's "MUST always be
enforced regardless of mode." Code and spec disagree here, and per this
project's rule the spec is authoritative — the code is wrong on this point
independent of anything else in this issue.

Current behavior

Blocklist matching scans the full command string passed to the shell
tool, with no distinction between text that will be interpreted as shell
syntax and text that is inert payload (here-doc bodies, quoted string
literals, comments).
Benign file content containing "deliberately", "format_value", "confirm",
"perform", "skillful", etc. is blocked, even though none of it causes rm,
del, format, shutdown, reboot, or kill to execute.
The check only runs if self.safe_mode:, so it is skipped entirely when an
agent has safe_mode: false — contradicting §13.5's unconditional
enforcement requirement.
Reproduction: run packages/calculator-app-actor.yaml; the calculator_builder
agent's shell heredoc writes to commands.py/engine.py are repeatedly
rejected with Dangerous command '...' blocked in safe mode, purely because
the Python source/comments contain "deliberately" / "format_value".

Expected behavior

Blocklist matching is scoped to the payload regions of the command string
that the invoked shell will actually parse and execute as command syntax —
not to the contents of here-doc bodies, quoted string/text literals, or
comments that are opaque data as far as the shell is concerned.
Within those in-scope regions, matching is resistant to the false positives
above: it identifies actual command tokens (e.g. by tokenizing on shell
metacharacters/word boundaries) rather than raw substring containment across
arbitrary prose.
The design does not simply become "naive substring match, minus here-doc
bodies" — it must also not be trivially defeated by shell-level obfuscation
that reassembles a blocked command at execution time (e.g. quoted/concatenated
fragments like 'r''m' or r\m, command substitution such as $(...)/
backticks, eval, bash -c "..."/sh -c "...", or piping through another
interpreter). The threat model is: block what would actually run, catch
attempts to smuggle a blocked command past the check, and stop flagging text
that never reaches the shell as a command.
The blocklist is enforced unconditionally, regardless of safe_mode, per
§13.5 — the if self.safe_mode: gate around the check is removed (this does
not relax anything else safe_mode currently governs, e.g. file_read/
file_write boundaries).
Because this changes the documented substring-matching semantics of §4.5.4.1
and the "MUST always be enforced" applicability language of §13.5, it is an
architectural change to the standard, not a bug-in-code-vs-already-accepted-spec
fix. Per this project's ADR-before-code rule, it requires: extending
docs/adr/ADR-2030-tool-calling-spec-extensions.md in place with a new
decision (next available slot: D-9) describing the parsing/matching design
and getting it accepted, then a spec revision to docs/index.md §4.5.4.1/§13.5
via the standard spec-revision procedure (bump the document Version, add a
§21.1 Revision History row attributing the change to the ADR) — before any
implementation code is merged.

Acceptance criteria

A shell call whose command line writes a here-doc/file payload containing
the words "deliberately", "format_value", or a call to format(...) is
not blocked when the actual command being run is cat > <file> <<'EOF'
(or equivalent) and contains no invocation of a blocklisted command.
A shell call that actually invokes rm, del, format, shutdown,
reboot, or kill as a command (including through common obfuscations
agreed in the ADR, e.g. quoted-fragment reassembly or bash -c "...") is
still blocked.
The blocklist check fires the same way whether the agent's safe_mode is
true or false (verifiable: a blocked command is rejected in both
configurations).
docs/adr/ADR-2030-tool-calling-spec-extensions.md contains an accepted
decision (D-9) describing the new matching design before the corresponding
code change is merged.
docs/index.md §4.5.4.1 and §13.5 reflect the new matching semantics, with
the document Version bumped and a §21.1 Revision History row attributing the
change to ADR-2030 D-9.
nox (all default sessions) is green and nox -s coverage_report stays
≥ 97%.

Supporting information

Repro log (from running packages/calculator-app-actor.yaml):
Tool execution failed: Dangerous command 'cat > calculator_app/calculator/commands.py <<'PYEOF' ... Undo/Memento is deliberately left out ... PYEOF' blocked in safe mode
and the same pattern against engine.py for text containing format_value/
format(...).
Actor Configuration Standard docs/index.md §4.5.4.1 (shell blocklist
definition) and §13.5 (unconditional enforcement requirement).
docs/adr/ADR-2030-tool-calling-spec-extensions.md — D-7 already covers
shell/python_exec tool registration and gating (allow_shell); this is
the natural home for the new decision (D-9) rather than a new ADR file.
Relevant symbols at commit 5520daf:
- cleveractors.agents.tool.ToolAgent.__init__ (sets self.safe_mode = cfg.get("safe_mode", True))
- cleveractors.agents.tool.ToolAgent._execute_shell_command (the blocklist check and the if self.safe_mode: gate)
- cleveractors.agents.tool.ToolAgent._shell_tool (passes the full command string into _execute_shell_command)
Note: allow_unsafe as used in packages/calculator-app-actor.yaml is not a
recognized config key anywhere in the spec or codebase (confirmed via
repo-wide search) and has no effect on this issue — unrelated to the fix.

Subtasks

ADR: extend ADR-2030-tool-calling-spec-extensions.md in place with a new decision (D-9) proposing shell-invocation-region-scoped blocklist matching plus an anti-obfuscation approach; submit for review and get it accepted before writing code
Spec: update docs/index.md §4.5.4.1 and §13.5 per the spec-revision procedure (bump Version, add a §21.1 Revision History row attributing to ADR-2030 D-9) — no inline edits without this
Implement the accepted matching design in ToolAgent._execute_shell_command (cleveractors.agents.tool)
Remove the if self.safe_mode: gate so the blocklist check runs unconditionally, per §13.5
Tests (Behave): here-doc/file-write payload containing "format"/"deliberately"-like benign text is NOT blocked; genuine (and commonly-obfuscated) invocations of rm, kill, etc. ARE blocked; blocklist enforced identically with safe_mode: true and safe_mode: false
Tests (Robot): integration scenario running the shell tool end-to-end with a here-doc file write followed by a chained dangerous command
Verify coverage >= 97% via nox -s coverage_report
Run nox (all default sessions), fix any errors

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
The ADR-2030 D-9 extension is accepted and docs/index.md §4.5.4.1/§13.5 are updated via the spec-revision procedure before implementation code is merged.
A Git commit is created where the first line matches the Commit Message in Metadata exactly.
The commit is pushed to the branch matching the Branch in Metadata exactly.
The commit is submitted as a PR to master, reviewed, and merged.

## Metadata Commit Message: `feat(agents): scope safe-mode shell blocklist to invocation regions` Branch: `feature/m3-shell-safe-mode-blocklist-matching` ## Background and context Actor Configuration Standard v1.1.0 (`docs/index.md`) defines the safe-mode shell blocklist twice: - §4.5.4.1: "The shell tool MUST refuse to execute any command containing any of the literal substrings: `rm`, `del`, `format`, `shutdown`, `reboot`, `kill` (case-insensitive comparison)." - §13.5: "The safe-mode blocklist defined in §4.5.4 ... MUST always be enforced regardless of mode." `cleveractors.agents.tool.ToolAgent._execute_shell_command` (commit `5520daf`) implements this as a single check: ```python if self.safe_mode: dangerous_commands = ["rm", "del", "format", "shutdown", "reboot", "kill"] if any(cmd in command.lower() for cmd in dangerous_commands): raise ExecutionError(f"Dangerous command '{command}' blocked in safe mode") ``` `command` is the *entire* shell string passed to the `shell` tool, including any here-doc payload the command writes to disk. Since the check is a raw substring scan over that whole string, it fires on ordinary English words and Python identifiers that happen to contain a blocked token, with no relation to what the shell will actually execute — `"del"` ⊂ `"deliberately"`, `"format"` ⊂ `"format_value"`/`format(...)`, etc. This was hit running `packages/calculator-app-actor.yaml` (a `calculator_builder` LLM agent with `allow_shell: true`, `tools: [file_read, file_write, shell]`, writing a Python calculator app via `cat > file.py <<'PYEOF' ... PYEOF` heredocs). Every attempt to write `commands.py` or `engine.py` was rejected — not because the agent tried to run `rm`/`kill`/etc., but because the *file content being written* contained the words "deliberately" and "format_value"/`format(...)`. Separately, the current `if self.safe_mode:` gate means the blocklist is *not* enforced when `safe_mode: false`, which contradicts §13.5's "MUST always be enforced regardless of mode." Code and spec disagree here, and per this project's rule the spec is authoritative — the code is wrong on this point independent of anything else in this issue. ## Current behavior 1. Blocklist matching scans the full `command` string passed to the `shell` tool, with no distinction between text that will be interpreted as shell syntax and text that is inert payload (here-doc bodies, quoted string literals, comments). 2. Benign file content containing "deliberately", "format_value", "confirm", "perform", "skillful", etc. is blocked, even though none of it causes `rm`, `del`, `format`, `shutdown`, `reboot`, or `kill` to execute. 3. The check only runs `if self.safe_mode:`, so it is skipped entirely when an agent has `safe_mode: false` — contradicting §13.5's unconditional enforcement requirement. 4. Reproduction: run `packages/calculator-app-actor.yaml`; the `calculator_builder` agent's `shell` heredoc writes to `commands.py`/`engine.py` are repeatedly rejected with `Dangerous command '...' blocked in safe mode`, purely because the Python source/comments contain "deliberately" / "format_value". ## Expected behavior 1. Blocklist matching is scoped to the payload regions of the command string that the invoked shell will actually parse and execute as command syntax — not to the contents of here-doc bodies, quoted string/text literals, or comments that are opaque data as far as the shell is concerned. 2. Within those in-scope regions, matching is resistant to the false positives above: it identifies actual command tokens (e.g. by tokenizing on shell metacharacters/word boundaries) rather than raw substring containment across arbitrary prose. 3. The design does not simply become "naive substring match, minus here-doc bodies" — it must also not be trivially defeated by shell-level obfuscation that reassembles a blocked command at execution time (e.g. quoted/concatenated fragments like `'r''m'` or `r\m`, command substitution such as `$(...)`/ backticks, `eval`, `bash -c "..."`/`sh -c "..."`, or piping through another interpreter). The threat model is: block what would actually run, catch attempts to smuggle a blocked command past the check, and stop flagging text that never reaches the shell as a command. 4. The blocklist is enforced unconditionally, regardless of `safe_mode`, per §13.5 — the `if self.safe_mode:` gate around the check is removed (this does not relax anything else `safe_mode` currently governs, e.g. `file_read`/ `file_write` boundaries). 5. Because this changes the documented substring-matching semantics of §4.5.4.1 and the "MUST always be enforced" applicability language of §13.5, it is an architectural change to the standard, not a bug-in-code-vs-already-accepted-spec fix. Per this project's ADR-before-code rule, it requires: extending `docs/adr/ADR-2030-tool-calling-spec-extensions.md` in place with a new decision (next available slot: D-9) describing the parsing/matching design and getting it accepted, then a spec revision to `docs/index.md` §4.5.4.1/§13.5 via the standard spec-revision procedure (bump the document Version, add a §21.1 Revision History row attributing the change to the ADR) — before any implementation code is merged. ## Acceptance criteria - A `shell` call whose command line writes a here-doc/file payload containing the words "deliberately", "format_value", or a call to `format(...)` is **not** blocked when the actual command being run is `cat > <file> <<'EOF'` (or equivalent) and contains no invocation of a blocklisted command. - A `shell` call that actually invokes `rm`, `del`, `format`, `shutdown`, `reboot`, or `kill` as a command (including through common obfuscations agreed in the ADR, e.g. quoted-fragment reassembly or `bash -c "..."`) is still blocked. - The blocklist check fires the same way whether the agent's `safe_mode` is `true` or `false` (verifiable: a blocked command is rejected in both configurations). - `docs/adr/ADR-2030-tool-calling-spec-extensions.md` contains an accepted decision (D-9) describing the new matching design before the corresponding code change is merged. - `docs/index.md` §4.5.4.1 and §13.5 reflect the new matching semantics, with the document Version bumped and a §21.1 Revision History row attributing the change to ADR-2030 D-9. - `nox` (all default sessions) is green and `nox -s coverage_report` stays ≥ 97%. ## Supporting information - Repro log (from running `packages/calculator-app-actor.yaml`): `Tool execution failed: Dangerous command 'cat > calculator_app/calculator/commands.py <<'PYEOF' ... Undo/Memento is deliberately left out ... PYEOF' blocked in safe mode` and the same pattern against `engine.py` for text containing `format_value`/ `format(...)`. - Actor Configuration Standard `docs/index.md` §4.5.4.1 (shell blocklist definition) and §13.5 (unconditional enforcement requirement). - `docs/adr/ADR-2030-tool-calling-spec-extensions.md` — D-7 already covers `shell`/`python_exec` tool registration and gating (`allow_shell`); this is the natural home for the new decision (D-9) rather than a new ADR file. - Relevant symbols at commit `5520daf`: - `cleveractors.agents.tool.ToolAgent.__init__` (sets `self.safe_mode = cfg.get("safe_mode", True)`) - `cleveractors.agents.tool.ToolAgent._execute_shell_command` (the blocklist check and the `if self.safe_mode:` gate) - `cleveractors.agents.tool.ToolAgent._shell_tool` (passes the full command string into `_execute_shell_command`) - Note: `allow_unsafe` as used in `packages/calculator-app-actor.yaml` is not a recognized config key anywhere in the spec or codebase (confirmed via repo-wide search) and has no effect on this issue — unrelated to the fix. ## Subtasks - [ ] ADR: extend `ADR-2030-tool-calling-spec-extensions.md` in place with a new decision (D-9) proposing shell-invocation-region-scoped blocklist matching plus an anti-obfuscation approach; submit for review and get it accepted before writing code - [ ] Spec: update `docs/index.md` §4.5.4.1 and §13.5 per the spec-revision procedure (bump Version, add a §21.1 Revision History row attributing to ADR-2030 D-9) — no inline edits without this - [ ] Implement the accepted matching design in `ToolAgent._execute_shell_command` (`cleveractors.agents.tool`) - [ ] Remove the `if self.safe_mode:` gate so the blocklist check runs unconditionally, per §13.5 - [ ] Tests (Behave): here-doc/file-write payload containing "format"/"deliberately"-like benign text is NOT blocked; genuine (and commonly-obfuscated) invocations of `rm`, `kill`, etc. ARE blocked; blocklist enforced identically with `safe_mode: true` and `safe_mode: false` - [ ] Tests (Robot): integration scenario running the `shell` tool end-to-end with a here-doc file write followed by a chained dangerous command - [ ] Verify coverage >= 97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - The ADR-2030 D-9 extension is accepted and `docs/index.md` §4.5.4.1/§13.5 are updated via the spec-revision procedure before implementation code is merged. - A Git commit is created where the first line matches the Commit Message in Metadata exactly. - The commit is pushed to the branch matching the Branch in Metadata exactly. - The commit is submitted as a PR to master, reviewed, and merged.

CoreRasurae added the

labels

2026-08-05 17:53:43 +00:00