Loud tool-argument validation: eliminate silent discard/degradation of LLM tool arguments

Directory.Packages.props

@@ -106,4 +106,11 @@
   <ItemGroup>
     <PackageVersion Include="Microsoft.SourceLink.GitHub" Version="10.0.300" />
   </ItemGroup>
+  <!-- Transitive security pin: Aspire.Hosting.AppHost → StreamJsonRpc pulls in
+       MessagePack 2.5.192, flagged by NuGetAudit (GHSA-hv8m-jj95-wg3x, LZ4
+       decompression DoS). Netclaw uses MessagePack nowhere — this is sample-only
+       Aspire tooling RPC — but pin to the patched v2 (2.5.301) to clear the audit. -->
+  <ItemGroup>
+    <PackageVersion Include="MessagePack" Version="2.5.301" />
+  </ItemGroup>
 </Project>

SILENT_FALLBACK_AUDIT.md

@@ -0,0 +1,108 @@
+# Silent-Fallback / Silent-Discard Audit — Tool-Call Surface
+
+**Origin:** session `D0AC6CKBK5K_1781115410_840529` (memory `e9a72b27`). The agent passed
+`"TimeoutSeconds":"1200"` to extend a shell timeout; Netclaw only recognizes the meta key
+`_timeout_seconds`, so the value was silently dropped, the timeout fell back to a 90s default,
+and the agent got **no signal** — forming a false belief that fed a stuck loop.
+
+**Constitution rule violated:** *"When something fails or is misconfigured, fail loudly — do
+not silently degrade to a default… on security-relevant paths they can silently escalate
+privileges."* The rule is enforced on the **config** surface (`additionalProperties:false` +
+`ConfigSchemaDoctorCheck`) but **not** on the live **tool-call argument** surface. That
+asymmetry is the gap.
+
+**Method:** 3 parallel auditors (arg/meta/pipeline layer; tool implementations; policy/security
+layer), identical rubric. CRITICAL finding independently re-verified by hand and down-graded.
+
+**Resolution status (change `loud-tool-arg-validation`):** findings #1, #2, #4, #5, #6, #7,
+#8, #9, #10 are **FIXED** by this change (unknown-key validator at the dispatcher,
+strict value binding in the generator + `ToolArgumentHelper`, pipeline-side meta-value
+rejection, `ComputeEffectiveTimeout` clamp/floor notices via `ToolExecutionContext.Notices`,
+provider-boundary args-parse sentinel, `web_fetch` format validation + truncation notice,
+`list_webhooks` Filter honored). The latent `GetInt32` uncaught-throw is also fixed
+(`TryGetInt32`). Findings #3, #11, #15, #17 (policy layer) remain **OPEN — parked** for a
+security owner, recorded as out-of-scope open questions in the change proposal. #12–#14, #16
+(BORDERLINE) remain open, unchanged.
+
+---
+
+## The class unifies into 3 mechanisms + standalones
+
+Most of the 17 sites are not independent bugs — they are three repeated shapes:
+
+- **M1 — unknown / near-miss key silently dropped.** The original bug. Lives at *two* layers:
+  generated `ParseArguments` (all tools) and `ToolCallMeta.ExtractFrom` (meta keys, exact-match).
+- **M2 — present-but-invalid value silently coerced to a default.** `_ => null`/`_ => false`
+  switch arms; malformed args JSON → null args.
+- **M3 — requested value silently clamped/overridden.** timeout floor & ceiling; format
+  fallback; output/byte truncation with no marker.
+
+A loud-by-default tool-arg seam (reject unknown keys + "did you mean" + surface every
+override) closes M1–M3 for all ~20 tools at ~3 chokepoints.
+
+---
+
+## Findings (severity-ranked, de-duplicated)
+
+| # | file:line | mechanism | what's silently handled | verdict | sev | minimal loud fix |
+|---|---|---|---|---|---|---|
+| 1 | `Netclaw.Tools.Generators/NetclawToolGenerator.cs:234-296` + `DispatchingToolExecutor.cs:67` | M1 | **any** unknown/misspelled arg key, all tools (the exact `TimeoutSeconds` mechanism) | VIOLATION | HIGH | diff supplied keys vs schema props (+ meta keys); return `Error: unrecognized argument 'X'. Did you mean 'Y'?` |
+| 2 | `Netclaw.Tools.Abstractions/ToolCallMeta.cs:69,80,95` | M1 | near-miss meta keys (`TimeoutSeconds`, `_timeoutSeconds`, `_timeout-seconds`) — exact-match `TryGetValue`, unlike normal args which use `ToolArgumentHelper.TryGetValueFlexible` | VIOLATION | HIGH | reuse the flexible matcher for meta keys; emit "did you mean `_timeout_seconds`?" notice on near-miss |
+| 3 | `ToolAudienceProfileResolver.cs:70-71,175-192` | (policy) | audience `Allowlist` profile silently does **not** govern tools outside a hardcoded set (memory tools, `search_tools`, `load_tool`, `spawn_agent`, `check_background_job`) → always allowed | VIOLATION / maybe by-design | HIGH (re-classified from CRITICAL) | product decision: default-deny unmanaged first-party tools under Allowlist, **or** document + surface that the allowlist is non-authoritative. Approval gate still applies (mitigates). |
+| 4 | `ToolArgumentHelper.cs:111,129,145` (`_ => null`) + generator `:260,274,288` (`?? 0/0.0/false`) | M2 | present-but-unparseable scalar (`"abc"` int, `"yes"` bool) → coerced to `0`/`false` | VIOLATION | HIGH | distinguish absent vs present-but-unparseable; throw/surface on the latter |
+| 5 | `ToolCallMeta.cs:89,103` (`_ => null`/`_ => false`) | M2 | malformed `_timeout_seconds`/`_background` value → meta silently empty → default | VIOLATION | HIGH | surface "ignored `_timeout_seconds=\"1200ms\"` — expected positive int; used default" |
+| 6 | `Netclaw.Providers/SelfHosted/OpenAiCompatibleChatClient.cs:779-792` (`TryDeserializeArguments`) | M2 | malformed/truncated tool-call args JSON → `return null`, call dispatched with **null args** | VIOLATION | HIGH | emit a tool-result error for that call id instead of an arg-less dispatch |
+| 7 | `Sessions/Pipelines/ToolCallMetaExtractor.cs:37-39` & `:41` | M3 | requested timeout below floor → default; above ceiling → silent `Math.Min` clamp (the literal prod scenario) | VIOLATION | HIGH | append `[timeout clamped 1200s→600s max]` / `[requested 10s below 60s floor]` to result |
+| 8 | `Tools/ListWebhooksTool.cs:31` | standalone | schema-advertised `Filter` arg is **never read** — `ListRouteFiles()` ignores it; complete no-op | VIOLATION | MEDIUM | honor filter (`definition.Enabled`) or reject unknown values; echo applied filter |
+| 9 | `Tools/WebFetchTool.cs:111` | M3 | `Format` ≠ `"text"` (e.g. `"markdown"`, typo) → silent raw-HTML fallback | VIOLATION | MEDIUM | validate `Format ∈ {raw,text}`; error/notice otherwise |
+| 10 | `Tools/WebFetchTool.cs:212-217` → `:107,149` | M3 | body > 5 MB cap → truncated, summary shows count but **no "truncated" marker** | VIOLATION | MEDIUM | append `[content truncated at 5 MB — N bytes not fetched]` |
+| 11 | `ToolAccessPolicy.cs:204-205` (non-interactive shell trust-zone) | (policy) | path token that `NormalizePathToken` returns null for → `continue` (unchecked); working-dir branch fails closed — inconsistent, allow-leaning | BORDERLINE | MEDIUM | fail closed on null-normalized token (`shell_unresolvable_path_token`) |
+| 12 | `Protocol/ChatMessageConverter.cs:94-97` | M2 | persisted media file missing at request-build → `log + continue`; attachment vanishes from LLM message (log-only, model blind) | BORDERLINE | MEDIUM | insert `[attachment unavailable: <name>]` placeholder into contents |
+| 13 | `Providers/SelfHosted/TextToolCallParser.cs:40` | M2 | text/XML tool-call params all coerced to trimmed **string**; arrays/objects/types lost (the **Qwen text-format path** — relevant: orchestrator was Qwen) | BORDERLINE | MEDIUM | try `JsonDocument.Parse` per value; keep structured form; surface unreconcilable values |
+| 14 | `OpenAiCompatibleChatClient.cs:749-752` | M2 | streamed tool call missing `function.name` → `?? string.Empty` → masquerades as "unknown tool", args lost | BORDERLINE | MEDIUM | treat nameless finished call as stream-assembly error (log + diagnostic) |
+| 15 | `ToolAccessPolicy.cs:493-525` (`ResolveApprovalMode`) | (policy) | a future `/`-bearing matcher key would skip `McpServerDefaults` (latent, not currently reachable) | BORDERLINE | MEDIUM | guard: matcher keys must be first-party (no `/`) |
+| 16 | `Tools/WebSearchTool.cs:40` / `Tools/FileReadTool.cs:84-85` | M3 | `MaxResults` clamp to 30 (documented); `StartLine`/`Limit` 0/neg treated as unspecified | BORDERLINE | LOW | optional `[capped at 30]` note; reject non-positive line numbers |
+| 17 | `ToolAccessPolicy.cs:338-346` (safe-verb short-circuit) | (policy) | read-only-verb auto-ALLOW with no audit line (cf. `LogApprovalNearMisses` which does log) | BORDERLINE | LOW | emit info/audit log on safe-verb auto-grant |
+
+**Separate defect class (not silent-fallback, flagged for awareness):**
+`ToolCallMeta.cs:87` — `je.GetInt32()` inside a `when` guard **throws** (uncaught) on a
+non-integral/overflow JSON number (`_timeout_seconds: 12.5` or `1e12`). `ExtractFrom` has no
+try/catch. Use `TryGetInt32`.
+
+---
+
+## Reusable "loud" patterns already in-repo (reuse before adding)
+
+- **`ToolOutputSpill.Compose` (`ToolOutputSpill.cs:108-115`)** — gold standard model-facing
+  truncation notice: `[output truncated to X of Y; saved to <path> — read a slice…]`.
+- **`SessionToolExecutionPipeline.AppendModelInputHandoffWarning` (`:924`)** — in-band,
+  model-facing notice driven by a requested-vs-actual gap. Exact mechanism for M3 overrides.
+- **`ToolArgumentHelper.TryGetValueFlexible` / `NormalizeKey` (`:17-68`)** — case/punct-insensitive
+  matcher; reuse inside `ToolCallMeta.ExtractFrom` to fix M1 meta-key drops *and* detect near-misses.
+- **`ApprovalNearMiss` facility (`ApprovalPatternMatching.cs:192-311`, logged via
+  `ToolApprovalActor.LogApprovalNearMisses:163-185`)** — already classifies
+  "expected-match-but-missed" with a `Describe()`. A `ToolArgNearMiss` modeled on it gives the
+  "supplied X, recognized form is Y, here's why" surface for M1. Read-only diagnostics — safe.
+- **`RedirectToken` throw-on-unknown-enum (`IToolApprovalMatcher.cs:447`)** — correct alternative
+  to `_ => default` on a security-relevant enum map.
+- **`ShellTool` cwd handling (`:118-133`)** and **`RouteToBackgroundJobAsync` trust-context guard
+  (`:734-736`, `throw … "trust context cannot be defaulted"`)** — constitution-aligned exemplars.
+
+---
+
+## Proposed spec spine (for the OpenSpec change)
+
+1. **No-silent-discard invariant at the tool-arg seam.** Unknown keys → loud, recoverable
+   tool-result error with near-miss "did you mean" (reuse `ApprovalNearMiss` shape + flexible
+   matcher). Covers M1 at `NetclawToolGenerator.ParseArguments` + `ToolCallMeta.ExtractFrom`.
+2. **Absent vs present-but-invalid.** Present-but-unparseable values surface, never coerce to
+   `0`/`false`/`null`-args. Covers M2.
+3. **Surface every override to the agent.** Clamp/floor/format-fallback/truncation emit a
+   model-facing note (reuse `ToolOutputSpill`/`AppendModelInputHandoffWarning`). Covers M3.
+4. **Policy decisions, separately.** #3 (allowlist authority), #11 (null token fail-closed),
+   #17 (audit auto-grants) need a security-owner decision, not just a notice.
+5. **Ergonomic fix alongside the safety net:** accept obvious aliases (`TimeoutSeconds` →
+   `_timeout_seconds`) so correct intent just works; the validator is the backstop.
+
+**Highest single leverage:** items 1 + 2 (the M1 seam) — closes the original bug's entire
+class in ~2 places.

