Skip to content

Commit 3a7e14f

Browse files
AkiKurisuclaude
andauthored
fix(core): honor thread-level RequireApprovalOutsideWorkspace in core tools (#215)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 1a7c74c commit 3a7e14f

10 files changed

Lines changed: 336 additions & 13 deletions

File tree

docs/developing/configuration.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,11 +228,10 @@ For Anthropic-compatible providers, `anthropicMessageContent` can declare how Do
228228
| Field | Description | Default |
229229
|---|---|---|
230230
| `Security.BlacklistedPaths` | Paths the agent must not access; subpaths are also checked | `[]` |
231-
| `Tools.File.RequireApprovalOutsideWorkspace` | Approve file ops outside workspace | `true` |
231+
| `Tools.File.RequireApprovalOutsideWorkspace` | Approve file and shell ops outside workspace; `false` blocks them | `true` |
232232
| `Tools.File.MaxFileSize` | Max readable file size in bytes | `10485760` |
233233
| `Tools.File.RipgrepPath` | Optional `rg` path; empty tries `DOTCRAFT_RG_PATH`, `PATH`, then fallback | `""` |
234234
| `Tools.File.SearchTimeoutSeconds` | Max `GrepFiles` content-search time before timeout | `30` |
235-
| `Tools.Shell.RequireApprovalOutsideWorkspace` | Approve shell commands outside workspace | `true` |
236235
| `Tools.Shell.Timeout` | Shell timeout in seconds | `300` |
237236
| `Tools.Shell.MaxOutputLength` | Max shell output length in characters | `10000` |
238237
| `Tools.Shell.Background.Enabled` | Enable background terminal sessions | `true` |
@@ -287,7 +286,6 @@ Personal local hardening example:
287286
"RequireApprovalOutsideWorkspace": true
288287
},
289288
"Shell": {
290-
"RequireApprovalOutsideWorkspace": true,
291289
"Timeout": 300
292290
}
293291
}

docs/zh/developing/configuration.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,11 +227,10 @@ Deep-thinking adapter 文件:
227227
| 配置项 | 说明 | 默认值 |
228228
|---|---|---|
229229
| `Security.BlacklistedPaths` | Agent 绝不能访问的路径,子路径也会接受检查 | `[]` |
230-
| `Tools.File.RequireApprovalOutsideWorkspace` | 工作区外文件操作是否需要审批 | `true` |
230+
| `Tools.File.RequireApprovalOutsideWorkspace` | 工作区外文件与 Shell 操作是否需要审批,`false` 时直接拒绝 | `true` |
231231
| `Tools.File.MaxFileSize` | 最大可读取文件大小(字节) | `10485760` |
232232
| `Tools.File.RipgrepPath` | 可选 `rg` 路径。为空时依次尝试 `DOTCRAFT_RG_PATH``PATH` 和内置回退 | `""` |
233233
| `Tools.File.SearchTimeoutSeconds` | `GrepFiles` 内容搜索最长运行时间,超时后返回超时结果 | `30` |
234-
| `Tools.Shell.RequireApprovalOutsideWorkspace` | 工作区外 Shell 命令是否需要审批 | `true` |
235234
| `Tools.Shell.Timeout` | Shell 命令超时时间(秒) | `300` |
236235
| `Tools.Shell.MaxOutputLength` | Shell 命令最大输出长度(字符) | `10000` |
237236
| `Tools.Shell.Background.Enabled` | 是否启用后台终端会话 | `true` |
@@ -286,7 +285,6 @@ Deep-thinking adapter 文件:
286285
"RequireApprovalOutsideWorkspace": true
287286
},
288287
"Shell": {
289-
"RequireApprovalOutsideWorkspace": true,
290288
"Timeout": 300
291289
}
292290
}

