feat: add shell tool

[akihikokuroda]

Pull Request

Issue

Fix #1024

Description

This PR doesn't include issues mentioned in #1087

Testing

• Tests added to the respective file if code was changed
• New code has 100% coverage if code was added
• Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

Attribution

• AI coding assistants used: claudecode

---

Adding a new component, requirement, sampling strategy, or tool?

If your PR adds or modifies one of the types below, check the matching box. A checklist of type-specific review items will be posted as a comment.

• Component
• Requirement
• Sampling Strategy
• Tool

NOTE: Please ensure you have an issue that has been acknowledged by a core contributor and routed you to open a pull request against this repository. Otherwise, please open an issue before continuing with this pull request.

[github-actions]

This comment is managed by a bot. Editing it is fine — checking off boxes, adding notes — but please leave the HTML comment marker on the first line alone, otherwise checklist updates will break.

Tool PR Checklist

Use this checklist when adding or modifying tools in mellea/stdlib/tools/.

Protocol Compliance

• Ensure compatibility with existing backends and providers
  • For most tools being added as functions, this means that calling convert_function_to_tool works

Integration

• Tool exported in mellea/stdlib/tools/__init__.py or, if you are adding a library of tools, from your sub-module

[planetf1]

Overall the design is solid — subprocess.run(shell=False) as the foundation, shlex.split() for tokenisation, and working-dir restriction are the right primitives. Two confirmed bugs block merge; a third issue means the _bash_patterns.py test suite gives false assurance about what's actually blocked at runtime.

[planetf1]

env bash -c <payload> bypasses the denylist

The code-execution check (lines 176–182) only fires when argv[0] is the interpreter. When argv[0] is env, that path is never reached — and lines 225–231 explicitly allow shells as nested arguments inside safe wrappers, so -c is never re-checked.

_is_dangerous_command(["env", "bash", "-c", "id"])          # (False, '')
_is_dangerous_command(["timeout", "60", "bash", "-c", "id"])  # (False, '')

Fix: re-check for -c/-e on the nested interpreter, or restrict the shell allowlist here to script-file invocations only (no -c/-e in the remaining argv).

[akihikokuroda]

additional check is added. The tests are added.

[planetf1]

Verified fixed — env/timeout/nice bash -c bypass is now blocked correctly. LGTM.

[planetf1]

Classic iterate-and-remove bug — filter results will be incomplete

results.remove() inside for violation in results causes Python's iterator to skip the element immediately after each removal. Filters will silently return the wrong set.

if session_id:
    results = [v for v in results if v.session_id == session_id]
if pattern:
    results = [v for v in results if v.pattern == pattern]
if category:
    results = [v for v in results if v.category == category]
if severity:
    results = [v for v in results if v.severity == severity]

[planetf1]

write_commands is missing several filesystem-write commands

chmod, chown, ln, and dd aren't in this set, so the dangerous-path checks never fire for them. A few things that currently slip through:

• chmod 777 /etc/passwd
• ln -sf /etc/passwd /allowed/path/link (symlink escape out of the allowed fence)
• dd if=/dev/urandom of=/boot/vmlinuz

Same set is duplicated on line 415 — both need updating.

[akihikokuroda]

Additional write commands are added. Add "ln" command handling.

[planetf1]

Verified fixed — WRITE_COMMANDS now covers chmod, chown, chgrp, ln, dd, install, truncate. All four cases blocked. LGTM.

[planetf1]

This framework isn't connected to the execution path

shell.py only imports record_bash_violation from _bash_audit — check_all_patterns() is never called from bash_executor(). The tests in test_bash_guardrails.py all pass, but none of it affects whether a command is actually blocked at runtime.

If this is intended as the extensible pattern layer, wire check_all_patterns(argv) into _is_dangerous_command(). If it's documentation of the threat model, the disconnect from the execution path should be made explicit.

[akihikokuroda]

wire check_all_patterns is wired into the execution path by updating _is_dangerous_command to use the pattern framework.

[planetf1]

Verified fixed — check_all_patterns is now imported and called at the top of _is_dangerous_command. LGTM.

[planetf1]

Two functions are both labelled "Example 3" — example_3_llm_with_forced_tool_use (line 85) already has that number. This one should be Example 4, with the subsequent examples renumbered.

[akihikokuroda]

After investigate, it may not be good to share the CapabilityPolicy , proposal a separate policy for shell execution.

@DataClass
class BashExecutionPolicy:
"""Execution constraints for bash commands.

  Args:
      timeout (int): Max seconds before subprocess.run timeout (enforced).
      stdout_max_bytes (int | None): Truncate stdout to this size (enforced).
      stderr_max_bytes (int | None): Truncate stderr to this size (enforced).
      allowed_paths (list[Path] | None): Write allowlist for filesystem safety check.
      working_dir (Path | None): Host directory for command execution.
  """

  timeout: int = 60
  stdout_max_bytes: int | None = 10 * 1024  # 10KB (matches current default)
  stderr_max_bytes: int | None = 10 * 1024
  allowed_paths: list[Path] | None = None
  working_dir: Path | None = None

  ENFORCED_timeout: ClassVar[bool] = True
  ENFORCED_stdout_max_bytes: ClassVar[bool] = True
  ENFORCED_stderr_max_bytes: ClassVar[bool] = True
  ENFORCED_allowed_paths: ClassVar[bool] = False  # Denylist override
  ENFORCED_working_dir: ClassVar[bool] = False    # Declarative

Benefits:

• Minimal, focused on enforced constraints
• Maintains the ENFORCED_* pattern for transparency
• Parallel to Python's CapabilityPolicy without forcing a bad fit
• Users understand it's bash-specific, not a general execution policy

[planetf1]

Second pass. Clarification replies added to the A, C, and D threads — none of those claimed fixes are reflected in the current code. Inline comments below cover the two unaddressed findings without author replies (B and E) and one new issue surfaced by this review.

[planetf1]

Verified fixed — filter is now a list comprehension, returns correct counts. LGTM.

[planetf1]

Verified fixed — renamed to example_4_with_working_dir with correct numbering; __main__ calls the LLM example. LGTM.

[akihikokuroda]

Would you check this comment? There may be some confusion.

[planetf1]

New finding: even if check_all_patterns were wired into the execution path, every violation would still be logged with category='unknown' and severity='MEDIUM'. The function reads these via getattr(pattern, 'category', 'unknown') and getattr(pattern, 'severity', 'MEDIUM'), but no concrete pattern class defines category or severity attributes — those live in _bash_guardrails.COMMAND_RULES, not on the pattern objects themselves.

To fix this, either add category and severity class attributes to each concrete pattern (populated from COMMAND_RULES), or return them as part of a richer result object from check() instead of a plain tuple[bool, str].

[planetf1]

Verified fixed in the latest commit — each pattern class now declares category and severity as class attributes, so the getattr defaults never fire. Violations are logged with correct metadata. LGTM.

[planetf1]

Correction to my previous reply — on closer inspection this is only partially fixed. The class-level attributes prevent the getattr defaults, but they apply a blanket value per pattern class rather than per command. For example DangerousCommandPattern hardcodes severity='HIGH', so a sudo attempt (which COMMAND_RULES defines as CRITICAL/PRIVILEGE_ESCALATION) gets logged as HIGH/dangerous_command. The granular data is already in COMMAND_RULES — the fix is to look up the matched command there rather than using a class-level constant.

[akihikokuroda]

Thanks. I made changes.