From 11a97d0ce0206f75ad105542274a5791b24a1a60 Mon Sep 17 00:00:00 2001 From: Eric Lee Date: Wed, 29 Jul 2026 02:39:54 -0700 Subject: [PATCH 1/2] test(ui-tui): hoist useComposerState import out of a 5s test timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `appendImageChip` was loaded with a per-test `await import()`. That module pulls in a large graph, and a cold transform is billed against the 5s testTimeout — under a loaded full-suite run it blew the budget and failed whichever test happened to run first, while passing in isolation. Load it once in beforeAll with its own generous timeout. It stays out of the file's static imports on purpose: the tests above it are pure protocol checks that should not pay for (or run the module-scope side effects of) the composer. Co-Authored-By: Claude Opus 5 --- ui-tui/src/__tests__/imageRef.test.ts | 39 +++++++++++++++------------ 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/ui-tui/src/__tests__/imageRef.test.ts b/ui-tui/src/__tests__/imageRef.test.ts index 0af9b1aa..428d0f8b 100644 --- a/ui-tui/src/__tests__/imageRef.test.ts +++ b/ui-tui/src/__tests__/imageRef.test.ts @@ -1,7 +1,22 @@ -import { describe, expect, it } from 'vitest' +import { beforeAll, describe, expect, it } from 'vitest' import { formatImageRef, IMAGE_REF_RE, parseImageRefs } from '../protocol/imageRef.js' +/** `useComposerState` pulls in a large module graph, so it stays out of this + * file's static imports — the tests above it are pure protocol checks that + * should not pay for (or run the module-scope side effects of) the composer. + * + * It is loaded ONCE in beforeAll rather than per-test: a cold `await import()` + * inside a test is billed against the 5s testTimeout, and under a loaded + * full-suite run that transform blew the budget and failed whichever test + * happened to be first. beforeAll takes its own generous timeout, so the cost + * is paid where it belongs. */ +let appendImageChip: typeof import('../app/useComposerState.js')['appendImageChip'] + +beforeAll(async () => { + ;({ appendImageChip } = await import('../app/useComposerState.js')) +}, 60_000) + describe('formatImageRef', () => { it('renders the chip the composer shows', () => { expect(formatImageRef(1)).toBe('[Image #1]') @@ -52,44 +67,34 @@ describe('chip round-trip against the backend contract', () => { }) describe('appendImageChip — composer placement', () => { - it('puts the chip on its own line under existing text', async () => { - const { appendImageChip } = await import('../app/useComposerState.js') - + it('puts the chip on its own line under existing text', () => { expect(appendImageChip('what this image is about?', 2).value).toBe( 'what this image is about?\n[Image #2] ' ) }) - it('is the only content in an empty composer', async () => { - const { appendImageChip } = await import('../app/useComposerState.js') - + it('is the only content in an empty composer', () => { expect(appendImageChip('', 1).value).toBe('[Image #1] ') }) - it('does not double the newline', async () => { - const { appendImageChip } = await import('../app/useComposerState.js') - + it('does not double the newline', () => { expect(appendImageChip('line\n', 3).value).toBe('line\n[Image #3] ') }) - it('leaves the cursor after the chip', async () => { - const { appendImageChip } = await import('../app/useComposerState.js') + it('leaves the cursor after the chip', () => { const out = appendImageChip('hi', 4) expect(out.cursor).toBe(out.value.length) }) - it('keeps a dropped-path remainder after the chip', async () => { - const { appendImageChip } = await import('../app/useComposerState.js') - + it('keeps a dropped-path remainder after the chip', () => { // No trailing space before a newline — it would be invisible whitespace. expect(appendImageChip('look:', 5, 'and this too').value).toBe( 'look:\n[Image #5]\nand this too' ) }) - it('stacks multiple attachments, each parseable', async () => { - const { appendImageChip } = await import('../app/useComposerState.js') + it('stacks multiple attachments, each parseable', () => { const one = appendImageChip('compare these', 1).value const two = appendImageChip(one, 2).value From e11c885c4ff132a158e0a24167a677da3a289dc5 Mon Sep 17 00:00:00 2001 From: Eric Lee Date: Wed, 29 Jul 2026 02:48:58 -0700 Subject: [PATCH 2/2] feat(permissions): /mode becomes /permissions, 3-level picker, Full Access by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The permission UX was hard to discover and hard to set: `/mode` took a raw engine mode name (`default|plan|acceptEdits|dontAsk|bypassPermissions`) and every session started in `default`, so the practical advice was "always pass --dangerously-skip-permissions". - `/mode` -> `/permissions` (old name kept as an alias). Bare `/permissions` opens a picker over three levels — Ask for approval / Approve for me / Full Access — mapped onto EXISTING engine modes, so the permission engine itself is unchanged. `/permissions ` remains as a power-user escape hatch for `plan` / `dontAsk`. The level catalog lives in src/permissions/levels.py with a TS mirror pinned equal by a test; descriptions are written against what the engine does rather than copied from the tool this was modeled on. - A bare interactive `clawcodex` on a TTY now starts in Full Access. - The chosen level persists to `permissions.defaultMode` in `~/.clawcodex/settings.json`, so dialing permissions DOWN survives a relaunch. This wires the read side of a writer that had been write-only since the C1 review (updates.py's standing TODO). Keeping the new default defensible: - `bypass_selectable` is a NEW capability, separate from `is_bypass_permissions_mode_available`. The latter ALSO relaxes plan mode (check.py `should_bypass`), so reusing it to make the picker work would have turned `/plan` into full access in every session. `_build_runtime` no longer derives availability from the launch mode for the same reason. - The plan-approval dialog's elevated arm now keys off the same disjunction as the set_permission_mode gate. Before, approving a plan in a full-access session silently downgraded it to acceptEdits with no way back. - Shift+Tab can reach Full Access via selectability, so the default mode is not a one-way exit while the footer advertises "(shift+tab to cycle)". - Repository settings files (`settings.json` AND `settings.local.json` — both are committable; `.local` describes a .gitignore convention, not a trust boundary) may only supply `default` or `dontAsk`, and only when that does not loosen what applies anyway. `plan` is excluded because its looseness depends on bypass availability, which a ranking cannot express. - `permissions.allowBypassPermissionsMode` is read from the user config only. It was also read from `/.clawcodex/config.local.json`, which is a committable repo file, so a checked-out repository could grant itself bypass availability and thereby turn `/plan` into a write-anywhere bypass. - Root outside a sandbox drops bypass availability and clamps an implicit or persisted Full Access; an explicit `--permission-mode` is still honored, so `docker run ... --permission-mode bypassPermissions -p` keeps working. - Persisting `defaultMode` goes through one policy (`_may_persist_mode`) on both doors: the `persist` flag and a `chosen_updates` setMode. Previously the latter was an ungated second path to `bypassPermissions` for any non-TUI agent-server client. - A saved Full Access does not arm `clawcodex -p`; persistence dials headless down, never up. Non-TTY and `--print` launches keep `default`, so CI and the Harbor eval harness are unaffected. Verified with the full Python suite, the ui-tui suite (typecheck clean, only the 8 pre-existing failures), and a live pyte end-to-end run against the real Ink client + agent-server covering the whole round trip in both directions. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 51 +++ README.md | 65 ++- src/cli.py | 47 +- src/command_system/permissions_command.py | 66 ++- src/entrypoints/agent_server_cli.py | 37 +- src/entrypoints/tui_launcher.py | 56 ++- src/permissions/cycle.py | 21 +- src/permissions/levels.py | 122 +++++ src/permissions/modes.py | 401 ++++++++++++++++- src/permissions/updates.py | 8 +- src/server/agent_server.py | 225 ++++++++-- src/skills/bundled/update_config.py | 3 +- .../server/test_bypass_permissions_wiring.py | 253 ++++++++++- tests/server/test_tui_launcher.py | 55 +++ tests/skills/test_update_config_skill.py | 19 +- tests/test_dangerous_skip_permissions.py | 112 ++++- tests/test_permission_levels.py | 419 ++++++++++++++++++ ui-tui/src/__tests__/brandingPermsRow.test.ts | 127 ++++++ ui-tui/src/__tests__/composerFooter.test.ts | 26 +- ui-tui/src/__tests__/gatewayClient.test.ts | 60 ++- .../src/__tests__/permissionsCommand.test.ts | 129 ++++++ .../src/__tests__/permissionsPicker.test.tsx | 161 +++++++ ui-tui/src/app/interfaces.ts | 3 + ui-tui/src/app/overlayStore.ts | 4 + ui-tui/src/app/slash/commands/session.ts | 57 +++ ui-tui/src/app/uiStore.ts | 1 + ui-tui/src/app/useMainApp.ts | 10 + ui-tui/src/components/appLayout.tsx | 1 + ui-tui/src/components/appOverlays.tsx | 18 +- ui-tui/src/components/branding.tsx | 47 +- ui-tui/src/components/composerFooter.tsx | 9 +- ui-tui/src/components/permissionsPicker.tsx | 99 +++++ ui-tui/src/gatewayClient.ts | 46 +- ui-tui/src/gatewayTypes.ts | 7 + ui-tui/src/lib/permissionLevels.ts | 71 +++ 35 files changed, 2621 insertions(+), 215 deletions(-) create mode 100644 src/permissions/levels.py create mode 100644 tests/test_permission_levels.py create mode 100644 ui-tui/src/__tests__/brandingPermsRow.test.ts create mode 100644 ui-tui/src/__tests__/permissionsCommand.test.ts create mode 100644 ui-tui/src/__tests__/permissionsPicker.test.tsx create mode 100644 ui-tui/src/components/permissionsPicker.tsx create mode 100644 ui-tui/src/lib/permissionLevels.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c4b979a..8144703c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,57 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **Permissions are now loose by default and easy to change.** `/mode` is + renamed `/permissions` (the old name still works as an alias) and bare + `/permissions` opens a three-option picker — *Ask for approval*, *Approve for + me*, *Full Access* — instead of requiring a raw mode name. A bare interactive + `clawcodex` now starts in **Full Access**, equivalent to + `--dangerously-skip-permissions`. The chosen level persists to + `permissions.defaultMode` in `~/.clawcodex/settings.json`, so dialing + permissions *down* survives a relaunch. + + Scope and safety of the new default: + + - Only a real terminal gets it. `--print`/headless is unchanged (`default`) — + CI and the eval harness drive it — and so is any non-TTY launch, even + without `-p`. A persisted `defaultMode` applies to every surface, with one + exception in the safe direction: a saved *Full Access* is interactive-only + (see below). + - Explicit intent always wins: `--permission-mode`, + `--dangerously-skip-permissions`, and a persisted `defaultMode` all outrank + the implicit default, and `--allow-dangerously-skip-permissions` (which + means "available without starting in it") suppresses it. + - Suppressed entirely under a `disableBypassPermissionsMode` lockdown, and + under root outside a sandbox — where it degrades to `default` rather than + refusing to start, and also clamps a persisted `bypassPermissions` and + settings-granted bypass availability. + - **Plan mode still restrains.** Choosing Full Access sets the mode only and + never grants engine bypass *availability*, which is what relaxes `plan`, so + `/plan` keeps asking in a full-access session. + - Repository settings files (`.clawcodex/settings.json` and + `settings.local.json` — both are committable, whatever the `.local` name + suggests) may only supply `default` or `dontAsk`, and only when that would + not loosen what applies anyway. A cloned repo cannot widen your + permissions. `plan` is excluded on purpose: with bypass availability it is + a full bypass, not a restriction. + - A saved Full Access applies only to interactive sessions. Persistence + exists so dialing *down* survives a relaunch; it never dials `-p` up. + - `permissions.allowBypassPermissionsMode` is now read from the user config + only. It was also read from `/.clawcodex/config.local.json`, + which — despite the `.local` name — is a committable repo file, so a + checked-out repository could grant itself bypass *availability* and thereby + turn `/plan` into a write-anywhere bypass. Operators who kept that key in a + repo-local config must move it to `~/.clawcodex/config.json`. + - Running as root outside a sandbox now also drops bypass availability, and + ignores a *saved* Full Access. An explicit `--permission-mode` is still + honored there; only the implicit and saved paths are clamped. + - `setMode` arriving inside a permission-ask reply (`chosen_updates`) is now + gated by the same capability as `/permissions`, and refused entirely for + persisted destinations — previously it was an ungated second door to + `bypassPermissions` for any non-TUI agent-server client. + ## [1.3.0] - 2026-07-29 ### Added diff --git a/README.md b/README.md index 0f1ce9dd..94d51d94 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ Then configure a provider and start coding: ```bash clawcodex login # interactive provider + API key setup → ~/.clawcodex/config.json -clawcodex --dangerously-skip-permissions # start the REPL in any project +clawcodex # start it in any project — Full Access by default, /permissions to change ``` The installer also ships `clawcodex` lifecycle helpers — `doctor` (diagnose your @@ -104,7 +104,7 @@ pip install -r requirements.txt python -m src.cli login # writes config to ~/.clawcodex/config.json -python -m src.cli --dangerously-skip-permissions # start the REPL +python -m src.cli # start it (Full Access by default; /permissions to change) ``` @@ -400,12 +400,58 @@ clawcodex -p --output-format stream-json --input-format stream-json < events.ndj clawcodex --provider anthropic --model claude-sonnet-4-6 -p "Hi" clawcodex --max-turns 10 --allowed-tools Read,Grep -p "Find TODOs" -# Permission control (REPL, TUI, and -p all honor these) -clawcodex --permission-mode plan # plan / acceptEdits / dontAsk +# Permission control (TUI and -p both honor these) +clawcodex --permission-mode plan # plan / default / acceptEdits / dontAsk clawcodex --dangerously-skip-permissions -p "ls" # bypass all permission checks -clawcodex --allow-dangerously-skip-permissions # allow /permission-mode bypass later +clawcodex --allow-dangerously-skip-permissions # Full Access selectable, but don't start in it ``` +### Permissions + +An interactive `clawcodex` starts in **Full Access** — it can edit any file and +run any command without asking. Use `/permissions` at any time to pick a level: + +| Level | What it allows | +| ----- | -------------- | +| **Ask for approval** | Read files and run read-only commands freely. Editing files, running other commands, or touching anything outside the workspace asks first. | +| **Approve for me** | Auto-accept file edits in the workspace and a small set of safe shell commands. Everything else still asks. | +| **Full Access** *(default)* | Edit any file and run any command without asking, except where you configured a deny or ask rule. | + +The level you pick is saved to `permissions.defaultMode` in +`~/.clawcodex/settings.json` and applies to later sessions, so dialing +permissions down sticks. `--permission-mode` and +`--dangerously-skip-permissions` override it for a single run without changing +it. + +`clawcodex -p` (headless) is **not** loose by default — it stays in `default` +unless you say otherwise, and a *saved* Full Access does not change that: +persistence dials headless down, never up. Say otherwise explicitly with +`--dangerously-skip-permissions` or `--permission-mode`. The same applies to +any launch that isn't attached to a terminal. + +To start in a lower level every time, either pick it once with `/permissions` +or set it directly: + +```json +// ~/.clawcodex/settings.json +{ "permissions": { "defaultMode": "default" } } +``` + +A repo can ship `.clawcodex/settings.json` (or `settings.local.json`) with a +`defaultMode` to *tighten* permissions for anyone who opens it. Repo-supplied +values are restricted to `default` and `dontAsk` — note `dontAsk` denies +anything that would otherwise prompt, so a repo can make the agent refuse +actions rather than ask about them — and are ignored if they'd be looser than +what would otherwise apply, so a cloned repo can never widen yours. (`plan` is +excluded deliberately: combined with bypass availability it is a full bypass, +not a restriction.) Administrators can disable Full Access outright with +`disableBypassPermissionsMode` in `~/.clawcodex/config.json` or managed policy. + +`/plan` keeps restraining edits even in a Full Access session — unless you +launched with `--allow-dangerously-skip-permissions` or set +`allowBypassPermissionsMode` in your user config, both of which relax plan mode +by design. + ### Claude Pro/Max subscription login Run `clawcodex login`, choose `anthropic`, then choose `subscription`. The CLI @@ -446,9 +492,12 @@ Use `clawcodex config` to check connection status and `clawcodex logout openai` to delete the stored OAuth credentials. > **`--dangerously-skip-permissions`** disables every tool permission check -> for the session. Recommended only inside sandboxed containers/VMs with no -> internet access. The flag is refused when the process is running as -> root/sudo unless `IS_SANDBOX=1` or `CLAUDE_CODE_BUBBLEWRAP=1` is set. +> for the session — the same thing an interactive `clawcodex` now does by +> default (see [Permissions](#permissions)). Recommended only inside sandboxed +> containers/VMs with no internet access. The *flag* is refused when the +> process is running as root/sudo unless `IS_SANDBOX=1` or +> `CLAUDE_CODE_BUBBLEWRAP=1` is set; under root the implicit default quietly +> falls back to `default` instead, so `sudo clawcodex` still starts. *** diff --git a/src/cli.py b/src/cli.py index 93639042..958d0da6 100644 --- a/src/cli.py +++ b/src/cli.py @@ -232,6 +232,7 @@ def main(): effort=args.effort, permission_mode=args._resolved_permission_mode, is_bypass_available=args._resolved_is_bypass_available, + bypass_selectable=args._resolved_bypass_selectable, workspace=(worktree_session.worktree_path if worktree_session else None), worktree=worktree_session, ) @@ -580,41 +581,49 @@ def _resolve_permission_state(args) -> None: from src.permissions.dangerous_safety import ( enforce_dangerous_skip_permissions_safety, ) - from src.permissions.modes import ( - has_allow_bypass_permissions_mode, - initial_permission_mode_from_cli, - is_bypass_permissions_mode_disabled, - ) + from src.permissions.modes import resolve_interactive_permission_state dangerously = bool(getattr(args, 'dangerously_skip_permissions', False)) allow_dangerously = bool(getattr(args, 'allow_dangerously_skip_permissions', False)) permission_mode_cli = getattr(args, 'permission_mode', None) - # Safety gate first — refuse to run as root outside a sandbox. + # Safety gate first — refuse to run as root outside a sandbox. Applies to the + # EXPLICIT flags only; the implicit full-access default degrades to `default` + # under the same condition instead of aborting (see + # modes.resolve_interactive_permission_state), because `sudo clawcodex` with + # no flags must still start. enforce_dangerous_skip_permissions_safety( bypass_requested=dangerously or allow_dangerously, ) - mode = initial_permission_mode_from_cli( + # The loose default is scoped to the INTERACTIVE launch — "the user typed + # clawcodex in a terminal". `--print`/headless keeps `default` because CI + # and the Harbor eval harness drive it, and silently altering benchmark runs + # is not acceptable. A persisted `permissions.defaultMode` still applies to + # both, so an explicit user choice is never honored on one surface and + # ignored on the other. + # + # "not --print" is NOT sufficient: a TTY check is what makes "a human is + # sitting there" true. Without it any piped/automated launch that merely + # omits -p would take the loose floor — and `_gate_folder_trust` grants + # trust implicitly on non-TTY stdin, so nothing else would stop it either. + interactive = ( + not bool(getattr(args, 'print', False)) + and sys.stdin.isatty() + and sys.stdout.isatty() + ) + + mode, is_bypass_available, bypass_selectable = resolve_interactive_permission_state( permission_mode_cli=permission_mode_cli, dangerously_skip_permissions=dangerously, + allow_dangerously_skip_permissions=allow_dangerously, + implicit_full_access=interactive, ) - # TS isBypassPermissionsModeAvailable (permissionSetup.ts:941-946): - # (bypass-requested OR allow-key) AND NOT disabled. The negative guard was - # dropped in the port → an operator's disableBypassPermissionsMode lockdown - # was silently ignored (critic C12, a live fail-open). A disable overrides - # even an explicit --dangerously-skip-permissions, matching TS's - # unconditional `&& !settingsDisableBypassPermissionsMode`. - is_bypass_available = ( - dangerously - or allow_dangerously - or has_allow_bypass_permissions_mode() - ) and not is_bypass_permissions_mode_disabled() - # Stash on args so downstream entrypoints don't need to re-derive. args._resolved_permission_mode = mode args._resolved_is_bypass_available = is_bypass_available + args._resolved_bypass_selectable = bypass_selectable if dangerously or allow_dangerously: _logging.getLogger("clawcodex.permissions").info( diff --git a/src/command_system/permissions_command.py b/src/command_system/permissions_command.py index b52f2657..7228cb09 100644 --- a/src/command_system/permissions_command.py +++ b/src/command_system/permissions_command.py @@ -1,5 +1,22 @@ """permissions — interactive ``/permissions`` command (port of TS local-jsx). +.. warning:: + + **Test-only reachable.** The live ``/permissions`` is the Ink TUI's local + slash command (``ui-tui/src/app/slash/commands/session.ts``), which routes + through the agent-server's gated ``set_permission_mode`` control. This one is + still registered (``builtins.py``) but its two original surfaces — the Rich + REPL and the Textual TUI — were deleted in the UI consolidation, and the Ink + client never dispatches unknown slashes to the Python registry. It survives + because ``tests/test_interactive_bridge.py`` uses it as the reference command + for the whole interactive-command bridge. + + Do not wire it to a live surface as-is: it writes ``AppState.permission_mode`` + through the store and never touches ``tool_context``, so it applies no mode to + the live permission gate **and enforces no bypass gate** — it would be an + ungated ``bypassPermissions`` selector. Route any new surface through + ``set_permission_mode`` instead. + Reference command for the P0-3 interactive-command bridge (see my-docs/get-parity-by-folder/commands-phase2-interactive-bridge-plan.md): a single ``select`` over the user-facing permission modes, backed by the reactive @@ -28,30 +45,33 @@ UIOption, ) -# User-facing permission modes, in the Shift+Tab cycle order -# (``src/permissions/cycle.py``). The internal modes (dontAsk / auto / bubble) -# are deliberately excluded from the picker — they're not user-addressable. +def _option_for(mode: str, fallback_label: str, fallback_description: str) -> UIOption: + """Build a :class:`UIOption` from :mod:`src.permissions.levels` when the mode + is one of the three user-facing levels, else from the supplied fallback. + + Labels/descriptions come from the shared catalog so this command and the Ink + TUI's ``/permissions`` picker never present two different vocabularies for + the same modes. ``plan`` has no level row (it is a workflow mode, not a point + on the looseness ladder) and keeps its own wording. + """ + from src.permissions.levels import level_for_mode + + level = level_for_mode(mode) + return UIOption( + value=mode, + label=level.label if level else fallback_label, + description=level.description if level else fallback_description, + ) + + +# In Shift+Tab cycle order (``src/permissions/cycle.py``). The internal modes +# (dontAsk / auto / bubble) are deliberately excluded — they're not +# user-addressable. _PERMISSION_MODE_OPTIONS: list[UIOption] = [ - UIOption( - value="default", - label="default", - description="Prompt before each tool's first use", - ), - UIOption( - value="acceptEdits", - label="acceptEdits", - description="Auto-accept file edits in the workspace", - ), - UIOption( - value="plan", - label="plan", - description="Plan only — no edits or commands", - ), - UIOption( - value="bypassPermissions", - label="bypassPermissions", - description="Skip all permission prompts", - ), + _option_for("default", "default", "Prompt before each tool's first use"), + _option_for("acceptEdits", "acceptEdits", "Auto-accept file edits in the workspace"), + _option_for("plan", "Plan mode", "Plan only — no edits or commands"), + _option_for("bypassPermissions", "bypassPermissions", "Skip all permission prompts"), ] diff --git a/src/entrypoints/agent_server_cli.py b/src/entrypoints/agent_server_cli.py index 31385e2f..68b33b50 100644 --- a/src/entrypoints/agent_server_cli.py +++ b/src/entrypoints/agent_server_cli.py @@ -88,8 +88,8 @@ def _sanitize_agent_server_mode(args: Any, dangerously: bool) -> None: The agent-server subcommand sets the mode STRAIGHT into AgentServerConfig (it does NOT route through ``initial_permission_mode_from_cli``, so that - function's candidate-skip doesn't apply). ``check.py:456`` bypasses on the - mode ALONE, so BOTH ``--dangerously-skip-permissions`` and a directly-passed + function's candidate-skip doesn't apply). ``check.py``'s ``should_bypass`` + bypasses on the mode ALONE, so BOTH ``--dangerously-skip-permissions`` and a directly-passed ``--permission-mode bypassPermissions`` must be sanitized here or they defeat a managed ``disableBypassPermissionsMode: "disable"`` lockdown. Downgrade -and-warn (TS ``continue`` semantics), never a hard error.""" @@ -155,8 +155,16 @@ def run_agent_server_subcommand(argv: list[str]) -> int: parser.add_argument( "--allow-dangerously-skip-permissions", action="store_true", dest="allow_dangerously_skip_permissions", - help="Make bypassPermissions AVAILABLE (Shift+Tab / /mode) without " - "starting in it.", + help="Make bypassPermissions AVAILABLE (Shift+Tab / /permissions) " + "without starting in it. Also relaxes plan mode.", + ) + parser.add_argument( + "--allow-select-bypass", action="store_true", + dest="allow_select_bypass", + help="Let /permissions CHOOSE Full Access. Weaker than " + "--allow-dangerously-skip-permissions: it does not relax plan " + "mode and is not inherited as bypass availability. Set by the " + "interactive launchers; this server is a carrier, not a resolver.", ) parser.add_argument("--workspace", default=None, help="Workspace root the agent operates in (default: cwd).") @@ -216,7 +224,7 @@ def run_agent_server_subcommand(argv: list[str]) -> int: # bypass AVAILABILITY. Flags always count. Trusted settings # (permissions.allowBypassPermissionsMode) count only on the # single-session --stdio transport: this server serves exactly the - # operator who launched it, so their own user/local settings apply. On + # operator who launched it, so their own user settings apply. On # the multi-session --http transport, folding host settings in would # unlock bypass for every remote client — resolve that per-launch # upstream and pass a flag instead. @@ -232,6 +240,24 @@ def run_agent_server_subcommand(argv: list[str]) -> int: if is_bypass_permissions_mode_disabled(): is_bypass_available = False + # bypass SELECTABILITY — whether /permissions may choose Full Access. This + # server is a CARRIER, not a resolver: the value is decided once at the + # interactive launch boundary (src/cli.py, tui_launcher) and forwarded, never + # re-derived from settings here. Re-reading settings would let the --http + # server host's own config unlock Full Access for every remote client + # session (the same hazard documented at agent_server._build_runtime). + # Availability implies selectability; a lockdown revokes both. + # + # Bounded claim: this keeps SELECTABILITY off the host's settings. It does + # not isolate the starting MODE — the launcher that spawned this server + # already resolved that from the host's own `permissions.defaultMode`, so a + # multi-tenant deployment still inherits the host's configured mode. + bypass_selectable = bool(getattr(args, "allow_select_bypass", False)) or is_bypass_available + if bypass_selectable and not is_bypass_available: + from src.permissions.modes import is_bypass_permissions_mode_disabled + if is_bypass_permissions_mode_disabled(): + bypass_selectable = False + workspace = str(Path(args.workspace).resolve()) if args.workspace else str(Path.cwd()) if args.fallback_model and args.fallback_model == args.model: @@ -245,6 +271,7 @@ def run_agent_server_subcommand(argv: list[str]) -> int: fallback_model=args.fallback_model, permission_mode=args.permission_mode, is_bypass_available=is_bypass_available, + bypass_selectable=bypass_selectable, max_turns=args.max_turns, ) diff --git a/src/entrypoints/tui_launcher.py b/src/entrypoints/tui_launcher.py index 61450bf7..8f99359c 100644 --- a/src/entrypoints/tui_launcher.py +++ b/src/entrypoints/tui_launcher.py @@ -57,7 +57,11 @@ def run_tui_launcher(argv: list[str]) -> int: parser.add_argument("--effort", default=None, choices=("low", "medium", "high", "xhigh", "max"), help="Reasoning effort — seeds the session's /effort level") - parser.add_argument("--permission-mode", default="default", dest="permission_mode") + # default=None (not "default") so "flag absent" is distinguishable from an + # explicit `--permission-mode default`: the former gets the loose interactive + # floor, the latter is an explicit request for prompts. src/cli.py's parser + # already uses None. + parser.add_argument("--permission-mode", default=None, dest="permission_mode") parser.add_argument( "--dangerously-skip-permissions", action="store_true", dest="dangerously_skip_permissions", @@ -67,8 +71,8 @@ def run_tui_launcher(argv: list[str]) -> int: parser.add_argument( "--allow-dangerously-skip-permissions", action="store_true", dest="allow_dangerously_skip_permissions", - help="Make bypassPermissions available (Shift+Tab / /mode) without " - "starting in it.", + help="Make bypassPermissions available (Shift+Tab / /permissions) " + "without starting in it.", ) parser.add_argument("--workspace", default=None, help="Workspace root the agent operates in (default: cwd).") @@ -94,31 +98,32 @@ def run_tui_launcher(argv: list[str]) -> int: return 2 # Same resolution the default `clawcodex` entry runs in - # src/cli.py::_resolve_permission_state — safety gate first, then - # flag > --permission-mode priority, then availability. + # src/cli.py::_resolve_permission_state — one shared resolver so the two + # interactive entrypoints cannot drift. from src.permissions.dangerous_safety import ( enforce_dangerous_skip_permissions_safety, ) - from src.permissions.modes import ( - has_allow_bypass_permissions_mode, - initial_permission_mode_from_cli, - ) + from src.permissions.modes import resolve_interactive_permission_state dangerously = bool(args.dangerously_skip_permissions) allow_dangerously = bool(args.allow_dangerously_skip_permissions) enforce_dangerous_skip_permissions_safety( bypass_requested=dangerously or allow_dangerously, ) - mode = initial_permission_mode_from_cli( + # --print-connect starts a listening agent-server, prints a token, and waits + # for a remote client: nobody is at the terminal, so the "the user typed + # clawcodex and is sitting there" premise behind the loose default does not + # hold. That surface stays opt-in via the explicit flags. Same TTY reasoning + # as src/cli.py::_resolve_permission_state. + mode, is_bypass_available, bypass_selectable = resolve_interactive_permission_state( permission_mode_cli=args.permission_mode, dangerously_skip_permissions=dangerously, + allow_dangerously_skip_permissions=allow_dangerously, + cwd=args.workspace, + implicit_full_access=( + not args.print_connect and sys.stdin.isatty() and sys.stdout.isatty() + ), ) - # ... AND NOT disabled (critic C12 — the dropped negative guard; - # an operator lockdown must override even --dangerously-skip-permissions). - from src.permissions.modes import is_bypass_permissions_mode_disabled - is_bypass_available = ( - dangerously or allow_dangerously or has_allow_bypass_permissions_mode() - ) and not is_bypass_permissions_mode_disabled() # --worktree: the trust gate already ran in cli.py::_run_tui_subcommand # (against the original cwd) before this launcher was invoked. Create or @@ -148,6 +153,7 @@ def run_tui_launcher(argv: list[str]) -> int: if args.print_connect: args.permission_mode = mode args.is_bypass_available = is_bypass_available + args.bypass_selectable = bypass_selectable if worktree_session is not None: # run_agent_server_subcommand runs IN THIS process; exporting the # session here is exactly how the server's worktree controls see @@ -160,6 +166,7 @@ def run_tui_launcher(argv: list[str]) -> int: effort=args.effort, permission_mode=mode, is_bypass_available=is_bypass_available, + bypass_selectable=bypass_selectable, workspace=args.workspace, tui_dir=args.tui_dir, worktree=worktree_session, @@ -173,6 +180,7 @@ def launch_ink_tui( effort: str | None = None, permission_mode: str = "default", is_bypass_available: bool = False, + bypass_selectable: bool = False, workspace: str | None = None, tui_dir: str | None = None, worktree=None, @@ -190,6 +198,12 @@ def launch_ink_tui( agent-server, which owns the Shift+Tab cycle and set_permission_mode guards. Without it, availability resolved by the CLI was silently dropped and bypass could never be reached at runtime. + + ``bypass_selectable`` is the DIFFERENT, weaker capability that lets + ``/permissions`` choose Full Access. It is deliberately not the same flag: + ``is_bypass_available`` also relaxes **plan mode** (``check.py`` + ``should_bypass``), so setting it just to make the picker work would turn + ``/plan`` into full access for every session. """ args = SimpleNamespace( provider=provider, @@ -197,6 +211,7 @@ def launch_ink_tui( effort=effort, permission_mode=permission_mode, is_bypass_available=is_bypass_available, + bypass_selectable=bypass_selectable, workspace=workspace, tui_dir=tui_dir, print_connect=False, @@ -259,8 +274,13 @@ def _agent_server_cmd(args) -> list[str]: if getattr(args, "is_bypass_available", False): # Availability only — the launch mode above decides whether the # session STARTS in bypass; this keeps bypass reachable via - # Shift+Tab / /mode either way. + # Shift+Tab / /permissions either way. cmd += ["--allow-dangerously-skip-permissions"] + if getattr(args, "bypass_selectable", False): + # The weaker capability: /permissions may CHOOSE Full Access. Kept + # separate from availability above because that one also relaxes plan + # mode; see launch_ink_tui's docstring. + cmd += ["--allow-select-bypass"] if args.provider: cmd += ["--provider", args.provider] if args.model: @@ -316,6 +336,8 @@ def _print_connect(args) -> int: "--permission-mode", args.permission_mode, *(["--allow-dangerously-skip-permissions"] if getattr(args, "is_bypass_available", False) else []), + *(["--allow-select-bypass"] + if getattr(args, "bypass_selectable", False) else []), *(["--provider", args.provider] if args.provider else []), *(["--model", args.model] if args.model else []), *(["--effort", args.effort] if getattr(args, "effort", None) else []), diff --git a/src/permissions/cycle.py b/src/permissions/cycle.py index 0e51149b..bed47b34 100644 --- a/src/permissions/cycle.py +++ b/src/permissions/cycle.py @@ -14,7 +14,11 @@ from .types import PermissionUpdateSetMode -def get_next_permission_mode(context: ToolPermissionContext) -> PermissionMode: +def get_next_permission_mode( + context: ToolPermissionContext, + *, + can_select_bypass: bool = False, +) -> PermissionMode: """Return the next mode when the user presses Shift+Tab. Mirrors ``getNextPermissionMode`` in @@ -24,11 +28,22 @@ def get_next_permission_mode(context: ToolPermissionContext) -> PermissionMode: - ``default`` → ``acceptEdits`` - ``acceptEdits`` → ``plan`` - - ``plan`` → ``bypassPermissions`` (when available) else ``default`` + - ``plan`` → ``bypassPermissions`` (when reachable) else ``default`` - ``bypassPermissions`` → ``default`` - ``dontAsk`` / ``auto`` / ``bubble`` → ``default`` (escape hatch — these modes are not part of the user-facing cycle but we still need a defined transition so Shift+Tab never strands the user.) + + ``can_select_bypass`` is the agent-server's ``bypass_selectable``: whether + ``/permissions`` may choose Full Access. It is a SECOND way to reach the + bypass stop, because the sessions that default to Full Access deliberately + leave ``is_bypass_permissions_mode_available`` False (that flag also relaxes + plan mode). Without it, a default session's very first Shift+Tab dropped out + of Full Access with no way back through the same key — while the footer + badge kept advertising "(shift+tab to cycle)". + + Reaching bypass this way sets the MODE only; availability is untouched, so a + later ``plan`` still restrains. """ mode = context.mode if mode == "default": @@ -36,7 +51,7 @@ def get_next_permission_mode(context: ToolPermissionContext) -> PermissionMode: if mode == "acceptEdits": return "plan" if mode == "plan": - if context.is_bypass_permissions_mode_available: + if context.is_bypass_permissions_mode_available or can_select_bypass: return "bypassPermissions" return "default" if mode == "bypassPermissions": diff --git a/src/permissions/levels.py b/src/permissions/levels.py new file mode 100644 index 00000000..cc66e902 --- /dev/null +++ b/src/permissions/levels.py @@ -0,0 +1,122 @@ +"""The three user-facing permission levels behind ``/permissions``. + +Single source of truth, mirrored by ``ui-tui/src/lib/permissionLevels.ts`` (the +two are pinned equal by ``tests/test_permission_levels.py``) and consumed by the +Python ``/permissions`` command (``src/command_system/permissions_command.py``). + +Each level is a *label* over an existing engine :class:`PermissionMode` — the +permission engine itself is untouched. ``plan`` and ``dontAsk`` are deliberately +absent: they are workflow / strict-deny modes, not points on a looseness ladder. +They stay reachable via ``/plan``, ``--permission-mode``, and the power-user form +``/permissions ``. + +The descriptions are written against what the engine actually does. They are NOT +copied from the picker this was modeled on (OpenAI Codex), whose wording +describes internet gating clawcodex does not implement and a classifier that +here lives only on the flag-gated ``auto`` mode. Sources for each claim: + +* ``ask`` → ``default``: read-only Bash auto-allows + (:mod:`src.permissions.bash_mode_validation`), out-of-root reads ask + (:func:`src.permissions.filesystem`), out-of-root Bash writes ask + (``src/tool_system/tools/bash/bash_tool.py``). +* ``approve`` → ``acceptEdits``: edits inside the working roots auto-allow + (``check.py`` acceptEdits branch) plus a *small fixed list* of write and + read-only commands (:mod:`src.permissions.bash_mode_validation`). ``git + commit``, ``npm install``, ``pytest`` and friends all still ask — hence "a + small set of safe shell commands" rather than "only unsafe things". +* ``full`` → ``bypassPermissions``: the mode short-circuit in ``check.py``. + Deny/ask *rules* still run first, and ``ExitPlanMode``'s interaction ask is + bypass-immune — hence the "except where you have configured…" clause. + +Level keys are chosen NOT to collide with any :data:`PermissionMode` name, so +``/permissions `` can accept a level key or a raw mode unambiguously. In +particular the middle level is ``approve``, not ``auto`` — ``auto`` is a real +(internal, classifier-backed) mode. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from .types import PermissionMode + + +@dataclass(frozen=True) +class PermissionLevel: + """One row of the ``/permissions`` picker.""" + + #: Stable identifier used by ``/permissions `` and the TUI overlay. + key: str + #: Title-case label shown in the picker. + label: str + #: The engine mode this level selects. + mode: PermissionMode + #: One-line explanation of what the level actually permits. + description: str + + +#: Ordered loosest-last, matching the reference picker's 1/2/3 ordering. +PERMISSION_LEVELS: tuple[PermissionLevel, ...] = ( + PermissionLevel( + key="ask", + label="Ask for approval", + mode="default", + description=( + "Read files and run read-only commands freely. Editing files, " + "running other commands, or touching anything outside this " + "workspace asks first." + ), + ), + PermissionLevel( + key="approve", + label="Approve for me", + mode="acceptEdits", + description=( + "Auto-accept file edits in this workspace and a small set of safe " + "shell commands. Everything else still asks." + ), + ), + PermissionLevel( + key="full", + label="Full Access", + mode="bypassPermissions", + description=( + "Edit any file and run any command without asking, except where " + "you have configured a deny or ask rule. Exercise caution when " + "using." + ), + ), +) + +PERMISSION_LEVEL_KEYS: tuple[str, ...] = tuple(lv.key for lv in PERMISSION_LEVELS) + +#: The mode every level maps to — used to decide whether a mode is on the ladder. +PERMISSION_LEVEL_MODES: tuple[PermissionMode, ...] = tuple(lv.mode for lv in PERMISSION_LEVELS) + + +def level_for_key(key: str) -> PermissionLevel | None: + """Look up a level by its ``key`` (case-insensitive). ``None`` if unknown.""" + wanted = key.strip().lower() + for level in PERMISSION_LEVELS: + if level.key == wanted: + return level + return None + + +def level_for_mode(mode: str | None) -> PermissionLevel | None: + """Return the level a mode belongs to, or ``None`` for off-ladder modes. + + ``plan`` / ``dontAsk`` / ``auto`` / ``bubble`` all return ``None`` — they are + real modes with no row in the picker, and callers must render them as + "no level selected" rather than silently showing one of the three. + """ + for level in PERMISSION_LEVELS: + if level.mode == mode: + return level + return None + + +def mode_for_key(key: str) -> PermissionMode | None: + """``/permissions `` → engine mode. ``None`` if ``key`` is not a level.""" + level = level_for_key(key) + return level.mode if level is not None else None diff --git a/src/permissions/modes.py b/src/permissions/modes.py index 10828a02..979058bf 100644 --- a/src/permissions/modes.py +++ b/src/permissions/modes.py @@ -83,6 +83,7 @@ def initial_permission_mode_from_cli( dangerously_skip_permissions: bool = False, settings_default_mode: str | None = None, disable_bypass_permissions_mode: bool | None = None, + fallback_mode: PermissionMode = "default", ) -> PermissionMode: """Resolve the effective :class:`PermissionMode` from CLI flags + settings. @@ -99,6 +100,16 @@ def initial_permission_mode_from_cli( Unknown / mistyped mode strings degrade to ``default`` via :func:`permission_mode_from_string`. + ``fallback_mode`` replaces the hardcoded ``default`` FLOOR reached when no + candidate matched. It is how the interactive entrypoints express "nothing + was configured, so start in full access" (the ``/permissions`` loose + default) WITHOUT inserting a non-TS candidate into the ladder above — the + priority list stays a faithful mirror, and "an explicit + ``--permission-mode default``" stays distinguishable from "nothing + matched", because any matched candidate returns before the floor. + Callers are responsible for passing ``default`` when a lockdown or an + elevated-privileges check should suppress their looser floor. + ``disable_bypass_permissions_mode`` (critic C12): when a bypass lockdown is in effect, the ``bypassPermissions`` candidate is SKIPPED (TS permissionSetup.ts:778-793 ``continue``s past it) so the resolved mode @@ -124,39 +135,385 @@ def initial_permission_mode_from_cli( ) continue # TS: skip this mode if it's disabled return candidate - return "default" + # The floor. TS returns "default" unconditionally here; ``fallback_mode`` + # keeps that as the default while letting an interactive launcher raise it. + if fallback_mode == "bypassPermissions" and disable_bypass_permissions_mode: + log.warning( + "Bypass permissions mode was disabled by settings/policy " + "(permissions.disableBypassPermissionsMode); falling back." + ) + return "default" + return fallback_mode + + +# The ONLY modes a repo-scoped settings file may supply as +# ``permissions.defaultMode``. An allowlist, not a "block bypassPermissions" +# denylist, because looseness is not a property of the mode name alone: +# +# ``plan`` LOOKS like the strictest mode, but ``check.py``'s ``should_bypass`` +# is ``mode == "bypassPermissions" or (mode == "plan" and +# is_bypass_permissions_mode_available)`` — and the same disjunction in +# ``tool_system/context.py`` is what lifts working-root containment. So in any +# session that has bypass availability (``--allow-dangerously-skip-permissions``, +# or ``allowBypassPermissionsMode`` plus a user who ran ``/permissions ask``), +# a repo shipping ``defaultMode: "plan"`` would get unconstrained +# write-anywhere. A ranking cannot express that, because the answer depends on +# a second variable the ranking never sees. +# +# ``default`` and ``dontAsk`` are unambiguously restrictive whatever else is +# true of the session, so they are the two a repo may ask for. That still covers +# the case worth supporting — "don't run wide open in this repo" — now that Full +# Access is the interactive default. +_REPO_SETTABLE_DEFAULT_MODES: tuple[str, ...] = ("default", "dontAsk") + +# How loose each mode is, low → high. Applied ON TOP of the allowlist above so a +# repo cannot loosen relative to what would otherwise apply (e.g. proposing +# ``default`` to a user who persisted ``dontAsk``). +_MODE_LOOSENESS: dict[str, int] = { + "dontAsk": 0, + "plan": 1, + "default": 2, + "auto": 3, + "acceptEdits": 4, + "bypassPermissions": 5, +} + + +def _read_settings_file_default_mode(path: str) -> PermissionMode | None: + """Read ``permissions.defaultMode`` from one settings file, or ``None``.""" + import json + import os + + try: + if not path or not os.path.exists(path): + return None + with open(path, encoding="utf-8") as f: + data = json.load(f) + except Exception: # noqa: BLE001 — an unreadable/!JSON tier is simply absent + log.debug("settings file unreadable for defaultMode: %s", path, exc_info=True) + return None + perms = (data or {}).get("permissions") + if not isinstance(perms, dict): + return None + mode = perms.get("defaultMode") + if not isinstance(mode, str) or mode not in EXTERNAL_PERMISSION_MODES: + return None + return mode # type: ignore[return-value] + + +def read_settings_default_mode( + cwd: str | None = None, + *, + baseline_mode: PermissionMode = "default", +) -> PermissionMode | None: + """Resolve ``permissions.defaultMode`` from the settings files. + + This is the READ side of the mode that ``persist_permission_update`` + (``updates.py`` ``PermissionUpdateSetMode`` branch) has been writing + write-only since the C1 review — its standing TODO ("setup_permissions does + not read defaultMode back at startup; wire the read side") is this function. + + The store is the canonical permission settings layer + (:mod:`src.permissions.settings_paths`: ``~/.clawcodex/settings.json``, + ``/.clawcodex/settings{,.local}.json``, managed policy) — NOT + ``~/.clawcodex/config.json["settings"]``. Writing a ``permissions`` DICT + into the latter would collide with :class:`~src.settings.types.SettingsSchema`, + whose ``permissions`` field is a typed rule LIST. + + Tiers split into TRUSTED and REPO-SCOPED, which is the security boundary: + + * **trusted** — managed (root-owned MDM policy), then **user** + (``~/.clawcodex/settings.json``). User comes before any repo file because + that is where ``/permissions`` writes the user's own explicit choice, and + a repo must not silently defeat it. First hit wins. + * **repo-scoped** — ``/.clawcodex/settings.local.json`` then + ``settings.json``. BOTH are treated as untrusted-for-loosening. The + ``.local`` suffix conventionally means "gitignored, operator-owned", but + that is a property of *your* ``.gitignore``: a hostile repo simply commits + the file. Trusting it let a cloned repo hand itself ``bypassPermissions``, + including on ``clawcodex -p``, which returns before the folder-trust gate. + + A repo-scoped value is honored only when it is both in + :data:`_REPO_SETTABLE_DEFAULT_MODES` and **no looser** than what would + otherwise apply, so a repo can tighten but never loosen. ``baseline_mode`` is + what the caller would use absent any setting — pass the interactive floor + (``bypassPermissions``) so a repo can tighten it. + + Returns ``None`` when nothing is configured. + """ + from .settings_paths import ( + local_settings_path, + project_settings_path, + user_settings_path, + ) + + trusted: PermissionMode | None = None + try: + from src.settings.managed_path import resolve_managed_settings_path + + managed = resolve_managed_settings_path() + if managed is not None: + trusted = _read_settings_file_default_mode(str(managed)) + except Exception: # noqa: BLE001 — no managed policy on this platform + log.debug("managed settings unavailable for defaultMode", exc_info=True) + + if trusted is None: + trusted = _read_settings_file_default_mode(user_settings_path()) + + effective = trusted if trusted is not None else baseline_mode + for path in (local_settings_path(cwd), project_settings_path(cwd)): + proposed = _read_settings_file_default_mode(path) + if proposed is None: + continue + if proposed not in _REPO_SETTABLE_DEFAULT_MODES: + log.warning( + "Ignoring permissions.defaultMode=%r from %s: a repository " + "settings file may only request %s.", + proposed, + path, + " or ".join(_REPO_SETTABLE_DEFAULT_MODES), + ) + continue + if _MODE_LOOSENESS.get(proposed, 99) > _MODE_LOOSENESS.get(effective, 99): + log.warning( + "Ignoring permissions.defaultMode=%r from %s: a repository " + "settings file may tighten permissions but not loosen them " + "(effective mode is %r).", + proposed, + path, + effective, + ) + continue + # A repo CAN force `dontAsk`, which denies everything that would prompt. + # Restrictive, so allowed — but say why, or a user whose agent suddenly + # refuses every action has no way to trace it back to the repo. + if proposed == "dontAsk": + log.warning( + "%s sets permissions.defaultMode='dontAsk': this session will " + "DENY anything that would otherwise prompt. Override with " + "--permission-mode or /permissions.", + path, + ) + return proposed + return trusted + + +def set_settings_default_mode(mode: PermissionMode) -> bool: + """Persist ``mode`` as ``permissions.defaultMode`` in the USER settings file. + + Reuses the existing :func:`~src.permissions.updates.persist_permission_update` + ``setMode`` writer rather than adding a second one, so the on-disk shape stays + whatever that function already produces. Returns the writer's success flag. + + Refuses anything :func:`read_settings_default_mode` would not read back — + notably ``auto``, which is a valid runtime mode but not an + :data:`EXTERNAL_PERMISSION_MODES` member. Writing it would clobber a real + prior choice with a value nothing consumes. + """ + from .settings_paths import settings_path_for_destination + from .types import PermissionUpdateSetMode + from .updates import persist_permission_update + + if mode not in EXTERNAL_PERMISSION_MODES: + log.debug("refusing to persist non-external defaultMode %r", mode) + return False + + return persist_permission_update( + PermissionUpdateSetMode( + type="setMode", + destination="userSettings", + mode=mode, + ), + settings_path_for_destination=settings_path_for_destination, + ) + + +def is_elevated_without_sandbox() -> bool: + """True when running as uid 0 outside a sandbox. + + The implicit full-access default is suppressed in that case. This is NOT + :func:`~src.permissions.dangerous_safety.enforce_dangerous_skip_permissions_safety` + — that helper ``SystemExit``s, which is right for an explicit + ``--dangerously-skip-permissions`` but would make a plain ``sudo clawcodex`` + fail to start. An implicit default must degrade, never abort. + """ + from .dangerous_safety import _is_running_as_root, is_sandbox_environment + + return _is_running_as_root() and not is_sandbox_environment() + + +def resolve_interactive_permission_state( + *, + permission_mode_cli: str | None, + dangerously_skip_permissions: bool, + allow_dangerously_skip_permissions: bool, + cwd: str | None = None, + implicit_full_access: bool = True, +) -> tuple[PermissionMode, bool, bool]: + """Resolve ``(mode, is_bypass_available, bypass_selectable)`` for a launch. + + The single resolver shared by ``src/cli.py::_resolve_permission_state`` and + ``src/entrypoints/tui_launcher.py`` so the two interactive entrypoints cannot + drift. + + Priority (via :func:`initial_permission_mode_from_cli`, unchanged): + ``--dangerously-skip-permissions`` → ``--permission-mode`` → persisted + ``permissions.defaultMode`` → the floor. ``implicit_full_access`` raises the + floor to ``bypassPermissions``; it is suppressed under an operator lockdown + (handled inside the ladder) and under root-outside-sandbox (here). + + The two booleans are deliberately distinct: + + ``is_bypass_available`` + the ENGINE flag (``ToolPermissionContext.is_bypass_permissions_mode_available``). + It also relaxes **plan mode** (``check.py`` ``should_bypass``), so it stays + strictly opt-in — flags or ``allowBypassPermissionsMode`` only. The + implicit default must NOT set it, or ``/plan`` would stop restraining. + + ``bypass_selectable`` + whether ``/permissions`` may select Full Access. True for an interactive + launch unless locked down or elevated. Selecting Full Access sets the + MODE only and never flips the engine flag, so a later ``/plan`` still asks. + """ + disabled = is_bypass_permissions_mode_disabled() + elevated = is_elevated_without_sandbox() + + # `--allow-dangerously-skip-permissions` means, in its own words, "make + # bypass AVAILABLE without starting in it". That is an explicit statement + # about the starting mode, so it suppresses the implicit floor — otherwise + # the flag would silently do the opposite of what it says. + asked_for_available_not_entered = ( + allow_dangerously_skip_permissions and not dangerously_skip_permissions + ) + + # The floor: full access only when this launch opted in AND nothing forbids + # it. The ladder re-checks the lockdown itself; the elevated check is ours. + floor: PermissionMode = "default" + if ( + implicit_full_access + and not asked_for_available_not_entered + and not disabled + and not elevated + ): + floor = "bypassPermissions" + + persisted_mode = read_settings_default_mode(cwd, baseline_mode=floor) + + # A persisted `bypassPermissions` does NOT arm non-interactive launches. + # Persistence exists so that dialing permissions DOWN survives a relaunch; + # letting it dial headless UP would mean one `/permissions full` click + # silently converts every future `clawcodex -p`, in every directory, into a + # full bypass — and `-p` returns before the folder-trust gate, so nothing + # would ask. That is precisely the outcome the interactive-only scoping of + # the implicit floor exists to prevent, and it would falsify the documented + # contract that headless stays in `default` unless you say otherwise. + # Saying otherwise is `--dangerously-skip-permissions` or an explicit + # `--permission-mode`, both of which outrank this candidate anyway. + # + # This drops a MANAGED-policy `bypassPermissions` too, not just a user's own + # `/permissions full` click. Deliberate: it fails safe, and an admin who + # wants to constrain bypass has `disableBypassPermissionsMode`, not this. + if not implicit_full_access and persisted_mode == "bypassPermissions": + log.debug( + "Ignoring persisted permissions.defaultMode='bypassPermissions' for " + "a non-interactive launch; pass --dangerously-skip-permissions to " + "opt in explicitly." + ) + persisted_mode = None + + mode = initial_permission_mode_from_cli( + permission_mode_cli=permission_mode_cli, + dangerously_skip_permissions=dangerously_skip_permissions, + settings_default_mode=persisted_mode, + disable_bypass_permissions_mode=disabled, + fallback_mode=floor, + ) + + is_bypass_available = ( + dangerously_skip_permissions + or allow_dangerously_skip_permissions + or has_allow_bypass_permissions_mode() + ) and not disabled + + bypass_selectable = is_bypass_available or ( + implicit_full_access and not disabled and not elevated + ) + + # Root outside a sandbox clamps the RESOLVED state, not just the floor: the + # floor guard alone left a persisted `defaultMode: bypassPermissions` + # (exactly what `/permissions full` writes) and settings-granted + # availability — which turns `plan` into a full bypass via check.py's + # `should_bypass` — both reaching root sessions untouched. + # + # But it must clamp only what the user did NOT ask for. `--permission-mode + # bypassPermissions` does NOT go through + # `enforce_dangerous_skip_permissions_safety` (that guard keys off the two + # `*-skip-permissions` flags and returns early otherwise), so it arrives here + # intact — and `docker run … clawcodex --permission-mode bypassPermissions + # -p …` as root is a routine invocation. Silently rewriting it to `default` + # would flip those runs from "everything allowed" to "everything denied" + # (headless denies every ask), announced only by a log line. + mode_was_explicit = bool(permission_mode_cli) or dangerously_skip_permissions + if elevated and (mode == "bypassPermissions" or is_bypass_available): + clamping_mode = mode == "bypassPermissions" and not mode_was_explicit + log.warning( + "Running as root outside a sandbox: %s (resolved mode %r, " + "availability %s). Set IS_SANDBOX=1 or CLAUDE_CODE_BUBBLEWRAP=1 if " + "this really is a sandbox.", + "refusing full access" if clamping_mode else "refusing bypass availability", + mode, + is_bypass_available, + ) + if clamping_mode: + mode = "default" + # Availability is dropped either way: under root it can only come from + # settings (both flags that grant it exit before this function), and it + # is the flag that lifts working-root containment in plan mode. + is_bypass_available = False + bypass_selectable = False + + return mode, is_bypass_available, bypass_selectable def has_allow_bypass_permissions_mode() -> bool: """Return True if a trusted settings source enables bypass availability. - Mirrors ``hasAllowBypassPermissionsMode`` in - ``typescript/src/utils/settings/settings.ts:897``: reads - ``permissions.allowBypassPermissionsMode`` from the user (global) and local - tiers, and INTENTIONALLY EXCLUDES the project tier - (``/.clawcodex/config.json``) — that file is committable, so a - cloned repo must not be able to auto-enable bypass (the TS comment: "a + Follows ``hasAllowBypassPermissionsMode`` in + ``typescript/src/utils/settings/settings.ts:897``, which reads + ``permissions.allowBypassPermissionsMode`` and EXCLUDES the committable + project tier so a cloned repo cannot auto-enable bypass (the TS comment: "a malicious project could otherwise enable bypass mode (security risk)"). + **The USER (global) tier only** — a deliberate divergence from the TS + reference, which also reads the *local* tier. Here the local tier is + ``/.clawcodex/config.local.json`` (``config.get_local_config_path`` + is git-root relative, not home relative), so it is just as committable as + the project tier the exclusion already names; ``.local`` describes a + convention in *your* ``.gitignore``, not a trust boundary. A repo shipping + that file could grant itself bypass availability — and availability is not a + minor capability: ``check.py``'s ``should_bypass`` is ``mode == + "bypassPermissions" or (mode == "plan" and + is_bypass_permissions_mode_available)``, and the same disjunction in + ``tool_system/context.py`` lifts working-root containment. So a committed + file could make ``plan`` — the mode whose whole purpose is changing nothing — + a full write-anywhere bypass, with ``clawcodex -p`` never reaching the + folder-trust gate. Same reasoning as + :func:`read_settings_default_mode`'s repo-scoped tiers. + We read the RAW per-tier config dicts rather than the merged ``SettingsSchema`` for two reasons: the schema has no structured slot for this scalar (``permissions`` is modeled as a flat rule *list*), and the - merged view can't tell the project tier apart from the trusted tiers, which - is exactly the distinction the security exclusion turns on. + merged view can't tell the tiers apart, which is exactly the distinction the + security exclusion turns on. """ try: from src.config import ConfigManager cm = ConfigManager() - # Global (user) + local (gitignored, operator-owned) only — never the - # committable project tier. - for loader in (cm.load_global, cm.load_local): - perms = loader().get("settings", {}).get("permissions") - if isinstance(perms, dict) and perms.get("allowBypassPermissionsMode"): - return True + perms = cm.load_global().get("settings", {}).get("permissions") + return bool(isinstance(perms, dict) and perms.get("allowBypassPermissionsMode")) except Exception: return False - return False def is_bypass_permissions_mode_disabled() -> bool: @@ -174,7 +531,17 @@ def is_bypass_permissions_mode_disabled() -> bool: ``disable`` only ever REMOVES capability, so honoring it from ANY tier — including the managed/policy tier (the org-admin lockdown) and the project tier — is always safe. Reads the raw per-tier dicts (the SettingsSchema - models ``permissions`` as a flat rule list, no scalar slot).""" + models ``permissions`` as a flat rule list, no scalar slot). + + .. warning:: + + The FILE is ``~/.clawcodex/config.json`` (under its ``"settings"`` key), + plus the managed policy file — the :class:`~src.config.ConfigManager` + tiers, NOT the ``settings.json`` layer that + :func:`read_settings_default_mode` and ``setup_permissions`` read. Two + adjacent permission knobs, two different files; an operator who sets + ``disableBypassPermissionsMode`` in ``~/.clawcodex/settings.json`` gets + silence, not a lockdown.""" try: from src.config import ConfigManager diff --git a/src/permissions/updates.py b/src/permissions/updates.py index ec52b731..ffba7fe1 100644 --- a/src/permissions/updates.py +++ b/src/permissions/updates.py @@ -357,9 +357,11 @@ def persist_permission_update( permissions["additionalDirectories"] = [d for d in existing if d not in target] elif isinstance(update, PermissionUpdateSetMode): - # Write-only today: setup_permissions does not read defaultMode - # back at startup (mode comes from CLI/config). Asymmetry noted in - # the C1 review; wire the read side with the C8 mode-gate work. + # The read side is :func:`src.permissions.modes.read_settings_default_mode`, + # which the interactive launchers consult so `/permissions` choices + # survive a relaunch. (This was write-only for a long time — the C1 + # review's noted asymmetry — so anything asserting "defaultMode has no + # effect" is stale.) permissions["defaultMode"] = update.mode return _write_json(path, settings) diff --git a/src/server/agent_server.py b/src/server/agent_server.py index acd43478..c36acd58 100644 --- a/src/server/agent_server.py +++ b/src/server/agent_server.py @@ -113,6 +113,15 @@ class AgentServerConfig: # isBypassPermissionsModeAvailable in # typescript/src/utils/permissions/permissionSetup.ts:941. is_bypass_available: bool = False + # bypassPermissions SELECTABILITY — whether `/permissions` may choose Full + # Access. Strictly weaker than is_bypass_available above, and deliberately a + # SEPARATE field: availability ALSO relaxes plan mode (check.py + # `should_bypass`), so reusing it to make the picker work would silently turn + # /plan into full access in every session that defaults to Full Access. + # Resolved once at the interactive launch boundary and forwarded via + # --allow-select-bypass; never derived from settings here (see the + # multi-tenant --http note in _build_runtime). + bypass_selectable: bool = False max_turns: int = DEFAULT_MAX_TURNS allowed_tools: tuple[str, ...] = () disallowed_tools: tuple[str, ...] = () @@ -426,20 +435,25 @@ async def _handle_control_request(self, msg: dict) -> None: self._reply(request_id, {"ok": False, "error": "session not ready"}) return # bypassPermissions is only settable when the session made it - # available (--dangerously-skip-permissions / --allow-…). Same - # guard the Shift+Tab cycle enforces (get_next_permission_mode) — - # without it, /mode bypassPermissions silently disabled the whole + # available (--dangerously-skip-permissions / --allow-…) or + # SELECTABLE (--allow-select-bypass, set by the interactive + # launchers so `/permissions full` works). Without a guard, + # `/permissions bypassPermissions` silently disabled the whole # permission gate in any session. Mirrors the onSetPermissionMode # contract in typescript/src/bridge/replBridge.ts:182-193. + # + # Selecting bypass sets the MODE only — it never flips the engine + # availability flag, so a later /plan still restrains (that flag + # also relaxes plan mode; see AgentServerConfig.bypass_selectable). pc = self.tool_context.permission_context - if mode == "bypassPermissions" and not getattr( - pc, "is_bypass_permissions_mode_available", False + if mode == "bypassPermissions" and not ( + getattr(pc, "is_bypass_permissions_mode_available", False) + or self.config.bypass_selectable ): self._reply(request_id, { "ok": False, - "error": "bypassPermissions is not available in this " - "session — launch with " - "--dangerously-skip-permissions or " + "error": "Full Access is not available in this session — " + "launch with --dangerously-skip-permissions or " "--allow-dangerously-skip-permissions", }) return @@ -448,7 +462,20 @@ async def _handle_control_request(self, msg: dict) -> None: # tool_context.permission_context; the store dispatch runs # the centralized seams (listeners; future persistence). _dispatch_app_state(self, permission_mode=mode) - self._reply(request_id, {"ok": True, "mode": mode}) + # `persist` is set only by `/permissions` choosing one of the three + # LEVELS — a standing preference that must survive relaunch, so a + # user who deliberately dials DOWN to "Ask for approval" is not + # silently returned to Full Access next launch. Shift+Tab cycling and + # the raw-mode escape hatch stay transient. + persisted = False + if inner.get("persist") and self._may_persist_mode(mode): + try: + from src.permissions.modes import set_settings_default_mode + + persisted = bool(set_settings_default_mode(mode)) # type: ignore[arg-type] + except Exception: # noqa: BLE001 — a failed write must not fail the set + logger.debug("[agent-server] defaultMode persist failed", exc_info=True) + self._reply(request_id, {"ok": True, "mode": mode, "persisted": persisted}) return if subtype == "cycle_permission_mode": # ch13 round-4 (critic B1) — shift+tab cycling MUST be computed @@ -465,7 +492,12 @@ async def _handle_control_request(self, msg: dict) -> None: from src.permissions.cycle import get_next_permission_mode pc = self.tool_context.permission_context - new_mode = get_next_permission_mode(pc) + # Pass selectability: a session that defaults to Full Access has + # availability False on purpose, so without this the first Shift+Tab + # was a one-way exit out of the default mode. + new_mode = get_next_permission_mode( + pc, can_select_bypass=self.config.bypass_selectable, + ) _set_mode(self.tool_context, new_mode) _dispatch_app_state(self, permission_mode=new_mode) self._reply(request_id, {"ok": True, "mode": new_mode}) @@ -2647,9 +2679,18 @@ def permission_handler(self, request: Any) -> Any: pc = getattr(self.tool_context, "permission_context", None) wire_request["plan"] = get_plan() wire_request["plan_file_path"] = str(get_plan_file_path()) + # Same disjunction as the set_permission_mode gate — NOT the + # engine availability flag alone. This drives which elevated + # option the approval box offers (prompts.tsx: "Yes, and bypass + # permissions" vs "Yes, auto-accept edits"). With availability + # alone, a session that started in Full Access and ran /plan + # would be offered only "auto-accept edits", whose chosen_updates + # setMode pre-empts ExitPlanMode's pre_plan_mode restore — so + # approving a plan silently DOWNGRADED the session out of full + # access with no notice and no way back but /permissions. wire_request["bypass_available"] = bool( getattr(pc, "is_bypass_permissions_mode_available", False) - ) + ) or bool(self.config.bypass_selectable) except Exception: # noqa: BLE001 — degrade to the generic box logger.debug("[agent-server] plan payload failed", exc_info=True) self._emit({ @@ -2666,34 +2707,139 @@ def permission_handler(self, request: Any) -> Any: return PermissionAskReply( behavior="deny", message="permission request timed out" ) - reply = pending.reply or {"behavior": "deny"} - behavior = reply.get("behavior") - if behavior == "allow": - updated = reply.get("updatedInput") - if not isinstance(updated, dict): - updated = reply.get("updated_input") - # ch13 round-4 — read the user's chosen "don't ask again" rules - # back off the reply and return them so handle_permission_ask / - # the can_use_tool adapter PERSIST them (registry.py:169 → - # _apply_and_persist_updates → settings). This is what makes - # "always allow" actually stick. - chosen_raw = reply.get("chosen_updates") or reply.get("chosenUpdates") - chosen: tuple = () - if isinstance(chosen_raw, list): - deserialized = [ - _deserialize_permission_update(u) - for u in chosen_raw if isinstance(u, dict) - ] - chosen = tuple(u for u in deserialized if u is not None) + return self._permission_reply(pending.reply or {"behavior": "deny"}) + + def _permission_reply(self, reply: dict) -> Any: + """Turn a client's permission-ask reply into a :class:`PermissionAskReply`. + + Extracted from :meth:`permission_handler` so the ``chosen_updates`` gate + below is directly testable — it is a privilege boundary, and the only + alternative was driving it through a blocking wire round-trip. + """ + from src.permissions.types import PermissionAskReply + + if reply.get("behavior") != "allow": return PermissionAskReply( - behavior="allow", - updated_input=updated if isinstance(updated, dict) else None, - chosen_updates=chosen, + behavior="deny", + message=str(reply.get("message", "")) or "denied by user", + ) + updated = reply.get("updatedInput") + if not isinstance(updated, dict): + updated = reply.get("updated_input") + # ch13 round-4 — read the user's chosen "don't ask again" rules + # back off the reply and return them so handle_permission_ask / + # the can_use_tool adapter PERSIST them (registry.py:169 → + # _apply_and_persist_updates → settings). This is what makes + # "always allow" actually stick. + chosen_raw = reply.get("chosen_updates") or reply.get("chosenUpdates") + chosen: tuple = () + if isinstance(chosen_raw, list): + deserialized = [ + _deserialize_permission_update(u) + for u in chosen_raw if isinstance(u, dict) + ] + chosen = tuple( + u for u in deserialized + if u is not None and self._permission_update_allowed(u) ) return PermissionAskReply( - behavior="deny", message=str(reply.get("message", "")) or "denied by user" + behavior="allow", + updated_input=updated if isinstance(updated, dict) else None, + chosen_updates=chosen, ) + def _may_persist_mode(self, mode: str) -> bool: + """Whether a wire request may write ``permissions.defaultMode`` to disk. + + The SINGLE policy for both doors into persistence: this control's + ``persist`` flag and a ``chosen_updates`` setMode with a non-session + destination. Persisting is a HOST-WIDE, durable change — the value is + read at every future launch, in every project, including headless — so + it is restricted to sessions with ``bypass_selectable``. + + The exact predicate, since it is broader than "an interactive launcher + owns this process": ``bypass_selectable`` is set by ``src/cli.py`` / + ``tui_launcher`` AND is False under a ``disableBypassPermissionsMode`` + lockdown or when running as root outside a sandbox. So in a locked-down + or elevated interactive session a level choice applies but does not + persist. That is an accidental coupling rather than a designed one, but + it fails safe — those sessions already floor at ``default``, so the only + thing lost is persisting a LOOSENING — and splitting a separate + settings-writable carrier is not worth the wire surface today. + + Without this, a ``--print-connect`` / ``--http`` client could answer with + ``{mode: "acceptEdits", persist: true}`` and silently overwrite a user + who had deliberately chosen "Ask for approval", or install a durable + ``dontAsk`` the user has to find in a settings file to undo. + + Only modes the READER accepts are written: ``EXTERNAL_PERMISSION_MODES`` + excludes ``auto``, which ``set_permission_mode`` otherwise allows — so + persisting it would clobber a real prior choice with a value + ``read_settings_default_mode`` silently ignores. + """ + from src.permissions.types import EXTERNAL_PERMISSION_MODES + + if mode not in EXTERNAL_PERMISSION_MODES: + logger.debug("[agent-server] refusing to persist non-external mode %r", mode) + return False + if not self.config.bypass_selectable: + logger.warning( + "[agent-server] refusing to persist permissions.defaultMode=%r — " + "this session was not launched by an interactive client", mode, + ) + return False + return True + + def _permission_update_allowed(self, update: Any) -> bool: + """Gate a permission update that arrived inside a permission-ask REPLY. + + ``chosen_updates`` is the "always allow Bash(ls:*)" channel, but it also + carries ``setMode`` (the plan-approval dialog's "Yes, and bypass + permissions" arm). That made it a SECOND door to ``bypassPermissions``, + bypassing the ``set_permission_mode`` gate entirely — a client could + answer any prompt with a setMode and take Full Access in a session where + ``bypass_selectable`` is False. The honest TUI never does this, but the + agent-server also serves ``--print-connect`` / ``--http`` clients, which + is exactly the population the selectability split exists to bound. + + Two rules for a wire-supplied ``setMode``: + + * ``bypassPermissions`` needs the same capability ``set_permission_mode`` + requires; + * a persisted destination needs :meth:`_may_persist_mode` — the same + policy the ``set_permission_mode`` ``persist`` flag goes through. + Writing ``permissions.defaultMode`` into the HOST's settings file makes + one message from a remote client a permanent host-wide setting, since + that value is read at every launch. + + Non-``setMode`` updates (rule grants) are unaffected. + """ + from src.permissions.types import PermissionUpdateSetMode + + if not isinstance(update, PermissionUpdateSetMode): + return True + if getattr(update, "destination", "session") != "session" and not ( + self._may_persist_mode(update.mode) + ): + logger.warning( + "[agent-server] refusing chosen_updates setMode with " + "destination=%r", getattr(update, "destination", None), + ) + return False + if update.mode != "bypassPermissions": + return True + pc = getattr(self.tool_context, "permission_context", None) + allowed = bool( + getattr(pc, "is_bypass_permissions_mode_available", False) + or self.config.bypass_selectable + ) + if not allowed: + logger.warning( + "[agent-server] refusing chosen_updates setMode:bypassPermissions " + "— Full Access is not available in this session", + ) + return allowed + # ─── /goal — completion-condition loop (src/goals) ───────────────────── def _goal_manager(self) -> Any: @@ -4506,7 +4652,16 @@ def _build_runtime(sess: _AgentSession, perm_mode: str | None) -> None: # single-session stdio case) and carried in. Availability alone does # NOT enter bypass; it only unlocks it for Shift+Tab / # set_permission_mode. Mirrors permissionSetup.ts:941-945. - bypass = mode == "bypassPermissions" or cfg.is_bypass_available + # + # NOT `or mode == "bypassPermissions"`: availability also relaxes PLAN + # mode (check.py `should_bypass`), so deriving it from the launch mode + # meant a session started in Full Access — now the interactive default — + # had /plan silently permitting every edit and command. Availability is + # flag/settings-derived only, matching TS isBypassPermissionsModeAvailable + # and the headless path (headless.py), which never had a mode-implies- + # availability rule. A session may still START in bypass without it: the + # engine's own `mode == "bypassPermissions"` clause covers that. + bypass = cfg.is_bypass_available perm_setup = setup_permissions( cwd=str(workspace_root), mode=mode, # type: ignore[arg-type] diff --git a/src/skills/bundled/update_config.py b/src/skills/bundled/update_config.py index eef56a0e..0360fed2 100644 --- a/src/skills/bundled/update_config.py +++ b/src/skills/bundled/update_config.py @@ -63,7 +63,8 @@ - `allow` / `deny` / `ask` are arrays of **rule strings** — the enforced permission set (read at startup). - Rule syntax: `Tool` alone (e.g. `Read`) matches all uses of that tool; `Tool(specifier)` scopes it. For `Bash`, `Bash(cmd:*)` is a prefix match (`Bash(git:*)` matches `git status`, `git commit`, …); `Bash(npm run test)` is exact. For file tools, `Edit(src/**)` / `Write(*.env)` match by path glob. - `additionalWorkingDirectories` (a **top-level** list of path strings) grants access to paths outside the workspace root — this is the key the port reads at startup (NOT a `permissions.additionalDirectories` key). -- The permission MODE is `default`, `plan`, `acceptEdits`, `bypassPermissions`, or `dontAsk`, but set it via the `--permission-mode` flag or the `/mode` command — a `defaultMode` key is not yet read back at startup, so writing it to settings.json has no effect today. +- `permissions.defaultMode` sets the mode a session STARTS in: `default`, `plan`, `acceptEdits`, `bypassPermissions`, or `dontAsk`. It IS read at startup (from `~/.clawcodex/settings.json`, `.clawcodex/settings{,.local}.json`, and managed policy), and it is what the `/permissions` command writes when the user picks a level. `--permission-mode` and `--dangerously-skip-permissions` are per-run overrides that outrank it without rewriting it. Interactive sessions with none of these set start in `bypassPermissions` (Full Access). +- A **committable** project `settings.json` may only TIGHTEN `defaultMode` — a value looser than what would otherwise apply is ignored, so a cloned repo cannot widen the reader's permissions. `settings.local.json` (gitignored) and the user file are honored in either direction. ## Environment variables (`.clawcodex/settings.json`) diff --git a/tests/server/test_bypass_permissions_wiring.py b/tests/server/test_bypass_permissions_wiring.py index 0baadce2..82eb4129 100644 --- a/tests/server/test_bypass_permissions_wiring.py +++ b/tests/server/test_bypass_permissions_wiring.py @@ -130,11 +130,46 @@ def test_default_session_has_no_bypass_availability(self) -> None: self.assertEqual(pc.mode, "default") self.assertFalse(pc.is_bypass_permissions_mode_available) - def test_launching_in_bypass_mode_implies_availability(self) -> None: + def test_launching_in_bypass_mode_does_not_imply_availability(self) -> None: + """Launching IN bypass must NOT set the engine availability flag. + + _build_runtime used to OR the launch mode into availability. That flag + also relaxes **plan mode** (``check.py`` ``should_bypass``: ``mode == + "plan" and is_bypass_permissions_mode_available``), so once Full Access + became the interactive default, every session's ``/plan`` silently + permitted every edit and command — the exact opposite of what plan mode + is for. + + Availability is now flag/settings-derived only (matching TS + ``isBypassPermissionsModeAvailable`` and the headless path, which never + had a mode-implies-availability rule). A session still bypasses while IN + bypass mode via the engine's own ``mode == "bypassPermissions"`` clause; + this test is the pin that stops the clause being re-added. + """ sess = self._build(permission_mode="bypassPermissions") pc = sess.tool_context.permission_context self.assertEqual(pc.mode, "bypassPermissions") - self.assertTrue(pc.is_bypass_permissions_mode_available) + self.assertFalse(pc.is_bypass_permissions_mode_available) + + def test_plan_mode_still_asks_in_an_implicit_full_access_session(self) -> None: + """The regression the availability split exists to prevent.""" + import dataclasses + + from src.permissions.check import has_permissions_to_use_tool + from src.tool_system.tools.write import WriteTool + + sess = self._build(permission_mode="bypassPermissions") + pc = sess.tool_context.permission_context + args = {"file_path": str(self.ws / "x.txt"), "content": "x"} + + # Full Access: the write is allowed outright. + self.assertEqual(has_permissions_to_use_tool(WriteTool, args, pc).behavior, "allow") + + # …but plan mode must restrain it rather than inherit the bypass, which + # is exactly what is_bypass_permissions_mode_available=True would have + # made it do (check.py `should_bypass`). + plan_pc = dataclasses.replace(pc, mode="plan") + self.assertNotEqual(has_permissions_to_use_tool(WriteTool, args, plan_pc).behavior, "allow") def test_allow_flag_grants_availability_without_entering_bypass(self) -> None: sess = self._build(is_bypass_available=True) @@ -195,6 +230,193 @@ def test_plain_modes_still_settable(self) -> None: self.assertIs(reply.get("ok"), True) self.assertEqual(sess.tool_context.permission_context.mode, "acceptEdits") + def test_selectable_allows_bypass_without_granting_engine_availability(self) -> None: + """`/permissions full` must work in an interactive session — but choosing + it sets the MODE only. Flipping engine availability too would relax plan + mode for the rest of the session.""" + sess = self._build(bypass_selectable=True) + reply = self._set_mode(sess, "bypassPermissions") + self.assertIs(reply.get("ok"), True) + pc = sess.tool_context.permission_context + self.assertEqual(pc.mode, "bypassPermissions") + self.assertFalse(pc.is_bypass_permissions_mode_available) + + def _set_mode_persist(self, sess, mode) -> dict: + asyncio.run(sess._handle_control_request({ + "request_id": f"r-persist-{mode}", + "request": { + "subtype": "set_permission_mode", "mode": mode, "persist": True, + }, + })) + return self._control_replies(sess)[-1] + + def test_persist_writes_default_mode_only_when_asked(self) -> None: + """Picking a level is a standing preference; Shift+Tab is not.""" + from src.permissions.modes import read_settings_default_mode + + # bypass_selectable marks a session an interactive launcher owns, which + # is what licenses a host-wide write. + sess = self._build(bypass_selectable=True) + + self._set_mode(sess, "acceptEdits") # no persist flag + self.assertIsNone(read_settings_default_mode(str(self.ws))) + + reply = self._set_mode_persist(sess, "default") + self.assertIs(reply.get("ok"), True) + self.assertIs(reply.get("persisted"), True) + self.assertEqual(read_settings_default_mode(str(self.ws)), "default") + + def test_persist_refused_for_a_session_no_interactive_client_owns(self) -> None: + """Persisting is HOST-WIDE and durable — it is read at every future + launch, in every project, including headless. A --print-connect / --http + client must not be able to overwrite a user who deliberately chose "Ask + for approval", or install a durable `dontAsk` they have to find in a + settings file to undo.""" + from src.permissions.modes import read_settings_default_mode + + sess = self._build() # bypass_selectable=False + + reply = self._set_mode_persist(sess, "acceptEdits") + self.assertIs(reply.get("ok"), True) # the MODE change still applies… + self.assertEqual(sess.tool_context.permission_context.mode, "acceptEdits") + self.assertIs(reply.get("persisted"), False) # …but nothing was written + self.assertIsNone(read_settings_default_mode(str(self.ws))) + + def test_persist_refuses_a_mode_the_reader_would_ignore(self) -> None: + """`auto` is settable but is not an EXTERNAL_PERMISSION_MODE, so + read_settings_default_mode drops it — persisting it would clobber a real + prior choice with a value nothing reads back.""" + from src.permissions.modes import read_settings_default_mode, set_settings_default_mode + + sess = self._build(bypass_selectable=True) + set_settings_default_mode("default") + + reply = self._set_mode_persist(sess, "auto") + self.assertIs(reply.get("persisted"), False) + self.assertEqual(read_settings_default_mode(str(self.ws)), "default") + + +class TestChosenUpdatesGate(_SessionHarness): + """`chosen_updates` is a SECOND door to the permission mode. + + It rides on a permission-ask REPLY (the plan dialog's "Yes, and bypass + permissions" arm), so it never passed through the `set_permission_mode` + gate. A client could answer any prompt with a `setMode` and take Full Access + in a session where it is not available — and with a persisted destination, + write `permissions.defaultMode` into the HOST's settings file, which is now + read at every launch. + """ + + def _reply_with_setmode(self, sess, mode, destination="session"): + return sess._permission_reply({ + "behavior": "allow", + "chosen_updates": [ + {"type": "setMode", "destination": destination, "mode": mode}, + ], + }) + + def test_wire_setmode_bypass_refused_without_the_capability(self) -> None: + sess = self._build() + self.assertEqual(self._reply_with_setmode(sess, "bypassPermissions").chosen_updates, ()) + + def test_wire_setmode_bypass_allowed_with_selectability(self) -> None: + sess = self._build(bypass_selectable=True) + chosen = self._reply_with_setmode(sess, "bypassPermissions").chosen_updates + self.assertEqual(len(chosen), 1) + self.assertEqual(chosen[0].mode, "bypassPermissions") + + def test_wire_setmode_to_a_persisted_destination_is_refused(self) -> None: + # A mode must not become a permanent host-wide setting because a client + # asked for it in a prompt reply. Same policy as the set_permission_mode + # `persist` flag — one `_may_persist_mode`, both doors. + sess = self._build() + self.assertEqual( + self._reply_with_setmode(sess, "acceptEdits", "userSettings").chosen_updates, (), + ) + + def test_rule_grants_are_untouched(self) -> None: + # The gate must not break the "always allow Bash(ls:*)" channel, which + # is what chosen_updates exists for. + sess = self._build() + reply = sess._permission_reply({ + "behavior": "allow", + "chosen_updates": [{ + "type": "addRules", + "destination": "session", + "behavior": "allow", + "rules": [{"toolName": "Bash", "ruleContent": "ls:*"}], + }], + }) + self.assertEqual(len(reply.chosen_updates), 1) + + +class TestShiftTabCycle(_SessionHarness): + def test_cycle_can_return_to_full_access_when_selectable(self) -> None: + """A session that DEFAULTS to Full Access leaves engine availability + False on purpose, so the cycle needs selectability too — otherwise the + first Shift+Tab was a one-way exit out of the default mode while the + footer kept advertising "(shift+tab to cycle)".""" + from src.permissions.cycle import get_next_permission_mode + + sess = self._build(permission_mode="bypassPermissions", bypass_selectable=True) + pc = sess.tool_context.permission_context + pc.mode = "plan" + self.assertEqual( + get_next_permission_mode(pc, can_select_bypass=sess.config.bypass_selectable), + "bypassPermissions", + ) + + def test_cycle_still_skips_bypass_without_either_capability(self) -> None: + from src.permissions.cycle import get_next_permission_mode + + sess = self._build() + pc = sess.tool_context.permission_context + pc.mode = "plan" + self.assertEqual( + get_next_permission_mode(pc, can_select_bypass=sess.config.bypass_selectable), + "default", + ) + + +class TestPlanApprovalBypassArm(_SessionHarness): + """Regression: `/plan` must not be a one-way exit out of Full Access. + + The ExitPlanMode approval box picks its elevated option from the wire's + `bypass_available` (prompts.tsx `planApprovalOptions`). When that read only + the ENGINE availability flag — which an implicit-full-access session + deliberately leaves False — the box offered only "Yes, auto-accept edits", + whose `chosen_updates` setMode pre-empts ExitPlanMode's `pre_plan_mode` + restore. Approving a plan silently downgraded the session with no notice. + """ + + def _exit_plan_wire(self, sess) -> dict: + from src.permissions.types import PermissionAskRequest + + sess.config.permission_timeout_s = 0.05 # no client will answer + sess.permission_handler( + PermissionAskRequest( + tool_name="ExitPlanMode", message="Exit plan mode?", tool_input={}, + ), + ) + for call in sess.loop.call_soon_threadsafe.call_args_list: + args = call.args + if ( + len(args) == 2 + and isinstance(args[1], dict) + and args[1].get("type") == "control_request" + and args[1]["request"].get("tool_name") == "ExitPlanMode" + ): + return args[1]["request"] + self.fail("no ExitPlanMode control_request was emitted") + + def test_selectable_session_is_offered_the_bypass_arm(self) -> None: + sess = self._build(permission_mode="bypassPermissions", bypass_selectable=True) + self.assertTrue(self._exit_plan_wire(sess)["bypass_available"]) + + def test_plain_session_is_not(self) -> None: + sess = self._build() + self.assertFalse(self._exit_plan_wire(sess)["bypass_available"]) + class TestAgentServerCliFlags(unittest.TestCase): """The subcommand flags land in AgentServerConfig before serving.""" @@ -242,6 +464,33 @@ def test_no_flags_no_availability(self) -> None: cfg = self._run([]) self.assertEqual(cfg.permission_mode, "default") self.assertFalse(cfg.is_bypass_available) + # The child is a CARRIER, not a resolver: no implicit full-access floor + # here. That is decided once at the interactive launch boundary and + # forwarded, so a directly-launched agent-server (the VS Code extension) + # is unaffected by the loose interactive default. + self.assertFalse(cfg.bypass_selectable) + + def test_allow_select_bypass_grants_selectability_only(self) -> None: + """`--allow-select-bypass` lets `/permissions` choose Full Access without + granting engine bypass availability — that one also relaxes plan mode.""" + cfg = self._run(["--allow-select-bypass"]) + self.assertTrue(cfg.bypass_selectable) + self.assertFalse(cfg.is_bypass_available) + self.assertEqual(cfg.permission_mode, "default") + + def test_availability_implies_selectability(self) -> None: + cfg = self._run(["--allow-dangerously-skip-permissions"]) + self.assertTrue(cfg.is_bypass_available) + self.assertTrue(cfg.bypass_selectable) + + def test_lockdown_revokes_selectability(self) -> None: + with patch( + "src.permissions.modes.is_bypass_permissions_mode_disabled", + return_value=True, + ): + cfg = self._run(["--allow-select-bypass"]) + self.assertFalse(cfg.bypass_selectable) + self.assertFalse(cfg.is_bypass_available) def test_no_flags_defaults_max_turns_to_shared_constant(self) -> None: # Pins the --max-turns CLI default to the same DEFAULT_MAX_TURNS the diff --git a/tests/server/test_tui_launcher.py b/tests/server/test_tui_launcher.py index 214e8243..c24a8382 100644 --- a/tests/server/test_tui_launcher.py +++ b/tests/server/test_tui_launcher.py @@ -74,6 +74,61 @@ class _NoEffort: assert "--effort" not in _agent_server_cmd(_NoEffort()) +def test_agent_server_cmd_forwards_bypass_selectable(): + """``--allow-select-bypass`` is what makes `/permissions full` work. + + Same class of bug as the dropped ``--effort`` hop: the launcher resolves it + and the SERVER enforces it, so a missing hop is invisible until a user picks + Full Access and is told it "is not available in this session". + + It must stay SEPARATE from ``--allow-dangerously-skip-permissions``: that + one grants engine bypass availability, which also relaxes plan mode. + """ + from src.entrypoints.tui_launcher import _agent_server_cmd + + class _Selectable: + permission_mode = "bypassPermissions" + provider = None + model = None + effort = None + is_bypass_available = False + bypass_selectable = True + + cmd = _agent_server_cmd(_Selectable()) + assert "--allow-select-bypass" in cmd + assert "--allow-dangerously-skip-permissions" not in cmd + + class _Neither: + permission_mode = "default" + provider = None + model = None + effort = None + + assert "--allow-select-bypass" not in _agent_server_cmd(_Neither()) + + +def test_print_connect_forwards_bypass_selectable(): + """The ``--print-connect`` route builds its own argv — the hop historically + missed (cf. the --effort test below).""" + from types import SimpleNamespace + from unittest.mock import patch + + seen: dict = {} + + with patch( + "src.entrypoints.tui_launcher.run_agent_server_subcommand", + lambda argv: seen.setdefault("argv", argv) and 0 or 0, + ): + from src.entrypoints.tui_launcher import _print_connect + + _print_connect(SimpleNamespace( + workspace=None, permission_mode="default", is_bypass_available=False, + bypass_selectable=True, provider=None, model=None, effort=None, + )) + + assert "--allow-select-bypass" in seen["argv"] + + def test_print_connect_forwards_effort(): """The ``--print-connect`` route runs the server in-process, so it needs the flag on its own argv list — a separate hop from _agent_server_cmd.""" diff --git a/tests/skills/test_update_config_skill.py b/tests/skills/test_update_config_skill.py index 06a27910..7dfacadc 100644 --- a/tests/skills/test_update_config_skill.py +++ b/tests/skills/test_update_config_skill.py @@ -9,6 +9,7 @@ from __future__ import annotations import json +import re import pytest @@ -79,12 +80,20 @@ def test_permissions_allow_deny_ask_strings(self): assert "additionalWorkingDirectories" in p assert '"additionalDirectories"' not in p # never as a JSON key - def test_default_mode_caveated_as_write_only(self): - # critic MA3: defaultMode is not read back at startup — must be caveated, - # not taught as a functional settings.json key. + def test_default_mode_is_taught_as_functional(self): + # The old caveat ("not yet read back at startup") became FALSE when + # `/permissions` started persisting the user's chosen level and the + # launchers started reading it. Teaching the stale caveat would send + # users to a flag when the settings key now works. p = UPDATE_CONFIG_PROMPT - assert "--permission-mode" in p and "/mode" in p - assert "not yet read back at startup" in p + assert "defaultMode" in p + assert "not yet read back at startup" not in p + assert "/permissions" in p + # The renamed command. Word-boundary: `/model` legitimately appears. + assert not re.search(r"/mode\b", p) + # The direction rule matters more than the key: a cloned repo must not + # be able to widen the reader's permissions. + assert "TIGHTEN" in p def test_permission_example_rules_parse_with_semantics(self): # critic N1: assert the SEMANTICS (the parser never returns None, so the diff --git a/tests/test_dangerous_skip_permissions.py b/tests/test_dangerous_skip_permissions.py index 3570bfda..b93e77e0 100644 --- a/tests/test_dangerous_skip_permissions.py +++ b/tests/test_dangerous_skip_permissions.py @@ -174,21 +174,64 @@ def test_has_allow_bypass_reads_global_tier(_tiered_configs): assert has_allow_bypass_permissions_mode() is True -def test_has_allow_bypass_reads_local_tier(_tiered_configs): +def test_has_allow_bypass_ignores_local_tier(_tiered_configs): + # SECURITY: the local tier is /.clawcodex/config.local.json — + # get_local_config_path is GIT-ROOT relative, not home relative, so it is + # exactly as committable as the project tier below. `.local` names a + # convention in your own .gitignore, not a trust boundary. + # + # This is a deliberate divergence from the TS reference, which does read the + # local tier: availability is not a minor capability. check.py's + # `should_bypass` is `mode == "bypassPermissions" or (mode == "plan" and + # is_bypass_permissions_mode_available)`, and the same disjunction in + # tool_system/context.py lifts working-root containment — so a committed + # file could turn `plan` into a write-anywhere bypass, with `clawcodex -p` + # never reaching the folder-trust gate. _, _, local_path = _tiered_configs _write_perms_config(local_path, True) - assert has_allow_bypass_permissions_mode() is True + assert has_allow_bypass_permissions_mode() is False def test_has_allow_bypass_ignores_project_tier(_tiered_configs): - # SECURITY: the committable /.claude/config.json must NOT be able - # to auto-enable bypass availability (parity with the TS projectSettings - # exclusion). Only the project tier is set here. + # SECURITY: the committable /.clawcodex/config.json must NOT be + # able to auto-enable bypass availability (parity with the TS + # projectSettings exclusion). Only the project tier is set here. _, project_path, _ = _tiered_configs _write_perms_config(project_path, True) assert has_allow_bypass_permissions_mode() is False +def test_a_repo_granted_availability_cannot_lift_plan_containment(_tiered_configs): + """The end-to-end consequence, through the real gate. + + Availability + `plan` is a full bypass, so a repo-writable source for it + would defeat the headline property that `/plan` restrains. + """ + from src.permissions.check import has_permissions_to_use_tool + from src.permissions.modes import resolve_interactive_permission_state + from src.permissions.types import ToolPermissionContext + from src.tool_system.tools.write import WriteTool + + _, project_path, local_path = _tiered_configs + _write_perms_config(local_path, True) + _write_perms_config(project_path, True) + + mode, available, _sel = resolve_interactive_permission_state( + permission_mode_cli="plan", + dangerously_skip_permissions=False, + allow_dangerously_skip_permissions=False, + implicit_full_access=False, + ) + assert available is False + ctx = ToolPermissionContext( + mode=mode, is_bypass_permissions_mode_available=available, + ) + decision = has_permissions_to_use_tool( + WriteTool, {"file_path": "/etc/evil.txt", "content": "x"}, ctx, + ) + assert decision.behavior != "allow" + + def test_has_allow_bypass_false_when_all_tiers_absent(_tiered_configs): assert has_allow_bypass_permissions_mode() is False @@ -432,15 +475,68 @@ def test_resolve_permission_state_stashes_resolved_mode_on_args(): assert args._resolved_is_bypass_available is True -def test_resolve_permission_state_default_mode_when_no_flag(): +def _as_tty(monkeypatch): + """Make the process look like it is attached to a terminal. + + The loose default is TTY-gated, and pytest runs with pipes — so without this + every "interactive" assertion would silently test the non-TTY path instead. + """ + import sys as _sys + + monkeypatch.setattr(_sys.stdin, "isatty", lambda: True, raising=False) + monkeypatch.setattr(_sys.stdout, "isatty", lambda: True, raising=False) + + +def test_resolve_permission_state_full_access_when_no_flag_interactive(monkeypatch): + """A bare interactive `clawcodex` starts in Full Access. + + This replaced the previous `default`: the permission UX was reworked so the + tool is loose by default and `/permissions` dials it back. Note what does + NOT change — engine bypass AVAILABILITY stays False, because that flag also + relaxes plan mode (`check.py` `should_bypass`) and `/plan` must keep + restraining even in a full-access session. + """ from src.cli import _build_parser, _resolve_permission_state + _as_tty(monkeypatch) parser = _build_parser() args = parser.parse_args([]) _resolve_permission_state(args) + assert args._resolved_permission_mode == "bypassPermissions" + assert args._resolved_is_bypass_available is False + # …but the picker can still offer Full Access, which is a separate capability. + assert args._resolved_bypass_selectable is True + + +def test_resolve_permission_state_non_tty_launch_is_not_loose(): + """"Not --print" is not the same as "a human is sitting there". + + A piped/automated launch that merely omits -p must not take the loose floor + — `_gate_folder_trust` grants trust implicitly on non-TTY stdin, so nothing + downstream would stop it either. pytest's own pipes make this the default + state here, which is exactly the case being pinned. + """ + from src.cli import _build_parser, _resolve_permission_state + + parser = _build_parser() + args = parser.parse_args([]) + _resolve_permission_state(args) + assert args._resolved_permission_mode == "default" + assert args._resolved_bypass_selectable is False + + +def test_resolve_permission_state_print_mode_keeps_default(): + """`--print` / headless is deliberately excluded from the loose default: + CI and the Harbor eval harness drive it, and silently changing what a + benchmark run is permitted to do is not acceptable.""" + from src.cli import _build_parser, _resolve_permission_state + + parser = _build_parser() + args = parser.parse_args(["--print", "hello"]) + _resolve_permission_state(args) assert args._resolved_permission_mode == "default" - # is_bypass_available depends on settings; default config has no bypass. - assert isinstance(args._resolved_is_bypass_available, bool) + assert args._resolved_is_bypass_available is False + assert args._resolved_bypass_selectable is False def test_resolve_permission_state_allow_dangerously_only_does_not_flip_mode(): diff --git a/tests/test_permission_levels.py b/tests/test_permission_levels.py new file mode 100644 index 00000000..e2cfab88 --- /dev/null +++ b/tests/test_permission_levels.py @@ -0,0 +1,419 @@ +"""The three `/permissions` levels, and the launch resolution behind them. + +Covers the contract added when `/mode` became `/permissions` and interactive +launches started defaulting to Full Access: + +* the level catalog and its TypeScript mirror, +* `resolve_interactive_permission_state` — precedence, and the suppressors that + must be able to override the loose default, +* `permissions.defaultMode` read/write against the canonical settings store. +""" + +from __future__ import annotations + +import json +import re +from pathlib import Path + +import pytest + +from src.permissions.levels import ( + PERMISSION_LEVEL_KEYS, + PERMISSION_LEVELS, + level_for_key, + level_for_mode, + mode_for_key, +) +from src.permissions.modes import ( + read_settings_default_mode, + resolve_interactive_permission_state, + set_settings_default_mode, +) +from src.permissions.types import PERMISSION_MODES + +_TS_MIRROR = Path(__file__).resolve().parents[1] / "ui-tui" / "src" / "lib" / "permissionLevels.ts" + + +# --------------------------------------------------------------------------- # +# The catalog +# --------------------------------------------------------------------------- # +class TestLevelCatalog: + def test_three_levels_loosest_last(self): + assert [lv.key for lv in PERMISSION_LEVELS] == ["ask", "approve", "full"] + assert [lv.mode for lv in PERMISSION_LEVELS] == [ + "default", + "acceptEdits", + "bypassPermissions", + ] + + def test_keys_never_collide_with_engine_mode_names(self): + """`/permissions ` takes a level key OR a raw mode; the two + namespaces must stay disjoint or the arg is ambiguous. In particular the + middle level is `approve`, not `auto` — `auto` is a real mode.""" + assert not (set(PERMISSION_LEVEL_KEYS) & set(PERMISSION_MODES)) + + def test_off_ladder_modes_have_no_level(self): + # plan / dontAsk / auto are real modes with no picker row. Callers must + # get None so they render "no level" rather than mislabeling them. + for mode in ("plan", "dontAsk", "auto", "bubble", "nonsense"): + assert level_for_mode(mode) is None + + def test_lookup_helpers(self): + assert mode_for_key("FULL") == "bypassPermissions" + assert mode_for_key(" ask ") == "default" + assert mode_for_key("plan") is None + assert level_for_key("approve").label == "Approve for me" + assert level_for_mode("acceptEdits").key == "approve" + + def test_typescript_mirror_matches(self): + """`ui-tui/src/lib/permissionLevels.ts` is a hand-kept mirror; the picker + and every backend consumer must describe the levels identically. A drift + here is a picker that misrepresents what it is about to permit.""" + block = ( + _TS_MIRROR.read_text(encoding="utf-8") + .split("export const PERMISSION_LEVELS", 1)[1] + .split("\n]", 1)[0] + ) + # Prettier wraps long string literals onto their own line, so compare + # against a whitespace-normalized source rather than a shape-sensitive + # per-object regex. + flat = " ".join(block.split()) + + assert len(re.findall(r"key: '", flat)) == len(PERMISSION_LEVELS) + assert [m for m in re.findall(r"key: '([^']+)'", flat)] == [ + lv.key for lv in PERMISSION_LEVELS + ], "level ORDER must match — the picker numbers rows 1-3 from it" + + for lv in PERMISSION_LEVELS: + assert f"key: '{lv.key}'" in flat + assert f"label: '{lv.label}'" in flat + assert f"mode: '{lv.mode}'" in flat + assert " ".join(lv.description.split()) in flat, f"description drift: {lv.key}" + + +# --------------------------------------------------------------------------- # +# Launch resolution +# --------------------------------------------------------------------------- # +@pytest.fixture +def settings_home(tmp_path, monkeypatch): + """`(user_settings_file, work_dir)` for the four-tier settings store. + + The user tier is resolved through :mod:`src.permissions.settings_paths` + rather than assumed to be ``~/.clawcodex/settings.json``: conftest's autouse + isolation already redirects that function so tests never read the + developer's real settings, and hardcoding the home path here would write to + a file nothing reads. + """ + from src.permissions import settings_paths + + work = tmp_path / "work" + work.mkdir() + monkeypatch.chdir(work) + return Path(settings_paths.user_settings_path()), work + + +def _write_settings(path: Path, **permissions): + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps({"permissions": permissions}), encoding="utf-8") + + +def _resolve(**kw): + kw.setdefault("permission_mode_cli", None) + kw.setdefault("dangerously_skip_permissions", False) + kw.setdefault("allow_dangerously_skip_permissions", False) + return resolve_interactive_permission_state(**kw) + + +def _tool_ctx(mode, available): + from src.permissions.types import ToolPermissionContext + + return ToolPermissionContext( + mode=mode, is_bypass_permissions_mode_available=available, + ) + + +class TestInteractiveDefault: + def test_bare_interactive_launch_is_full_access(self, settings_home): + mode, available, selectable = _resolve() + assert mode == "bypassPermissions" + assert selectable is True + # …but NOT engine availability: that flag also relaxes plan mode, so + # setting it here would make /plan permit everything. + assert available is False + + def test_explicit_permission_mode_default_beats_the_loose_floor(self, settings_home): + # "flag absent" vs "explicitly asked for prompts" must be distinguishable. + mode, _available, _sel = _resolve(permission_mode_cli="default") + assert mode == "default" + + def test_dangerously_skip_permissions_is_unchanged(self, settings_home): + mode, available, selectable = _resolve(dangerously_skip_permissions=True) + assert (mode, available, selectable) == ("bypassPermissions", True, True) + + def test_allow_flag_grants_availability_without_entering_bypass(self, settings_home): + mode, available, selectable = _resolve( + permission_mode_cli="default", allow_dangerously_skip_permissions=True + ) + assert (mode, available, selectable) == ("default", True, True) + + def test_allow_flag_alone_suppresses_the_loose_floor(self, settings_home): + """`--allow-dangerously-skip-permissions` says "available WITHOUT + starting in it". Letting the implicit floor start the session in Full + Access anyway would make the flag do the opposite of what it says.""" + mode, available, selectable = _resolve(allow_dangerously_skip_permissions=True) + assert (mode, available, selectable) == ("default", True, True) + + def test_non_interactive_keeps_default(self, settings_home): + """`--print` / headless must not inherit the loose default: CI and the + Harbor eval harness drive it.""" + mode, available, selectable = _resolve(implicit_full_access=False) + assert (mode, available, selectable) == ("default", False, False) + + +class TestSuppressors: + def test_operator_lockdown_suppresses_the_implicit_default(self, settings_home, monkeypatch): + monkeypatch.setattr( + "src.permissions.modes.is_bypass_permissions_mode_disabled", lambda: True + ) + mode, available, selectable = _resolve() + assert mode == "default" + assert available is False + assert selectable is False + + def test_lockdown_also_overrides_an_explicit_flag(self, settings_home, monkeypatch): + monkeypatch.setattr( + "src.permissions.modes.is_bypass_permissions_mode_disabled", lambda: True + ) + mode, available, _sel = _resolve(dangerously_skip_permissions=True) + assert mode == "default" + assert available is False + + def test_root_outside_sandbox_degrades_instead_of_exiting(self, settings_home, monkeypatch): + """`sudo clawcodex` with no flags must still START — just not wide open. + + The explicit-flag path keeps its SystemExit + (`enforce_dangerous_skip_permissions_safety`); an IMPLICIT default has + to degrade, or elevating would break the app entirely. + """ + monkeypatch.setattr("src.permissions.modes.is_elevated_without_sandbox", lambda: True) + mode, _available, selectable = _resolve() # must not raise SystemExit + assert mode == "default" + assert selectable is False + + def test_root_clamps_a_persisted_full_access_choice(self, settings_home, monkeypatch): + """The suppressor has to clamp the RESOLVED mode, not just the floor. + + `/permissions full` writes `defaultMode: bypassPermissions`, which is a + settings candidate that outranks the floor — so guarding only the floor + left `sudo clawcodex` running root-full-access silently. + """ + user_settings, _work = settings_home + _write_settings(user_settings, defaultMode="bypassPermissions") + monkeypatch.setattr("src.permissions.modes.is_elevated_without_sandbox", lambda: True) + mode, available, selectable = _resolve() + assert (mode, available, selectable) == ("default", False, False) + + def test_root_does_not_discard_an_explicit_permission_mode(self, settings_home, monkeypatch): + """The clamp must not silently rewrite a flag the user typed. + + `--permission-mode bypassPermissions` does NOT go through + `enforce_dangerous_skip_permissions_safety` (that guard keys off the two + `*-skip-permissions` flags), so it reaches the clamp intact — and + `docker run … clawcodex --permission-mode bypassPermissions -p …` as root + is a routine invocation. Rewriting it to `default` would flip those runs + from everything-allowed to everything-DENIED (headless denies every + ask), announced only by a log line. + """ + monkeypatch.setattr("src.permissions.modes.is_elevated_without_sandbox", lambda: True) + mode, available, selectable = _resolve( + permission_mode_cli="bypassPermissions", implicit_full_access=False + ) + assert mode == "bypassPermissions" + # Availability is still dropped: under root it can only come from + # settings, and it is what lifts containment in plan mode. + assert (available, selectable) == (False, False) + + def test_root_clamps_settings_granted_availability(self, settings_home, monkeypatch): + """Availability alone is enough for a full bypass via plan mode + (`check.py` `should_bypass`), so root must drop it too. The explicit + flags never get here — the caller exits on those first.""" + monkeypatch.setattr("src.permissions.modes.is_elevated_without_sandbox", lambda: True) + monkeypatch.setattr( + "src.permissions.modes.has_allow_bypass_permissions_mode", lambda: True + ) + _mode, available, selectable = _resolve() + assert available is False + assert selectable is False + + +class TestPersistedDefaultMode: + def test_persisted_choice_beats_the_implicit_default(self, settings_home): + user_settings, _work = settings_home + _write_settings(user_settings, defaultMode="default") + mode, _available, selectable = _resolve() + assert mode == "default" + # …and Full Access stays SELECTABLE so /permissions can go back. + assert selectable is True + + def test_explicit_flags_beat_a_persisted_choice_without_overwriting_it(self, settings_home): + path, _work = settings_home + _write_settings(path, defaultMode="default") + mode, _a, _s = _resolve(dangerously_skip_permissions=True) + assert mode == "bypassPermissions" + # A per-run override must not rewrite the standing preference. + assert json.loads(path.read_text())["permissions"]["defaultMode"] == "default" + + def test_round_trip_through_the_public_writer(self, settings_home): + assert set_settings_default_mode("acceptEdits") is True + assert read_settings_default_mode() == "acceptEdits" + + def test_persisted_choice_applies_to_headless_too(self, settings_home): + """An explicit user choice must not be honored interactively and ignored + by `clawcodex -p`; only the implicit FLOOR is interactive-only.""" + user_settings, _work = settings_home + _write_settings(user_settings, defaultMode="acceptEdits") + mode, _a, _s = _resolve(implicit_full_access=False) + assert mode == "acceptEdits" + + def test_a_persisted_full_access_does_not_arm_headless(self, settings_home): + """…with one exception, in the safe direction. + + Persistence exists so dialing DOWN survives a relaunch. Letting it dial + headless UP would mean one `/permissions full` click silently converts + every future `clawcodex -p`, in every directory, into a full bypass — + and `-p` returns before the folder-trust gate, so nothing would ask. + That is the outcome the interactive-only floor exists to prevent. + """ + user_settings, _work = settings_home + _write_settings(user_settings, defaultMode="bypassPermissions") + mode, _a, _s = _resolve(implicit_full_access=False) + assert mode == "default" + # Saying otherwise is still possible, explicitly. + assert _resolve( + dangerously_skip_permissions=True, implicit_full_access=False + )[0] == "bypassPermissions" + assert _resolve( + permission_mode_cli="bypassPermissions", implicit_full_access=False + )[0] == "bypassPermissions" + + def test_a_persisted_full_access_still_applies_interactively(self, settings_home): + user_settings, _work = settings_home + _write_settings(user_settings, defaultMode="bypassPermissions") + assert _resolve()[0] == "bypassPermissions" + + def test_unknown_or_malformed_values_are_ignored(self, settings_home): + path, _work = settings_home + _write_settings(path, defaultMode="nonsense") + assert read_settings_default_mode() is None + path.write_text("{not json", encoding="utf-8") + assert read_settings_default_mode() is None + + +class TestSettingsTierPrecedence: + def test_user_tier_beats_a_repo_file_trying_to_loosen(self, settings_home): + """The picker writes the USER tier, so a committable repo file must not + silently defeat the user's explicit choice by loosening it.""" + user_settings, work = settings_home + _write_settings(user_settings, defaultMode="default") + _write_settings(work / ".clawcodex" / "settings.json", defaultMode="bypassPermissions") + assert read_settings_default_mode(str(work)) == "default" + + def test_a_repo_may_still_tighten_over_the_user_tier(self, settings_home): + """The other direction is the point of honoring the project tier at all: + with Full Access as the default, "don't run wide open in this repo" is + the most valuable thing a project can declare.""" + user_settings, work = settings_home + _write_settings(user_settings, defaultMode="acceptEdits") + _write_settings(work / ".clawcodex" / "settings.json", defaultMode="default") + assert read_settings_default_mode(str(work)) == "default" + + def test_project_tier_may_tighten(self, settings_home): + _home, work = settings_home + _write_settings(work / ".clawcodex" / "settings.json", defaultMode="default") + # Nothing in the trusted tiers; the interactive floor is Full Access, so + # a repo asking for prompts is a tightening and is honored. + assert read_settings_default_mode(str(work), baseline_mode="bypassPermissions") == "default" + + def test_project_tier_may_not_loosen(self, settings_home): + _home, work = settings_home + _write_settings(work / ".clawcodex" / "settings.json", defaultMode="bypassPermissions") + assert read_settings_default_mode(str(work), baseline_mode="default") is None + + def test_project_tier_may_not_loosen_via_accept_edits_either(self, settings_home): + """The guard is a direction rule, not a `bypassPermissions` blocklist: + acceptEdits also loosens `default` (auto-accepted edits + write commands) + for a user who deliberately chose "Ask for approval".""" + user_settings, work = settings_home + _write_settings(user_settings, defaultMode="default") + _write_settings(work / ".clawcodex" / "settings.json", defaultMode="acceptEdits") + assert read_settings_default_mode(str(work)) == "default" + + def test_local_tier_is_repo_scoped_and_cannot_loosen(self, settings_home): + """`settings.local.json` is NOT trusted. + + The `.local` suffix conventionally means "gitignored, operator-owned", + but that is a property of *your* `.gitignore` — a hostile repo simply + commits the file. Treating it as trusted let `git clone && clawcodex -p` + run with the permission gate off, and `-p` returns before the + folder-trust gate, so nothing would have prompted. + """ + _home, work = settings_home + _write_settings(work / ".clawcodex" / "settings.local.json", defaultMode="bypassPermissions") + assert read_settings_default_mode(str(work), baseline_mode="default") is None + + def test_local_tier_may_tighten_and_beats_project(self, settings_home): + _home, work = settings_home + _write_settings(work / ".clawcodex" / "settings.local.json", defaultMode="default") + _write_settings(work / ".clawcodex" / "settings.json", defaultMode="dontAsk") + assert read_settings_default_mode(str(work), baseline_mode="bypassPermissions") == "default" + + def test_a_repo_may_not_supply_plan(self, settings_home): + """`plan` looks like the strictest mode and is not. + + `check.py`'s `should_bypass` is `mode == "bypassPermissions" or (mode == + "plan" and is_bypass_permissions_mode_available)`, and the same + disjunction in `tool_system/context.py` lifts working-root containment. + So in any session with bypass availability, a repo shipping + `defaultMode: "plan"` would get unconstrained write-anywhere — which is + why the repo tier is an ALLOWLIST, not a looseness ranking. + """ + _home, work = settings_home + _write_settings(work / ".clawcodex" / "settings.json", defaultMode="plan") + assert read_settings_default_mode(str(work), baseline_mode="bypassPermissions") is None + + def test_a_repo_supplied_plan_cannot_reach_write_anywhere(self, settings_home): + """The end-to-end version of the above, through the real gate.""" + import dataclasses + + from src.permissions.check import has_permissions_to_use_tool + from src.permissions.types import ToolPermissionContext + from src.tool_system.tools.write import WriteTool + + _home, work = settings_home + _write_settings(work / ".clawcodex" / "settings.json", defaultMode="plan") + + mode, available, _sel = _resolve( + allow_dangerously_skip_permissions=True, cwd=str(work) + ) + ctx = ToolPermissionContext( + mode=mode, is_bypass_permissions_mode_available=available, + ) + decision = has_permissions_to_use_tool( + WriteTool, {"file_path": "/etc/evil.txt", "content": "x"}, ctx, + ) + assert decision.behavior != "allow", ( + f"a repo-supplied defaultMode reached write-anywhere (mode={mode!r}, " + f"available={available})" + ) + # Sanity: the same context WOULD allow it if plan had been honored, so + # the assertion above is not passing for an unrelated reason. + plan_ctx = dataclasses.replace(ctx, mode="plan") + assert has_permissions_to_use_tool( + WriteTool, {"file_path": "/etc/evil.txt", "content": "x"}, plan_ctx, + ).behavior == "allow" + + def test_a_repo_may_still_force_dont_ask(self, settings_home): + # Restrictive, so allowed — a repo-side annoyance, not an escalation. + _home, work = settings_home + _write_settings(work / ".clawcodex" / "settings.json", defaultMode="dontAsk") + assert read_settings_default_mode(str(work), baseline_mode="bypassPermissions") == "dontAsk" diff --git a/ui-tui/src/__tests__/brandingPermsRow.test.ts b/ui-tui/src/__tests__/brandingPermsRow.test.ts new file mode 100644 index 00000000..358c13ce --- /dev/null +++ b/ui-tui/src/__tests__/brandingPermsRow.test.ts @@ -0,0 +1,127 @@ +import { PassThrough } from 'stream' + +import { renderSync } from '@clawcodex/ink' +import React from 'react' +import { afterEach, describe, expect, it } from 'vitest' + +import { SessionPanel } from '../components/branding.js' +import { stripAnsi } from '../lib/text.js' +import { DEFAULT_THEME } from '../theme.js' +import type { SessionInfo } from '../types.js' + +/** + * The startup header's permission row. + * + * It matters more than an ordinary identity row: Full Access is the default, and + * the live indicator (the composer badge) hides whenever the user is typing, so + * this row is the one unmissable disclosure of what the session may do. It also + * names the command that changes it. + * + * SessionPanel reads its width from useStdout(), which is hardcoded to + * process.stdout — pin that, not the PassThrough (see brandingRenderWidth's + * note; assigning `columns` to the stream leaves the component on its `?? 100` + * fallback and silently tests one layout at every width). + */ +const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)) + +const originalColumns = Object.getOwnPropertyDescriptor(process.stdout, 'columns') + +afterEach(() => { + if (originalColumns) { + Object.defineProperty(process.stdout, 'columns', originalColumns) + + return + } + + delete (process.stdout as { columns?: number }).columns +}) + +const info = (permission_mode?: string): SessionInfo => ({ + cwd: '/Users/dev/workspace/proj', + model: 'anthropic/claude-opus-5', + permission_mode, + skills: {}, + tools: {}, + version: '1.3.0' +}) + +async function render(permission_mode: string | undefined, cols = 100): Promise { + Object.defineProperty(process.stdout, 'columns', { configurable: true, value: cols, writable: true }) + + const stdout = new PassThrough() + const stdin = new PassThrough() + const stderr = new PassThrough() + + Object.assign(stdout, { columns: cols, isTTY: false, rows: 60 }) + Object.assign(stdin, { isTTY: false }) + + let captured = '' + + stdout.on('data', chunk => { + captured += chunk.toString() + }) + + const instance = renderSync( + React.createElement(SessionPanel, { info: info(permission_mode), sid: 'a1b2c3d4', t: DEFAULT_THEME }), + { + patchConsole: false, + stderr: stderr as NodeJS.WriteStream, + stdin: stdin as NodeJS.ReadStream, + stdout: stdout as NodeJS.WriteStream + } + ) + + try { + await delay(30) + + return stripAnsi(captured) + } finally { + instance.unmount() + } +} + +describe('SessionPanel — permission row', () => { + it('names the level and the command that changes it', async () => { + const out = await render('bypassPermissions') + + expect(out).toContain('Full Access') + expect(out).toContain('/permissions') + }) + + it('is not truncated at the default width', async () => { + // The row is in optimalLeftWidth's sizing set precisely because it isn't + // covered by the other rows — sized off them it rendered "…/permission…", + // which is the row that must stay legible now that Full Access is default. + const out = await render('bypassPermissions') + + expect(out).toContain('Full Access · /permissions') + expect(out).not.toContain('/permission…') + }) + + it('renders the other levels by their picker labels', async () => { + expect(await render('default')).toContain('Ask for approval') + expect(await render('acceptEdits')).toContain('Approve for me') + }) + + it('shows an off-ladder mode under its raw name, not a level label', async () => { + // plan / dontAsk are real modes the picker deliberately does not list. + // They still render — labeling them as one of the three would be a lie, + // but omitting them hides them entirely. + const out = await render('plan') + + expect(out).toContain('plan') + expect(out).not.toContain('Full Access') + expect(out).not.toContain('Ask for approval') + }) + + it('surfaces a repo-forced dontAsk, which denies everything that would prompt', async () => { + // A repository settings file may set this. The composer badge is the only + // other indicator and it hides while the user types, so without this row a + // user whose agent refuses every action has nothing on screen to trace it to. + expect(await render('dontAsk')).toContain('dontAsk') + }) + + it('omits the row when the backend reported no mode', async () => { + expect(await render(undefined)).not.toContain('Perms') + }) +}) diff --git a/ui-tui/src/__tests__/composerFooter.test.ts b/ui-tui/src/__tests__/composerFooter.test.ts index d9a81535..be2a118a 100644 --- a/ui-tui/src/__tests__/composerFooter.test.ts +++ b/ui-tui/src/__tests__/composerFooter.test.ts @@ -10,7 +10,8 @@ vi.hoisted(() => { delete process.env.NO_COLOR }) -import { ComposerFooter } from '../components/composerFooter.js' +import { ComposerFooter, MODE_BADGES } from '../components/composerFooter.js' +import { PERMISSION_LEVELS } from '../lib/permissionLevels.js' import { stripAnsi } from '../lib/text.js' import { DEFAULT_THEME } from '../theme.js' @@ -76,17 +77,36 @@ describe('ComposerFooter', () => { // hint coexists with the badge expect(stripAnsi(plan)).toContain('? for shortcuts') + // Labels track the /permissions level vocabulary (lib/permissionLevels.ts) + // so the badge and the picker never name the same mode two ways. const accept = footer({ mode: 'acceptEdits' }) - expect(stripAnsi(accept)).toContain('▶▶ accept edits on') + expect(stripAnsi(accept)).toContain('▶▶ approve for me on') expect(accept).toContain('175;135;255') // autoAccept violet const bypass = footer({ mode: 'bypassPermissions' }) - expect(stripAnsi(bypass)).toContain('▶▶ bypass permissions on') + expect(stripAnsi(bypass)).toContain('▶▶ full access on') expect(bypass).toContain('255;107;128') // error red }) + it('names badges with the same labels the /permissions picker shows', () => { + // A drifting label here is how a user ends up unable to tell which level + // they picked, given full access is the default. + // + // `default` deliberately has no badge (CC parity: the base mode is the + // absence of one), so it is named explicitly rather than skipped by an + // `if (badge)` guard — that guard would make this test vacuous the moment + // someone deleted a badge. + expect(Object.keys(MODE_BADGES).sort()).toContain('acceptEdits') + expect(Object.keys(MODE_BADGES).sort()).toContain('bypassPermissions') + expect(MODE_BADGES.default).toBeUndefined() + + for (const level of PERMISSION_LEVELS.filter(l => l.mode !== 'default')) { + expect(MODE_BADGES[level.mode]!.label).toBe(`${level.label.toLowerCase()} on`) + } + }) + it('shows bash-mode hint in pink when the input is in ! mode', () => { const out = footer({ sh: true }) diff --git a/ui-tui/src/__tests__/gatewayClient.test.ts b/ui-tui/src/__tests__/gatewayClient.test.ts index 95417390..4afa055e 100644 --- a/ui-tui/src/__tests__/gatewayClient.test.ts +++ b/ui-tui/src/__tests__/gatewayClient.test.ts @@ -296,28 +296,35 @@ describe('GatewayClient NDJSON adapter', () => { expect(last('session.stats')).toBeUndefined() }) - it('confirms the applied mode from the server reply on /mode', async () => { - const p = gw.request('slash.exec', { command: 'mode acceptEdits' }) - await replyToControl('set_permission_mode', { mode: 'acceptEdits', ok: true }) - await expect(p).resolves.toEqual({ output: 'Permission mode: acceptEdits.', type: 'exec' }) - }) - - it('treats bare /mode as a no-op query, not an empty set', async () => { - // No arg → must NOT send an empty mode the server would reject; report - // unchanged instead (the pre-hardening behavior). - const p = gw.request('slash.exec', { command: 'mode' }) - const r: any = await p - expect(r).toEqual({ output: 'Permission mode: (unchanged).', type: 'exec' }) - // And it must not have hit the backend at all. - expect(stdinFrames().some(f => f.request?.subtype === 'set_permission_mode')).toBe(false) - }) + // `/mode` is gone — `/permissions` replaced it as a LOCAL slash command + // (app/slash/commands/session.ts), so dispatchSlash no longer has a case for + // either name. What remains on the gateway is the RPC that command uses. it('reflects the server rejection through config.set permission_mode', async () => { - // The settings-panel write path must not report success when the server - // refuses (bypassPermissions gated on availability). + // The write path must not report success when the server refuses (Full + // Access is gated on selectability), and must hand back the REASON so + // /permissions can print it rather than a generic failure. const p = gw.request('config.set', { key: 'permission_mode', value: 'bypassPermissions' }) await replyToControl('set_permission_mode', { error: 'not available', ok: false }) - await expect(p).resolves.toEqual({ ok: false }) + await expect(p).resolves.toMatchObject({ error: 'not available', ok: false }) + }) + + it('returns the applied mode from config.set permission_mode', async () => { + // /permissions badges `mode` from the reply, not the arg it sent. + const p = gw.request('config.set', { key: 'permission_mode', value: 'acceptEdits' }) + await replyToControl('set_permission_mode', { mode: 'acceptEdits', ok: true, persisted: true }) + await expect(p).resolves.toMatchObject({ mode: 'acceptEdits', ok: true, persisted: true }) + }) + + it('forwards the persist flag on config.set permission_mode', async () => { + // persist is what makes a deliberate down-shift to "Ask for approval" + // survive relaunch; it must reach the control request, not be dropped. + void gw.request('config.set', { key: 'permission_mode', persist: true, value: 'default' }) + await vi.waitFor(() => { + const f = stdinFrames().find(x => x.request?.subtype === 'set_permission_mode') + + expect(f?.request?.persist).toBe(true) + }) }) it('routes config.set logoColor to the set_logo_color control and echoes the value', async () => { @@ -388,21 +395,6 @@ describe('GatewayClient NDJSON adapter', () => { await expect(p).rejects.toThrow("model 'm' expects provider 'other' but this session is on 'deepseek'") }) - it('surfaces the server rejection when /mode bypassPermissions is unavailable', async () => { - // The server gates bypassPermissions on availability (same guard as the - // Shift+Tab cycle) — the client must show the refusal, not pretend the - // mode changed. - const p = gw.request('slash.exec', { command: 'mode bypassPermissions' }) - await replyToControl('set_permission_mode', { - error: 'bypassPermissions is not available in this session', - ok: false - }) - const r: any = await p - expect(r.type).toBe('exec') - expect(r.output).toContain('not available') - expect(r.output).not.toContain('Permission mode:') - }) - it('dispatches an unknown slash as a backend workflow command (send)', async () => { const p = gw.request('slash.exec', { command: 'deep-research what is love' }) await replyToControl('workflow_command', { @@ -538,12 +530,12 @@ describe('GatewayClient NDJSON adapter', () => { ok: true }) const r = await p - expect(r.hints['/mode']).toBe('[default|plan|acceptEdits|dontAsk|bypassPermissions]') expect(r.hints['/deep-research']).toBe('') // Names shadowed by TUI-local commands carry no gateway hint — the local // registry's argumentHint is the truthful one (dispatch order). expect(r.hints['/compact']).toBeUndefined() expect(r.hints['/model']).toBeUndefined() + expect(r.hints['/permissions']).toBeUndefined() }) it('skills.manage list groups backend skills by category', async () => { diff --git a/ui-tui/src/__tests__/permissionsCommand.test.ts b/ui-tui/src/__tests__/permissionsCommand.test.ts new file mode 100644 index 00000000..6777fd0b --- /dev/null +++ b/ui-tui/src/__tests__/permissionsCommand.test.ts @@ -0,0 +1,129 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { getOverlayState, resetOverlayState } from '../app/overlayStore.js' +import { findSlashCommand } from '../app/slash/registry.js' +import type { SlashRunCtx } from '../app/slash/types.js' +import { getUiState, patchUiState, resetUiState } from '../app/uiStore.js' +import { PERMISSION_LEVELS } from '../lib/permissionLevels.js' + +/** Minimal SlashRunCtx: transcript + gateway.rpc + the guarded() wrappers + * createSlashHandler builds per invocation (the /logo test's shape). */ +const makeCtx = (rpcResult: unknown) => { + const sys = vi.fn() + const rpc = vi.fn().mockResolvedValue(rpcResult) + + const ctx = { + gateway: { rpc }, + guarded: + (fn: (r: T) => void) => + (r: null | T) => { + if (r) { + fn(r) + } + }, + guardedErr: vi.fn(), + transcript: { sys } + } as unknown as SlashRunCtx + + return { ctx, rpc, sys } +} + +const cmd = findSlashCommand('permissions')! + +describe('/permissions TUI-local command', () => { + beforeEach(() => { + resetOverlayState() + resetUiState() + }) + + it('is registered with the level grammar', () => { + expect(cmd).toBeDefined() + expect(cmd.argumentHint).toBe('[ask|approve|full]') + }) + + it('still answers to the old /mode name', () => { + // The command was renamed; muscle memory and older docs should not 404. + expect(findSlashCommand('mode')).toBe(cmd) + }) + + it('bare /permissions opens the picker overlay', () => { + const { ctx, rpc } = makeCtx({ ok: true }) + cmd.run('', ctx, '/permissions') + + expect(getOverlayState().permissionsPicker).toBe(true) + expect(rpc).not.toHaveBeenCalled() + }) + + it('maps each level key to its engine mode and persists it', async () => { + for (const level of PERMISSION_LEVELS) { + const { ctx, rpc, sys } = makeCtx({ mode: level.mode, ok: true, persisted: true }) + + cmd.run(level.key, ctx, `/permissions ${level.key}`) + + expect(rpc).toHaveBeenCalledWith('config.set', { + key: 'permission_mode', + // Choosing a level is a standing preference — it must survive relaunch, + // or a user who deliberately dials DOWN is silently returned to Full + // Access next launch. + persist: true, + value: level.mode + }) + await vi.waitFor(() => expect(sys).toHaveBeenCalledWith(`Permissions: ${level.label}.`)) + expect(getUiState().permissionMode).toBe(level.mode) + } + }) + + it('persists a raw mode that IS a level, e.g. the settings.json spelling', async () => { + // `/permissions default` is what a user copies out of settings.json. It + // lands in the same state as `/permissions ask`, so it must be equally + // durable — otherwise two spellings of one choice differ only in whether + // they survive a relaunch. + const { ctx, rpc } = makeCtx({ mode: 'default', ok: true, persisted: true }) + + cmd.run('default', ctx, '/permissions default') + + expect(rpc).toHaveBeenCalledWith('config.set', { + key: 'permission_mode', + persist: true, + value: 'default' + }) + }) + + it('accepts a raw engine mode as a power-user escape hatch, without persisting', async () => { + // plan / dontAsk are real modes with no picker row. Persisting them would + // make every future session start in plan mode, which nobody means by it. + const { ctx, rpc, sys } = makeCtx({ mode: 'plan', ok: true }) + + cmd.run('plan', ctx, '/permissions plan') + + expect(rpc).toHaveBeenCalledWith('config.set', { + key: 'permission_mode', + persist: false, + value: 'plan' + }) + await vi.waitFor(() => expect(sys).toHaveBeenCalledWith('Permission mode: plan.')) + }) + + it('badges the mode the SERVER confirmed, not the one requested', async () => { + const { ctx, sys } = makeCtx({ mode: 'acceptEdits', ok: true }) + + cmd.run('full', ctx, '/permissions full') + + await vi.waitFor(() => expect(sys).toHaveBeenCalledWith('Permissions: Approve for me.')) + expect(getUiState().permissionMode).toBe('acceptEdits') + }) + + it('surfaces a server refusal and leaves the badge alone', async () => { + patchUiState({ permissionMode: 'default' }) + + const { ctx, sys } = makeCtx({ error: 'Full Access is not available in this session', ok: false }) + + cmd.run('full', ctx, '/permissions full') + + await vi.waitFor(() => + expect(sys).toHaveBeenCalledWith('Full Access is not available in this session') + ) + // A rejected set must not flip the composer indicator. + expect(getUiState().permissionMode).toBe('default') + }) +}) diff --git a/ui-tui/src/__tests__/permissionsPicker.test.tsx b/ui-tui/src/__tests__/permissionsPicker.test.tsx new file mode 100644 index 00000000..14ad5fde --- /dev/null +++ b/ui-tui/src/__tests__/permissionsPicker.test.tsx @@ -0,0 +1,161 @@ +import { PassThrough } from 'node:stream' + +import { renderSync } from '@clawcodex/ink' +import React from 'react' +import { describe, expect, it, vi } from 'vitest' + +import { PermissionsPicker } from '../components/permissionsPicker.js' +import { PERMISSION_LEVELS } from '../lib/permissionLevels.js' +import { stripAnsi } from '../lib/text.js' +import { DEFAULT_THEME } from '../theme.js' + +const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)) + +/** Mount the picker with a writable stdin so keys can actually be driven — + * the digit/arrow/Enter/Esc branches ARE this component's behavior, and a + * render-only test leaves all of them uncovered. */ +function mount(current: string) { + const stdout = new PassThrough() + const stdin = new PassThrough() + const stderr = new PassThrough() + let out = '' + + stdout.on('data', (c: Buffer) => { + out += c.toString() + }) + Object.assign(stdout, { columns: 100, rows: 40 }) + Object.assign(stdin, { isTTY: true, ref: () => {}, setRawMode: () => {}, unref: () => {} }) + + const onClose = vi.fn() + const onSelect = vi.fn() + + const app = renderSync( + React.createElement(PermissionsPicker, { current, onClose, onSelect, t: DEFAULT_THEME }), + { + exitOnCtrlC: false, + patchConsole: false, + stderr: stderr as NodeJS.WriteStream, + stdin: stdin as NodeJS.ReadStream, + stdout: stdout as NodeJS.WriteStream + } + ) + + return { + onClose, + onSelect, + output: () => stripAnsi(out), + async press(seq: string, settle = 20) { + stdin.write(seq) + await delay(settle) + }, + unmount: () => app.unmount() + } +} + +const render = (current: string): string => { + const p = mount(current) + + p.unmount() + + return p.output() +} + +const ARROW_UP = '' +const ARROW_DOWN = '' +const ENTER = '\r' +const ESC = '' + +describe('PermissionsPicker', () => { + it('lists all three levels, numbered, with their descriptions', () => { + const out = render('bypassPermissions') + + PERMISSION_LEVELS.forEach((level, i) => { + expect(out).toContain(`${i + 1}. ${level.label}`) + // The description is the whole point — a row that only says "Full Access" + // does not tell the user what they are about to permit. + expect(out.replace(/\s+/g, ' ')).toContain(level.description.split('.')[0]!) + }) + }) + + it('marks the live mode as current', () => { + const out = render('bypassPermissions').replace(/\s+/g, ' ') + + expect(out).toContain('Full Access · current') + expect(out).not.toContain('Ask for approval · current') + }) + + it('marks nothing current for a mode with no level row', () => { + // plan / dontAsk are real modes the picker deliberately does not list. + // Showing one of the three as "current" there would be a lie. + const out = render('plan') + + expect(out).not.toContain('· current') + }) + + it('states the keys that work', () => { + expect(render('default')).toContain('1-3') + }) +}) + +describe('PermissionsPicker — key handling', () => { + it('selects directly by number', async () => { + for (const [i, level] of PERMISSION_LEVELS.entries()) { + const p = mount('bypassPermissions') + + await p.press(String(i + 1)) + expect(p.onSelect).toHaveBeenCalledWith(level.key) + p.unmount() + } + }) + + it('ignores a digit with no row', async () => { + const p = mount('bypassPermissions') + + await p.press('4') + await p.press('0') + expect(p.onSelect).not.toHaveBeenCalled() + p.unmount() + }) + + it('Enter applies the highlighted row, which starts on the current level', async () => { + const p = mount('default') + + await p.press(ENTER) + expect(p.onSelect).toHaveBeenCalledWith('ask') + p.unmount() + }) + + it('arrows move the highlight and clamp at both ends', async () => { + const p = mount('default') // starts on row 1 + + await p.press(ARROW_UP) // already at the top — must not wrap or go negative + await p.press(ENTER) + expect(p.onSelect).toHaveBeenLastCalledWith('ask') + + await p.press(ARROW_DOWN) + await p.press(ARROW_DOWN) + await p.press(ARROW_DOWN) // already at the bottom — must clamp + await p.press(ENTER) + expect(p.onSelect).toHaveBeenLastCalledWith('full') + p.unmount() + }) + + it('starts on row 1 for a mode with no row, instead of a negative index', async () => { + const p = mount('plan') + + await p.press(ENTER) + expect(p.onSelect).toHaveBeenCalledWith('ask') + p.unmount() + }) + + it('Esc cancels without selecting', async () => { + const p = mount('bypassPermissions') + + // A lone ESC is the prefix of every escape SEQUENCE, so the input parser + // holds it until it can rule one out — give it longer than a plain key. + await p.press(ESC, 200) + expect(p.onClose).toHaveBeenCalled() + expect(p.onSelect).not.toHaveBeenCalled() + p.unmount() + }) +}) diff --git a/ui-tui/src/app/interfaces.ts b/ui-tui/src/app/interfaces.ts index c313f2f9..aba3ee76 100644 --- a/ui-tui/src/app/interfaces.ts +++ b/ui-tui/src/app/interfaces.ts @@ -144,6 +144,7 @@ export interface OverlayState { memoryPicker: boolean modelPicker: boolean pager: null | PagerState + permissionsPicker: boolean petPicker: boolean planApproval: null | PlanApprovalReq pluginsHub: boolean @@ -417,6 +418,7 @@ export interface AppLayoutActions { onLogoSelect: (value: string) => void onMemorySelect: (path: string) => void onModelSelect: (value: string) => void + onPermissionsSelect: (levelKey: string) => void resumeById: (id: string) => void setStickyPrompt: (value: string) => void } @@ -484,6 +486,7 @@ export interface AppOverlaysProps { onModelSelect: (value: string) => void onNewLiveSession: () => void onNewPromptSession: (prompt: string, modelArg?: string) => void + onPermissionsSelect: (levelKey: string) => void onResumeSelect: (sessionId: string) => void onSecretSubmit: (value: string) => void onSudoSubmit: (pw: string) => void diff --git a/ui-tui/src/app/overlayStore.ts b/ui-tui/src/app/overlayStore.ts index 24c8db64..cbe01133 100644 --- a/ui-tui/src/app/overlayStore.ts +++ b/ui-tui/src/app/overlayStore.ts @@ -13,6 +13,7 @@ const buildOverlayState = (): OverlayState => ({ memoryPicker: false, modelPicker: false, pager: null, + permissionsPicker: false, petPicker: false, planApproval: null, pluginsHub: false, @@ -37,6 +38,7 @@ export const $isBlocked = computed( memoryPicker, modelPicker, pager, + permissionsPicker, petPicker, planApproval, pluginsHub, @@ -56,6 +58,7 @@ export const $isBlocked = computed( memoryPicker || modelPicker || pager || + permissionsPicker || petPicker || planApproval || pluginsHub || @@ -91,6 +94,7 @@ export const resetFlowOverlays = () => logoPicker: $overlayState.get().logoPicker, memoryPicker: $overlayState.get().memoryPicker, modelPicker: $overlayState.get().modelPicker, + permissionsPicker: $overlayState.get().permissionsPicker, petPicker: $overlayState.get().petPicker, pluginsHub: $overlayState.get().pluginsHub, sessions: $overlayState.get().sessions, diff --git a/ui-tui/src/app/slash/commands/session.ts b/ui-tui/src/app/slash/commands/session.ts index 4ee7bff6..b9e18cff 100644 --- a/ui-tui/src/app/slash/commands/session.ts +++ b/ui-tui/src/app/slash/commands/session.ts @@ -12,6 +12,7 @@ import type { VoiceToggleResponse } from '../../../gatewayTypes.js' import { isLogoPaletteName, LOGO_PALETTE_LABELS, LOGO_PALETTE_NAMES } from '../../../lib/logoPalettes.js' +import { levelForKey, levelForMode, PERMISSION_LEVEL_KEYS } from '../../../lib/permissionLevels.js' import { formatVoiceRecordKey, parseVoiceRecordKey } from '../../../lib/platform.js' import { fmtK } from '../../../lib/text.js' import type { PanelSection } from '../../../types.js' @@ -424,6 +425,62 @@ export const sessionCommands: SlashCommand[] = [ } }, + { + aliases: ['mode'], + argumentHint: `[${PERMISSION_LEVEL_KEYS.join('|')}]`, + help: 'choose what clawcodex is allowed to do', + name: 'permissions', + usage: `/permissions [${PERMISSION_LEVEL_KEYS.join('|')}]`, + // Bare /permissions opens the three-level picker (lib/permissionLevels.ts); + // an argument applies directly. The arg accepts a level KEY (ask|approve| + // full) or a RAW engine mode, so `plan` / `dontAsk` — real modes with no + // picker row — stay reachable without one. `mode` is kept as an alias for + // the command's former name. + // + // persist is deliberately LEVEL-only: choosing one of the three is a + // standing preference and is written to settings.json, but the raw-mode + // escape hatch is session-scoped. Persisting `/permissions plan` would make + // every future session start in plan mode, which nobody means by it. + // + // Keyed off the resolved MODE, not just the key, so `/permissions default` + // — the spelling a user copies out of settings.json — is as durable as + // `/permissions ask`. Same end state, so it should have the same durability. + run: (arg, ctx) => { + const raw = arg.trim() + + if (!raw) { + return patchOverlayState({ permissionsPicker: true }) + } + + const level = levelForKey(raw) + const mode = level ? level.mode : raw + const persist = Boolean(level ?? levelForMode(mode)) + + ctx.gateway + .rpc('config.set', { key: 'permission_mode', persist, value: mode }) + .then( + ctx.guarded(r => { + if (r.ok === false) { + return ctx.transcript.sys(r.error ?? `Could not set permissions to ${mode}.`) + } + + // Badge only what the server confirmed — a rejected set must not + // flip the composer indicator. + const applied = r.mode ?? mode + + patchUiState({ permissionMode: applied }) + + const appliedLevel = levelForMode(applied) + + ctx.transcript.sys( + appliedLevel ? `Permissions: ${appliedLevel.label}.` : `Permission mode: ${applied}.` + ) + }) + ) + .catch(ctx.guardedErr) + } + }, + { argumentHint: '[]', help: 'switch theme skin (fires skin.changed)', diff --git a/ui-tui/src/app/uiStore.ts b/ui-tui/src/app/uiStore.ts index fcd7b930..9cf64ab3 100644 --- a/ui-tui/src/app/uiStore.ts +++ b/ui-tui/src/app/uiStore.ts @@ -44,6 +44,7 @@ export const $uiState = atom(buildUiState()) export const $uiTheme = computed($uiState, state => state.theme) export const $uiSessionId = computed($uiState, state => state.sid) export const $uiLogoPalette = computed($uiState, state => state.logoPalette) +export const $uiPermissionMode = computed($uiState, state => state.permissionMode) export const getUiState = () => $uiState.get() diff --git a/ui-tui/src/app/useMainApp.ts b/ui-tui/src/app/useMainApp.ts index e25280de..af81115f 100644 --- a/ui-tui/src/app/useMainApp.ts +++ b/ui-tui/src/app/useMainApp.ts @@ -1128,6 +1128,14 @@ export function useMainApp(gw: GatewayClient) { slashRef.current(`/logo ${value}`) }, []) + // /permissions picker choice → re-enter the slash command with the chosen + // LEVEL KEY so the single command path owns the RPC, the persist and the + // transcript line (the /model + /logo picker pattern). + const onPermissionsSelect = useCallback((levelKey: string) => { + patchOverlayState({ permissionsPicker: false }) + slashRef.current(`/permissions ${levelKey}`) + }, []) + // /memory picker choice → the memory.tsx flow: ensure-create, $EDITOR under // the alt-screen suspend, "Opened memory file at …" system line, and a // memory_edited cache bust so the next turn re-reads the file. @@ -1250,6 +1258,7 @@ export function useMainApp(gw: GatewayClient) { onLogoSelect, onMemorySelect, onModelSelect, + onPermissionsSelect, // Resuming a cold session from the overlay CLOSES the current one, so it // must respect the busy guard just like the `/resume` slash path. // (Switching between live sessions and `+ new` keep the current session @@ -1275,6 +1284,7 @@ export function useMainApp(gw: GatewayClient) { onLogoSelect, onMemorySelect, onModelSelect, + onPermissionsSelect, session ] ) diff --git a/ui-tui/src/components/appLayout.tsx b/ui-tui/src/components/appLayout.tsx index 769421cc..26eb8219 100644 --- a/ui-tui/src/components/appLayout.tsx +++ b/ui-tui/src/components/appLayout.tsx @@ -308,6 +308,7 @@ const ComposerPane = memo(function ComposerPane({ onModelSelect={actions.onModelSelect} onNewLiveSession={actions.newLiveSession} onNewPromptSession={actions.newPromptSession} + onPermissionsSelect={actions.onPermissionsSelect} onResumeSelect={actions.resumeById} pagerPageSize={composer.pagerPageSize} /> diff --git a/ui-tui/src/components/appOverlays.tsx b/ui-tui/src/components/appOverlays.tsx index f8cf509d..635a7d14 100644 --- a/ui-tui/src/components/appOverlays.tsx +++ b/ui-tui/src/components/appOverlays.tsx @@ -5,7 +5,7 @@ import { useGateway } from '../app/gatewayContext.js' import type { AppOverlaysProps } from '../app/interfaces.js' import { $overlayState, patchOverlayState } from '../app/overlayStore.js' import { argumentHintFor } from '../app/slash/argumentHints.js' -import { $uiLogoPalette, $uiSessionId, $uiTheme } from '../app/uiStore.js' +import { $uiLogoPalette, $uiPermissionMode, $uiSessionId, $uiTheme } from '../app/uiStore.js' import { ActiveSessionSwitcher } from './activeSessionSwitcher.js' import { FloatBox } from './appChrome.js' @@ -15,6 +15,7 @@ import { MaskedPrompt } from './maskedPrompt.js' import { MemoryPicker } from './memoryPicker.js' import { ModelPicker } from './modelPicker.js' import { OverlayHint } from './overlayControls.js' +import { PermissionsPicker } from './permissionsPicker.js' import { PetPicker } from './petPicker.js' import { PluginsHub } from './pluginsHub.js' import { ApprovalPrompt, ClarifyPrompt, ConfirmPrompt, PlanApprovalPrompt } from './prompts.js' @@ -146,6 +147,7 @@ export function FloatingOverlays({ onModelSelect, onNewLiveSession, onNewPromptSession, + onPermissionsSelect, onResumeSelect, pagerPageSize }: Pick< @@ -160,6 +162,7 @@ export function FloatingOverlays({ | 'onModelSelect' | 'onNewLiveSession' | 'onNewPromptSession' + | 'onPermissionsSelect' | 'onResumeSelect' | 'pagerPageSize' >) { @@ -168,11 +171,13 @@ export function FloatingOverlays({ const sid = useStore($uiSessionId) const theme = useStore($uiTheme) const logoPalette = useStore($uiLogoPalette) + const permissionMode = useStore($uiPermissionMode) const hasAny = overlay.logoPicker || overlay.memoryPicker || overlay.modelPicker || + overlay.permissionsPicker || overlay.pager || overlay.petPicker || overlay.sessions || @@ -232,6 +237,17 @@ export function FloatingOverlays({ )} + {overlay.permissionsPicker && ( + + patchOverlayState({ permissionsPicker: false })} + onSelect={onPermissionsSelect} + t={theme} + /> + + )} + {overlay.memoryPicker && ( {shownCwd} + {/* Launch-time permission level. Worth a row of its own because full + access is the default: the composer badge is the LIVE indicator but + hides while the user types (composerFooter suppressHint), so this + states it once, unmissably, and names the command that changes it. + Like Model above, this is a committed transcript row the renderer + never re-emits — it describes the session's STARTING state and does + not follow a later /permissions change; the badge is what tracks + "right now". */} + {permsLabel && ( + + {PERMS_LABEL} + + {permsLabel} + + {PERMS_HINT} + + )} + {worktree && ( Tree diff --git a/ui-tui/src/components/composerFooter.tsx b/ui-tui/src/components/composerFooter.tsx index 106de376..da006fe1 100644 --- a/ui-tui/src/components/composerFooter.tsx +++ b/ui-tui/src/components/composerFooter.tsx @@ -22,10 +22,13 @@ interface ModeBadge { // Original PermissionMode.ts symbols/titles: ⏸ (U+23F8) for plan, ▶▶ // (U+25B6 ×2) for the rest. -const MODE_BADGES: Record = { - acceptEdits: { color: t => t.color.autoAccept, label: 'accept edits on', symbol: '▶▶' }, +// Labels track the /permissions picker's vocabulary (lib/permissionLevels.ts) +// so the badge and the picker never name the same mode two different ways. +// `plan` / `dontAsk` / `auto` have no picker row and keep their own wording. +export const MODE_BADGES: Record = { + acceptEdits: { color: t => t.color.autoAccept, label: 'approve for me on', symbol: '▶▶' }, auto: { color: t => t.color.warn, label: 'auto mode on', symbol: '▶▶' }, - bypassPermissions: { color: t => t.color.error, label: 'bypass permissions on', symbol: '▶▶' }, + bypassPermissions: { color: t => t.color.error, label: 'full access on', symbol: '▶▶' }, dontAsk: { color: t => t.color.error, label: "don't ask on", symbol: '▶▶' }, plan: { color: t => t.color.planMode, label: 'plan mode on', symbol: '⏸' } } diff --git a/ui-tui/src/components/permissionsPicker.tsx b/ui-tui/src/components/permissionsPicker.tsx new file mode 100644 index 00000000..c22e1a77 --- /dev/null +++ b/ui-tui/src/components/permissionsPicker.tsx @@ -0,0 +1,99 @@ +import { Box, Text, useInput } from '@clawcodex/ink' +import { useState } from 'react' + +import { levelForMode, PERMISSION_LEVELS } from '../lib/permissionLevels.js' +import type { Theme } from '../theme.js' + +import { OverlayHint } from './overlayControls.js' + +/** + * Interactive `/permissions` picker — the three levels from + * `lib/permissionLevels.ts` (Python twin: `src/permissions/levels.py`). + * + * Shaped on `LogoPicker`: Enter hands the chosen level key to `onSelect`, which + * re-enters `/permissions ` so the result lands in the transcript and the + * single slash-command path owns the RPC. Esc cancels. Rows are numbered and + * `1`–`3` select directly, matching the reference picker. + * + * `current` is the live engine mode, not a level key: `plan` / `dontAsk` are + * real modes with no row here, so `levelForMode` returns undefined and no row + * is marked current — deliberately, rather than mislabeling plan mode as one of + * the three. + */ +export function PermissionsPicker({ current, onClose, onSelect, t }: PermissionsPickerProps) { + const currentLevel = levelForMode(current) + const currentIdx = currentLevel ? PERMISSION_LEVELS.findIndex(l => l.key === currentLevel.key) : -1 + const [idx, setIdx] = useState(Math.max(0, currentIdx)) + + useInput((input, key) => { + if (key.escape) { + return onClose() + } + + if (key.upArrow) { + return setIdx(i => Math.max(0, i - 1)) + } + + if (key.downArrow) { + return setIdx(i => Math.min(PERMISSION_LEVELS.length - 1, i + 1)) + } + + // Direct 1/2/3 selection — applies immediately, like the reference picker. + const digit = Number.parseInt(input, 10) + + if (Number.isInteger(digit) && digit >= 1 && digit <= PERMISSION_LEVELS.length) { + const picked = PERMISSION_LEVELS[digit - 1] + + return picked ? onSelect(picked.key) : undefined + } + + if (key.return) { + const picked = PERMISSION_LEVELS[idx] + + return picked ? onSelect(picked.key) : undefined + } + }) + + return ( + + + Choose what clawcodex is allowed to do + + + {PERMISSION_LEVELS.map((level, i) => { + const at = i === idx + const isCurrent = currentLevel?.key === level.key + + return ( + + + + {at ? '▸ ' : ' '} + {i + 1}.{' '} + + + {level.label} + + {isCurrent && · current} + + + {' '} + {level.description} + + + ) + })} + + ↑/↓ or 1-3 select · Enter apply · Esc cancel + + ) +} + +interface PermissionsPickerProps { + /** The live engine permission mode (not a level key). */ + current: string + onClose: () => void + /** Receives the chosen level KEY (`ask` / `approve` / `full`). */ + onSelect: (key: string) => void + t: Theme +} diff --git a/ui-tui/src/gatewayClient.ts b/ui-tui/src/gatewayClient.ts index 15797c70..119a3595 100644 --- a/ui-tui/src/gatewayClient.ts +++ b/ui-tui/src/gatewayClient.ts @@ -454,7 +454,7 @@ const SLASHES: ReadonlyArray<{ desc: string; hint?: string; name: string }> = [ { desc: 'Switch the model', name: '/model' }, { desc: 'Set the output style', hint: '[]', name: '/output-style' }, { desc: 'Change the startup logo color scheme', name: '/logo' }, - { desc: 'Set the permission mode', hint: '[default|plan|acceptEdits|dontAsk|bypassPermissions]', name: '/mode' }, + { desc: 'Choose what clawcodex is allowed to do', name: '/permissions' }, { desc: 'Compact the conversation to save context', name: '/compact' }, { desc: 'Show context-window usage', name: '/context' }, { desc: 'Show the total cost and duration of the current session', name: '/cost' }, @@ -736,10 +736,20 @@ export class GatewayClient extends EventEmitter { if (key === 'permission_mode') { // The server can reject this (bypassPermissions is gated on - // availability); reflect its verdict so a settings-panel write - // doesn't falsely report success while the server refused. - return this.controlQuery('set_permission_mode', { mode: value }) - .then(r => ({ ok: (r as any)?.ok !== false } as T)) + // selectability); reflect its FULL verdict — `/permissions` needs the + // applied mode and the rejection text, not just a boolean, so a + // refused set can neither flip the badge nor report success. + return this.controlQuery('set_permission_mode', { mode: value, persist: Boolean(p.persist) }) + .then(r => { + const res = (r ?? {}) as { error?: string; mode?: string; ok?: boolean; persisted?: boolean } + + return { + error: res.error, + mode: res.mode, + ok: res.ok !== false, + persisted: res.persisted + } as T + }) } if (key === 'model') {return this.setModel(String(value ?? '')) as Promise} @@ -1329,28 +1339,10 @@ export class GatewayClient extends EventEmitter { return out(`Effort: ${r?.effort ?? arg ?? '(unchanged)'}.${note}`) } - case 'mode': { - // Bare `/mode` is a no-op query, not a set — don't send an empty mode - // (the server would reject it as invalid). Matches the prior behavior. - if (arg == null || arg.trim() === '') {return out('Permission mode: (unchanged).')} - - // The server validates the mode and gates bypassPermissions on - // availability (same guard as the Shift+Tab cycle) — surface its - // verdict instead of echoing the arg as if it took effect. - const r = (await this.controlQuery('set_permission_mode', { mode: arg.trim() })) as any - - if (r && r.ok === false) {return out(`mode: ${r.error ?? 'invalid mode'}`)} - - // Only badge the mode the server actually confirmed — a rejected set - // must not flip the composer's permission-mode indicator. - const mode = typeof r?.mode === 'string' ? r.mode : arg.trim() - - if (mode) { - this.publish({ payload: { mode: String(mode) }, type: 'permission.mode' }) - } - - return out(`Permission mode: ${mode}.`) - } + // `/permissions` (formerly `/mode`) is a LOCAL slash command + // (app/slash/commands/session.ts) — createSlashHandler resolves the local + // registry first and returns, so a gateway case here would be unreachable. + // The RPC it uses is `config.set{key:'permission_mode'}` above. case 'model': await this.controlQuery('set_model', { model: arg }) diff --git a/ui-tui/src/gatewayTypes.ts b/ui-tui/src/gatewayTypes.ts index b370fb98..36dfef08 100644 --- a/ui-tui/src/gatewayTypes.ts +++ b/ui-tui/src/gatewayTypes.ts @@ -254,8 +254,15 @@ export interface ConfigSetResponse { confirm_message?: string confirm_required?: boolean credential_warning?: string + /** Server-side rejection reason (permission_mode: invalid / unavailable). */ + error?: string history_reset?: boolean info?: SessionInfo + /** permission_mode only: the mode the server actually applied. */ + mode?: string + ok?: boolean + /** permission_mode only: whether the choice was written to settings.json. */ + persisted?: boolean value?: string warning?: string } diff --git a/ui-tui/src/lib/permissionLevels.ts b/ui-tui/src/lib/permissionLevels.ts new file mode 100644 index 00000000..f480f0bc --- /dev/null +++ b/ui-tui/src/lib/permissionLevels.ts @@ -0,0 +1,71 @@ +/** + * The three user-facing permission levels behind `/permissions`. + * + * Mirror of the Python twin `src/permissions/levels.py`, which is the source of + * truth; `tests/test_permission_levels.py` pins the two equal (keys, labels, + * modes and descriptions), so edit both together. + * + * Each level is a label over an existing engine permission mode — the + * permission engine is untouched. `plan` and `dontAsk` deliberately have no row: + * they are workflow / strict-deny modes, not points on a looseness ladder, and + * stay reachable via `/plan`, `--permission-mode`, and `/permissions `. + * + * Level keys are chosen not to collide with any engine mode name so + * `/permissions ` can accept either a key or a raw mode unambiguously — + * note the middle level is `approve`, not `auto` (`auto` is a real mode). + */ + +export interface PermissionLevel { + /** One-line explanation of what the level actually permits. */ + description: string + /** Stable identifier used by `/permissions ` and the picker overlay. */ + key: string + /** Title-case label shown in the picker. */ + label: string + /** The engine mode this level selects. */ + mode: string +} + +/** Ordered loosest-last, matching the picker's 1/2/3 ordering. */ +export const PERMISSION_LEVELS: readonly PermissionLevel[] = [ + { + description: + 'Read files and run read-only commands freely. Editing files, running other commands, or touching anything outside this workspace asks first.', + key: 'ask', + label: 'Ask for approval', + mode: 'default' + }, + { + description: + 'Auto-accept file edits in this workspace and a small set of safe shell commands. Everything else still asks.', + key: 'approve', + label: 'Approve for me', + mode: 'acceptEdits' + }, + { + description: + 'Edit any file and run any command without asking, except where you have configured a deny or ask rule. Exercise caution when using.', + key: 'full', + label: 'Full Access', + mode: 'bypassPermissions' + } +] + +export const PERMISSION_LEVEL_KEYS: readonly string[] = PERMISSION_LEVELS.map(l => l.key) + +/** Look up a level by key (case-insensitive); undefined when not a level. */ +export const levelForKey = (key: string): PermissionLevel | undefined => + PERMISSION_LEVELS.find(l => l.key === key.trim().toLowerCase()) + +/** + * The level a mode belongs to, or undefined for off-ladder modes. + * + * `plan` / `dontAsk` / `auto` / `bubble` return undefined — real modes with no + * picker row. Callers must render those as "no level selected" rather than + * silently showing one of the three. + */ +export const levelForMode = (mode: null | string | undefined): PermissionLevel | undefined => + PERMISSION_LEVELS.find(l => l.mode === mode) + +/** `/permissions ` → engine mode; undefined when key is not a level. */ +export const modeForKey = (key: string): string | undefined => levelForKey(key)?.mode