Skip to content

feat: netclaw approvals CLI (list / revoke / TUI) (closes #921)#927

Merged
Aaronontheweb merged 6 commits into
netclaw-dev:devfrom
Aaronontheweb:claude-wt-approvals-cli
May 8, 2026
Merged

feat: netclaw approvals CLI (list / revoke / TUI) (closes #921)#927
Aaronontheweb merged 6 commits into
netclaw-dev:devfrom
Aaronontheweb:claude-wt-approvals-cli

Conversation

@Aaronontheweb
Copy link
Copy Markdown
Collaborator

Summary

  • Adds netclaw approvals — a Termina TUI plus single-shot list / revoke subcommands — for inspecting and revoking persistent tool approvals in ~/.netclaw/config/tool-approvals.json.
  • Operates on the file directly via ToolApprovalStore. The daemon re-reads the file on every approval check (no cache), so revocations take effect on the next prompt without a restart.
  • Centralizes the platform-correct comparer (Ordinal POSIX / OrdinalIgnoreCase Windows) into Netclaw.Configuration.ToolApprovalEntryComparer so the daemon gate and the CLI agree on what "the same entry" means.
  • Updates the netclaw-operations system skill (1.26.0 → 1.27.0) so the running agent recommends the CLI instead of hand-editing JSON. Keeps the file-edit recipe as a last-resort recovery path.

Why

PR #896 shipped directory-scoped persistent approvals; the file accumulates one entry per trusted directory per audience per tool. Until now there was no operator surface to audit or revoke those grants without hand-editing JSON. The longer the file grows, the harder it is to reason about — defeating the friction-reduction goal of #896. Issue #921 asks for this CLI.

Behavior contract

netclaw approvals                                # → TUI
netclaw approvals tui                            # alias
netclaw approvals list [--audience ...] [--tool ...] [--json]
netclaw approvals revoke <pattern> [--audience ...] [--tool ...]
netclaw approvals revoke --tool <name> --all [--audience ...]
netclaw approvals help
  • Exit 0 success, 1 user error / no-match for revoke, 2 reserved for malformed-file conditions.
  • revoke <pattern> is exact-match only (matches ApprovalPatternMatching.MatchesShellApprovalEntry's no-prefix-widening discipline).
  • revoke --all requires --tool. No-match revoke is loud, never silent.
  • Empty file is a normal state for list (exits 0 with No persistent approvals.); for revoke it exits 1.
  • A sibling tool-approvals.json.invalid (existing quarantine path) prints a warning before any list/revoke output.

OpenSpec

  • New change openspec/changes/approvals-management-cli/
    • MODIFIED tool-approval-gates "Persistent approval storage" — clarifies operator-editable + no-restart contract.
    • ADDED under netclaw-cli — "Operator CLI for persistent tool approvals" with 9 scenarios.
  • Will be archived after merge per /opsx-archive.

Tests

  • 9 ToolApprovalStoreTests covering platform case sensitivity, empty-section pruning, snapshot independence.
  • 13 ApprovalsCommandTests covering every list/revoke flag combination and exit-code contract (filter, JSON shape, exact-match revoke, no-match exit 1, --all scoping, --all without --tool, unknown audience, unscoped multi-audience revoke).
  • dotnet build, dotnet test (Configuration + CLI + Actors), dotnet slopwatch analyze (0 violations), Add-FileHeaders.ps1 -Verify all clean.

Test plan

  • Reviewer pulls the branch and runs dotnet test src/Netclaw.Cli.Tests and dotnet test src/Netclaw.Configuration.Tests.
  • NETCLAW_HOME=$(mktemp -d) dotnet run --project src/Netclaw.Cli -- approvals list → "No persistent approvals.", exit 0.
  • Seed tool-approvals.json with a personal/shell_execute/git push entry. netclaw approvals list shows it; netclaw approvals revoke "git push" --audience personal --tool shell_execute removes it; second revoke exits 1.
  • Bare netclaw approvals launches the TUI; Up/Down navigate, Delete or R triggers the revoke confirmation, Enter confirms, file changes match a follow-up list.
  • With a daemon running, perform an "Approve always" in a session, then revoke via the CLI, then re-trigger the same shell command — daemon should prompt again without a restart.
  • netclaw doctor continues to parse tool-approvals.json post-mutation.

Out of scope

Companion docs issue

Will be filed against netclaw-dev/netclaw-website once this PR is up — the website maintainer owns the docs PR.

Adds an operator surface for inspecting and revoking persistent tool
approvals stored in ~/.netclaw/config/tool-approvals.json. Bare
`netclaw approvals` (and `netclaw approvals tui`) launches an interactive
Termina page; single-shot subcommands `list` and `revoke` cover scripting.
The CLI talks to the file directly via ToolApprovalStore — the daemon
re-reads the file on every approval check (no cache), so revocations take
effect on the next prompt without a restart.

Storage layer
- ToolApprovalStore gains RemoveApproval, RemoveAllForTool, and Snapshot
  with empty-section pruning. All writes stay under the existing _lock.
- New ToolApprovalEntryComparer in Netclaw.Configuration centralizes the
  platform-correct StringComparison (Ordinal POSIX, OrdinalIgnoreCase
  Windows). ApprovalPatternMatching now consumes it so the daemon gate
  and the operator CLI agree on what "the same entry" means.

CLI surface
- list supports --audience, --tool, and --json with stable ordering and
  an empty-state message that exits 0.
- revoke is exact-match-only across the same comparer; --tool --all
  bulk-clears a tool's entries; --all without --tool fails fast.
- Unknown audience flag, no-match revoke, and quarantined-file warnings
  all surface explicitly — never silent success.

TUI
- ApprovalsManagerPage and ViewModel mirror ProviderManagerPage. Read
  via Snapshot(), revoke through RemoveApproval, refresh after each
  action.

OpenSpec
- New change `approvals-management-cli` adds a requirement under the
  netclaw-cli capability and clarifies the persistent-storage requirement
  in tool-approval-gates so the daemon's no-restart contract is explicit.

Skill update
- netclaw-operations bumped to 1.27.0; Approval Prompts section now
  recommends `netclaw approvals` and keeps the file-edit recovery path
  documented as a last resort.

Tests
- 9 ToolApprovalStoreTests covering platform case sensitivity, section
  cleanup, and snapshot independence.
- 13 ApprovalsCommandTests covering every list/revoke flag combination
  and exit-code contract.

Closes netclaw-dev#921
- ToolApprovalStore.AddApproval now uses ToolApprovalEntryComparer for
  dedup. Closes a Windows-only consistency hole where "Git Push" and
  "git push" deduped as distinct on add but a single revoke wiped both.
- Snapshot() returns a read-only projection
  (IReadOnlyDictionary<string, IReadOnlyDictionary<string, IReadOnlyList<string>>>)
  instead of the mutable serialization type so callers can't mutate it
  and discard their changes. Matches the documented "read-only iteration"
  contract from the OpenSpec tasks.
- Quarantine path is now exposed as ToolApprovalStore.QuarantinePath so
  the CLI's ".invalid" sniff doesn't duplicate the store's convention.
- Replaced 5 out-params and a duplicated --audience/--tool parsing
  block with ListOptions/RevokeOptions records and a shared
  TryConsumeSharedFlag helper. Distinguishes "missing value for --audience"
  from "unknown flag" instead of collapsing both into one misleading
  error.
- Removed the "exit code 2" claim from `netclaw approvals help` since
  the malformed-file path is surfaced as a warning + exit 0/1, not 2.
- Added two regression tests for the missing-value error paths.

All Approvals (15) + ToolApprovalStore (9) + Configuration/CLI/Actors
suites green. Slopwatch clean, file headers verified.
Three call sites hardcoded `[Personal, Team, Public]` as a "this is
every audience" array. If a future PR adds a new TrustAudience value,
those arrays go silently stale and would skip the new audience —
a real privilege-escalation hazard for the unscoped `netclaw approvals
revoke <pattern>` path, where a user expects "remove this everywhere"
to actually remove it everywhere.

- Added `TrustAudiences.All` (ImmutableArray) sourced from
  `Enum.GetValues<TrustAudience>()` so it can never drift from the
  enum definition.
- Updated all three call sites:
  - `ApprovalsCommand.RunRevokeAll` — security-relevant; uses canonical
    enum-declaration order (no UI ordering requirement).
  - `McpToolPermissionsViewModel` and `ChannelsStepView` — preserve
    their existing UI cycling order (most-trusted-first) by deriving
    `[.. TrustAudiences.All.Reverse()]`. New audiences still flow in
    automatically; the order is just inverted to keep user-visible
    behavior identical.
- New invariant test in TrustAudiencesTests pins the security
  contract: TrustAudiences.All must cover every TrustAudience enum
  value. The test fails fast if anyone adds an audience without
  thinking about iteration coverage.

Configuration (273) + CLI (616) + Actors (1475) suites green.
Slopwatch clean, file headers verified.
Mirrors the InitWizardPageTests / McpToolPermissionsPageTests pattern:
VirtualTerminal + VirtualInputSource exercise the full Termina render
and input pipeline against a temporary tool-approvals.json. Coverage:

- Empty-store rendering (page title + "No persistent approvals.")
- Seeded entries appear in the list (audience, tool, pattern)
- R + Enter revokes the highlighted entry; file is updated; state
  returns to List
- R + Esc cancels; file unchanged
- Revoking the final entry transitions the page into the Empty state

5 facts, all green. Total CLI surface coverage for this feature:
- ApprovalsCommandTests (single-shot): 14 facts
- ApprovalsManagerPageTests (TUI):     5 facts
- ToolApprovalStoreTests (storage):    9 facts
- TrustAudiencesTests (invariant):     1 fact
The page was missing a Ctrl+Q handler, so users had no way to exit
once the approvals manager was open — the established convention
across InitWizardPage, McpToolPermissionsPage, ChatPage,
ReminderCreatePage, and the provider/model/stats/sessions VMs.

Discovered via /simplify efficiency review: the new
ApprovalsManagerPageTests ran 50s instead of <1s because each test's
enqueued Ctrl+Q was silently dropped, forcing the loop to wait out the
10s cancellation token. After fix:

  ApprovalsManagerPageTests: 50s → 168ms (5 facts)
  Full CLI test suite:       51s → 2s

Also folded in two cosmetic fixes from the same review:
- ApprovalsCommand.RunRevokeAll: collapse the if/else split back to
  the single-loop form. The compiler infers IEnumerable<TrustAudience>
  for both branches via target-typing; no need for the duplication.
- Trim the duplicated 3-line cycling comment in McpToolPermissionsViewModel
  and ChannelsStepView to a single line. The "preserves the original UI
  behavior (Personal → Team → Public on right-arrow)" half is now part
  of the same line as the order — no information lost.

621 CLI / 274 Configuration tests green. Slopwatch clean, headers verified.
@Aaronontheweb Aaronontheweb merged commit a4ab853 into netclaw-dev:dev May 8, 2026
6 checks passed
@Aaronontheweb Aaronontheweb deleted the claude-wt-approvals-cli branch May 8, 2026 02:13
Aaronontheweb added a commit that referenced this pull request May 8, 2026
PR #927 (`netclaw approvals` CLI) is merged. This follow-up:

- Syncs the change's delta specs into the canonical specs:
  - `netclaw-cli`: appends "Operator CLI for persistent tool approvals"
    requirement with 9 scenarios. Adjusted from the delta-spec wording
    to match what shipped — drops the "exit code 2 for malformed-file
    conditions" claim since the implementation surfaces that as a
    warning + exit 0/1, not 2.
  - `tool-approval-gates`: extends the "Persistent approval storage"
    requirement with the operator-editable + no-restart-on-revoke
    contract, plus a new "Operator-applied revocation visible without
    restart" scenario.
- Drive-by: gives `netclaw-cli/spec.md` the `## Purpose` and
  `## Requirements` headers it was missing on `dev` so it validates
  against the OpenSpec schema. Pre-existing structural debt; this PR
  was already editing the file so fixing it here costs nothing.
- Archives the change directory to
  `openspec/changes/archive/2026-05-08-approvals-management-cli/`.
- Marks tasks 9.1 (PR #927) and 9.2 (companion docs issue
  netclaw-website#15) complete in the archived `tasks.md`. The
  remaining 4 unchecked items (3.4 manual TUI smoke, 8.2-8.4
  daemon-running end-to-end) are explicitly deferred per the change's
  plan; 8.2 is now blocked on issue #930 (release cadence).

`openspec validate netclaw-cli --strict` and
`openspec validate tool-approval-gates --strict` both clean.
@Aaronontheweb Aaronontheweb added the shell Issues related to the shell tool, since it has the largest security perimeter. label May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

shell Issues related to the shell tool, since it has the largest security perimeter.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant