Problem
PR #3444 replaced the shell denylist with an executable allowlist in
`autobot-slm-backend/api/nodes_execution.py`. The allowlist approach is
correct, but several entries are write-capable and have no argument-level
enforcement despite the inline comment claiming otherwise.
Dangerous entries with no enforcement
| Executable |
Risk |
Comment in code |
| `apt`, `yum`, `dnf`, `rpm` |
Can install/remove packages — `apt install ` runs as root via sudo |
"Package management (query-only)" — NOT enforced |
| `wget`, `curl` |
Can download arbitrary files, exfiltrate data, POST to internal services |
No restriction |
| `git` |
Can `git clone`, `git push`, modify repo state |
"read-only operations are enforced at argument level by callers" — NO such enforcement exists |
Fix
Option A — remove the dangerous entries from ALLOWED_EXECUTABLES entirely:
- Remove `apt`, `yum`, `dnf`, `rpm`, `wget`, `curl`, `git`
- Add read-only variants if needed: `apt-cache`, `apt list` via a separate subcommand guard
Option B — add per-executable argument guards in `_validate_command`:
```python
_READ_ONLY_ARGS: dict[str, frozenset[str]] = {
"apt": frozenset({"list", "show", "search", "policy", "depends"}),
"git": frozenset({"log", "status", "diff", "show", "branch", "tag"}),
}
After allowlist check, if executable in _READ_ONLY_ARGS:
require first arg in _READ_ONLY_ARGS[executable]
```
Option A is safer and simpler. Option B enables legitimate use cases.
Discovery
Found during gap review of PR #3444 (#3421 fix).
Problem
PR #3444 replaced the shell denylist with an executable allowlist in
`autobot-slm-backend/api/nodes_execution.py`. The allowlist approach is
correct, but several entries are write-capable and have no argument-level
enforcement despite the inline comment claiming otherwise.
Dangerous entries with no enforcement
Fix
Option A — remove the dangerous entries from ALLOWED_EXECUTABLES entirely:
Option B — add per-executable argument guards in `_validate_command`:
```python
_READ_ONLY_ARGS: dict[str, frozenset[str]] = {
"apt": frozenset({"list", "show", "search", "policy", "depends"}),
"git": frozenset({"log", "status", "diff", "show", "branch", "tag"}),
}
After allowlist check, if executable in _READ_ONLY_ARGS:
require first arg in _READ_ONLY_ARGS[executable]
```
Option A is safer and simpler. Option B enables legitimate use cases.
Discovery
Found during gap review of PR #3444 (#3421 fix).