Skip to content

Commit 991eeca

Browse files
wyckitclaude
andauthored
fix(mcp): per-tool forward-arg allowlist for all built-in tools (#19)
Extends the v1.14.0 forward-arg stripping from filesystem-only to EVERY built-in tool, and corrects an over-strip: - ForwardArgAllowlist maps each built-in tool to its full legitimate arg surface: send_email {to,subject,body}, run_command {cmd}, read_calendar {range}, plus the full @modelcontextprotocol/server-filesystem tool set (read_file {path,head,tail}, edit_file {path,edits,dryRun}, search_files {path,pattern,excludePatterns}, etc.). Any arg outside a tool's surface is stripped before forwarding. - Fixes the v1.14.0 blanket fs key-set (path/source/destination/paths/content) which would have dropped legitimate args of richer fs tools (edits, pattern, head/tail, sortBy, excludePatterns) — strictness no longer breaks function. - Custom-mapper tools not in the allowlist are forwarded unchanged (mapper owns the surface). - NormalizeForForward restructured: fs path-canonicalization (under a root) then the per-tool allowlist for any built-in. Tests: non-fs strip (read_calendar), full-surface preservation (search_files keeps pattern, strips unknown). 251 passing + 3 env-gated skipped. Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
1 parent 8aa2152 commit 991eeca

6 files changed

Lines changed: 148 additions & 43 deletions

File tree

CHANGELOG.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,25 @@
33
All notable changes to IntentMesh. Claims are test-backed; see [docs/MATURITY.md](docs/MATURITY.md)
44
for the production-ready / experimental / future breakdown.
55

6+
## v1.14.1 — Per-tool MCP forward-arg allowlist (all built-ins)
7+
8+
Extends (and corrects) the v1.14.0 forward-arg stripping. **251 passing + 3 env-gated skipped.**
9+
10+
- **Strict allowlist for every built-in tool**, not just filesystem: `send_email``{to,subject,body}`,
11+
`run_command``{cmd}`, `read_calendar``{range}`, plus the full `@modelcontextprotocol/server-filesystem`
12+
tool set. Any argument outside a tool's known surface is stripped before forwarding, so an
13+
unrecognized/unchecked key can't be honored by the server.
14+
- **Fixes a v1.14.0 over-strip:** the previous blanket fs key-set (`path/source/destination/paths/content`)
15+
would have dropped legitimate args of richer fs tools (`edit_file`'s `edits`, `search_files`'s `pattern`,
16+
`directory_tree`'s `excludePatterns`, `read_text_file`'s `head/tail`, `list_directory_with_sizes`'s
17+
`sortBy`). Each tool now lists its full legitimate arg surface, so strictness no longer breaks function.
18+
- A custom-mapper tool not in the allowlist is forwarded unchanged (the mapper owns its arg surface).
19+
- New tests cover a non-fs strip (`read_calendar`), full-surface preservation (`search_files` keeps
20+
`pattern`), and the live FS-E2E continues to exercise read/write against the real server.
21+
22+
> The filesystem allowlists track the pinned server version (`@modelcontextprotocol/[email protected]`);
23+
> bump them alongside the package.
24+
625
## v1.14.0 — Audit fidelity, verification & supply-chain hardening (seventh review pass)
726

827
Closes a seventh external review (7 High + 3 Medium). **249 passing + 3 env-gated skipped.**

Directory.Build.props

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
Demos, tools, the web host, the E2E/bench runners, the template, and tests stay non-packable. -->
66

77
<PropertyGroup>
8-
<Version>1.14.0</Version>
8+
<Version>1.14.1</Version>
99
<Authors>Chad Sandor</Authors>
1010
<Company>wyckit</Company>
1111
<Product>IntentMesh</Product>
@@ -18,7 +18,7 @@
1818
<!-- MIT-licensed (SPDX expression in package metadata). -->
1919
<PackageLicenseExpression>MIT</PackageLicenseExpression>
2020
<PackageRequireLicenseAcceptance>false</PackageRequireLicenseAcceptance>
21-
<PackageReleaseNotes>v1.14.0seventh review pass: MCP bundles record applied approvals + strip unknown forward args; granular email/calendar verification; CI requires real FS-MCP + packs before npx; digest-pinned Docker with .dockerignore + writable runs volume; proxy-mode dedicated auth key; explain no longer honors caller approvals. See CHANGELOG.md.</PackageReleaseNotes>
21+
<PackageReleaseNotes>v1.14.1per-tool MCP forward-arg allowlist for ALL built-in tools (send_email/run_command/read_calendar + the full filesystem tool set), replacing the v1.14.0 fs key-set that over-stripped richer tools (edit_file/search_files/etc.). See CHANGELOG.md.</PackageReleaseNotes>
2222

2323
<!-- Reproducible restore: lock files are honored in CI via locked-mode restore. -->
2424
<RestorePackagesWithLockFile>true</RestorePackagesWithLockFile>

README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ dotnet run --project src/IntentMesh.Cli -- --trace "plan my Friday and draft Sar
104104
dotnet run --project src/IntentMesh.Web # then open the printed localhost URL
105105

106106
# tests
107-
dotnet test IntentMesh.slnx # 249 passing (+3 env-gated skipped)
107+
dotnet test IntentMesh.slnx # 251 passing (+3 env-gated skipped)
108108
```
109109

110110
### Wrap your own agent (the SDK on-ramp)
@@ -190,7 +190,7 @@ v1.7 platform:** Phase 1 (clarity) ✓ · Phase 2 (signed artifacts, replay, con
190190
Phase 3 (Control Room v1) ✓ · Phase 4 (IntentBench 25/25) ✓ · Phase 5 (SDK + MCP proxy / OpenAPI
191191
import / real-adapter example) ✓ · Phase 6 (manifesto, whitepaper, landing) ✓. **v1.7** adds the
192192
adoptable platform surface (full-lifecycle SDK + host template, real-LLM-proposer hardening,
193-
operator workflow, audit operations). **249 passing (+3 env-gated skipped) tests · IntentBench 25/25 · TLM 7/7.**
193+
operator workflow, audit operations). **251 passing (+3 env-gated skipped) tests · IntentBench 25/25 · TLM 7/7.**
194194

195195
**Proven vs. experimental vs. future (claims discipline).** [docs/MATURITY.md](docs/MATURITY.md) is
196196
the canonical statement: every *proven* claim has a passing test that would fail if it stopped being
@@ -207,7 +207,7 @@ and the [CHANGELOG](CHANGELOG.md).
207207
## Status
208208

209209
Research prototype with a production-shaped core, **v1.8.0**. Symbolic layer: 7 TLMs, ~125 concepts,
210-
7/7 round-trip verify; typed action contracts across four domains. **xUnit 249 passing (+3 env-gated skipped).** Five demo
210+
7/7 round-trip verify; typed action contracts across four domains. **xUnit 251 passing (+3 env-gated skipped).** Five demo
211211
scenarios. See [docs/MATURITY.md](docs/MATURITY.md) for the proven / experimental / future breakdown.
212212
Delivered beyond v0.1:
213213

docs/MATURITY.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
The single source of truth for **what is production-ready, what is experimental, and what is future
44
work.** Every "proven" claim below is backed by a test that would fail if the claim stopped being
5-
true (`dotnet test IntentMesh.slnx`**249 passing, 3 env-gated skipped**). Nothing here is aspirational unless it says so.
5+
true (`dotnet test IntentMesh.slnx`**251 passing, 3 env-gated skipped**). Nothing here is aspirational unless it says so.
66

77
> IntentMesh is a **research prototype with a production-shaped core**: the security kernel and its
88
> guarantees are proven and stable; the *operational backends* around it (KMS, DB persistence,

src/IntentMesh.Integrations/McpProxy.cs

Lines changed: 72 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -323,53 +323,88 @@ public McpForwardResult GateAndForward(McpToolCall call, IMcpClient client,
323323
return new McpForwardResult(gate, ServerResponse: response);
324324
}
325325

326-
/// <summary>For a filesystem call under an allowed root, rewrite each path-bearing arg to the exact
327-
/// canonical in-root path the gate validated — so the forwarded call can't re-resolve a relative or
328-
/// aliased original arg to a different target than was checked. Non-fs calls (or no allowed root)
329-
/// are forwarded unchanged.</summary>
326+
/// <summary>
327+
/// The EXACT argument keys forwarded to the server for each BUILT-IN tool — every other key is stripped
328+
/// before the call leaves the proxy, so an argument the typed mapping never represented (and the policy
329+
/// never checked) can't be honored unsigned/unchecked by the server. Each entry is the tool's FULL
330+
/// legitimate arg surface (so strictness doesn't break the tool). The filesystem entries track the
331+
/// pinned <c>@modelcontextprotocol/server-filesystem</c> version; bump them with the server. A
332+
/// custom-mapper tool that is NOT listed here is forwarded unchanged — the custom mapper owns its
333+
/// server's arg surface (and is responsible for its own allowlisting).
334+
/// </summary>
335+
private static readonly IReadOnlyDictionary<string, string[]> ForwardArgAllowlist =
336+
new Dictionary<string, string[]>(StringComparer.Ordinal)
337+
{
338+
// Non-filesystem built-ins (small, fully-modeled surfaces).
339+
["send_email"] = new[] { "to", "subject", "body" },
340+
["run_command"] = new[] { "cmd" },
341+
["read_calendar"] = new[] { "range" },
342+
// @modelcontextprotocol/server-filesystem tools.
343+
["read_file"] = new[] { "path", "head", "tail" },
344+
["read_text_file"] = new[] { "path", "head", "tail" },
345+
["read_media_file"] = new[] { "path" },
346+
["read_multiple_files"] = new[] { "paths" },
347+
["get_file_info"] = new[] { "path" },
348+
["list_directory"] = new[] { "path" },
349+
["list_directory_with_sizes"] = new[] { "path", "sortBy" },
350+
["directory_tree"] = new[] { "path", "excludePatterns" },
351+
["search_files"] = new[] { "path", "pattern", "excludePatterns" },
352+
["list_allowed_directories"] = Array.Empty<string>(),
353+
["write_file"] = new[] { "path", "content" },
354+
["edit_file"] = new[] { "path", "edits", "dryRun" },
355+
["create_directory"] = new[] { "path" },
356+
["move_file"] = new[] { "source", "destination" },
357+
};
358+
359+
/// <summary>Normalize a call for forwarding: (1) for a filesystem call under a sandbox root, rewrite
360+
/// each path-bearing arg to the exact canonical in-root path the gate validated (so the server can't
361+
/// re-resolve a relative/aliased original to a different target than was checked); (2) for any built-in
362+
/// tool, strip every argument outside that tool's allowlist so an unrecognized/unchecked key can't be
363+
/// honored. A custom-mapper tool not in the allowlist is forwarded with its args intact.</summary>
330364
private McpToolCall NormalizeForForward(McpToolCall call)
331365
{
332-
if (_allowedRoot is null) return call;
333366
var (action, _) = _customMapper?.Invoke(call) ?? MapToAction(call);
334-
if (action is not (FsReadAction or FsWriteAction)) return call;
335-
336-
var root = Canonicalize(Path.TrimEndingDirectorySeparator(Path.GetFullPath(_allowedRoot)));
337367
var args = new Dictionary<string, string>(call.Args, StringComparer.Ordinal);
338-
bool hadPath = false;
339-
foreach (var key in new[] { "path", "source", "destination" })
340-
if (args.TryGetValue(key, out var p) && !string.IsNullOrEmpty(p)) { args[key] = Resolve(p, root); hadPath = true; }
341368

342-
// Custom-mapper paths: a per-server mapper may carry the path in a NON-standard arg (e.g. "target",
343-
// "filepath"). The typed action already exposes the validated path(s); rewrite whichever raw arg
344-
// holds that value to the exact canonical in-root path, so the forwarded call can't re-resolve a
345-
// relative/aliased original to a different target than the gate checked.
346-
foreach (var typed in TypedPaths(action))
347-
{
348-
var canonical = Resolve(typed, root);
349-
foreach (var key in args.Keys.ToList())
350-
if (!string.IsNullOrEmpty(args[key]) && Resolve(args[key], root) == canonical) { args[key] = canonical; hadPath = true; }
351-
}
352-
// No-path filesystem tool under a sandbox: scope it explicitly to the root rather than letting the
353-
// server fall back to its own working directory (defense in depth over the server's own sandbox).
354-
if (!hadPath && !args.ContainsKey("paths"))
355-
args["path"] = root;
356-
if (args.TryGetValue("paths", out var multi) && !string.IsNullOrWhiteSpace(multi))
369+
if (_allowedRoot is not null && action is FsReadAction or FsWriteAction)
357370
{
358-
try
371+
var root = Canonicalize(Path.TrimEndingDirectorySeparator(Path.GetFullPath(_allowedRoot)));
372+
bool hadPath = false;
373+
foreach (var key in new[] { "path", "source", "destination" })
374+
if (args.TryGetValue(key, out var p) && !string.IsNullOrEmpty(p)) { args[key] = Resolve(p, root); hadPath = true; }
375+
376+
// Custom-mapper paths: a per-server mapper may carry the path in a NON-standard arg (e.g.
377+
// "target"). The typed action exposes the validated path(s); rewrite whichever raw arg holds
378+
// that value to the canonical in-root path.
379+
foreach (var typed in TypedPaths(action))
359380
{
360-
var parsed = JsonSerializer.Deserialize<List<string>>(multi);
361-
if (parsed is not null)
362-
args["paths"] = JsonSerializer.Serialize(parsed.Select(e => string.IsNullOrEmpty(e) ? e : Resolve(e, root)).ToList());
381+
var canonical = Resolve(typed, root);
382+
foreach (var key in args.Keys.ToList())
383+
if (!string.IsNullOrEmpty(args[key]) && Resolve(args[key], root) == canonical) { args[key] = canonical; hadPath = true; }
384+
}
385+
// No-path filesystem tool under a sandbox: scope it explicitly to the root rather than letting
386+
// the server fall back to its own working directory (defense in depth over the server's sandbox).
387+
if (!hadPath && !args.ContainsKey("paths"))
388+
args["path"] = root;
389+
if (args.TryGetValue("paths", out var multi) && !string.IsNullOrWhiteSpace(multi))
390+
{
391+
try
392+
{
393+
var parsed = JsonSerializer.Deserialize<List<string>>(multi);
394+
if (parsed is not null)
395+
args["paths"] = JsonSerializer.Serialize(parsed.Select(e => string.IsNullOrEmpty(e) ? e : Resolve(e, root)).ToList());
396+
}
397+
catch { /* not a JSON array — it was validated as a single path; leave as-is */ }
363398
}
364-
catch { /* not a JSON array — it was validated as a single path; leave as-is */ }
365399
}
366400

367-
// Forward ONLY the recognized filesystem arguments — strip any extra/unknown key so an argument the
368-
// typed action never represented (and the path policy never checked) cannot be honored by the
369-
// server. The forwarded payload is therefore exactly the policy-checked fields.
370-
var allowed = new HashSet<string>(new[] { "path", "source", "destination", "paths", "content" }, StringComparer.Ordinal);
371-
foreach (var key in args.Keys.Where(k => !allowed.Contains(k)).ToList())
372-
args.Remove(key);
401+
// Forward-arg allowlist for every built-in tool — strip anything outside the tool's known surface.
402+
if (ForwardArgAllowlist.TryGetValue(call.Tool, out var allowed))
403+
{
404+
var allowedSet = new HashSet<string>(allowed, StringComparer.Ordinal);
405+
foreach (var key in args.Keys.Where(k => !allowedSet.Contains(k)).ToList())
406+
args.Remove(key);
407+
}
373408

374409
return call with { Args = args };
375410

tests/IntentMesh.Tests/IntegrationTests.cs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,57 @@ public void Filesystem_forward_strips_unknown_args()
508508
finally { Directory.Delete(root, true); }
509509
}
510510

511+
/// <summary>A non-filesystem built-in forward (read_calendar) also carries ONLY its allowlisted args —
512+
/// an extra/unknown key is stripped before the call reaches the server.</summary>
513+
[Fact]
514+
public void Non_filesystem_forward_strips_unknown_args()
515+
{
516+
var root = TempRoot();
517+
try
518+
{
519+
IReadOnlyDictionary<string, string>? forwarded = null;
520+
var client = new CapturingMcpClient(args => { forwarded = args; return "{}"; });
521+
var proxy = new McpProxy(Runtime(), Workspace.CreateDemo(), allowedRoot: root,
522+
auditStore: new FileRunArtifactStore(TempRoot()), auditKeyProvider: McpTestKeyProvider,
523+
approvalService: NewApprovalService(), tenantId: "test");
524+
525+
var fwd = proxy.GateAndForward(
526+
new McpToolCall("read_calendar", new Dictionary<string, string> { ["range"] = "Friday", ["evil"] = "x" }), client);
527+
528+
Assert.True(fwd.Gate.Allowed);
529+
Assert.NotNull(forwarded);
530+
Assert.True(forwarded!.ContainsKey("range")); // the legitimate arg is kept
531+
Assert.False(forwarded.ContainsKey("evil")); // the unknown arg is stripped
532+
}
533+
finally { Directory.Delete(root, true); }
534+
}
535+
536+
/// <summary>The allowlist keeps each tool's FULL legitimate surface — a tool with more args than
537+
/// path/content (search_files: path + pattern) is not over-stripped, while an unknown arg still is.</summary>
538+
[Fact]
539+
public void Forward_allowlist_keeps_a_tools_full_arg_surface()
540+
{
541+
var root = TempRoot();
542+
try
543+
{
544+
IReadOnlyDictionary<string, string>? forwarded = null;
545+
var client = new CapturingMcpClient(args => { forwarded = args; return "[]"; });
546+
var proxy = new McpProxy(Runtime(), Workspace.CreateDemo(), allowedRoot: root,
547+
auditStore: new FileRunArtifactStore(TempRoot()), auditKeyProvider: McpTestKeyProvider,
548+
approvalService: NewApprovalService(), tenantId: "test");
549+
550+
var fwd = proxy.GateAndForward(
551+
new McpToolCall("search_files", new Dictionary<string, string> { ["path"] = root, ["pattern"] = "note", ["evil"] = "x" }), client);
552+
553+
Assert.True(fwd.Gate.Allowed);
554+
Assert.NotNull(forwarded);
555+
Assert.True(forwarded!.ContainsKey("pattern")); // a legitimate non-path arg is preserved
556+
Assert.True(forwarded.ContainsKey("path"));
557+
Assert.False(forwarded.ContainsKey("evil")); // ...but an unknown one is still stripped
558+
}
559+
finally { Directory.Delete(root, true); }
560+
}
561+
511562
/// <summary>The audit sink and challenge service are MANDATORY for the dangerous operations: a proxy
512563
/// not wired with them cannot forward (no audit-less side effect) and cannot accept a raw approval —
513564
/// both throw rather than silently taking an unsafe path. A pure Gate decision still works.</summary>

0 commit comments

Comments
 (0)