TUI: Incomplete dangerous command detection in `shell_exec.py` #8887

New issue

Closed

opened 2026-04-14 03:26:33 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-14 03:26:33 +00:00

Owner

Metadata

Commit Message: fix(tui): strengthen looks_dangerous detection in shell_exec to prevent bypass
Branch: fix/tui-shell-exec-looks-dangerous-bypass

Background and Context

The looks_dangerous function in src/cleveragents/tui/input/shell_exec.py (lines 21–31) uses a simple substring match against a hardcoded list of five patterns to detect dangerous shell commands:

def looks_dangerous(command: str) -> bool:
    """Best-effort dangerous command detector."""
    lowered = command.strip().lower()
    patterns = (
        "rm -rf /",
        "git push --force",
        "mkfs.",
        "dd if=",
        ":(){:|:&};:",
    )
    return any(pattern in lowered for pattern in patterns)

This approach is trivially bypassed. For example:

" rm -rf /" — leading space before rm causes the .strip() to still match, but "rm -rf /" (double space) does not.
"rm -rf /" — extra space between -rf and / evades the match entirely.
"rm -rf ~/" — targets home directory, not caught.
"rm -rf ./important" — targets a relative path, not caught.
"git push --force origin master" — the pattern "git push --force" is present, but "git push --force-with-lease" is not.
"dd if=/dev/zero of=/dev/sda" — "dd if=" is matched, but "dd if =/dev/zero" (with space) is not.

