diff --git a/docs/runbooks/tool-approval-gates.md b/docs/runbooks/tool-approval-gates.md index 9abea1028..c6cf0b98e 100644 --- a/docs/runbooks/tool-approval-gates.md +++ b/docs/runbooks/tool-approval-gates.md @@ -7,13 +7,16 @@ channel. ## Overview -Tool invocations pass through three layers: - -1. **Hard deny** — commands that are always blocked (e.g., `netclaw daemon stop`, - `rm -rf /`). Never approvable. Checked first. -2. **Tool access** — per-audience allowlists (`AllowedTools`, +Tool invocations pass through four layers: + +1. **Operation hard deny** — shell commands that are always blocked + (e.g., `netclaw daemon stop`, `rm -rf /`). Never approvable. Checked first. +2. **Resource hard deny** — protected files and directories (secrets, keys, + lifecycle/control-plane files) that are blocked for file tools and shell + path references. Never approvable. +3. **Tool access** — per-audience allowlists (`AllowedTools`, `AllowedMcpServers`). Binary: the tool is available or it isn't. -3. **Approval gate** — for tools that pass layers 1 and 2, does this specific +4. **Approval gate** — for tools that pass layers 1-3, does this specific invocation need user sign-off? The approval gate is transparent to the LLM — it never knows approval is @@ -132,8 +135,14 @@ For **compound commands** (`&&`, `||`, `;`, `|`), each segment is checked independently. If any segment is unapproved, all unapproved patterns are batched into one prompt. -For **non-shell tools** (MCP tools, `file_write`, etc.), approval is at the -tool-name level — either the tool is approved or it isn't. +For most **non-shell tools** (MCP tools, `file_read`, etc.), approval is at the +tool-name level. + +For `file_write` and `file_edit`, approval is path-aware for Netclaw +control-plane targets. Writes under the control-plane root use mode keys like +`file_write:control-plane` / `file_edit:control-plane` and persist approvals as +path-scoped patterns (for example, +`file_write:control-plane:netclaw.json`). ### Persistent approvals @@ -165,6 +174,11 @@ mode: The hard deny check runs even in `Auto` mode (no approval configured). +In addition to command hard deny, Netclaw enforces path hard deny for protected +resources (for example `secrets.json`, key material, webhook secrets, and +control-plane lifecycle files). Those accesses are blocked for file tools and +for shell commands that reference those paths. + ### Custom hard deny patterns Add patterns via `HardDenyPatterns` in `netclaw.json`: diff --git a/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/.openspec.yaml b/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/.openspec.yaml new file mode 100644 index 000000000..33d01f784 --- /dev/null +++ b/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-04-12 diff --git a/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/design.md b/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/design.md new file mode 100644 index 000000000..8bd4b289c --- /dev/null +++ b/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/design.md @@ -0,0 +1,119 @@ +## Context + +`tool-approval-gates` added approval interception across `ToolAccessPolicy`, +`DispatchingToolExecutor`, and session approval retry handling. Follow-on +testing exposed composition edge cases: + +- File mutation gating now uses argument-aware matcher keys for control-plane + paths, but key resolution needs explicit precedence when both path-specific + and base tool overrides exist. +- Approve-once currently depends on one-time context state and matcher patterns; + retry matching must align with the filtered unapproved set returned by + `IToolApprovalService`, not the pre-filter candidate set. +- Shell deny checks are split between operation-level command hard deny and + resource-level path denial; precedence and user-visible deny semantics need a + single contract. + +The implementation spans `Netclaw.Actors.Tools`, `Netclaw.Security`, and +session retry flow state. Actor boundary remains unchanged: session actor owns +approval decisions and temporary one-time retry grants. + +## Goals / Non-Goals + +**Goals:** + +- Make approval mode resolution deterministic when matcher-derived keys and base + tool keys both exist. +- Ensure approve-once retry acceptance checks use the same filtered unapproved + pattern set shown in the interaction prompt. +- Define shell hard-deny composition semantics between operation hard-deny and + resource hard-deny, including precedence for deny reasons. +- Add regression scenarios to the capability spec so future refactors preserve + these compositions. + +**Non-Goals:** + +- Redesign of approval UI/options or interaction protocol. +- New persistence model for approvals. +- Changes to trust audiences, grant categories, or Slack channel UX. + +## Decisions + +### Decision 1: Approval mode key precedence uses most-specific to least-specific + +**Choice:** Resolve approval mode in this order: + +1. Matcher-derived key override (for example `file_write:control-plane`) +2. Base tool key override (`file_write`) +3. Matcher fail-closed behavior for Personal audience +4. Audience `DefaultMode` + +**Alternatives considered:** + +- Matcher key only with no base fallback: rejected because adding path-specific + matchers unintentionally bypasses existing tool-level policy intent. +- Base key first: rejected because it prevents finer-grained overrides from + taking effect. + +**Rationale:** This preserves backward compatibility for existing overrides while +letting operators tighten high-risk subsets without broadening unrelated calls. + +### Decision 2: Approve-once matching is evaluated after unapproved filtering + +**Choice:** For approval-gated calls, first compute unapproved patterns via +`IToolApprovalService`; then evaluate one-time retry bypass against that filtered +set. + +**Alternatives considered:** + +- Check one-time bypass against pre-filter matcher patterns: rejected because it + can reprompt even when the user just approved the exact prompt set. +- Persist approve-once to shared approval service: rejected because it breaks + one-shot scope guarantees. + +**Rationale:** Prompt set and retry set must be identical to avoid UX/security +drift. One-time state remains in-memory and call-retry scoped. + +### Decision 3: Shell deny composition is fail-closed with operation precedence + +**Choice:** Shell invocation remains denied if either operation hard-deny +(`ShellCommandPolicy`) or resource hard-deny (`ToolPathPolicy`) matches. +Operation hard-deny is evaluated first; if it matches, resource checks are not +consulted for the result reason. + +**Alternatives considered:** + +- Resource deny first: rejected because known self-destructive operations should + short-circuit early and return stable hard-deny categorization. +- Merge both policies into one matcher: rejected for now to keep policy modules + independently testable. + +**Rationale:** This keeps a strict deny floor while preserving diagnosable, +deterministic denial reasons. + +## Risks / Trade-offs + +- **[Risk] Key fallback could broaden approval-gated scope unexpectedly** -> + Mitigation: explicit spec scenarios for matcher-key override and base-key + fallback; add policy tests for both branches. +- **[Risk] Retry-path reordering may miss existing one-time checks** -> + Mitigation: add executor and pipeline tests that assert no reprompt on the + immediate retry but prompt on later calls. +- **[Risk] Deny precedence can hide secondary violations** -> Mitigation: + preserve first-deny reason in user result and audit log while keeping + independent tests for both operation and resource deny paths. + +## Migration Plan + +1. Update approval-mode resolution logic and unit tests. +2. Update executor retry matching order to use filtered unapproved patterns. +3. Update shell policy composition checks and denial reason assertions. +4. Update OpenSpec delta scenarios and run targeted test suites. + +Rollback is straightforward: revert this change set to restore prior composition +behavior. + +## Open Questions + +- None for this scope; behavior is constrained to composition clarifications and + regression coverage. diff --git a/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/proposal.md b/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/proposal.md new file mode 100644 index 000000000..048c527f8 --- /dev/null +++ b/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/proposal.md @@ -0,0 +1,50 @@ +## Why + +Tool approval behavior in the shipped pipeline has three composition gaps that can +produce surprising security outcomes: control-plane file mutations can miss +intended approval overrides, approve-once retries can reprompt because matching +uses pre-filter patterns, and shell policy layering between operation-level and +resource-level hard denies is under-specified. This follow-up closes those gaps +to keep approval gates deterministic and auditable under PRD-002 security +constraints. + +## What Changes + +- Define deterministic approval-mode key precedence for matcher-derived keys + (for example `file_write:control-plane`) versus base tool keys (`file_write`) + and default mode fallback. +- Align approve-once retry matching with the same filtered unapproved pattern + set that was shown to the user in the prompt, including path-aware matcher + patterns for control-plane file mutations. +- Clarify shell policy composition so operation hard-deny and resource hard-deny + are both enforced with explicit precedence and denial reasons. +- Add targeted behavior scenarios in the capability spec for key precedence, + approve-once retry behavior, and shell deny composition. +- In scope: tool approval composition and requirement/test updates in the + existing approval capability. +- Out of scope: new approval UX options, non-tool interaction types, sandbox + shell implementation, and broad ACL model redesign. + +## Capabilities + +### New Capabilities + +- None. + +### Modified Capabilities + +- `tool-approval-gates`: Refine approval key resolution precedence, + approve-once retry matching semantics, and shell hard-deny composition + semantics; extend normative scenarios for these behaviors. + +## Impact + +- **Security / policy surface**: `ToolAccessPolicy`, `DispatchingToolExecutor`, + matcher implementations, and shell deny checks gain explicit composition + rules (PRD-002: SEC-003, SEC-006, SEC-009). +- **Behavioral consistency**: Approval prompts and immediate retries use the + same pattern identity set, reducing false reprompts. +- **Operational clarity**: Deny reason precedence is documented for actor logs, + tool audit entries, and troubleshooting. +- **Validation impact**: Update capability scenarios and matching tests in + approval gate and executor suites; no config schema changes expected. diff --git a/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/specs/tool-approval-gates/spec.md b/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/specs/tool-approval-gates/spec.md new file mode 100644 index 000000000..13573677a --- /dev/null +++ b/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/specs/tool-approval-gates/spec.md @@ -0,0 +1,182 @@ +## 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`. + +Approval mode resolution SHALL use deterministic precedence: + +1. Matcher-derived approval-mode key override (for example + `file_write:control-plane`) +2. Base tool key override (for example `file_write`) +3. Matcher fail-closed behavior for Personal audience +4. Audience `DefaultMode` + +Runtime audience defaults SHALL NOT implicitly place `shell_execute` in +`Approval` mode. Instead, the init-generated Personal config SHALL explicitly +write `ApprovalPolicy.ToolOverrides.shell_execute = Approval` as the +recommended shell-safe default. + +#### 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** the system checks the approval cache before execution +- **AND** if the command pattern is not approved, an approval prompt is emitted + +#### Scenario: Tool in Auto mode executes without approval + +- **GIVEN** a tool whose 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 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** the system checks approval cache and may prompt +- **AND** when a Team session invokes `shell_execute` +- **THEN** the system denies immediately without prompting + +#### Scenario: Matcher-specific override key takes precedence over base tool key + +- **GIVEN** `ApprovalPolicy.ToolOverrides.file_write = Auto` +- **AND** `ApprovalPolicy.ToolOverrides.file_write:control-plane = Approval` +- **WHEN** the agent invokes `file_write` targeting a control-plane path +- **THEN** the resolved mode is `Approval` +- **AND** the call is approval-gated unless already approved for that path pattern + +#### Scenario: Base tool key applies when matcher-specific key is absent + +- **GIVEN** `ApprovalPolicy.ToolOverrides.file_write = Approval` +- **AND** no override exists for `file_write:control-plane` +- **WHEN** the agent invokes `file_write` targeting a control-plane path +- **THEN** the resolved mode is `Approval` +- **AND** mode resolution does NOT fall directly to `DefaultMode` + +### Requirement: Configurable hard deny list + +The system SHALL enforce shell hard-deny composition across both +operation-level hard deny and resource-level hard deny: + +- **Operation hard-deny**: command intent patterns evaluated by + `ShellCommandPolicy` (for example self-destructive/system-destructive verbs) +- **Resource hard-deny**: protected-path checks evaluated by `ToolPathPolicy` + +Shell execution SHALL be denied when either hard-deny layer matches. Operation +hard-deny SHALL be evaluated first and SHALL short-circuit result reason when it +matches. Denied commands SHALL never be approvable. The system SHALL ship with +sensible defaults: commands that kill the Netclaw daemon process, `rm -rf /`, +`rm -rf ~/`, and fork bombs. Operators SHALL be able to add or remove operation +hard-deny patterns via configuration. + +#### Scenario: Hard-denied command blocked before approval + +- **GIVEN** a command matching the hard deny list (e.g., `netclaw daemon stop`) +- **WHEN** the agent invokes `shell_execute` with that command +- **THEN** the command is denied with reason `hard_deny_self_destructive` +- **AND** no approval prompt is offered +- **AND** the denial is logged + +#### Scenario: Hard deny enforced even in HostAllowed mode + +- **GIVEN** `ShellMode` is `HostAllowed` (no approval config) +- **WHEN** the agent runs a hard-denied command +- **THEN** the command is still blocked + +#### Scenario: Operator adds custom hard deny pattern + +- **GIVEN** the operator adds `docker rm` to the hard deny list in config +- **WHEN** the agent runs `docker rm my-container` +- **THEN** the command is denied + +#### Scenario: Compound command with hard-denied segment + +- **GIVEN** a compound command `git add . && netclaw daemon stop` +- **WHEN** the agent invokes `shell_execute` +- **THEN** the entire command is denied because one segment matches hard deny + +#### Scenario: Operation hard-deny reason takes precedence over resource deny + +- **GIVEN** a shell command matches both operation and resource deny checks +- **WHEN** the command is evaluated +- **THEN** operation hard-deny is applied first +- **AND** the surfaced deny reason is the operation hard-deny reason + +#### Scenario: Resource hard-deny blocks when operation hard-deny does not match + +- **GIVEN** a shell command does not match operation hard-deny patterns +- **AND** the command references a protected file path +- **WHEN** the command is executed +- **THEN** execution is denied by resource hard-deny + +### Requirement: Persistent approval storage + +The system SHALL store persistent approvals ("Approve Always" decisions) in +`~/.netclaw/config/tool-approvals.json`, separate from `netclaw.json`. The file +SHALL NOT be monitored by `ConfigWatcherService`. The file SHALL contain +per-audience sections with per-tool approval lists. For shell, the lists SHALL +contain command patterns. For other tools, approval SHALL be tool-level or +matcher-pattern-level as defined by that matcher. The file SHALL be read at +startup and written immediately on "Approve Always" decisions. + +The retry path for "Approve Once" SHALL match against the filtered unapproved +pattern set presented in the approval prompt, not against pre-filter pattern +candidates. + +#### Scenario: Approve Always persists to file + +- **GIVEN** the user clicks "Approve Always" for pattern `git push` +- **WHEN** the approval is processed +- **THEN** `git push` is added to the Personal shell_execute list in + `tool-approvals.json` +- **AND** the daemon does NOT restart + +#### Scenario: Persistent approvals loaded at startup + +- **GIVEN** `tool-approvals.json` contains `{"personal":{"shell_execute":["git push"]}}` +- **WHEN** the daemon starts +- **THEN** `git push` is pre-approved for Personal audience shell commands + +#### Scenario: Approve Once is retry-scoped only + +- **GIVEN** the user clicks "Approve Once" for pattern `docker build` +- **WHEN** the approval is processed +- **THEN** the blocked `docker build` call is retried immediately +- **AND** a later `docker build` call in the same session prompts again +- **AND** `tool-approvals.json` is NOT modified + +#### Scenario: Approve Once retry uses filtered unapproved patterns + +- **GIVEN** a command yields candidate patterns where some are already approved +- **AND** the prompt shows only filtered unapproved patterns +- **WHEN** the user selects "Approve Once" +- **THEN** the immediate retry succeeds without a second prompt for that call +- **AND** the one-time bypass checks only the filtered unapproved set + +#### Scenario: Approve Once for control-plane file path is path-scoped + +- **GIVEN** `file_write` on `.netclaw/tooling/AGENTS.md` prompts with matcher + pattern `file_write:control-plane:.netclaw/tooling/AGENTS.md` +- **WHEN** the user selects "Approve Once" +- **THEN** the blocked retry for that same path proceeds without reprompt +- **AND** a subsequent control-plane write to a different path prompts again +- **AND** no persistent approval file entry is written + +#### Scenario: Approve For This Chat is session-scoped only + +- **GIVEN** the user clicks "Approve For This Chat" for pattern `docker build` +- **WHEN** the approval is processed +- **THEN** `docker build` is approved for the current session only +- **AND** `tool-approvals.json` is NOT modified diff --git a/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/tasks.md b/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/tasks.md new file mode 100644 index 000000000..d3b2bcb94 --- /dev/null +++ b/openspec/changes/archive/2026-04-12-tool-approval-composition-fixes/tasks.md @@ -0,0 +1,24 @@ +## 1. Approval Mode Resolution Precedence + +- [x] 1.1 Update `ToolAccessPolicy.ResolveApprovalMode` to resolve matcher-derived override key before base tool key and then fall back to fail-closed/default mode +- [x] 1.2 Add policy tests for control-plane file mutation precedence (`file_write:control-plane` override beats `file_write`) +- [x] 1.3 Add policy tests for base-key fallback when matcher-specific override is absent + +## 2. Approve-Once Retry Matching Alignment + +- [x] 2.1 Update `DispatchingToolExecutor` approval flow so one-time bypass checks run against the filtered unapproved pattern set returned by `IToolApprovalService` +- [x] 2.2 Preserve approve-once scope to immediate retry only (no persistent writes, no broader session cache) +- [x] 2.3 Add executor and pipeline tests verifying approve-once does not reprompt on immediate retry but prompts on a later invocation +- [x] 2.4 Add path-aware tests for control-plane file mutation approve-once matching (same path bypass, different path reprompt) + +## 3. Shell Hard-Deny Composition + +- [x] 3.1 Codify shell deny composition order (operation hard-deny before resource hard-deny) with explicit deny reason precedence +- [x] 3.2 Add tests for operation-first precedence when both deny categories match +- [x] 3.3 Add tests for resource hard-deny when operation hard-deny does not match + +## 4. Spec and Validation Sync + +- [x] 4.1 Update `openspec/changes/tool-approval-composition-fixes/specs/tool-approval-gates/spec.md` scenarios as implemented +- [x] 4.2 Run targeted test suites for tool approval policy/executor/pipeline and shell deny behavior +- [x] 4.3 Run `dotnet slopwatch analyze` and address any new violations diff --git a/openspec/specs/tool-approval-gates/spec.md b/openspec/specs/tool-approval-gates/spec.md index fc19c9f3c..2c9f26f9f 100644 --- a/openspec/specs/tool-approval-gates/spec.md +++ b/openspec/specs/tool-approval-gates/spec.md @@ -16,12 +16,20 @@ approval capability. 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` (no -approval required). Runtime audience defaults SHALL NOT implicitly place -`shell_execute` in `Approval` mode. Instead, the init-generated Personal config -SHALL explicitly write -`ApprovalPolicy.ToolOverrides.shell_execute = Approval` as the recommended -shell-safe default. +overrides in `ToolOverrides`. + +Approval mode resolution SHALL use deterministic precedence: + +1. Matcher-derived approval-mode key override (for example + `file_write:control-plane`) +2. Base tool key override (for example `file_write`) +3. Matcher fail-closed behavior for Personal audience +4. Audience `DefaultMode` + +Runtime audience defaults SHALL NOT implicitly place `shell_execute` in +`Approval` mode. Instead, the init-generated Personal config SHALL explicitly +write `ApprovalPolicy.ToolOverrides.shell_execute = Approval` as the +recommended shell-safe default. #### Scenario: Shell requires approval in init-generated Personal config @@ -52,13 +60,37 @@ shell-safe default. - **AND** when a Team session invokes `shell_execute` - **THEN** the system denies immediately without prompting +#### Scenario: Matcher-specific override key takes precedence over base tool key + +- **GIVEN** `ApprovalPolicy.ToolOverrides.file_write = Auto` +- **AND** `ApprovalPolicy.ToolOverrides.file_write:control-plane = Approval` +- **WHEN** the agent invokes `file_write` targeting a control-plane path +- **THEN** the resolved mode is `Approval` +- **AND** the call is approval-gated unless already approved for that path pattern + +#### Scenario: Base tool key applies when matcher-specific key is absent + +- **GIVEN** `ApprovalPolicy.ToolOverrides.file_write = Approval` +- **AND** no override exists for `file_write:control-plane` +- **WHEN** the agent invokes `file_write` targeting a control-plane path +- **THEN** the resolved mode is `Approval` +- **AND** mode resolution does NOT fall directly to `DefaultMode` + ### Requirement: Configurable hard deny list -The system SHALL enforce a configurable hard deny list of command patterns that -are blocked before the approval gate is consulted. Denied commands SHALL never -be approvable. The system SHALL ship with sensible defaults: commands that kill -the Netclaw daemon process, `rm -rf /`, `rm -rf ~/`, and fork bombs. Operators -SHALL be able to add or remove patterns via configuration. +The system SHALL enforce shell hard-deny composition across both +operation-level hard deny and resource-level hard deny: + +- **Operation hard-deny**: command intent patterns evaluated by + `ShellCommandPolicy` (for example self-destructive/system-destructive verbs) +- **Resource hard-deny**: protected-path checks evaluated by `ToolPathPolicy` + +Shell execution SHALL be denied when either hard-deny layer matches. Operation +hard-deny SHALL be evaluated first and SHALL short-circuit result reason when it +matches. Denied commands SHALL never be approvable. The system SHALL ship with +sensible defaults: commands that kill the Netclaw daemon process, `rm -rf /`, +`rm -rf ~/`, and fork bombs. Operators SHALL be able to add or remove operation +hard-deny patterns via configuration. #### Scenario: Hard-denied command blocked before approval @@ -86,6 +118,20 @@ SHALL be able to add or remove patterns via configuration. - **WHEN** the agent invokes `shell_execute` - **THEN** the entire command is denied because one segment matches hard deny +#### Scenario: Operation hard-deny reason takes precedence over resource deny + +- **GIVEN** a shell command matches both operation and resource deny checks +- **WHEN** the command is evaluated +- **THEN** operation hard-deny is applied first +- **AND** the surfaced deny reason is the operation hard-deny reason + +#### Scenario: Resource hard-deny blocks when operation hard-deny does not match + +- **GIVEN** a shell command does not match operation hard-deny patterns +- **AND** the command references a protected file path +- **WHEN** the command is executed +- **THEN** execution is denied by resource hard-deny + ### Requirement: Shell command pattern matching The system SHALL extract verb-chain prefix patterns from shell commands using @@ -226,9 +272,13 @@ The system SHALL store persistent approvals ("Approve Always" decisions) in `~/.netclaw/config/tool-approvals.json`, separate from `netclaw.json`. The file SHALL NOT be monitored by `ConfigWatcherService`. The file SHALL contain per-audience sections with per-tool approval lists. For shell, the lists SHALL -contain command patterns. For other tools, approval SHALL be tool-level -(`true`). The file SHALL be read at startup and written immediately on "Approve -Always" decisions. +contain command patterns. For other tools, approval SHALL be tool-level or +matcher-pattern-level as defined by that matcher. The file SHALL be read at +startup and written immediately on "Approve Always" decisions. + +The retry path for "Approve Once" SHALL match against the filtered unapproved +pattern set presented in the approval prompt, not against pre-filter pattern +candidates. #### Scenario: Approve Always persists to file @@ -252,13 +302,29 @@ Always" decisions. - **AND** a later `docker build` call in the same session prompts again - **AND** `tool-approvals.json` is NOT modified +#### Scenario: Approve Once retry uses filtered unapproved patterns + +- **GIVEN** a command yields candidate patterns where some are already approved +- **AND** the prompt shows only filtered unapproved patterns +- **WHEN** the user selects "Approve Once" +- **THEN** the immediate retry succeeds without a second prompt for that call +- **AND** the one-time bypass checks only the filtered unapproved set + +#### Scenario: Approve Once for control-plane file path is path-scoped + +- **GIVEN** `file_write` on `.netclaw/tooling/AGENTS.md` prompts with matcher + pattern `file_write:control-plane:.netclaw/tooling/AGENTS.md` +- **WHEN** the user selects "Approve Once" +- **THEN** the blocked retry for that same path proceeds without reprompt +- **AND** a subsequent control-plane write to a different path prompts again +- **AND** no persistent approval file entry is written + #### Scenario: Approve For This Chat is session-scoped only - **GIVEN** the user clicks "Approve For This Chat" for pattern `docker build` - **WHEN** the approval is processed - **THEN** `docker build` is approved for the current session only - **AND** `tool-approvals.json` is NOT modified -- **AND** a new session will prompt for `docker build` again ### Requirement: Channel approval capability diff --git a/src/Netclaw.Actors.Tests/Tools/DispatchingToolExecutorTests.cs b/src/Netclaw.Actors.Tests/Tools/DispatchingToolExecutorTests.cs index 28cc91811..a75cdb782 100644 --- a/src/Netclaw.Actors.Tests/Tools/DispatchingToolExecutorTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/DispatchingToolExecutorTests.cs @@ -501,6 +501,170 @@ public async Task One_time_approval_bypasses_policy_for_matching_shell_patterns( Assert.Contains("bypass", retryResult, StringComparison.OrdinalIgnoreCase); } + [Fact] + public async Task One_time_approval_bypasses_policy_for_path_aware_file_patterns() + { + var controlPlaneRoot = Path.Combine(Path.GetTempPath(), $"netclaw-control-plane-{Guid.NewGuid():N}"); + var targetPath = Path.Combine(controlPlaneRoot, "netclaw.json"); + var secondPath = Path.Combine(controlPlaneRoot, "devices.json"); + Directory.CreateDirectory(controlPlaneRoot); + + try + { + var config = new ToolConfig { ShellMode = ShellExecutionMode.HostAllowed }; + config.AudienceProfiles.Personal.ApprovalPolicy = new ToolApprovalConfig + { + ToolOverrides = new Dictionary(StringComparer.Ordinal) + { + ["shell_execute"] = ToolApprovalMode.Approval + } + }; + + var registry = new ToolRegistry(); + registry.WithFirstPartyTools(config); + + var executor = new DispatchingToolExecutor( + registry, + new ToolAccessPolicy( + config, + new EffectivePolicyDefaults( + DeploymentPosture.Personal, + TrustAudience.Personal, + ShellExecutionMode.HostAllowed, + UsedStrictFallback: false), + fileApprovalMatcher: new FilePathApprovalMatcher(controlPlaneRoot))); + + var toolCall = new FunctionCallContent( + "call-file-approve-once-bypass", + "file_write", + new Dictionary + { + ["Path"] = targetPath, + ["Content"] = "approved once" + }); + + var context = new Netclaw.Tools.ToolExecutionContext("signalr/thread-1", null) + { + Audience = TrustAudience.Personal.ToWireValue(), + Boundary = SecurityPolicyDefaults.TrustedInstanceBoundary, + ChannelType = "signalr", + SupportsInteractiveApproval = true + }; + + var firstAttempt = await Assert.ThrowsAsync(() => + executor.ExecuteAsync(toolCall, context, TestContext.Current.CancellationToken)); + + context.OneTimeApprovedToolName = toolCall.Name; + context.SetOneTimeApprovedPatterns(firstAttempt.ApprovalContext.UnapprovedPatterns); + + var retryResult = await executor.ExecuteAsync(toolCall, context, TestContext.Current.CancellationToken); + Assert.Contains("Successfully wrote", retryResult, StringComparison.Ordinal); + Assert.True(File.Exists(targetPath)); + + var secondCall = new FunctionCallContent( + "call-file-approve-once-bypass-second", + "file_write", + new Dictionary + { + ["Path"] = secondPath, + ["Content"] = "different path" + }); + + await Assert.ThrowsAsync(() => + executor.ExecuteAsync(secondCall, context, TestContext.Current.CancellationToken)); + + context.OneTimeApprovedToolName = null; + context.SetOneTimeApprovedPatterns([]); + + await Assert.ThrowsAsync(() => + executor.ExecuteAsync(toolCall, context, TestContext.Current.CancellationToken)); + } + finally + { + if (Directory.Exists(controlPlaneRoot)) + Directory.Delete(controlPlaneRoot, recursive: true); + } + } + + [Fact] + public async Task One_time_approval_uses_filtered_unapproved_patterns_on_retry() + { + var config = new ToolConfig { ShellMode = ShellExecutionMode.HostAllowed }; + config.AudienceProfiles.Personal.ApprovalPolicy = new ToolApprovalConfig + { + ToolOverrides = new Dictionary(StringComparer.Ordinal) + { + ["shell_execute"] = ToolApprovalMode.Approval + } + }; + + var registry = new ToolRegistry(); + registry.WithFirstPartyTools(config); + + var system = ActorSystem.Create($"tool-approval-filtered-once-{Guid.NewGuid():N}"); + try + { + var approvalActor = system.ActorOf(ToolApprovalActor.CreateProps(), "tool-approval"); + var approvalService = new AkkaToolApprovalService(new StubRequiredActor(approvalActor)); + var executor = new DispatchingToolExecutor( + registry, + new ToolAccessPolicy( + config, + new EffectivePolicyDefaults( + DeploymentPosture.Personal, + TrustAudience.Personal, + ShellExecutionMode.HostAllowed, + UsedStrictFallback: false)), + approvalService); + + var context = new Netclaw.Tools.ToolExecutionContext("signalr/thread-filtered", null) + { + Audience = TrustAudience.Personal.ToWireValue(), + Boundary = SecurityPolicyDefaults.TrustedInstanceBoundary, + ChannelType = "signalr", + SupportsInteractiveApproval = true + }; + + await approvalService.RecordApprovalAsync( + "signalr/thread-filtered", + TrustAudience.Personal, + "shell_execute", + ["pwd"], + persistent: false, + TestContext.Current.CancellationToken); + + var call = new FunctionCallContent( + "call-filtered-once", + "shell_execute", + new Dictionary + { + ["Command"] = "pwd && ls" + }); + + var firstAttempt = await Assert.ThrowsAsync(() => + executor.ExecuteAsync(call, context, TestContext.Current.CancellationToken)); + + Assert.DoesNotContain("pwd", firstAttempt.ApprovalContext.UnapprovedPatterns); + Assert.Contains("ls", firstAttempt.ApprovalContext.UnapprovedPatterns); + + context.OneTimeApprovedToolName = call.Name; + context.SetOneTimeApprovedPatterns(firstAttempt.ApprovalContext.UnapprovedPatterns); + + var retryResult = await executor.ExecuteAsync(call, context, TestContext.Current.CancellationToken); + Assert.Contains("Exit code: 0", retryResult, StringComparison.Ordinal); + + context.OneTimeApprovedToolName = null; + context.SetOneTimeApprovedPatterns([]); + + await Assert.ThrowsAsync(() => + executor.ExecuteAsync(call, context, TestContext.Current.CancellationToken)); + } + finally + { + await system.Terminate(); + } + } + [Fact] public async Task Session_approval_allows_same_session_but_not_different_session() { diff --git a/src/Netclaw.Actors.Tests/Tools/ShellToolTests.cs b/src/Netclaw.Actors.Tests/Tools/ShellToolTests.cs index aff1ce1ae..ac8ca1ca3 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellToolTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ShellToolTests.cs @@ -203,4 +203,23 @@ public async Task Hard_deny_checked_before_path_policy() Assert.Contains("hard deny policy", result); Assert.DoesNotContain("protected file path", result); } + + [Fact] + public async Task Path_policy_still_blocks_sensitive_paths_when_command_is_not_hard_denied() + { + var commandPolicy = new ShellCommandPolicy(); + var pathPolicy = new ToolPathPolicy(["/home/user/.netclaw/config/secrets.json"]); + var tool = new ShellTool(new ToolConfig(), pathPolicy, commandPolicy); + + var args = new Dictionary + { + ["Command"] = "cat /home/user/.netclaw/config/secrets.json" + }; + + var result = await tool.ExecuteAsync(args, CancellationToken.None); + + Assert.DoesNotContain("hard deny policy", result); + Assert.Contains("protected file path", result); + Assert.Contains("Access denied", result); + } } diff --git a/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs b/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs index 827f86de4..377678c03 100644 --- a/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs @@ -155,4 +155,190 @@ public void Hard_denied_shell_command_is_blocked_before_approval() Assert.False(decision.NeedsApproval); Assert.Equal("hard_deny_self_destructive", decision.DenyReason); } + + private const string ControlPlaneRoot = "/home/user/.netclaw/config"; + + private static ToolAccessPolicy CreateFileWritePolicy(ToolApprovalConfig? approvalPolicy = null) + { + var config = new ToolConfig(); + config.AudienceProfiles.Personal.ApprovalPolicy = approvalPolicy; + return new ToolAccessPolicy( + config, + new EffectivePolicyDefaults( + DeploymentPosture.Personal, + TrustAudience.Personal, + ShellExecutionMode.HostAllowed, + UsedStrictFallback: false), + fileApprovalMatcher: new FilePathApprovalMatcher(ControlPlaneRoot)); + } + + private static INetclawTool FileWriteToolInstance() => new FileWriteTool(); + private static INetclawTool FileEditToolInstance() => new FileEditTool(); + + [Fact] + public void file_write_to_netclaw_json_requires_approval_under_fail_closed_default() + { + var policy = CreateFileWritePolicy(approvalPolicy: null); + var args = new Dictionary + { + ["Path"] = ControlPlaneRoot + "/netclaw.json", + ["Content"] = "{}" + }; + + var decision = policy.AuthorizeInvocation(FileWriteToolInstance(), PersonalContext(), args); + + Assert.True(decision.NeedsApproval); + Assert.NotNull(decision.ApprovalContext); + Assert.Contains( + decision.ApprovalContext!.UnapprovedPatterns, + p => p.StartsWith("file_write:control-plane:", StringComparison.Ordinal)); + } + + [Fact] + public void file_write_to_control_plane_still_requires_approval_when_policy_exists_without_override() + { + var approvalPolicy = new ToolApprovalConfig + { + ToolOverrides = new Dictionary(StringComparer.Ordinal) + { + ["shell_execute"] = ToolApprovalMode.Approval + } + }; + var policy = CreateFileWritePolicy(approvalPolicy); + var args = new Dictionary + { + ["Path"] = ControlPlaneRoot + "/netclaw.json", + ["Content"] = "{}" + }; + + var decision = policy.AuthorizeInvocation(FileWriteToolInstance(), PersonalContext(), args); + + Assert.True(decision.NeedsApproval); + Assert.NotNull(decision.ApprovalContext); + Assert.Contains( + decision.ApprovalContext!.UnapprovedPatterns, + p => p.StartsWith("file_write:control-plane:", StringComparison.Ordinal)); + } + + [Fact] + public void file_write_to_non_control_plane_path_auto_approves_under_null_policy() + { + var policy = CreateFileWritePolicy(approvalPolicy: null); + var args = new Dictionary + { + ["Path"] = "/tmp/scratch.txt", + ["Content"] = "hello" + }; + + var decision = policy.AuthorizeInvocation(FileWriteToolInstance(), PersonalContext(), args); + + Assert.True(decision.Allowed); + Assert.False(decision.NeedsApproval); + } + + [Fact] + public void file_edit_of_netclaw_json_requires_approval() + { + var policy = CreateFileWritePolicy(approvalPolicy: null); + var args = new Dictionary + { + ["Path"] = ControlPlaneRoot + "/netclaw.json", + ["OldString"] = "a", + ["NewString"] = "b" + }; + + var decision = policy.AuthorizeInvocation(FileEditToolInstance(), PersonalContext(), args); + + Assert.True(decision.NeedsApproval); + Assert.Contains( + decision.ApprovalContext!.UnapprovedPatterns, + p => p.StartsWith("file_edit:control-plane:", StringComparison.Ordinal)); + } + + [Fact] + public void file_write_emits_distinct_per_path_patterns() + { + var matcher = new FilePathApprovalMatcher(ControlPlaneRoot); + var netclawJson = matcher.ExtractPatterns("file_write", + new Dictionary { ["Path"] = ControlPlaneRoot + "/netclaw.json" }); + var toolApprovals = matcher.ExtractPatterns("file_write", + new Dictionary { ["Path"] = ControlPlaneRoot + "/tool-approvals.json" }); + var devices = matcher.ExtractPatterns("file_write", + new Dictionary { ["Path"] = ControlPlaneRoot + "/devices.json" }); + + Assert.NotEqual(netclawJson[0], toolApprovals[0]); + Assert.NotEqual(netclawJson[0], devices[0]); + Assert.NotEqual(toolApprovals[0], devices[0]); + } + + [Fact] + public void file_write_control_plane_approval_honors_explicit_auto_override() + { + var approvalPolicy = new ToolApprovalConfig + { + ToolOverrides = new Dictionary(StringComparer.Ordinal) + { + ["file_write:control-plane"] = ToolApprovalMode.Auto + } + }; + var policy = CreateFileWritePolicy(approvalPolicy); + var args = new Dictionary + { + ["Path"] = ControlPlaneRoot + "/netclaw.json", + ["Content"] = "{}" + }; + + var decision = policy.AuthorizeInvocation(FileWriteToolInstance(), PersonalContext(), args); + + Assert.True(decision.Allowed); + Assert.False(decision.NeedsApproval); + } + + [Fact] + public void file_write_control_plane_override_takes_precedence_over_base_tool_override() + { + var approvalPolicy = new ToolApprovalConfig + { + ToolOverrides = new Dictionary(StringComparer.Ordinal) + { + ["file_write"] = ToolApprovalMode.Auto, + ["file_write:control-plane"] = ToolApprovalMode.Approval + } + }; + var policy = CreateFileWritePolicy(approvalPolicy); + var args = new Dictionary + { + ["Path"] = ControlPlaneRoot + "/netclaw.json", + ["Content"] = "{}" + }; + + var decision = policy.AuthorizeInvocation(FileWriteToolInstance(), PersonalContext(), args); + + Assert.True(decision.NeedsApproval); + Assert.NotNull(decision.ApprovalContext); + } + + [Fact] + public void file_write_control_plane_falls_back_to_base_tool_override_when_specific_override_missing() + { + var approvalPolicy = new ToolApprovalConfig + { + DefaultMode = ToolApprovalMode.Approval, + ToolOverrides = new Dictionary(StringComparer.Ordinal) + { + ["file_write"] = ToolApprovalMode.Auto + } + }; + var policy = CreateFileWritePolicy(approvalPolicy); + var args = new Dictionary + { + ["Path"] = ControlPlaneRoot + "/netclaw.json", + ["Content"] = "{}" + }; + + var decision = policy.AuthorizeInvocation(FileWriteToolInstance(), PersonalContext(), args); + + Assert.True(decision.Allowed); + Assert.False(decision.NeedsApproval); + } } diff --git a/src/Netclaw.Actors/Tools/DispatchingToolExecutor.cs b/src/Netclaw.Actors/Tools/DispatchingToolExecutor.cs index 4a0d56d21..248e50c78 100644 --- a/src/Netclaw.Actors/Tools/DispatchingToolExecutor.cs +++ b/src/Netclaw.Actors/Tools/DispatchingToolExecutor.cs @@ -53,39 +53,6 @@ public async Task ExecuteAsync(FunctionCallContent toolCall, ToolExecuti return $"Unknown tool: {toolCall.Name}"; } - if (context is not null - && string.Equals(context.OneTimeApprovedToolName, toolCall.Name, StringComparison.Ordinal) - && IsOneTimeApprovalSatisfied(context, toolCall)) - { - _logger.LogInformation( - "Applying one-time approval bypass for tool {ToolName} in session {SessionId}", - toolCall.Name, - context.SessionId ?? "unknown"); - - var swBypass = Stopwatch.StartNew(); - try - { - var bypassResult = await tool.ExecuteAsync(toolCall.Arguments, context, ct); - swBypass.Stop(); - _logger.LogInformation( - "Tool executed via one-time approval bypass: {ToolName} ({Duration}ms, {ResultLength} chars)", - toolCall.Name, - swBypass.ElapsedMilliseconds, - bypassResult.Length); - return bypassResult; - } - catch (Exception ex) - { - swBypass.Stop(); - _logger.LogError( - ex, - "Tool execution failed via one-time approval bypass: {ToolName} ({Duration}ms)", - toolCall.Name, - swBypass.ElapsedMilliseconds); - throw; - } - } - var accessDecision = _policy.AuthorizeInvocation(tool, context, toolCall.Arguments); if (accessDecision.NeedsApproval && _approvalService is not null) @@ -116,6 +83,17 @@ public async Task ExecuteAsync(FunctionCallContent toolCall, ToolExecuti } } + if (accessDecision.NeedsApproval + && context is not null + && IsOneTimeApprovalSatisfied(context, toolCall, accessDecision.ApprovalContext)) + { + _logger.LogInformation( + "Applying one-time approval bypass for tool {ToolName} in session {SessionId}", + toolCall.Name, + context.SessionId ?? "unknown"); + accessDecision = ToolAccessDecision.Allow(); + } + if (accessDecision.NeedsApproval) { _logger.LogInformation("Tool requires approval: {ToolName}", toolCall.Name); @@ -152,19 +130,23 @@ public async Task ExecuteAsync(FunctionCallContent toolCall, ToolExecuti } } - private static bool IsOneTimeApprovalSatisfied(ToolExecutionContext context, FunctionCallContent toolCall) + private static bool IsOneTimeApprovalSatisfied( + ToolExecutionContext context, + FunctionCallContent toolCall, + ToolApprovalContext? approvalContext) { + if (approvalContext is null) + return false; + if (context.OneTimeApprovedPatterns.Count == 0) return false; - if (!string.Equals(toolCall.Name, "shell_execute", StringComparison.Ordinal)) - return context.OneTimeApprovedPatterns.Contains(toolCall.Name); + if (approvalContext.UnapprovedPatterns.Count == 0) + return false; - var matcher = ShellApprovalMatcher.Instance; - var commandPatterns = matcher.ExtractPatterns(toolCall.Name, toolCall.Arguments); - if (commandPatterns.Count == 0) + if (!string.Equals(context.OneTimeApprovedToolName, toolCall.Name, StringComparison.Ordinal)) return false; - return commandPatterns.All(pattern => context.OneTimeApprovedPatterns.Contains(pattern)); + return approvalContext.UnapprovedPatterns.All(pattern => context.OneTimeApprovedPatterns.Contains(pattern)); } } diff --git a/src/Netclaw.Actors/Tools/FileEditTool.cs b/src/Netclaw.Actors/Tools/FileEditTool.cs index cf2f07bc0..5cc07320a 100644 --- a/src/Netclaw.Actors/Tools/FileEditTool.cs +++ b/src/Netclaw.Actors/Tools/FileEditTool.cs @@ -10,11 +10,13 @@ namespace Netclaw.Actors.Tools; /// Makes targeted text replacements in an existing file without rewriting the entire file. /// Matches literal text (not regex). Fails if OldString is not found or is ambiguous. /// -[NetclawTool("file_edit", +[NetclawTool(ToolName, "Make targeted text replacements in an existing file without rewriting the entire file", Grant = "file")] public sealed partial class FileEditTool : NetclawTool { + public const string ToolName = "file_edit"; + private readonly ToolPathPolicy? _pathPolicy; private readonly ScopedFileAccessPolicy _fileAccessPolicy; @@ -54,7 +56,7 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon return accessError; if (_pathPolicy?.IsDenied(authorizedPath) == true) - return "Error: Access denied — this file is protected by security policy."; + return FileToolErrors.ControlPlaneWriteDenied(authorizedPath); if (!File.Exists(authorizedPath)) return $"Error: File not found: {authorizedPath}"; diff --git a/src/Netclaw.Actors/Tools/FilePathApprovalMatcher.cs b/src/Netclaw.Actors/Tools/FilePathApprovalMatcher.cs new file mode 100644 index 000000000..30c855f2a --- /dev/null +++ b/src/Netclaw.Actors/Tools/FilePathApprovalMatcher.cs @@ -0,0 +1,140 @@ +using Netclaw.Security; + +namespace Netclaw.Actors.Tools; + +/// +/// Argument-aware approval matcher for the file_write and file_edit +/// tools. Routes writes under a configured control-plane root to a distinct +/// approval-mode key so those invocations can be gated without requiring +/// approval for every ordinary file write. +/// +public sealed class FilePathApprovalMatcher : IToolApprovalMatcher +{ + public const string ControlPlaneModeKeySuffix = ":control-plane"; + + private readonly string _controlPlaneRoot; + + public FilePathApprovalMatcher(string controlPlaneRoot) + { + _controlPlaneRoot = NormalizePath(controlPlaneRoot); + } + + public string GetApprovalModeKey(string toolName, IDictionary? arguments) + { + return TryGetControlPlaneRelativePath(arguments, out _) + ? toolName + ControlPlaneModeKeySuffix + : toolName; + } + + public bool IsFailClosedOnPersonal(string toolName, IDictionary? arguments) + => TryGetControlPlaneRelativePath(arguments, out _); + + public IReadOnlyList ExtractPatterns(string toolName, IDictionary? arguments) + { + if (TryGetControlPlaneRelativePath(arguments, out var relativePath)) + return [toolName + ControlPlaneModeKeySuffix + ":" + relativePath]; + + return [toolName]; + } + + public bool IsApproved(string toolName, IDictionary? arguments, IEnumerable approvedPatterns) + { + var patterns = ExtractPatterns(toolName, arguments); + foreach (var pattern in patterns) + { + var matched = false; + foreach (var approved in approvedPatterns) + { + if (string.Equals(pattern, approved, StringComparison.OrdinalIgnoreCase)) + { + matched = true; + break; + } + } + + if (!matched) + return false; + } + + return true; + } + + public string FormatForDisplay(string toolName, IDictionary? arguments) + { + if (TryGetPath(arguments, out var path)) + return $"{toolName}: {path}"; + + return toolName; + } + + private bool TryGetControlPlaneRelativePath( + IDictionary? arguments, + out string relativePath) + { + relativePath = string.Empty; + + if (!TryGetPath(arguments, out var rawPath)) + return false; + + if (!TryNormalizePath(rawPath, out var normalized)) + return false; + + if (!IsUnderRoot(normalized, _controlPlaneRoot)) + return false; + + relativePath = Path.GetRelativePath(_controlPlaneRoot, normalized) + .Replace(Path.DirectorySeparatorChar, '/'); + return true; + } + + private static bool TryGetPath(IDictionary? arguments, out string path) + { + path = string.Empty; + if (arguments is null) + return false; + + if (arguments.TryGetValue("Path", out var value) || arguments.TryGetValue("path", out value)) + { + if (value is string s && !string.IsNullOrWhiteSpace(s)) + { + path = s; + return true; + } + } + + return false; + } + + private static bool TryNormalizePath(string rawPath, out string normalized) + { + normalized = string.Empty; + try + { + var baseDir = Environment.CurrentDirectory; + var combined = Path.IsPathRooted(rawPath) + ? rawPath + : Path.Combine(baseDir, rawPath); + normalized = NormalizePath(combined); + return true; + } + catch + { + return false; + } + } + + private static string NormalizePath(string path) + => Path.GetFullPath(path).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + + private static bool IsUnderRoot(string candidate, string root) + { + if (!candidate.StartsWith(root, StringComparison.OrdinalIgnoreCase)) + return false; + + if (candidate.Length == root.Length) + return true; + + var boundary = candidate[root.Length]; + return boundary == Path.DirectorySeparatorChar || boundary == Path.AltDirectorySeparatorChar; + } +} diff --git a/src/Netclaw.Actors/Tools/FileReadTool.cs b/src/Netclaw.Actors/Tools/FileReadTool.cs index 783e6ee2b..f64d905f9 100644 --- a/src/Netclaw.Actors/Tools/FileReadTool.cs +++ b/src/Netclaw.Actors/Tools/FileReadTool.cs @@ -43,8 +43,8 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon if (!_fileAccessPolicy.TryResolveReadPath(args.Path, context, out var authorizedPath, out var accessError)) return accessError; - if (_pathPolicy?.IsDenied(authorizedPath) == true) - return "Error: Access denied — this file is protected by security policy."; + if (_pathPolicy?.IsReadDenied(authorizedPath) == true) + return FileToolErrors.CredentialReadDenied(authorizedPath); if (!File.Exists(authorizedPath)) return $"Error: File not found: {authorizedPath}"; diff --git a/src/Netclaw.Actors/Tools/FileToolErrors.cs b/src/Netclaw.Actors/Tools/FileToolErrors.cs new file mode 100644 index 000000000..fefd8f448 --- /dev/null +++ b/src/Netclaw.Actors/Tools/FileToolErrors.cs @@ -0,0 +1,14 @@ +namespace Netclaw.Actors.Tools; + +internal static class FileToolErrors +{ + public static string ControlPlaneWriteDenied(string path) + => $"Error: Access denied: '{path}' is part of Netclaw's control plane " + + "(secrets, keys, database, or lifecycle files) and cannot be modified by agent tools, " + + "even with approval. If the user wants this change, ask them to run a dedicated command " + + "(e.g. `netclaw doctor --fix`, `netclaw secrets set`) or edit the file directly."; + + public static string CredentialReadDenied(string path) + => $"Error: Access denied: '{path}' contains credentials or keys " + + "and cannot be read by agent tools."; +} diff --git a/src/Netclaw.Actors/Tools/FileWriteTool.cs b/src/Netclaw.Actors/Tools/FileWriteTool.cs index 1e4b526ac..4c46cd39b 100644 --- a/src/Netclaw.Actors/Tools/FileWriteTool.cs +++ b/src/Netclaw.Actors/Tools/FileWriteTool.cs @@ -9,11 +9,13 @@ namespace Netclaw.Actors.Tools; /// /// Writes content to a file as UTF-8, creating parent directories if needed. /// -[NetclawTool("file_write", +[NetclawTool(ToolName, "Write content to a file, creating parent directories if needed", Grant = "file")] public sealed partial class FileWriteTool : NetclawTool { + public const string ToolName = "file_write"; + private readonly ToolPathPolicy? _pathPolicy; private readonly ScopedFileAccessPolicy _fileAccessPolicy; @@ -45,7 +47,7 @@ protected override async Task ExecuteAsync(Params args, ToolExecutionCon return accessError; if (_pathPolicy?.IsDenied(authorizedPath) == true) - return "Error: Access denied — this file is protected by security policy."; + return FileToolErrors.ControlPlaneWriteDenied(authorizedPath); try { diff --git a/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs b/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs index fa7ba915b..19cb9c852 100644 --- a/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs +++ b/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs @@ -13,13 +13,19 @@ public sealed class ToolAccessPolicy private readonly EffectivePolicyDefaults _defaults; private readonly ToolAudienceProfileResolver _profileResolver; private readonly ShellCommandPolicy? _shellCommandPolicy; + private readonly IToolApprovalMatcher _fileApprovalMatcher; - public ToolAccessPolicy(ToolConfig toolConfig, EffectivePolicyDefaults defaults, ShellCommandPolicy? shellCommandPolicy = null) + public ToolAccessPolicy( + ToolConfig toolConfig, + EffectivePolicyDefaults defaults, + ShellCommandPolicy? shellCommandPolicy = null, + IToolApprovalMatcher? fileApprovalMatcher = null) { _toolConfig = toolConfig; _defaults = defaults; _profileResolver = new ToolAudienceProfileResolver(toolConfig); _shellCommandPolicy = shellCommandPolicy; + _fileApprovalMatcher = fileApprovalMatcher ?? DefaultApprovalMatcher.Instance; } public IReadOnlyList FilterExposedTools( @@ -87,7 +93,7 @@ public ToolAccessDecision AuthorizeInvocation( return ToolAccessDecision.Deny("tool_not_allowed_for_audience_profile"); if (!IsShellTool(tool)) - return CheckApprovalGate(tool.Name, context, arguments, DefaultApprovalMatcher.Instance); + return CheckApprovalGate(tool.Name, context, arguments, SelectMatcherForTool(tool.Name)); var shellMode = ResolveShellMode(); if (shellMode == ShellExecutionMode.Off) @@ -134,8 +140,14 @@ private ToolAccessDecision CheckApprovalGate( var audience = ResolveAudience(context); var profile = ToolAudienceProfileDefaults.GetResolvedProfile(_toolConfig.AudienceProfiles, audience); var approvalPolicy = profile.ApprovalPolicy; - var mode = approvalPolicy?.GetEffectiveMode(toolName) - ?? GetMissingApprovalPolicyDefaultMode(toolName, audience); + var approvalModeKey = matcher.GetApprovalModeKey(toolName, arguments); + var mode = ResolveApprovalMode( + approvalPolicy, + approvalModeKey, + toolName, + arguments, + audience, + matcher); if (mode == ToolApprovalMode.Deny) return ToolAccessDecision.Deny("tool_denied_by_approval_policy"); @@ -162,16 +174,55 @@ private ToolAccessDecision CheckApprovalGate( return ToolAccessDecision.RequiresApproval(approvalContext); } - private static ToolApprovalMode GetMissingApprovalPolicyDefaultMode(string toolName, TrustAudience audience) + private static ToolApprovalMode GetMissingApprovalPolicyDefaultMode( + string toolName, + IDictionary? arguments, + TrustAudience audience, + IToolApprovalMatcher matcher) { - // Fail-closed for personal shell: if the approval policy was omitted, - // still require interactive approval rather than silently auto-approving. - if (audience == TrustAudience.Personal && string.Equals(toolName, ShellTool.ToolName, StringComparison.Ordinal)) + if (audience == TrustAudience.Personal && matcher.IsFailClosedOnPersonal(toolName, arguments)) return ToolApprovalMode.Approval; return ToolApprovalMode.Auto; } + private static ToolApprovalMode ResolveApprovalMode( + ToolApprovalConfig? approvalPolicy, + string approvalModeKey, + string toolName, + IDictionary? arguments, + TrustAudience audience, + IToolApprovalMatcher matcher) + { + if (approvalPolicy is null) + return GetMissingApprovalPolicyDefaultMode(toolName, arguments, audience, matcher); + + if (approvalPolicy.ToolOverrides.TryGetValue(approvalModeKey, out var mode)) + return mode; + + if (!string.Equals(approvalModeKey, toolName, StringComparison.Ordinal) + && approvalPolicy.ToolOverrides.TryGetValue(toolName, out mode)) + { + return mode; + } + + if (audience == TrustAudience.Personal && matcher.IsFailClosedOnPersonal(toolName, arguments)) + return ToolApprovalMode.Approval; + + return approvalPolicy.DefaultMode; + } + + private IToolApprovalMatcher SelectMatcherForTool(string toolName) + { + if (string.Equals(toolName, FileWriteTool.ToolName, StringComparison.Ordinal) + || string.Equals(toolName, FileEditTool.ToolName, StringComparison.Ordinal)) + { + return _fileApprovalMatcher; + } + + return DefaultApprovalMatcher.Instance; + } + private ShellExecutionMode ResolveShellMode() => _toolConfig.ShellMode ?? _defaults.ShellExecutionMode; diff --git a/src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json b/src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json index c9a4950fd..09c151a57 100644 --- a/src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json +++ b/src/Netclaw.Configuration/Schemas/netclaw-config.v1.schema.json @@ -608,7 +608,7 @@ }, "ToolOverrides": { "type": "object", - "description": "Per-tool approval mode overrides. Keys are tool names (e.g., shell_execute, mcp:server:tool).", + "description": "Per-tool approval mode overrides. Keys are tool names (e.g., shell_execute, mcp:server:tool) or matcher-specific keys such as file_write:control-plane and file_edit:control-plane.", "additionalProperties": { "type": "string", "enum": ["Auto", "Approval", "Deny"] diff --git a/src/Netclaw.Daemon/Program.cs b/src/Netclaw.Daemon/Program.cs index 46a75713d..ade69c6ce 100644 --- a/src/Netclaw.Daemon/Program.cs +++ b/src/Netclaw.Daemon/Program.cs @@ -551,14 +551,43 @@ static void ConfigureDaemonServices( .Get() ?? new SearchConfig(); var searchBackend = CreateSearchBackend(searchConfig); - // Tool path deny-list: prevent agent tools from accessing secrets - var toolPathPolicy = new ToolPathPolicy([paths.SecretsPath, paths.WebhooksDirectory, paths.KeysDirectory]); + var writeDenyList = new[] + { + paths.SecretsPath, + paths.KeysDirectory, + paths.SqliteDbPath, + paths.PidFilePath, + paths.LockFilePath, + paths.RestartManifestPath, + }; + var readDenyList = new[] + { + paths.SecretsPath, + paths.KeysDirectory, + paths.WebhooksDirectory, + }; + var shellIndicatorList = new[] + { + paths.SecretsPath, + paths.WebhooksDirectory, + paths.KeysDirectory, + paths.SqliteDbPath, + paths.PidFilePath, + paths.LockFilePath, + paths.RestartManifestPath, + }; + var toolPathPolicy = new ToolPathPolicy(writeDenyList, readDenyList, shellIndicatorList); services.AddSingleton(toolPathPolicy); var shellCommandPolicy = new ShellCommandPolicy(toolConfig.HardDenyPatterns); services.AddSingleton(shellCommandPolicy); - var toolAccessPolicy = new ToolAccessPolicy(toolConfig, effectivePolicyDefaults, shellCommandPolicy); + var fileApprovalMatcher = new FilePathApprovalMatcher(paths.ConfigDirectory); + var toolAccessPolicy = new ToolAccessPolicy( + toolConfig, + effectivePolicyDefaults, + shellCommandPolicy, + fileApprovalMatcher); services.AddSingleton(toolAccessPolicy); var toolApprovalStore = new ToolApprovalStore(paths.ToolApprovalsPath); diff --git a/src/Netclaw.Security.Tests/ToolPathPolicyTests.cs b/src/Netclaw.Security.Tests/ToolPathPolicyTests.cs index a43b22847..975843a7e 100644 --- a/src/Netclaw.Security.Tests/ToolPathPolicyTests.cs +++ b/src/Netclaw.Security.Tests/ToolPathPolicyTests.cs @@ -142,4 +142,153 @@ public void CommandReferencesDeniedPath_detects_high_risk_archive_of_config_dire Assert.True(policy.CommandReferencesDeniedPath("tar czf /tmp/netclaw-config.tgz ~/.netclaw/config")); } + + private static ToolPathPolicy CreateProductionPolicy() + { + var writeDeny = new[] + { + "/home/user/.netclaw/config/secrets.json", + "/home/user/.netclaw/keys", + "/home/user/.netclaw/netclaw.db", + "/home/user/.netclaw/netclaw.pid", + "/home/user/.netclaw/netclaw.lock", + "/home/user/.netclaw/cache/restart-manifest.json", + }; + var readDeny = new[] + { + "/home/user/.netclaw/config/secrets.json", + "/home/user/.netclaw/keys", + "/home/user/.netclaw/config/webhooks", + }; + var shellIndicators = new[] + { + "/home/user/.netclaw/config/secrets.json", + "/home/user/.netclaw/config/webhooks", + "/home/user/.netclaw/keys", + "/home/user/.netclaw/netclaw.db", + "/home/user/.netclaw/netclaw.pid", + "/home/user/.netclaw/netclaw.lock", + "/home/user/.netclaw/cache/restart-manifest.json", + }; + return new ToolPathPolicy(writeDeny, readDeny, shellIndicators); + } + + [Fact] + public void IsDenied_blocks_sqlite_db_when_listed() + { + var policy = CreateProductionPolicy(); + Assert.True(policy.IsDenied("/home/user/.netclaw/netclaw.db")); + } + + [Fact] + public void IsDenied_blocks_pid_and_lock_files() + { + var policy = CreateProductionPolicy(); + Assert.True(policy.IsDenied("/home/user/.netclaw/netclaw.pid")); + Assert.True(policy.IsDenied("/home/user/.netclaw/netclaw.lock")); + } + + [Fact] + public void IsDenied_blocks_restart_manifest() + { + var policy = CreateProductionPolicy(); + Assert.True(policy.IsDenied("/home/user/.netclaw/cache/restart-manifest.json")); + } + + [Fact] + public void IsDenied_allows_writes_to_netclaw_config_json_so_approval_gate_can_fire() + { + var policy = CreateProductionPolicy(); + Assert.False(policy.IsDenied("/home/user/.netclaw/config/netclaw.json")); + Assert.False(policy.IsDenied("/home/user/.netclaw/config/devices.json")); + Assert.False(policy.IsDenied("/home/user/.netclaw/config/tool-approvals.json")); + Assert.False(policy.IsDenied("/home/user/.netclaw/config/mcp-oauth-metadata.json")); + } + + [Fact] + public void IsDenied_allows_writes_to_identity_directory() + { + var policy = CreateProductionPolicy(); + Assert.False(policy.IsDenied("/home/user/.netclaw/identity/SOUL.md")); + Assert.False(policy.IsDenied("/home/user/.netclaw/identity/AGENTS.md")); + } + + [Fact] + public void IsDenied_allows_writes_to_skills_directory() + { + var policy = CreateProductionPolicy(); + Assert.False(policy.IsDenied("/home/user/.netclaw/skills/my-skill/SKILL.md")); + } + + [Fact] + public void IsDenied_allows_writes_to_arbitrary_user_paths() + { + var policy = CreateProductionPolicy(); + Assert.False(policy.IsDenied("/tmp/foo.json")); + Assert.False(policy.IsDenied("/home/user/Documents/notes.txt")); + } + + [Fact] + public void IsReadDenied_blocks_secrets_json() + { + var policy = CreateProductionPolicy(); + Assert.True(policy.IsReadDenied("/home/user/.netclaw/config/secrets.json")); + } + + [Fact] + public void IsReadDenied_blocks_keys_directory_children() + { + var policy = CreateProductionPolicy(); + Assert.True(policy.IsReadDenied("/home/user/.netclaw/keys/keyring.xml")); + } + + [Fact] + public void IsReadDenied_blocks_webhook_configs() + { + var policy = CreateProductionPolicy(); + Assert.True(policy.IsReadDenied("/home/user/.netclaw/config/webhooks/github-issues.json")); + } + + [Fact] + public void IsReadDenied_allows_netclaw_json() + { + var policy = CreateProductionPolicy(); + Assert.False(policy.IsReadDenied("/home/user/.netclaw/config/netclaw.json")); + } + + [Fact] + public void IsReadDenied_allows_netclaw_db() + { + var policy = CreateProductionPolicy(); + Assert.False(policy.IsReadDenied("/home/user/.netclaw/netclaw.db")); + } + + [Fact] + public void CommandReferencesDeniedPath_still_allows_ls_of_config_directory() + { + // Regression guard: directory-scoped writeDeny entries must not bleed + // into the shell substring indicator set, otherwise every shell command + // whose text contains ".netclaw/config" would be rejected. + var policy = CreateProductionPolicy(); + Assert.False(policy.CommandReferencesDeniedPath("ls ~/.netclaw/config")); + Assert.False(policy.CommandReferencesDeniedPath("stat ~/.netclaw/config")); + } + + [Fact] + public void CommandReferencesDeniedPath_still_blocks_cat_of_secrets_json() + { + var policy = CreateProductionPolicy(); + Assert.True(policy.CommandReferencesDeniedPath("cat ~/.netclaw/config/secrets.json")); + } + + [Fact] + public void CommandReferencesDeniedPath_blocks_control_plane_lifecycle_files() + { + var policy = CreateProductionPolicy(); + + Assert.True(policy.CommandReferencesDeniedPath("cat ~/.netclaw/netclaw.db")); + Assert.True(policy.CommandReferencesDeniedPath("cat ~/.netclaw/netclaw.pid")); + Assert.True(policy.CommandReferencesDeniedPath("cat ~/.netclaw/netclaw.lock")); + Assert.True(policy.CommandReferencesDeniedPath("cat ~/.netclaw/cache/restart-manifest.json")); + } } diff --git a/src/Netclaw.Security/IToolApprovalMatcher.cs b/src/Netclaw.Security/IToolApprovalMatcher.cs index 2df2dcb25..17de58c5b 100644 --- a/src/Netclaw.Security/IToolApprovalMatcher.cs +++ b/src/Netclaw.Security/IToolApprovalMatcher.cs @@ -7,6 +7,24 @@ namespace Netclaw.Security; /// public interface IToolApprovalMatcher { + /// + /// Returns the key used to look up this invocation's approval mode in + /// ToolApprovalConfig.ToolOverrides. Most matchers return the tool + /// name unchanged; argument-aware matchers may return a context-specific + /// key so different invocations of the same tool (e.g., a write to a + /// control-plane file vs. a write to a user file) can be gated + /// independently. + /// + string GetApprovalModeKey(string toolName, IDictionary? arguments); + + /// + /// Returns true if this invocation must require interactive approval on + /// the Personal audience when no explicit approval policy is configured. + /// Encapsulates the fail-closed decision so callers do not have to inspect + /// tool names or approval-key string formats. + /// + bool IsFailClosedOnPersonal(string toolName, IDictionary? arguments); + /// /// Extracts the intent-level pattern from a tool call's arguments. /// For shell: verb-chain prefix (e.g., "git push" from "git push origin main"). @@ -33,6 +51,12 @@ public sealed class ShellApprovalMatcher : IToolApprovalMatcher { public static readonly ShellApprovalMatcher Instance = new(); + public string GetApprovalModeKey(string toolName, IDictionary? arguments) + => toolName; + + public bool IsFailClosedOnPersonal(string toolName, IDictionary? arguments) + => true; + public IReadOnlyList ExtractPatterns(string toolName, IDictionary? arguments) { var command = GetCommand(arguments); @@ -108,6 +132,12 @@ public sealed class DefaultApprovalMatcher : IToolApprovalMatcher { public static readonly DefaultApprovalMatcher Instance = new(); + public string GetApprovalModeKey(string toolName, IDictionary? arguments) + => toolName; + + public bool IsFailClosedOnPersonal(string toolName, IDictionary? arguments) + => false; + public IReadOnlyList ExtractPatterns(string toolName, IDictionary? arguments) { return [toolName]; diff --git a/src/Netclaw.Security/ToolPathPolicy.cs b/src/Netclaw.Security/ToolPathPolicy.cs index 7a070ff4c..bbcc780d0 100644 --- a/src/Netclaw.Security/ToolPathPolicy.cs +++ b/src/Netclaw.Security/ToolPathPolicy.cs @@ -2,11 +2,20 @@ namespace Netclaw.Security; /// /// Evaluates whether a file path is denied for agent tool access. -/// Used to prevent the LLM from reading/writing sensitive files like secrets.json. /// +/// +/// Three independent deny surfaces: write (), read +/// (), and shell indicators +/// (). The shell indicator list must +/// stay narrow — file-level only — because that path does a raw substring scan +/// against the command text, so directory-scoped entries would block legitimate +/// commands whose arguments happen to contain the directory name. +/// public sealed class ToolPathPolicy { - private readonly HashSet _deniedPaths; + private readonly HashSet _writeDeniedPaths; + private readonly HashSet _readDeniedPaths; + private readonly HashSet _shellDeniedPaths; private readonly HashSet _commandIndicators; private static readonly HashSet HighRiskVerbs = new(StringComparer.OrdinalIgnoreCase) { @@ -17,41 +26,78 @@ public sealed class ToolPathPolicy public ToolPathPolicy(IEnumerable deniedPaths) { - var paths = deniedPaths.ToList(); - var normalizedPaths = paths.Select(NormalizePath).ToList(); + var materialized = deniedPaths.ToList(); + _writeDeniedPaths = BuildNormalizedSet(materialized); + _readDeniedPaths = _writeDeniedPaths; + _shellDeniedPaths = _writeDeniedPaths; + _commandIndicators = BuildCommandIndicators(materialized); + } - _deniedPaths = new HashSet(normalizedPaths, StringComparer.OrdinalIgnoreCase); - _commandIndicators = new HashSet(StringComparer.OrdinalIgnoreCase); + public ToolPathPolicy( + IEnumerable writeDeniedPaths, + IEnumerable readDeniedPaths, + IEnumerable shellIndicatorPaths) + { + _writeDeniedPaths = BuildNormalizedSet(writeDeniedPaths); + _readDeniedPaths = BuildNormalizedSet(readDeniedPaths); + var shellList = shellIndicatorPaths.ToList(); + _shellDeniedPaths = BuildNormalizedSet(shellList); + _commandIndicators = BuildCommandIndicators(shellList); + } - foreach (var path in paths.Concat(normalizedPaths)) + private static HashSet BuildNormalizedSet(IEnumerable paths) + { + var normalized = paths.Select(NormalizePath); + return new HashSet(normalized, StringComparer.OrdinalIgnoreCase); + } + + private static HashSet BuildCommandIndicators(IEnumerable paths) + { + var materialized = paths.ToList(); + var normalizedPaths = materialized.Select(NormalizePath).ToList(); + var indicators = new HashSet(StringComparer.OrdinalIgnoreCase); + + foreach (var path in materialized.Concat(normalizedPaths)) { var slashPath = path.Replace('\\', '/'); - _commandIndicators.Add(slashPath); + indicators.Add(slashPath); var netclawSegmentIdx = slashPath.IndexOf("/.netclaw/", StringComparison.OrdinalIgnoreCase); if (netclawSegmentIdx >= 0) - _commandIndicators.Add(slashPath[(netclawSegmentIdx + 1)..]); + indicators.Add(slashPath[(netclawSegmentIdx + 1)..]); var fileName = Path.GetFileName(path); if (!string.IsNullOrWhiteSpace(fileName) && fileName.Contains('.', StringComparison.Ordinal)) - _commandIndicators.Add(fileName); + indicators.Add(fileName); } + + return indicators; } /// - /// Returns true if the given path is denied by policy. + /// Returns true if the given path is denied for write by policy. /// Normalizes the path (resolves "..", removes trailing separators) before checking. /// public bool IsDenied(string path) + => IsDeniedAgainst(path, _writeDeniedPaths); + + /// + /// Returns true if the given path is denied for read by policy. Narrower + /// than : only covers files that leak credentials. + /// + public bool IsReadDenied(string path) + => IsDeniedAgainst(path, _readDeniedPaths); + + private static bool IsDeniedAgainst(string path, HashSet deniedSet) { if (string.IsNullOrWhiteSpace(path)) return false; - if (TryNormalizePath(path, null, out var normalized) && IsDeniedNormalized(normalized)) + if (TryNormalizePath(path, null, out var normalized) && IsDeniedNormalized(normalized, deniedSet)) return true; return TryResolveSymlinkTarget(path, out var resolvedTarget) - && IsDeniedNormalized(resolvedTarget); + && IsDeniedNormalized(resolvedTarget, deniedSet); } /// @@ -79,7 +125,7 @@ public bool CommandReferencesDeniedPath(string command, string? workingDirectory var expanded = ExpandHomeAndEnv(token); if (TryNormalizePath(expanded, workingDirectory, out var normalized) - && IsDeniedNormalized(normalized)) + && IsDeniedNormalized(normalized, _shellDeniedPaths)) { return true; } @@ -117,9 +163,9 @@ private static bool ContainsHighRiskVerb(IEnumerable tokens) return false; } - private bool IsDeniedNormalized(string candidate) + private static bool IsDeniedNormalized(string candidate, HashSet deniedSet) { - foreach (var denied in _deniedPaths) + foreach (var denied in deniedSet) { if (IsSamePathOrChild(candidate, denied)) return true;