Repository navigation
fix(doctor): align Personal shell approval fallback #1744
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 2 additions & 0 deletions
2
openspec/changes/archive/2026-08-03-align-personal-shell-approval-fallback/.openspec.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: spec-driven | ||
| created: 2026-08-03 |
59 changes: 59 additions & 0 deletions
59
...pec/changes/archive/2026-08-03-align-personal-shell-approval-fallback/design.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| ## Context | ||
|
|
||
| The Personal init path writes `ShellMode = HostAllowed` and an explicit `shell_execute = Approval` override. `ToolAccessPolicy` also selects `Approval` when a Personal shell call has no exact override. | ||
|
|
||
| `ToolAudienceProfilesDoctorCheck` instead treats a missing policy as `Auto`. The main specification also says that runtime defaults do not place shell in `Approval` mode. | ||
|
|
||
| The daemon actor boundary and approval persistence do not change. The change only aligns diagnostics and the contract with the existing authorization decision. | ||
|
|
||
| ## Goals / Non-Goals | ||
|
|
||
| **Goals:** | ||
|
|
||
| - Make the doctor report the effective Personal shell mode. | ||
| - State the clean-install and fail-closed fallback rules. | ||
| - Preserve explicit `Auto`, `Approval`, and `Deny` shell overrides. | ||
| - Add focused regression proof for doctor output. | ||
|
|
||
| **Non-Goals:** | ||
|
|
||
| - Change shell authorization or approval persistence. | ||
| - Add the approval disposition matrix. | ||
| - Change shell parsing or PR #1733. | ||
| - Change Team or Public shell access. | ||
|
|
||
| ## Decisions | ||
|
|
||
| ### Use the exact shell override as the doctor signal | ||
|
|
||
| The doctor will warn only when the Personal profile explicitly sets `shell_execute` to `Auto`. A missing policy or missing exact override resolves to `Approval` in the runtime. | ||
|
|
||
| This approach uses `ToolApprovalConfig.TryGetExplicitMode`. It does not add a second general approval resolver. | ||
|
|
||
| Alternative: move the runtime resolver into a new shared service. This change rejects that option because the contract correction needs no new runtime abstraction. | ||
|
|
||
| ### Keep the clean install explicit | ||
|
|
||
| The init wizard will continue to write `shell_execute = Approval`. Operators can inspect the normal security posture without knowledge of the runtime backstop. | ||
|
|
||
| The fallback remains necessary for old or partial configuration. It prevents a missing field from enabling host shell without approval. | ||
|
|
||
| ### Keep actor and persistence behavior unchanged | ||
|
|
||
| The session actor, approval actor, and `tool-approvals.json` format do not change. Existing approvals remain valid. | ||
|
|
||
| ## Risks / Trade-offs | ||
|
|
||
| - **Risk:** A future runtime change could diverge from the doctor again. **Mitigation:** Tests cover missing policy, missing override, and explicit override cases. | ||
| - **Risk:** Operators can misread a missing warning as an explicit configuration endorsement. **Mitigation:** The specification distinguishes the generated override from the runtime backstop. | ||
| - **Risk:** A doctor message change can affect documentation or scripts. **Mitigation:** The new message remains a warning and names the explicit `Auto` override. | ||
|
|
||
| ## Migration Plan | ||
|
|
||
| No configuration migration is required. Existing Personal configurations retain their current runtime behavior. | ||
|
|
||
| Rollback restores the old diagnostic and specification text. It does not change stored approvals or daemon state. | ||
|
|
||
| ## Open Questions | ||
|
|
||
| None. |
35 changes: 35 additions & 0 deletions
35
...c/changes/archive/2026-08-03-align-personal-shell-approval-fallback/proposal.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| ## Why | ||
|
|
||
| A clean Personal install writes an explicit `shell_execute = Approval` override. The runtime also applies this fail-closed result when the override is absent, but the specification and doctor describe `Auto` instead. | ||
|
|
||
| This conflict makes diagnostics incorrect and obscures the deployed security contract before PR #1733 changes shell parsing. This change aligns the contract before the approval matrix work begins. | ||
|
|
||
| ## What Changes | ||
|
|
||
| - State that Personal shell execution requires approval unless an explicit shell override selects `Auto` or `Deny`. | ||
| - Keep the clean Personal install as the normal configuration source. | ||
| - Keep the runtime fallback as a safety backstop for legacy or partial configuration. | ||
| - Fix the doctor check so it reports the effective runtime mode. | ||
| - Add focused doctor regression tests. | ||
| - Keep the approval matrix and PR #1733 changes out of scope. | ||
|
|
||
| ## Capabilities | ||
|
|
||
| ### New Capabilities | ||
|
|
||
| None. | ||
|
|
||
| ### Modified Capabilities | ||
|
|
||
| - `tool-approval-gates`: Align the Personal shell fallback requirement with the shipped runtime and generated configuration. | ||
| - `netclaw-cli`: Make the doctor warn for an explicit Personal shell `Auto` override, not for the fail-closed fallback. | ||
|
|
||
| ## Impact | ||
|
|
||
| - **PRDs:** PRD-002 gateway security and PRD-004 CLI onboarding and configuration. | ||
| - **Code:** `ToolAudienceProfilesDoctorCheck` effective-mode diagnostics. | ||
| - **Tests:** Focused doctor tests for missing and explicit Personal shell modes. | ||
| - **Documentation:** Correct the approval runbook for existing configurations. | ||
| - **Security:** The change preserves fail-closed shell behavior. It adds no new grant or bypass. | ||
| - **Operations:** Operators stop receiving a false missing-gate warning for a configuration that the runtime gates. | ||
| - **MVP scope:** The change corrects the existing contract. It adds no new capability. |
41 changes: 41 additions & 0 deletions
41
...ive/2026-08-03-align-personal-shell-approval-fallback/specs/netclaw-cli/spec.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| ## MODIFIED Requirements | ||
|
|
||
| ### Requirement: Doctor checks for approval configuration | ||
|
|
||
| `netclaw doctor` SHALL validate approval configuration consistency. It SHALL | ||
| warn when the Personal audience enables host shell access with an exact | ||
| `shell_execute = Auto` override. It SHALL NOT warn when the Personal approval | ||
| policy is absent. It SHALL NOT warn when the exact shell override is absent. | ||
| The runtime resolves both states to `Approval`. The doctor SHALL warn when | ||
| `tool-approvals.json` contains patterns for audiences or tools that are no | ||
| longer configured. | ||
|
|
||
| #### Scenario: Doctor accepts the missing-policy fail-closed fallback | ||
|
|
||
| - **GIVEN** the Personal audience has host shell access enabled | ||
| - **AND** the Personal profile has no `ApprovalPolicy` | ||
| - **WHEN** `netclaw doctor` runs | ||
| - **THEN** it emits no warning that shell lacks an approval gate | ||
|
|
||
| #### Scenario: Doctor accepts a policy without an exact shell override | ||
|
|
||
| - **GIVEN** the Personal audience has host shell access enabled | ||
| - **AND** `ApprovalPolicy.ToolOverrides` does not contain `shell_execute` | ||
| - **WHEN** `netclaw doctor` runs | ||
| - **THEN** it emits no warning that shell lacks an approval gate | ||
|
|
||
| #### Scenario: Doctor warns about an explicit Personal shell Auto override | ||
|
|
||
| - **GIVEN** the Personal audience has host shell access enabled | ||
| - **AND** `ApprovalPolicy.ToolOverrides.shell_execute` is `Auto` | ||
| - **WHEN** `netclaw doctor` runs | ||
| - **THEN** it emits a warning that Personal host shell explicitly runs without approval | ||
| - **AND** the warning recommends changing the exact override to `Approval` | ||
|
|
||
| #### Scenario: Doctor warns about stale approval patterns | ||
|
|
||
| - **GIVEN** `tool-approvals.json` has patterns for `team.shell_execute` | ||
| - **AND** the Team audience has shell mode Off | ||
| - **WHEN** `netclaw doctor` runs | ||
| - **THEN** it emits an info advisory: "Persistent approvals exist for | ||
| team.shell_execute but shell is disabled for Team audience." |
70 changes: 70 additions & 0 deletions
70
...-08-03-align-personal-shell-approval-fallback/specs/tool-approval-gates/spec.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| ## MODIFIED Requirements | ||
|
|
||
| ### Requirement: Tool approval configuration per audience | ||
|
|
||
| The system SHALL support per-audience tool approval configuration via | ||
| `ToolApprovalConfig` on `ToolAudienceProfile`. Each audience profile SHALL | ||
| independently specify a `DefaultMode` (Auto, Approval, Deny) and per-tool | ||
| overrides in `ToolOverrides`. The default `DefaultMode` SHALL be `Auto` for | ||
| tools without a stricter invocation-specific rule. | ||
|
|
||
| The init-generated Personal config SHALL explicitly write | ||
| `ApprovalPolicy.ToolOverrides.shell_execute = Approval` as the normal | ||
| shell-safe configuration. For a Personal shell invocation, an exact | ||
| `shell_execute` override SHALL select `Auto`, `Approval`, or `Deny`. The runtime | ||
| SHALL select `Approval` when that exact override is absent. This rule SHALL | ||
| apply when `ApprovalPolicy` is absent. It SHALL also apply when `DefaultMode` | ||
| is `Auto`. This fallback SHALL prevent a missing field from enabling host shell | ||
| without approval. | ||
|
|
||
| #### Scenario: Shell requires approval in init-generated Personal config | ||
|
|
||
| - **GIVEN** a Personal audience session whose generated config explicitly sets | ||
| `ApprovalPolicy.ToolOverrides.shell_execute` to `Approval` | ||
| - **WHEN** the agent invokes `shell_execute` | ||
| - **THEN** `ToolAccessPolicy` marks the call as approval-gated | ||
| - **AND** `DispatchingToolExecutor` consults `IToolApprovalService` before execution | ||
| - **AND** if the command pattern is not approved, an approval prompt is emitted | ||
|
|
||
| #### Scenario: Missing Personal approval policy fails closed for shell | ||
|
|
||
| - **GIVEN** a Personal audience session with `ShellMode` set to `HostAllowed` | ||
| - **AND** the Personal profile has no `ApprovalPolicy` | ||
| - **WHEN** the agent invokes `shell_execute` | ||
| - **THEN** the runtime resolves the invocation to `Approval` | ||
| - **AND** the missing policy does not enable automatic shell execution | ||
|
|
||
| #### Scenario: Personal policy without an exact shell override fails closed | ||
|
|
||
| - **GIVEN** a Personal approval policy whose `DefaultMode` is `Auto` | ||
| - **AND** `ToolOverrides` has no exact `shell_execute` entry | ||
| - **WHEN** the agent invokes `shell_execute` | ||
| - **THEN** the runtime resolves the invocation to `Approval` | ||
|
|
||
| #### Scenario: Explicit Personal shell Auto override executes without approval | ||
|
|
||
| - **GIVEN** a Personal approval policy with an exact `shell_execute = Auto` override | ||
| - **WHEN** the agent invokes a command that passes earlier security gates | ||
| - **THEN** the tool executes without an approval prompt | ||
|
|
||
| #### Scenario: Tool in Auto mode executes without approval | ||
|
|
||
| - **GIVEN** a tool whose effective approval mode is `Auto` for the session's audience | ||
| - **WHEN** the agent invokes the tool | ||
| - **THEN** the tool executes immediately without an approval prompt | ||
|
|
||
| #### Scenario: Tool in Deny mode is always blocked | ||
|
|
||
| - **GIVEN** a tool whose effective approval mode is `Deny` for the session's audience | ||
| - **WHEN** the agent invokes the tool | ||
| - **THEN** the tool is denied with reason `tool_denied_by_approval_policy` | ||
| - **AND** no approval prompt is offered | ||
|
|
||
| #### Scenario: Per-audience independence | ||
|
|
||
| - **GIVEN** Personal sets `shell_execute` to `Approval` and Team sets it to `Deny` | ||
| - **WHEN** a Personal session invokes `shell_execute` | ||
| - **THEN** `ToolAccessPolicy` marks the call as approval-gated | ||
| - **AND** `DispatchingToolExecutor` may prompt if `IToolApprovalService` reports unapproved patterns | ||
| - **AND** when a Team session invokes `shell_execute` | ||
| - **THEN** the system denies immediately without prompting |
15 changes: 15 additions & 0 deletions
15
...spec/changes/archive/2026-08-03-align-personal-shell-approval-fallback/tasks.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| ## 1. Doctor Contract | ||
|
|
||
| - [x] 1.1 Make the Personal shell doctor warning follow the runtime fallback and exact override precedence. | ||
| - [x] 1.2 Add focused doctor tests for missing policy, missing override, explicit `Approval`, and explicit `Auto`. | ||
|
|
||
| ## 2. Documentation and Specification | ||
|
|
||
| - [x] 2.1 Correct the approval runbook for clean and existing Personal configurations. | ||
| - [x] 2.2 Sync the `tool-approval-gates` and `netclaw-cli` delta specifications. | ||
|
|
||
| ## 3. Verification | ||
|
|
||
| - [x] 3.1 Run the focused doctor test class. | ||
| - [x] 3.2 Run Slopwatch, the file-header check, and `git diff --check`. | ||
| - [x] 3.3 Verify the implementation against the OpenSpec change and archive it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 0666f8d. The runbook now states that the fallback applies only to Personal shell_execute calls.