This is especially relevant because looks_dangerous() is the only active safety gate in the TUI shell execution path (see also issue #6361, which documents that the richer ShellSafetyService infrastructure is currently dead code in the TUI). Even as a "best-effort" detector, the current implementation provides a false sense of security.

Code Evidence:
src/cleveragents/tui/input/shell_exec.py — looks_dangerous function (module path: cleveragents.tui.input.shell_exec.looks_dangerous)

Expected Behavior

The looks_dangerous function should use more robust pattern matching (e.g., regex with word boundaries, tokenization, or normalisation of whitespace) so that trivial variations of known-dangerous commands are reliably detected. At minimum:

Whitespace normalisation before matching (collapse multiple spaces/tabs).
Regex-based matching with word boundaries where appropriate.
Coverage of common dangerous command variants (e.g., rm -rf ~/, rm -rf ./, rm -r /).
The function's docstring should accurately reflect its limitations and threat model.

Acceptance Criteria

looks_dangerous normalises internal whitespace before pattern matching (e.g., "rm -rf /" is detected).
looks_dangerous detects rm -rf ~/ and rm -rf ./ as dangerous.
looks_dangerous detects rm -r / (without the f flag) as dangerous.
Pattern matching uses regex or equivalent to avoid trivial whitespace-based bypasses.
All existing patterns continue to be detected correctly.
The function's docstring is updated to accurately describe its detection approach and known limitations.
BDD scenarios cover bypass attempts (whitespace variants, home-directory targets, relative-path targets).
Test coverage remains ≥ 97% (nox -s coverage_report).
nox (all default sessions) passes with no errors.

Subtasks

Audit all five existing patterns for whitespace-bypass and variant-bypass gaps.
Implement whitespace normalisation (collapse runs of spaces/tabs to a single space) before matching.
Extend pattern list to cover rm -r /, rm -rf ~/, rm -rf ./.
Convert string patterns to compiled regex patterns with appropriate anchoring/word boundaries.
Update the looks_dangerous docstring to describe the detection strategy and its limitations.
Tests (Behave): Add scenarios for whitespace-bypass attempts and new pattern variants in features/.
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.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly (fix(tui): strengthen looks_dangerous detection in shell_exec to prevent bypass), followed by a blank line, then additional lines providing relevant implementation details.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly (fix/tui-shell-exec-looks-dangerous-bypass).
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.

Automated by CleverAgents Bot
Agent: new-issue-creator

## Metadata - **Commit Message**: `fix(tui): strengthen looks_dangerous detection in shell_exec to prevent bypass` - **Branch**: `fix/tui-shell-exec-looks-dangerous-bypass` ## Background and Context The `looks_dangerous` function in `src/cleveragents/tui/input/shell_exec.py` (lines 21–31) uses a simple substring match against a hardcoded list of five patterns to detect dangerous shell commands: ```python def looks_dangerous(command: str) -> bool: """Best-effort dangerous command detector.""" lowered = command.strip().lower() patterns = ( "rm -rf /", "git push --force", "mkfs.", "dd if=", ":(){:|:&};:", ) return any(pattern in lowered for pattern in patterns) ``` This approach is trivially bypassed. For example: - `" rm -rf /"` — leading space before `rm` causes the `.strip()` to still match, but `"rm -rf /"` (double space) does not. - `"rm -rf /"` — extra space between `-rf` and `/` evades the match entirely. - `"rm -rf ~/"` — targets home directory, not caught. - `"rm -rf ./important"` — targets a relative path, not caught. - `"git push --force origin master"` — the pattern `"git push --force"` is present, but `"git push --force-with-lease"` is not. - `"dd if=/dev/zero of=/dev/sda"` — `"dd if="` is matched, but `"dd if =/dev/zero"` (with space) is not. This is especially relevant because `looks_dangerous()` is the **only** active safety gate in the TUI shell execution path (see also issue #6361, which documents that the richer `ShellSafetyService` infrastructure is currently dead code in the TUI). Even as a "best-effort" detector, the current implementation provides a false sense of security. **Code Evidence:** `src/cleveragents/tui/input/shell_exec.py` — `looks_dangerous` function (module path: `cleveragents.tui.input.shell_exec.looks_dangerous`) ## Expected Behavior The `looks_dangerous` function should use more robust pattern matching (e.g., regex with word boundaries, tokenization, or normalisation of whitespace) so that trivial variations of known-dangerous commands are reliably detected. At minimum: - Whitespace normalisation before matching (collapse multiple spaces/tabs). - Regex-based matching with word boundaries where appropriate. - Coverage of common dangerous command variants (e.g., `rm -rf ~/`, `rm -rf ./`, `rm -r /`). - The function's docstring should accurately reflect its limitations and threat model. ## Acceptance Criteria - [ ] `looks_dangerous` normalises internal whitespace before pattern matching (e.g., `"rm -rf /"` is detected). - [ ] `looks_dangerous` detects `rm -rf ~/` and `rm -rf ./` as dangerous. - [ ] `looks_dangerous` detects `rm -r /` (without the `f` flag) as dangerous. - [ ] Pattern matching uses regex or equivalent to avoid trivial whitespace-based bypasses. - [ ] All existing patterns continue to be detected correctly. - [ ] The function's docstring is updated to accurately describe its detection approach and known limitations. - [ ] BDD scenarios cover bypass attempts (whitespace variants, home-directory targets, relative-path targets). - [ ] Test coverage remains ≥ 97% (`nox -s coverage_report`). - [ ] `nox` (all default sessions) passes with no errors. ## Subtasks - [ ] Audit all five existing patterns for whitespace-bypass and variant-bypass gaps. - [ ] Implement whitespace normalisation (collapse runs of spaces/tabs to a single space) before matching. - [ ] Extend pattern list to cover `rm -r /`, `rm -rf ~/`, `rm -rf ./`. - [ ] Convert string patterns to compiled regex patterns with appropriate anchoring/word boundaries. - [ ] Update the `looks_dangerous` docstring to describe the detection strategy and its limitations. - [ ] Tests (Behave): Add scenarios for whitespace-bypass attempts and new pattern variants in `features/`. - [ ] 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. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly (`fix(tui): strengthen looks_dangerous detection in shell_exec to prevent bypass`), followed by a blank line, then additional lines providing relevant implementation details. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly (`fix/tui-shell-exec-looks-dangerous-bypass`). - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. --- **Automated by CleverAgents Bot** Agent: new-issue-creator

HAL9000 added the

labels

2026-04-14 03:29:33 +00:00

HAL9000 added this to the v3.7.0 milestone

2026-04-14 03:29:44 +00:00

HAL9000 referenced this issue

2026-04-14 03:30:09 +00:00

Epic: Settings Screen, Content Pruning & Safety Behaviors (v3.7.0) #8606

HAL9000 commented

2026-04-14 03:31:10 +00:00

Author

Owner

This issue is a duplicate of #8466 (shell_exec.looks_dangerous() is easily bypassed by spacing/case variations — ShellSafetyService regex patterns not used), which is already State/Verified with Priority/High. Please track the fix there.

Automated by CleverAgents Bot
Agent: new-issue-creator

This issue is a duplicate of #8466 (`shell_exec.looks_dangerous() is easily bypassed by spacing/case variations — ShellSafetyService regex patterns not used`), which is already `State/Verified` with `Priority/High`. Please track the fix there. --- **Automated by CleverAgents Bot** Agent: new-issue-creator

HAL9000

2026-04-14 03:31:10 +00:00