specs/architecture/session-core.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2113,7 +2113,7 @@ Approval-related fields are normative:
21132113
- `ApprovalPolicy = interrupt` means any approval-gated operation is rejected without prompting; the active tool receives the rejection and the turn may continue.
21142114
- `RequireApprovalOutsideWorkspace = true` allows outside-workspace file or shell operations to proceed through the approval service.
21152115
- `RequireApprovalOutsideWorkspace = false` rejects outside-workspace file or shell operations without prompting.
2116-
- `RequireApprovalOutsideWorkspace = null` falls back to the workspace-level defaults in `AppConfig.Tools.File` and `AppConfig.Tools.Shell`.
2116+
- `RequireApprovalOutsideWorkspace = null` falls back to the workspace-level default in `AppConfig.Tools.File`, which governs both file and shell operations.
21172117

21182118
When a thread is created or its configuration changes, Session Core recreates the effective agent/tool set from that configuration.
21192119

src/DotCraft.Core/Agents/Runtime/AgentFactory.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,8 @@ private static ToolPlanningContext CreateHostPlanningContext(
473473
profile: null,
474474
providerCapabilities: context.CurrentThreadSource?.SubAgent is null ? [] : ["subagent-child"],
475475
revision: 1,
476-
workspaceRoots: context.WorkspaceRoots);
476+
workspaceRoots: context.WorkspaceRoots,
477+
requireApprovalOutsideWorkspace: context.RequireApprovalOutsideWorkspace);
477478

478479
/// <summary>
479480
/// Creates an AI agent with the specified tools.

src/DotCraft.Core/Sessions/Runtime/SessionService.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5921,7 +5921,8 @@ source is SandboxToolSource
59215921
config.ProviderId,
59225922
config.Model,
59235923
toolContext.WorkspaceRoots,
5924-
config.SubAgentModelCatalogSnapshot);
5924+
config.SubAgentModelCatalogSnapshot,
5925+
config.RequireApprovalOutsideWorkspace);
59255926
var toolSnapshot = await agentFactory.BuildToolSnapshotAsync(
59265927
snapshotSources,
59275928
planningContext,

src/DotCraft.Core/Tools/Architecture/ToolPlanningContracts.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ public ToolPlanningContext(
2020
string? effectiveProviderId = null,
2121
string? effectiveMainModel = null,
2222
IReadOnlyList<string>? workspaceRoots = null,
23-
SubAgentModelCatalogSnapshot? subAgentModelCatalogSnapshot = null)
23+
SubAgentModelCatalogSnapshot? subAgentModelCatalogSnapshot = null,
24+
bool? requireApprovalOutsideWorkspace = null)
2425
{
2526
if (string.IsNullOrWhiteSpace(threadId))
2627
throw new ArgumentException("A thread identifier is required.", nameof(threadId));
@@ -44,6 +45,7 @@ public ToolPlanningContext(
4445
EffectiveProviderId = effectiveProviderId;
4546
EffectiveMainModel = effectiveMainModel;
4647
SubAgentModelCatalogSnapshot = subAgentModelCatalogSnapshot;
48+
RequireApprovalOutsideWorkspace = requireApprovalOutsideWorkspace;
4749
}
4850

4951
/// <summary>Gets the thread identifier.</summary>
@@ -72,4 +74,11 @@ public ToolPlanningContext(
7274
public string? EffectiveMainModel { get; }
7375
/// <summary>Gets the durable SubAgent model catalog frozen for this thread.</summary>
7476
public SubAgentModelCatalogSnapshot? SubAgentModelCatalogSnapshot { get; }
77+
/// <summary>
78+
/// Gets the thread-scoped outside-workspace boundary override.
79+
/// When set, it overrides <c>AppConfig.Tools.File.RequireApprovalOutsideWorkspace</c> for
80+
/// file/shell tool assembly: <see langword="true"/> routes outside-workspace operations
81+
/// through the approval service, <see langword="false"/> rejects them without prompting.
82+
/// </summary>
83+
public bool? RequireApprovalOutsideWorkspace { get; }
7584
}

src/DotCraft.Core/Tools/CoreToolProvider.cs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ protected override string GetDescription(AIFunction function, ToolPlanningContex
7777
};
7878
}
7979

80-
if (!config.Tools.File.RequireApprovalOutsideWorkspace)
80+
if (!RequiresApprovalOutsideWorkspace(context))
8181
return null;
8282