docs/runbooks/background-jobs.md

@@ -83,9 +83,9 @@ category as `shell_execute`).
 
 ## Configuration
 
-Background jobs inherit the session's tool timeout ceiling
-(`MaxToolTimeoutSeconds`, default 600s). The `_timeout_seconds` metadata field
-on the tool call controls per-job timeout within that ceiling.
+The `_timeout_seconds` metadata field on the tool call sets the per-job timeout
+and is honored as requested; when omitted, the session's default tool timeout
+(`SessionConfig.ToolExecutionTimeout`) applies.
 
 No separate configuration surface exists — background jobs use the same
 approval policy and audience ACL as regular shell execution.

docs/spec/configuration.md

@@ -189,7 +189,6 @@ shape, confirm that strict-default fallback is active, or verify that
 {
   "Tools": {
     "ShellMode": "HostAllowed",
-    "ShellTimeoutSeconds": 60,
     "MaxOutputChars": 32000,
     "AudienceProfiles": {
       "Public": {
@@ -230,7 +229,6 @@ shape, confirm that strict-default fallback is active, or verify that
 | Field | Type | Default | Description |
 |-------|------|---------|-------------|
 | `ShellMode` | string? | `null` | Optional shell mode override (`Off`, `SandboxOnly`, `HostAllowed`). Falls back to security posture defaults when omitted. |
-| `ShellTimeoutSeconds` | int | `60` | Timeout for shell command execution. |
 | `MaxOutputChars` | int | `32000` | Maximum characters captured from tool output. |
 | `AudienceProfiles` | object | built-in defaults | Per-audience tool, MCP server, and filesystem scopes. Default tool grants are monotonic — `public` ⊆ `team` ⊆ `personal`. `public` gets read-only file tools only (`file_read`, `file_list`, `attach_file`) — no file-mutation and no outbound web tools; `team` adds the file-mutation, web (`web_search`/`web_fetch`), scheduling, and skill tools but not `shell_execute`, the webhook tools, or any MCP server; `personal` defaults to unrestricted tool/file access and all MCP servers. `public` and `team` keep session-scoped file access until the operator opts in. |
 
@@ -536,7 +534,6 @@ export NETCLAW_Session__MaxToolIterationsPerTurn="60"
     "ToolExecutionTimeoutSeconds": 90
   },
   "Tools": {
-    "ShellTimeoutSeconds": 60,
     "MaxOutputChars": 32000
   }
 }

evals/run-evals.sh

@@ -1017,6 +1017,15 @@ assert_tool_file_list() {
     stdout_contains '\[tool:call\] file_list'
 }
 
+assert_tool_timeout_arg_recovery() {
+    # Loud arg validation: if the model emits a near-miss timeout key
+    # (TimeoutSeconds, timeout_seconds), the rejection's did-you-mean must
+    # steer it to the canonical _timeout_seconds within the turn — the
+    # command actually running is the proof of recovery.
+    stdout_contains '\[tool:call\] shell_execute' \
+        && stdout_contains 'netclaw-timeout-eval-ok'
+}
+
 # Category 5: Grounding & Alignment
 assert_grounding_no_hallucinate_version() {
     stdout_contains '\[tool:call\]'
@@ -1477,6 +1486,10 @@ run_all() {
     run_case tool_file_list "file_list called" \
         "What files are in my session directory?"
 
+    run_case tool_timeout_arg_recovery "long-timeout shell call lands on _timeout_seconds" \
+        "Run 'echo netclaw-timeout-eval-ok' in the shell with a 5 minute timeout." \
+        "Use the shell to run: echo netclaw-timeout-eval-ok — give it a 300 second timeout since it might be slow."
+
     end_category
 
     # ── Category 5: Grounding & Alignment ──

feeds/skills/.system/files/netclaw-operations/SKILL.md

@@ -3,7 +3,7 @@ name: netclaw-operations
 description: "REQUIRED when the user asks about scheduling, reminders, cron jobs, timers, background jobs, diagnostics, troubleshooting, MCP tools, daemon health, identity updates, or Netclaw capabilities and self-maintenance."
 metadata:
   author: netclaw
-  version: "2.11.2"
+  version: "2.12.1"
 ---
 
 # Netclaw Operations
@@ -258,6 +258,14 @@ Rules:
 
 - Only `shell_execute` supports background mode. Other tools ignore `_background`.
 - `_timeout_seconds` alone does NOT trigger background execution.
+- `_timeout_seconds` is honored as you set it (there is no ceiling or floor) —
+  set it to however long the work genuinely needs. When omitted, the default
+  tool timeout applies.
+- **Long-running delegation calls** (e.g. `curl` to a local coding-agent or
+  model server that takes minutes to respond) should run as background jobs and
+  carry a `_timeout_seconds` large enough for the work. A synchronous call set
+  to a short timeout (or left at the default) will be killed mid-flight while
+  the remote server is still working.
 - The user must approve the command before it starts running in the background.
 - Maximum 5 concurrent background jobs; overflow queues FIFO.
 - Job definitions persist to `~/.netclaw/jobs/{id}.json`.
@@ -279,6 +287,18 @@ results proactively when the job completes.
 Active background jobs appear in the `[active-background-jobs]` section of the
 session context on every turn.
 
+## Tool argument validation
+
+Tool argument names are validated strictly — unrecognized keys reject the call
+before execution with a `did you mean '<canonical>'?` suggestion and the list
+of valid argument names. Meta keys are exact-match: `_timeout_seconds` and
+`_background` (a leading underscore, snake_case). `TimeoutSeconds`,
+`timeout_seconds`, or `_timeoutSeconds` are rejected, never silently dropped.
+Values must parse as their declared type: `_timeout_seconds: "1200ms"` or
+`_background: "yes"` rejects the call instead of silently using defaults. When
+a call is rejected this way the tool did NOT run — fix the argument and
+re-issue once; do not retry the same shape.
+
 ## Large tool output
 
 Tool output is bounded to a small inline budget

openspec/changes/archive/2026-06-11-loud-tool-arg-validation/.openspec.yaml

@@ -0,0 +1,2 @@
+schema: spec-driven
+created: 2026-06-11