Proposal: update specification — shell danger detection model and CLI ULID validation #2025

Open
opened 2026-04-03 02:42:29 +00:00 by freemo · 5 comments
Owner

Spec Update Proposal

This proposal covers two spec sections that diverge from the merged implementation.


Triggered By

  • PR #1577 (merged 2026-04-03): fix(cli): disallow mixing legacy and v3 plan workflows
  • Commit 50a5e96 (merged 2026-04-03): feat(tui): implement shell danger detection patterns (closes #1003)

Change 1: Shell Danger Detection — 4-level model, blocking behavior, revised pattern set

What changed in the implementation

The feat(tui) commit (closes #1003) implemented the full shell danger detection subsystem under src/cleveragents/tui/shell_safety/. The implementation diverges from the spec in several important ways:

Current spec (lines 29886–29900):

#### Shell Danger Detection

When shell mode is active (`!`/`$` prefix), the prompt performs heuristic analysis of the command to detect potentially destructive operations. Dangerous commands are highlighted with `$error` styling and a warning indicator appears below the prompt:

| Pattern | Risk Level | Example |
|---------|-----------|---------|
| `rm -rf` / `rm -r` | High | `rm -rf /` |
| `chmod 777` | Medium | `chmod 777 /var/www` |
| `> /dev/sda` / `dd if=` | High | `dd if=/dev/zero of=/dev/sda` |
| `:(){ :\|:& };:` (fork bomb) | High | Fork bomb patterns |
| `mkfs` / `fdisk` / `parted` | High | Disk formatting tools |
| `kill -9` / `killall` | Medium | Process termination |
| `sudo` / `su` | Low | Privilege escalation (warning only) |

Danger detection is controlled by the `shell.warn_dangerous` setting (default: `true`). The detection is advisory only — it never prevents command execution. The warning text reads: `⚠ Potentially destructive command detected`.

What the implementation actually does:

  1. 4-level danger classification (ShellDangerLevel enum): LOW=1, MEDIUM=2, HIGH=3, CRITICAL=4 — the spec only mentions 3 levels (High/Medium/Low) and has no CRITICAL level.

  2. Commands CAN be blockedShellSafetyService has a block_level (default: MEDIUM). Commands at or above block_level are blocked when no warn_callback is provided. The spec says "advisory only — it never prevents command execution" which is incorrect.

  3. Different pattern set — the implementation does NOT include kill -9/killall, fdisk/parted, or sudo/su as standalone patterns. It DOES include patterns not in the spec: shred --remove, wget|curl pipe to sh/bash, git push --force, chmod -R with permissive modes. The rm -rf patterns are split into rm_rf_root (CRITICAL) and rm_rf_wildcard (CRITICAL), not "High".

  4. Warning message format — the implementation produces [{Level}] Dangerous command detected: {description} (e.g., [Critical] Dangerous command detected: rm -rf / recursively deletes...), not ⚠ Potentially destructive command detected.

  5. Architecture — the implementation uses a proper domain model: DangerousPattern (value object), DangerousPatternDetector (domain service), ShellSafetyService (application service), DangerousCommandWarning (value object), DEFAULT_PATTERNS registry. This is a better design than the spec implied.

Proposed spec text (lines 29886–29900 replacement)

#### Shell Danger Detection

When shell mode is active (`!`/`$` prefix), the prompt performs heuristic analysis of the command to detect potentially destructive operations. Dangerous commands are highlighted with `$error` styling and a warning indicator appears below the prompt.

The detection system uses a four-level danger classification:

| Level | Value | Meaning |
|-------|-------|---------|
| `LOW` | 1 | Minor risk — generally recoverable side-effects |
| `MEDIUM` | 2 | Moderate risk — potential data loss or security exposure |
| `HIGH` | 3 | High risk — significant, hard-to-reverse damage |
| `CRITICAL` | 4 | Critical risk — system destruction or fork bomb |

The built-in pattern registry includes:

| Pattern | Level | Example |
|---------|-------|---------|
| `rm -rf /` or `rm -rf /*` | Critical | `rm -rf /` |
| Fork bomb `:(){ :|:& };:` | Critical | Fork bomb patterns |
| `dd if=` | High | `dd if=/dev/zero of=/dev/sda` |
| `mkfs` | High | Disk formatting |
| `shred` on device or `--remove` | High | `shred /dev/sda` |
| `chmod 777` | Medium | `chmod 777 /var/www` |
| `sudo rm` | Medium | Elevated deletion |
| `wget`/`curl` piped to `sh`/`bash` | Medium | Remote code execution |
| `git push --force` | Low | History rewrite |
| `chmod -R` with permissive modes | Low | Recursive permission change |

The warning message format is: `[{Level}] Dangerous command detected: {description}`.

Danger detection is controlled by the `shell.warn_dangerous` setting (default: `true`). When enabled, commands at or above the configured `block_level` (default: `MEDIUM`) are **blocked** — the user must confirm before execution proceeds. Commands below `block_level` show a warning but allow execution. The `block_level` is configurable via `ShellSafetyService`.

The pattern registry is extensible: callers can add, remove, or replace patterns at runtime via `DangerousPatternDetector`.

Also update the settings table at line 30131:

  • Current: shell.warn_dangerous | boolean | true | — | Highlight potentially dangerous commands
  • Proposed: shell.warn_dangerous | boolean | true | — | Enable shell danger detection; commands at or above block_level are blocked

Rationale

The implementation is better than the spec — it introduces a proper 4-level classification, a blocking capability for truly dangerous commands (CRITICAL/HIGH), and a richer pattern set. The spec's "advisory only" statement was aspirational but the implementation correctly chose to block CRITICAL/HIGH commands. This is a genuine improvement that should be reflected in the spec.


Change 2: CLI ULID Validation and Legacy/v3 Workflow Incompatibility

What changed in the implementation

PR #1577 added ULID format validation to all v3 plan commands (execute, apply, status, errors, cancel) and documented the incompatibility between the legacy agents tell workflow and the v3 agents plan workflow.

The spec currently documents <PLAN_ID> as a positional argument for these commands but does not specify:

  • That <PLAN_ID> must be a valid ULID (Crockford Base32, 26 chars, alphabet 0-9A-HJKMNP-TV-Z)
  • That legacy plans created via agents tell are invisible to v3 commands
  • The actionable error message shown when a non-ULID is passed

Spec sections affected

The agents plan execute, agents plan apply, agents plan status, agents plan cancel, and agents plan errors command descriptions (around lines 12922–14311) should note that <PLAN_ID> must be a valid ULID.

Proposed addition

After the <PLAN_ID> argument description in each affected command, add a note:

Note

: <PLAN_ID> must be a valid ULID (26-character Crockford Base32 string, e.g., 01HXM8C2ZK4Q7C2B3F2R4VYV6J). Plans created with the legacy agents tell command are stored in a separate system and are not accessible via v3 plan commands. Passing a legacy plan name to a v3 command produces an actionable error explaining the incompatibility and migration path.

Also add a note in the Plan Lifecycle glossary entry (line 74) clarifying that plan IDs use Crockford Base32 encoding (alphabet excludes I, L, O, U).

Rationale

The ULID validation is a correctness improvement that prevents confusing "plan not found" errors when users accidentally mix legacy and v3 workflows. Documenting this behavior helps users understand the system and the migration path.


Scope

Section Lines (approx.) Change Type
#### Shell Danger Detection 29886–29900 Replace table + description
shell.warn_dangerous settings row 30131 Update description
agents plan execute/apply/status/cancel/errors 12922–14311 Add ULID validation note
Plan glossary entry 74 Clarify ULID encoding

Awaiting human approval before any branch or PR is created.


Automated by CleverAgents Bot
Supervisor: Spec Evolution | Agent: ca-spec-updater

## Spec Update Proposal This proposal covers two spec sections that diverge from the merged implementation. --- ### Triggered By - **PR #1577** (merged 2026-04-03): `fix(cli): disallow mixing legacy and v3 plan workflows` - **Commit `50a5e96`** (merged 2026-04-03): `feat(tui): implement shell danger detection patterns` (closes #1003) --- ## Change 1: Shell Danger Detection — 4-level model, blocking behavior, revised pattern set ### What changed in the implementation The `feat(tui)` commit (closes #1003) implemented the full shell danger detection subsystem under `src/cleveragents/tui/shell_safety/`. The implementation diverges from the spec in several important ways: **Current spec (lines 29886–29900):** ``` #### Shell Danger Detection When shell mode is active (`!`/`$` prefix), the prompt performs heuristic analysis of the command to detect potentially destructive operations. Dangerous commands are highlighted with `$error` styling and a warning indicator appears below the prompt: | Pattern | Risk Level | Example | |---------|-----------|---------| | `rm -rf` / `rm -r` | High | `rm -rf /` | | `chmod 777` | Medium | `chmod 777 /var/www` | | `> /dev/sda` / `dd if=` | High | `dd if=/dev/zero of=/dev/sda` | | `:(){ :\|:& };:` (fork bomb) | High | Fork bomb patterns | | `mkfs` / `fdisk` / `parted` | High | Disk formatting tools | | `kill -9` / `killall` | Medium | Process termination | | `sudo` / `su` | Low | Privilege escalation (warning only) | Danger detection is controlled by the `shell.warn_dangerous` setting (default: `true`). The detection is advisory only — it never prevents command execution. The warning text reads: `⚠ Potentially destructive command detected`. ``` **What the implementation actually does:** 1. **4-level danger classification** (`ShellDangerLevel` enum): `LOW=1`, `MEDIUM=2`, `HIGH=3`, `CRITICAL=4` — the spec only mentions 3 levels (High/Medium/Low) and has no CRITICAL level. 2. **Commands CAN be blocked** — `ShellSafetyService` has a `block_level` (default: `MEDIUM`). Commands at or above `block_level` are blocked when no `warn_callback` is provided. The spec says "advisory only — it never prevents command execution" which is incorrect. 3. **Different pattern set** — the implementation does NOT include `kill -9`/`killall`, `fdisk`/`parted`, or `sudo`/`su` as standalone patterns. It DOES include patterns not in the spec: `shred --remove`, `wget|curl pipe to sh/bash`, `git push --force`, `chmod -R` with permissive modes. The `rm -rf` patterns are split into `rm_rf_root` (CRITICAL) and `rm_rf_wildcard` (CRITICAL), not "High". 4. **Warning message format** — the implementation produces `[{Level}] Dangerous command detected: {description}` (e.g., `[Critical] Dangerous command detected: rm -rf / recursively deletes...`), not `⚠ Potentially destructive command detected`. 5. **Architecture** — the implementation uses a proper domain model: `DangerousPattern` (value object), `DangerousPatternDetector` (domain service), `ShellSafetyService` (application service), `DangerousCommandWarning` (value object), `DEFAULT_PATTERNS` registry. This is a better design than the spec implied. ### Proposed spec text (lines 29886–29900 replacement) ```markdown #### Shell Danger Detection When shell mode is active (`!`/`$` prefix), the prompt performs heuristic analysis of the command to detect potentially destructive operations. Dangerous commands are highlighted with `$error` styling and a warning indicator appears below the prompt. The detection system uses a four-level danger classification: | Level | Value | Meaning | |-------|-------|---------| | `LOW` | 1 | Minor risk — generally recoverable side-effects | | `MEDIUM` | 2 | Moderate risk — potential data loss or security exposure | | `HIGH` | 3 | High risk — significant, hard-to-reverse damage | | `CRITICAL` | 4 | Critical risk — system destruction or fork bomb | The built-in pattern registry includes: | Pattern | Level | Example | |---------|-------|---------| | `rm -rf /` or `rm -rf /*` | Critical | `rm -rf /` | | Fork bomb `:(){ :|:& };:` | Critical | Fork bomb patterns | | `dd if=` | High | `dd if=/dev/zero of=/dev/sda` | | `mkfs` | High | Disk formatting | | `shred` on device or `--remove` | High | `shred /dev/sda` | | `chmod 777` | Medium | `chmod 777 /var/www` | | `sudo rm` | Medium | Elevated deletion | | `wget`/`curl` piped to `sh`/`bash` | Medium | Remote code execution | | `git push --force` | Low | History rewrite | | `chmod -R` with permissive modes | Low | Recursive permission change | The warning message format is: `[{Level}] Dangerous command detected: {description}`. Danger detection is controlled by the `shell.warn_dangerous` setting (default: `true`). When enabled, commands at or above the configured `block_level` (default: `MEDIUM`) are **blocked** — the user must confirm before execution proceeds. Commands below `block_level` show a warning but allow execution. The `block_level` is configurable via `ShellSafetyService`. The pattern registry is extensible: callers can add, remove, or replace patterns at runtime via `DangerousPatternDetector`. ``` Also update the settings table at line 30131: - **Current**: `shell.warn_dangerous | boolean | true | — | Highlight potentially dangerous commands` - **Proposed**: `shell.warn_dangerous | boolean | true | — | Enable shell danger detection; commands at or above block_level are blocked` ### Rationale The implementation is **better than the spec** — it introduces a proper 4-level classification, a blocking capability for truly dangerous commands (CRITICAL/HIGH), and a richer pattern set. The spec's "advisory only" statement was aspirational but the implementation correctly chose to block CRITICAL/HIGH commands. This is a genuine improvement that should be reflected in the spec. --- ## Change 2: CLI ULID Validation and Legacy/v3 Workflow Incompatibility ### What changed in the implementation PR #1577 added ULID format validation to all v3 plan commands (`execute`, `apply`, `status`, `errors`, `cancel`) and documented the incompatibility between the legacy `agents tell` workflow and the v3 `agents plan` workflow. The spec currently documents `<PLAN_ID>` as a positional argument for these commands but does not specify: - That `<PLAN_ID>` must be a valid ULID (Crockford Base32, 26 chars, alphabet `0-9A-HJKMNP-TV-Z`) - That legacy plans created via `agents tell` are invisible to v3 commands - The actionable error message shown when a non-ULID is passed ### Spec sections affected The `agents plan execute`, `agents plan apply`, `agents plan status`, `agents plan cancel`, and `agents plan errors` command descriptions (around lines 12922–14311) should note that `<PLAN_ID>` must be a valid ULID. ### Proposed addition After the `<PLAN_ID>` argument description in each affected command, add a note: > **Note**: `<PLAN_ID>` must be a valid ULID (26-character Crockford Base32 string, e.g., `01HXM8C2ZK4Q7C2B3F2R4VYV6J`). Plans created with the legacy `agents tell` command are stored in a separate system and are not accessible via v3 plan commands. Passing a legacy plan name to a v3 command produces an actionable error explaining the incompatibility and migration path. Also add a note in the Plan Lifecycle glossary entry (line 74) clarifying that plan IDs use Crockford Base32 encoding (alphabet excludes I, L, O, U). ### Rationale The ULID validation is a correctness improvement that prevents confusing "plan not found" errors when users accidentally mix legacy and v3 workflows. Documenting this behavior helps users understand the system and the migration path. --- ## Scope | Section | Lines (approx.) | Change Type | |---------|----------------|-------------| | `#### Shell Danger Detection` | 29886–29900 | Replace table + description | | `shell.warn_dangerous` settings row | 30131 | Update description | | `agents plan execute/apply/status/cancel/errors` | 12922–14311 | Add ULID validation note | | Plan glossary entry | 74 | Clarify ULID encoding | --- **Awaiting human approval before any branch or PR is created.** --- **Automated by CleverAgents Bot** Supervisor: Spec Evolution | Agent: ca-spec-updater
freemo added this to the v3.7.0 milestone 2026-04-03 02:42:45 +00:00
Author
Owner

Documentation Update — Cycle 1 (2026-04-03)

The ca-docs-writer agent has completed its initial documentation pass for the recent merged commits. Commit: 108d87e1

Docs Updated

File Change
CHANGELOG.md Added 6 new [Unreleased] entries for commits merged on 2026-04-03
docs/api/a2a.md Updated A2aRequest/A2aResponse field names to JSON-RPC 2.0 wire format; updated code examples

CHANGELOG entries added

  • Added: TUI shell danger detection patterns (#1003)
  • Changed (BREAKING): A2A JSON-RPC 2.0 wire format compliance — field rename table (#1501)
  • Changed: CLI disallows mixing legacy and v3 plan workflows (#1577)
  • Fixed: agents actor add rich output missing Type/Config/Capabilities/Tools panels
  • Fixed: E2E suite centralized database initialization (#1023)
  • Fixed: CI parallelized static analysis pipeline

Docs Skipped (already current)

  • README.md — accurate and comprehensive; no new user-facing features to add
  • docs/architecture.md — component map and layer descriptions remain current
  • docs/api/ (all other modules) — no API surface changes in recent commits
  • docs/reference/ — no reference doc changes needed
  • docs/timeline.mdnot modified (maintained exclusively by ca-timeline-updater)

Notes

  • The A2A JSON-RPC 2.0 field rename is a breaking change for any code constructing A2aRequest or reading A2aResponse directly. The migration table is now in both CHANGELOG.md and docs/api/a2a.md.
  • The refactor(agents) commit (agent permission model) is internal tooling and does not require user-facing documentation.

Automated by CleverAgents Bot
Supervisor: Documentation | Agent: ca-docs-writer

## Documentation Update — Cycle 1 (2026-04-03) The `ca-docs-writer` agent has completed its initial documentation pass for the recent merged commits. Commit: `108d87e1` ### Docs Updated | File | Change | |------|--------| | `CHANGELOG.md` | Added 6 new `[Unreleased]` entries for commits merged on 2026-04-03 | | `docs/api/a2a.md` | Updated `A2aRequest`/`A2aResponse` field names to JSON-RPC 2.0 wire format; updated code examples | ### CHANGELOG entries added - **Added**: TUI shell danger detection patterns (#1003) - **Changed (BREAKING)**: A2A JSON-RPC 2.0 wire format compliance — field rename table (#1501) - **Changed**: CLI disallows mixing legacy and v3 plan workflows (#1577) - **Fixed**: `agents actor add` rich output missing Type/Config/Capabilities/Tools panels - **Fixed**: E2E suite centralized database initialization (#1023) - **Fixed**: CI parallelized static analysis pipeline ### Docs Skipped (already current) - `README.md` — accurate and comprehensive; no new user-facing features to add - `docs/architecture.md` — component map and layer descriptions remain current - `docs/api/` (all other modules) — no API surface changes in recent commits - `docs/reference/` — no reference doc changes needed - `docs/timeline.md` — **not modified** (maintained exclusively by `ca-timeline-updater`) ### Notes - The A2A JSON-RPC 2.0 field rename is a **breaking change** for any code constructing `A2aRequest` or reading `A2aResponse` directly. The migration table is now in both `CHANGELOG.md` and `docs/api/a2a.md`. - The `refactor(agents)` commit (agent permission model) is internal tooling and does not require user-facing documentation. --- **Automated by CleverAgents Bot** Supervisor: Documentation | Agent: ca-docs-writer
Author
Owner

Documentation Update — Cycle 2 (2026-04-03)

New commits detected since last cycle. PR #2050 created with documentation updates.

New commits documented

Commit Change Doc action
977cacc2 refactor(domain): extract shared model_config into DomainBaseModel (#1941) Added to CHANGELOG [Unreleased] ### Changed
28ddfe38 chore(agents): add finding validation requirements to bug hunter Internal agent config — no user-facing docs needed
36ac24f2 chore(ci): refactor nightly-quality workflow to use nox sessions (#1537) CI-only change — no user-facing docs needed
c38be2ee fix(ci): address review feedback on nightly-quality workflow (#1537) CI-only change — no user-facing docs needed

PR created

  • PR #2050: docs: restructure [Unreleased] CHANGELOG and add entries for recent merged PRs
    • Consolidates duplicate ### Added/### Fixed headings in [Unreleased]
    • Adds ### Changed section with A2A JSON-RPC 2.0, CLI workflow mixing, and DomainBaseModel entries
    • Adds ### Fixed section with actor add output, E2E init, and CI parallelization entries

Note on master branch protection

Master branch is now protected — direct pushes rejected. All future documentation changes will be submitted via PR.


Automated by CleverAgents Bot
Supervisor: Documentation | Agent: ca-docs-writer

## Documentation Update — Cycle 2 (2026-04-03) New commits detected since last cycle. PR #2050 created with documentation updates. ### New commits documented | Commit | Change | Doc action | |--------|--------|------------| | `977cacc2` | `refactor(domain): extract shared model_config into DomainBaseModel` (#1941) | Added to CHANGELOG `[Unreleased] ### Changed` | | `28ddfe38` | `chore(agents): add finding validation requirements to bug hunter` | Internal agent config — no user-facing docs needed | | `36ac24f2` | `chore(ci): refactor nightly-quality workflow to use nox sessions` (#1537) | CI-only change — no user-facing docs needed | | `c38be2ee` | `fix(ci): address review feedback on nightly-quality workflow` (#1537) | CI-only change — no user-facing docs needed | ### PR created - **PR #2050**: `docs: restructure [Unreleased] CHANGELOG and add entries for recent merged PRs` - Consolidates duplicate `### Added`/`### Fixed` headings in `[Unreleased]` - Adds `### Changed` section with A2A JSON-RPC 2.0, CLI workflow mixing, and DomainBaseModel entries - Adds `### Fixed` section with actor add output, E2E init, and CI parallelization entries ### Note on master branch protection Master branch is now protected — direct pushes rejected. All future documentation changes will be submitted via PR. --- **Automated by CleverAgents Bot** Supervisor: Documentation | Agent: ca-docs-writer
Author
Owner

Documentation Update — Cycle 4 (2026-04-03)

New feature commit detected. PR #2181 created with documentation updates.

New commits documented

Commit Change Doc action
0be3f85c feat(tui): implement Permission Question Widget (#997) Added ### PermissionQuestionWidget to docs/api/tui.md; added CHANGELOG [Unreleased] ### Added entry
f66fb5a1 docs(spec): align ASCII UI tables in specification and related pages (#1171) Formatting-only change to existing docs — no CHANGELOG entry needed

PR created

  • PR #2181: docs(tui): document PermissionQuestionWidget and add CHANGELOG entry
    • docs/api/tui.md: New ### PermissionQuestionWidget section with method table, key bindings, PermissionDecisionEvent docs, and render_permission_question() reference
    • CHANGELOG.md: New [Unreleased] ### Added entry for PermissionQuestionWidget (#997)

Automated by CleverAgents Bot
Supervisor: Documentation | Agent: ca-docs-writer

## Documentation Update — Cycle 4 (2026-04-03) New feature commit detected. PR #2181 created with documentation updates. ### New commits documented | Commit | Change | Doc action | |--------|--------|------------| | `0be3f85c` | `feat(tui): implement Permission Question Widget` (#997) | Added `### PermissionQuestionWidget` to `docs/api/tui.md`; added CHANGELOG `[Unreleased] ### Added` entry | | `f66fb5a1` | `docs(spec): align ASCII UI tables in specification and related pages` (#1171) | Formatting-only change to existing docs — no CHANGELOG entry needed | ### PR created - **PR #2181**: `docs(tui): document PermissionQuestionWidget and add CHANGELOG entry` - `docs/api/tui.md`: New `### PermissionQuestionWidget` section with method table, key bindings, `PermissionDecisionEvent` docs, and `render_permission_question()` reference - `CHANGELOG.md`: New `[Unreleased] ### Added` entry for `PermissionQuestionWidget` (#997) --- **Automated by CleverAgents Bot** Supervisor: Documentation | Agent: ca-docs-writer
freemo self-assigned this 2026-04-03 16:58:14 +00:00
Author
Owner

Approved

Approved
Author
Owner

Issue triaged by project owner:

  • State: Verified
  • Priority: Backlog (elevated to Medium consideration)
  • Milestone: v3.7.0
  • MoSCoW: Should Have — spec updates for shell danger detection model and CLI ULID validation improve safety and correctness; these are spec-alignment items that should be addressed

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: Backlog (elevated to Medium consideration) - **Milestone**: v3.7.0 - **MoSCoW**: Should Have — spec updates for shell danger detection model and CLI ULID validation improve safety and correctness; these are spec-alignment items that should be addressed --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core#2025
No description provided.