8383
return function.Name switch
@@ -101,6 +101,14 @@ protected override string GetDescription(AIFunction function, ToolPlanningContex
101101
};
102102
}
103103

104+
/// <summary>
105+
/// Resolves the effective outside-workspace boundary policy for one planning context.
106+
/// The thread-scoped override wins over the workspace-level default; false means
107+
/// outside-workspace file/shell operations are rejected without prompting.
108+
/// </summary>
109+
private bool RequiresApprovalOutsideWorkspace(ToolPlanningContext context) =>
110+
context.RequireApprovalOutsideWorkspace ?? config.Tools.File.RequireApprovalOutsideWorkspace;
111+
104112
private object FileApproval(
105113
string targetArgument,
106114
string operation,
@@ -128,7 +136,7 @@ protected override IEnumerable<AIFunction> CreateFunctions(ToolPlanningContext c
128136
return [];
129137

130138
var tools = new List<AIFunction>();
131-
var requireOutside = config.Tools.File.RequireApprovalOutsideWorkspace;
139+
var requireOutside = RequiresApprovalOutsideWorkspace(context);
132140
var fileSearchTimeout = TimeSpan.FromSeconds(Math.Max(1, config.Tools.File.SearchTimeoutSeconds));
133141

134142
var mainRuntime = chatClientRegistry.ResolveMainRuntime(

src/DotCraft.Core/Tools/Sandbox/SandboxToolSource.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ protected override IEnumerable<AIFunction> CreateFunctions(ToolPlanningContext c
8989
backgroundTerminalService: null,
9090
maxConcurrency: config.SubagentMaxConcurrency,
9191
shellTimeout: config.Tools.Shell.Timeout,
92-
requireApprovalOutsideWorkspace: config.Tools.File.RequireApprovalOutsideWorkspace,
92+
requireApprovalOutsideWorkspace: context.RequireApprovalOutsideWorkspace
93+
?? config.Tools.File.RequireApprovalOutsideWorkspace,
9394
reasoningConfig: subAgentPreference?.Reasoning ?? config.Reasoning,
9495
promptCachingConfig: config.PromptCaching,
9596
model: subAgentRuntime.Model,
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
using System.Text.Json.Nodes;
2+
using DotCraft.Configuration;
3+
using DotCraft.Security;
4+
using DotCraft.Skills;
5+
using DotCraft.Tools;
6+
using Xunit;
7+
8+
namespace DotCraft.Tests.Tools;
9+
10+
/// <summary>
11+
/// Verifies that the thread-scoped RequireApprovalOutsideWorkspace override reaches the
12+
/// core file/shell tool assembly. A thread that disables it must hard-reject
13+
/// outside-workspace operations instead of routing them through an (auto-approving)
14+
/// approval service.
15+
/// </summary>
16+
public sealed class CoreToolSourceWorkspaceBoundaryTests : IDisposable
17+
{
18+
private readonly string _tempRoot = Path.Combine(
19+
Path.GetTempPath(), "dotcraft-coretool-boundary-tests", Guid.NewGuid().ToString("N"));
20+
private readonly string _workspace;
21+
private readonly string _outsideFile;
22+
23+
public CoreToolSourceWorkspaceBoundaryTests()
24+
{
25+
_workspace = Path.Combine(_tempRoot, "workspace");
26+
Directory.CreateDirectory(_workspace);
27+
_outsideFile = Path.Combine(_tempRoot, "outside.txt");
28+
File.WriteAllText(_outsideFile, "TOP-PRIVATE-CONTENT");
29+
}
30+
31+
[Fact]
32+
public async Task Thread_override_false_hard_rejects_outside_workspace_read()
33+
{
34+
var registrations = await GetRegistrationsAsync(requireApprovalOutsideWorkspace: false);
35+
var readFile = Assert.Single(registrations, item => item.Definition.Name.Name == "ReadFile");
36+
37+
Assert.False(readFile.Definition.PolicyHints.RequiresApproval);
38+
Assert.False(readFile.Definition.Annotations.ContainsKey("dotcraft/nativeApproval"));
39+
40+
var result = await InvokeAsync(readFile, new JsonObject { ["path"] = _outsideFile });
41+
42+
Assert.Contains("outside workspace", result.Content, StringComparison.OrdinalIgnoreCase);
43+
Assert.DoesNotContain("TOP-PRIVATE-CONTENT", result.Content, StringComparison.Ordinal);
44+
}
45+
46+
[Fact]
47+
public async Task Thread_override_false_hard_rejects_exec_referencing_outside_paths()
48+
{
49+
var registrations = await GetRegistrationsAsync(requireApprovalOutsideWorkspace: false);
50+
var exec = Assert.Single(registrations, item => item.Definition.Name.Name == "Exec");
51+
52+
Assert.False(exec.Definition.PolicyHints.RequiresApproval);
53+
54+
var result = await InvokeAsync(exec, new JsonObject
55+
{
56+
["command"] = $"cat \"{_outsideFile}\""
57+
});
58+
59+
Assert.Contains("outside workspace", result.Content, StringComparison.OrdinalIgnoreCase);
60+
Assert.DoesNotContain("TOP-PRIVATE-CONTENT", result.Content, StringComparison.Ordinal);
61+
}
62+
63+
[Fact]
64+
public async Task Unset_thread_override_keeps_the_workspace_default_approval_routing()
65+
{
66+
var registrations = await GetRegistrationsAsync(requireApprovalOutsideWorkspace: null);
67+
var readFile = Assert.Single(registrations, item => item.Definition.Name.Name == "ReadFile");
68+
var exec = Assert.Single(registrations, item => item.Definition.Name.Name == "Exec");
69+
70+
Assert.True(readFile.Definition.PolicyHints.RequiresApproval);
71+
Assert.True(readFile.Definition.Annotations.ContainsKey("dotcraft/nativeApproval"));
72+
Assert.True(exec.Definition.PolicyHints.RequiresApproval);
73+
Assert.True(exec.Definition.Annotations.ContainsKey("dotcraft/nativeApproval"));
74+
}
75+
76+
public void Dispose()
77+
{
78+
try
79+
{
80+
if (Directory.Exists(_tempRoot))
81+
Directory.Delete(_tempRoot, recursive: true);
82+
}
83+
catch
84+
{
85+
// Best-effort cleanup for temp test directories.
86+
}
87+
}
88+
89+
private async Task<IReadOnlyList<ToolRegistration>> GetRegistrationsAsync(
90+
bool? requireApprovalOutsideWorkspace)
91+
{
92+
var config = AppConfigTestFactory.CreateOpenAI();
93+
Assert.True(config.Tools.File.RequireApprovalOutsideWorkspace);
94+
var skillsLoader = new SkillsLoader(_workspace);
95+
var source = new CoreToolSource(
96+
config,
97+
TestModelProviderRegistry.Create(),
98+
skillsLoader,
99+
new AutoApproveApprovalService(),
100+
new StubBackgroundTerminalService());
101+
return await source.GetRegistrationsAsync(new ToolPlanningContext(
102+
"thread-analyst",
103+
null,
104+
_workspace,
105+
Path.Combine(_workspace, ".craft"),
106+
"agent",
107+
null,
108+
[],
109+
1,
110+
workspaceRoots: [_workspace],
111+
requireApprovalOutsideWorkspace: requireApprovalOutsideWorkspace));
112+
}
113+
114+
private static async Task<ToolExecutionResult> InvokeAsync(
115+
ToolRegistration registration,
116+
JsonObject arguments) =>
117+
await registration.Binding.Runtime.InvokeAsync(
118+
new ToolInvocationContext(
119+
"thread-analyst",
120+
null,
121+
"call-1",
122+
ToolInvocationAudience.Model,
123+
registration.Definition.Name,
124+
registration.Definition.Id,
125+
registration.Binding.Id,
126+
registration.Binding.Revision,
127+
DateTimeOffset.UtcNow),
128+
arguments);
129+
}

0 commit comments

Comments
 (0)