agentscope-ai / agentscope-ai/agentscope

[Bug]: `&` and newline separators bypass the Bash read-only auto-allow, executing unapproved commands in DEFAULT permission mode

オープン
#2,470 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Python
スター
31.5k
フォーク
3.5k
平均マージ
1日 23時間
マージ済み PR(30日)
95

説明

## Prerequisites

- [x] I have searched the existing issues and discussions, and this is not a duplicate.
- [x] This is a bug, not a usage question.

## Background / Description

`Bash.check_permissions()` auto-ALLOWs a command in **every** permission mode — including
`DEFAULT` — when `BashCommandParser.is_read_only_command()` returns `True`
(`src/agentscope/tool/_builtin/_bash.py:270`). The safety checks that follow it (dangerous
command patterns, dangerous paths, dangerous removal paths) are never reached, because the
read-only branch returns first.

`is_read_only_command()` only treats a command as compound when one of `&&`, `||`, `;` or
`|` appears literally in the string:

```python
# src/agentscope/tool/_builtin/_bash_parser.py
if any(op in cmd for op in ["&&", "||", ";", "|"]):
... # split on the AST, require every subcommand to be read-only
# Single command - check directly
return self._is_single_command_read_only(cmd)
```

Bash has more command separators than those four. `&` (background) and a plain newline also
terminate a command, and neither is in the list. For `ls & rm -rf /tmp/pwn` or
`ls\nrm -rf /tmp/pwn` the classifier therefore skips the compound path and judges the whole
string by its read-only *head*, so it reports read-only and the tool auto-allows a command
that deletes files.

The same string is then handed to the shell unchanged, which runs both parts.

`check_injection_risk()` does not catch these either: `&` and newline produce plain
`command` nodes in the tree-sitter AST, not any of the `DANGEROUS_NODE_TYPES`
(`command_substitution`, `subshell`, control flow, ...). So the well-known
`ls $(rm -rf /)` vector is blocked, while `ls & rm -rf /` is not.

Affected separators, all classified read-only today:

| command | `is_read_only_command` | `check_permissions` |
|---|---|---|
| `ls; rm -rf /tmp/pwn` | `False` | ASK (correct) |
| `ls && rm -rf /tmp/pwn` | `False` | ASK (correct) |
| `ls $(rm -rf /tmp/pwn)` | `False` | ASK (correct) |
| `ls & rm -rf /tmp/pwn` | **`True`** | **ALLOW** |
| `ls -la&rm -rf /tmp/pwn` | **`True`** | **ALLOW** |
| `ls\nrm -rf /tmp/pwn` | **`True`** | **ALLOW** |
| `ls\r\nrm -rf /tmp/pwn` | **`True`** | **ALLOW** |
| `ls\n\nrm -rf /tmp/pwn` | **`True`** | **ALLOW** |
| `ls &\nrm -rf /tmp/pwn` | **`True`** | **ALLOW** |

This matters because the read-only fast path is exactly the path that exists to run without
asking the user. A model that emits a multi-line bash block — which is entirely ordinary
output — gets its trailing commands executed with no confirmation, and a prompt-injected one
can reach it deliberately. This is the same class of gap as #2003 (`find -delete` classified
read-only), just through the separator list rather than the predicate list.

## Expected Behavior

A command is read-only only when **every** subcommand it will actually run is read-only,
whatever separator joins them.

## Error Messages

```text
permission mode : PermissionMode.DEFAULT
command : 'ls /tmp/demo & rm -f /tmp/demo/important.txt'
decision : PermissionBehavior.ALLOW — Permission granted for read-only command
read_only : True
victim exists : True
victim exists : False <-- after execution
```

## Steps to Reproduce

Save and run against current `main`. It only touches a private temporary directory.

```python
import asyncio
import os
import tempfile

from agentscope.permission import PermissionContext
from agentscope.tool._builtin._bash import Bash

async def main() -> None:
for sep in ["&", "\n"]:
with tempfile.TemporaryDirectory() as d:
victim = os.path.join(d, "important.txt")
with open(victim, "w", encoding="utf-8") as f:
f.write("payroll")

command = f"ls {d}{sep}rm -f {victim}"
tool = Bash()
context = PermissionContext()

decision = await tool.check_permissions({"command": command}, context)
print("mode :", context.mode)
print("command :", repr(command))
print("decision :", decision.behavior, "-", decision.message)
print("read_only:", await tool.check_read_only({"command": command}))
print("exists :", os.path.exists(victim))

async for _ in await tool(command=command):
pass

print("exists :", os.path.exists(victim), "<-- after execution")
print()

asyncio.run(main())
```

Both separators print `PermissionBehavior.ALLOW` with `read_only: True`, and the file is
gone afterwards without any confirmation being requested.

## Environment

- AgentScope Version: 2.0.7.post1 (also reproduces on `main`)
- Python Version: 3.11.11
- OS: macOS 15 (the classifier is platform-independent)

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。