agentscope-ai / agentscope-ai/agentscope

[Bug]: indirect `rm` spellings (/bin/rm, \rm, env rm, FOO=1 rm) bypass the dangerous-removal check

Aberta Para iniciantes
#2,500 0 comentários 0 reações 0 responsáveis Ver no GitHub
Linguagem predominante
Python
Estrelas
31.6k
Forks
3.5k
Merge médio
1d 16h
PRs com merge (30d)
103

Descrição

## 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()` step 5 calls `_check_dangerous_removal_path()` in
`src/agentscope/tool/_builtin/_bash.py`. Its bypass-immune ASK is the last line of defence for a
command that would delete a critical system path, and it is explicitly documented as one that
"cannot be auto-allowed by permission rules".

That helper decides whether a subcommand is a removal command by comparing the first
whitespace-separated token literally:

```python
subcmd_tokens = subcmd.strip().split()
base = subcmd_tokens[0]
if base not in ("rm", "rmdir"):
continue
```

Bash runs several other spellings as the very same command, and none of them match that comparison:

- an absolute or relative path — `/bin/rm`, `/usr/bin/rm`
- a backslash that suppresses alias expansion — `\rm`
- a leading environment assignment — `FOO=1 rm`
- a wrapper that execs its argument — `env rm`, `command rm`, `env -i rm`

Writing the flags separately (`-r -f` instead of `-rf`) additionally keeps the command clear of the
`"rm -rf"` substring in `DANGEROUS_COMMANDS`, so step 2 does not catch it either. The whole
bypass-immune chain is therefore skipped, and `check_permissions()` returns `PASSTHROUGH` with
`bypass_immune=False` — so a user-configured Bash allow rule auto-approves the command, and the
decision can also be silenced, unlike the `rm -r -f /` form.

## Steps to reproduce

```python
import asyncio

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

CASES = [
"rm -rf /",
"rm -r -f /",
"rmdir /",
"/bin/rm -r -f /",
"/usr/bin/rm -r -f /etc",
"\\rm -r -f /",
"env rm -r -f /",
"env -i rm -r -f /",
"command rm -r -f /",
"FOO=1 rm -r -f /",
"sudo /bin/rm -r -f /",
"/bin/rm -r -f ~",
]

async def main() -> None:
tool = Bash()
context = PermissionContext(mode=PermissionMode.DEFAULT)
for command in CASES:
decision = await tool.check_permissions({"command": command}, context)
print(f"{command!r:<26} {decision.behavior.value:<12} bypass_immune={decision.bypass_immune}")

asyncio.run(main())
```

## Result on current `main` (`10eaaac`)

| command | behavior | bypass_immune |
|---|---|---|
| `rm -rf /` | ask | True |
| `rm -r -f /` | ask | True |
| `rmdir /` | ask | True |
| `/bin/rm -r -f /` | **passthrough** | **False** |
| `/usr/bin/rm -r -f /etc` | **passthrough** | **False** |
| `\rm -r -f /` | **passthrough** | **False** |
| `env rm -r -f /` | **passthrough** | **False** |
| `env -i rm -r -f /` | **passthrough** | **False** |
| `command rm -r -f /` | **passthrough** | **False** |
| `FOO=1 rm -r -f /` | **passthrough** | **False** |
| `sudo /bin/rm -r -f /` | **passthrough** | **False** |
| `/bin/rm -r -f ~` | **passthrough** | **False** |

The first three rows are the intended behaviour. Every `passthrough` row is a command that bash
executes identically to its flagged counterpart.

Note that `sudo rm -rf /` is still caught, but only by the `"sudo rm"` and `"rm -rf"` substring
patterns in step 2 — `sudo /bin/rm -r -f /` matches neither.

## Expected behavior

`_check_dangerous_removal_path()` should resolve the command name before comparing it, so that every
spelling bash treats as `rm` / `rmdir` reaches `_is_dangerous_removal_path()` and receives the same
bypass-immune ASK.

## Related

Same area as #2470, but a different function and a different defect: #2470 is about separator
handling in the parser's `is_read_only_command()`, while this one is the command-name comparison in
`_bash.py`'s removal check. The two do not overlap.

## Note

I would like to claim this one — I have a fix plus regression tests ready locally and will open a PR
referencing this issue.

Guia de contribuição

Abrir o guia de contribuição

Direção de pesquisa

Inspect `src/agentscope/tool/_builtin/_bash.py`, especially `check_permissions()` and `_check_dangerous_removal_path()`, where the subcommand is currently matched literally from the first token. Run the provided reproduction script to confirm which cases currently return `PASSTHROUGH` with `bypass_immune=False`. Add/extend tests for the Bash permission-check path with the listed variants (`/bin/rm`, `\\rm`, `env rm`, `env -i rm`, `command rm`, `FOO=1 rm`, `sudo /bin/rm`) and verify they now behave like `rm -rf` (`ASK`, `bypass_immune=True`).

Escrita pelo modelo de indexação a partir do texto da issue.

Avaliação

Stack de tecnologia
bash, python
Domínio
security
Tipo de issue
Bug
Dificuldade
2/5
Tempo estimado
1-3 horas
Status de atividade
Ativa
Clareza
Claramente especificada
Facilidade para iniciantes
82/100

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.