From ce075660d0fbaf204175eaae76f914f035c411a4 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Fri, 19 Jun 2026 13:11:55 +0300 Subject: [PATCH] feat(quarantine): trust-baseline tool approval + rug-pull flag + migration (MCP-2931) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the trust-based tool-approval model. A trusted (non-quarantined) server's CURRENT toolset auto-approves as its baseline (approved_by "auto-baseline") instead of stranding as pending. Post-baseline review still applies: an approved tool whose hash changes flips to changed (rug pull) and a genuinely-new tool flips to pending — both blocked — unless the per-server auto_approve_tool_changes flag (MCP-2930) is set, which auto-approves all post-baseline changes AND additions for that server. Existing installs are migrated automatically: a trusted server with no approved baseline promotes its already-stranded pending records (whose stored hash matches the live tool) to approved on the next discovery pass, clearing the reporter's case with no user action. A changed (rug-pull) record is NEVER cleared by migration. Two-gate consistency: index blocking (BlockedTools) now keys off stored pending status regardless of server-level quarantine, matching the call-time gate (internal/server/mcp.go), so a tool is never indexed/visible-but-uncallable. New transition reasons (ReasonBaselineTrust, ReasonAutoApproveChanges) keep the approval invariant explicit: only user action or the explicit per-server flag may clear a changed record. TDD red→green tests cover every transition. Parent: MCP-2916. --- CLAUDE.md | 2 +- docs/configuration.md | 4 +- docs/features/security-quarantine.md | 4 +- docs/features/tool-quarantine.md | 72 ++++- internal/runtime/tool_quarantine.go | 210 ++++++++++++- .../runtime/tool_quarantine_invariant_test.go | 59 ++-- internal/runtime/tool_quarantine_test.go | 19 +- .../tool_quarantine_trust_baseline_test.go | 279 ++++++++++++++++++ 8 files changed, 583 insertions(+), 66 deletions(-) create mode 100644 internal/runtime/tool_quarantine_trust_baseline_test.go diff --git a/CLAUDE.md b/CLAUDE.md index fdc629a3b..504df5ce5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -121,7 +121,7 @@ All server responses include a unified `health` field: `level` (healthy|degraded - **Localhost-only by default** (`127.0.0.1:8080`); **API key always required** (auto-generated and persisted if not provided). - **Agent tokens**: scoped credentials for AI agents (`mcp_agt_` prefix, HMAC-SHA256 hashed). See [docs/features/agent-tokens.md](docs/features/agent-tokens.md). -- **Quarantine**: new servers quarantined until approved; Tool Poisoning Attack (TPA) detection on descriptions. **Tool-level quarantine (Spec 032)**: SHA-256 hashes detect new ("pending") and changed ("changed", rug-pull) tools. Config: `quarantine_enabled` (global), `skip_quarantine` (per-server; successor `auto_approve_tool_changes` plumbed but not yet enforced — MCP-2930/2931). See [docs/features/security-quarantine.md](docs/features/security-quarantine.md). +- **Quarantine**: new servers quarantined until approved; Tool Poisoning Attack (TPA) detection on descriptions. **Tool-level quarantine (Spec 032)**: SHA-256 hashes detect new ("pending") and changed ("changed", rug-pull) tools. Trusted (non-quarantined) servers auto-approve their current toolset as a baseline; post-baseline changes/additions are reviewed unless per-server `auto_approve_tool_changes:true` (MCP-2931, deprecates `skip_quarantine`). Config: `quarantine_enabled` (global), `auto_approve_tool_changes` (per-server). See [docs/features/security-quarantine.md](docs/features/security-quarantine.md). - **`require_mcp_auth`**: when enabled, `/mcp` rejects unauthenticated requests (default off, for back-compat). - **Sensitive-data detection** (`internal/security/`): scans tool args/responses for secrets (cloud creds, private keys, API tokens, DB strings, Luhn-validated cards, sensitive file paths, high-entropy strings). On by default; integrates with the activity log. Config under `sensitive_data_detection`. See [docs/features/sensitive-data-detection.md](docs/features/sensitive-data-detection.md). diff --git a/docs/configuration.md b/docs/configuration.md index 565292855..b8582513f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -839,8 +839,8 @@ Per-server tool-change auto-approval is configured on the server entry: | Field | Type | Default | Description | |-------|------|---------|-------------| -| `skip_quarantine` | boolean | `false` | Skip tool-level quarantine for this server (auto-approve new tools). The current active per-server control. | -| `auto_approve_tool_changes` | boolean (tri-state) | unset | Successor to `skip_quarantine`. Accepted by config now; a legacy `skip_quarantine: true` is migrated onto it on load **only when it is unset** (an explicit `false` overrides the legacy flag). **Enforcement is not yet wired** — `skip_quarantine` remains the active control until an upcoming release. | +| `auto_approve_tool_changes` | boolean (tri-state) | unset (= `false`) | Auto-approve **all** post-baseline tool changes AND additions for this server (disables per-server rug-pull protection). The active per-server control. A trusted server's *baseline* is auto-approved regardless of this flag. | +| `skip_quarantine` | boolean | `false` | **Deprecated** — superseded by `auto_approve_tool_changes`. A legacy `skip_quarantine: true` is migrated onto `auto_approve_tool_changes` on load **only when it is unset** (an explicit `false` overrides the legacy flag). | See [Tool Quarantine](features/tool-quarantine.md) for complete details. diff --git a/docs/features/security-quarantine.md b/docs/features/security-quarantine.md index 073efbfd9..f19e805ef 100644 --- a/docs/features/security-quarantine.md +++ b/docs/features/security-quarantine.md @@ -175,7 +175,7 @@ In addition to server-level quarantine, MCPProxy provides **tool-level quarantin See [Tool Quarantine](./tool-quarantine.md) for complete documentation on: - SHA256 hash-based tool approval - CLI commands: `mcpproxy upstream inspect` and `mcpproxy upstream approve` -- Configuration: `quarantine_enabled` and `skip_quarantine` (successor key `auto_approve_tool_changes` is accepted but not yet enforced) +- Configuration: `quarantine_enabled` (global) and `auto_approve_tool_changes` (per-server; deprecates `skip_quarantine`) - REST API endpoints for tool approval management ### Block (approve + disable) @@ -215,7 +215,7 @@ When `quarantine_enabled` is `false`: An explicit `quarantined` field in an add-server request still wins over the default, so client code can always override on a per-server basis. -Per-server `skip_quarantine: true` continues to apply at the tool level (its successor `auto_approve_tool_changes` becomes the active control in an upcoming release). +Per-server `auto_approve_tool_changes: true` auto-approves all post-baseline tool changes and additions for that server (the deprecated `skip_quarantine: true` is migrated onto it automatically). Warning: Disabling quarantine exposes your system to Tool Poisoning Attacks. Only do this on machines where every MCP server you connect to diff --git a/docs/features/tool-quarantine.md b/docs/features/tool-quarantine.md index 78fef7c0d..c2e07d99b 100644 --- a/docs/features/tool-quarantine.md +++ b/docs/features/tool-quarantine.md @@ -29,13 +29,35 @@ This hash is compared against previously approved hashes to detect changes. | `pending` | New tool, never approved | No | No | | `changed` | Hash differs from approved hash | No | No | +### Trust-baseline model + +A **trusted** server is one that is *not* under server-level quarantine +(`quarantined: false`) — whether you added it that way, loaded it from config, +or approved it out of quarantine. When a trusted server is discovered and it has +**no prior tool-approval baseline**, its current toolset is treated as that +baseline: every current tool auto-approves (status `approved`, +`approved_by: "auto-baseline"`) rather than stranding as `pending`. Approving a +server *is* trusting the tools it ships with. + +Once the baseline exists, tool-level quarantine guards against later changes: + +- An existing approved tool whose hash later changes → `changed` (rug pull) → blocked. +- A genuinely-new tool that appears *after* the baseline → `pending` → blocked. + +A **quarantined** server (`quarantined: true`) is the exception: none of its +tools auto-approve — they all stay `pending` until you approve the server, which +then promotes them to the baseline. + ### Detection Flow ``` Server connects → Tools discovered → For each tool: - ├─ No existing record → Status: "pending" (new tool) - ├─ Hash matches approved hash → Status: "approved" (unchanged) - └─ Hash differs from approved hash → Status: "changed" (rug pull detected) + ├─ Quarantine disabled / per-server auto-approve → Status: "approved" (auto) + ├─ Trusted server, no baseline yet (this pass IS the baseline) + │ → Status: "approved" (auto-baseline) + ├─ No existing record, baseline already exists → Status: "pending" (new tool) + ├─ Hash matches approved hash → Status: "approved" (unchanged) + └─ Hash differs from approved hash → Status: "changed" (rug pull detected) ``` When a tool is `pending` or `changed`, it is: @@ -43,6 +65,9 @@ When a tool is `pending` or `changed`, it is: - **Blocked from execution** (tool calls return an error) - **Visible in the management UI** for review +Index visibility and callability are driven by the **same stored status**, so a +tool is never indexed/visible while being uncallable (or vice versa). + ## Configuration ### Global Setting @@ -61,7 +86,10 @@ Tool-level quarantine is enabled by default. To disable: ### Per-Server Auto-Approve -Trust specific servers by skipping per-server tool-change review: +By default a trusted server's baseline auto-approves, but **post-baseline +changes and additions are still reviewed** (`changed` / `pending`). To opt a +server out of that review entirely — auto-approving every change and addition, +including rug-pulls — set `auto_approve_tool_changes: true`: ```json { @@ -69,7 +97,7 @@ Trust specific servers by skipping per-server tool-change review: { "name": "trusted-internal-server", "command": "my-mcp-server", - "skip_quarantine": true + "auto_approve_tool_changes": true } ] } @@ -77,19 +105,31 @@ Trust specific servers by skipping per-server tool-change review: | Field | Type | Default | Description | |-------|------|---------|-------------| -| `skip_quarantine` | boolean | `false` | Skip tool-level quarantine for this server (auto-approve new tools). The current active per-server control. | -| `auto_approve_tool_changes` | boolean (tri-state) | unset | Successor to `skip_quarantine`. Accepted by config now; a legacy `skip_quarantine: true` is migrated onto it on load **only when it is unset**, so an explicit `auto_approve_tool_changes: false` overrides a legacy `skip_quarantine: true`. | - -> **Note — rollout:** `auto_approve_tool_changes` is config-plumbed but its **enforcement is not yet wired** — runtime auto-approval is still governed by `skip_quarantine`. The new flag becomes the active control (and gains the richer rug-pull / trust-baseline behavior) in an upcoming release. Existing `skip_quarantine` configs are unaffected and are migrated onto the new key automatically. To auto-approve a server today, set `skip_quarantine: true`. +| `auto_approve_tool_changes` | boolean (tri-state) | unset (= `false`) | Auto-approve **all** post-baseline tool changes AND additions for this server, disabling per-server rug-pull protection. The active per-server control. | +| `skip_quarantine` | boolean | `false` | **Deprecated** — superseded by `auto_approve_tool_changes`. A legacy `skip_quarantine: true` is migrated onto `auto_approve_tool_changes` on load **only when the latter is unset**, so an explicit `auto_approve_tool_changes: false` overrides a legacy `skip_quarantine: true`. | -When the active control is enabled for a server, new tools from it are automatically approved. +> **Security:** `auto_approve_tool_changes: true` turns off rug-pull detection for +> that server — a later malicious description/schema change is silently approved. +> Only enable it for servers you fully control. ### Auto-Approve Behavior Tools are automatically approved (no manual review needed) when: -- `quarantine_enabled` is `false` globally -- The server has `skip_quarantine: true` (the active per-server control; `auto_approve_tool_changes` becomes active in an upcoming release) -- The auto-approval is recorded with `approved_by: "auto"` in the approval record +- `quarantine_enabled` is `false` globally, or the server still carries the + deprecated `skip_quarantine: true` — recorded with `approved_by: "auto"`. +- A **trusted server establishes its baseline** (first discovery with no prior + baseline, or migrating already-stranded `pending` tools whose hash is + unchanged) — recorded with `approved_by: "auto-baseline"`. +- The server has `auto_approve_tool_changes: true` and a post-baseline change or + addition occurs — recorded with `approved_by: "auto-approve-changes"`. + +#### Migrating existing installs + +Earlier releases stranded trusted-server tools as `pending`. On upgrade, the +next discovery pass on a trusted server with no approved baseline promotes those +stranded `pending` records (whose stored hash matches the live tool) to +`approved` automatically — no user action required. A `changed` (rug-pull) +record is **never** cleared by this migration. ## Managing Tool Approvals @@ -279,7 +319,7 @@ Tool quarantine events are recorded in the activity log: | Event | Description | |-------|-------------| | `tool_discovered` | New tool found, pending approval | -| `tool_auto_approved` | New tool automatically approved (trusted server) | +| `tool_auto_approved` | Tool automatically approved (quarantine off, baseline trust, or `auto_approve_tool_changes`) | | `tool_approved` | Tool manually approved by user | | `tool_description_changed` | Tool description/schema changed since approval | @@ -309,7 +349,7 @@ Tool-level quarantine is a separate system from [server-level quarantine](./secu | **Scope** | Entire server | Individual tools | | **Trigger** | Server added via AI client | Tool description/schema changes | | **Detection** | Manual review | SHA256 hash comparison | -| **Config** | `quarantined: true/false` on server | `quarantine_enabled` global + `skip_quarantine` per-server (successor `auto_approve_tool_changes` not yet enforced) | +| **Config** | `quarantined: true/false` on server | `quarantine_enabled` global + `auto_approve_tool_changes` per-server (deprecates `skip_quarantine`) | | **Approval** | `POST /servers/{name}/unquarantine` | `POST /servers/{name}/tools/approve` | Both systems work together: a quarantined server's tools are never indexed regardless of tool approval status. @@ -317,7 +357,7 @@ Both systems work together: a quarantined server's tools are never indexed regar ## Best Practices 1. **Review changed tools carefully**: A `changed` status may indicate a rug pull attack where a malicious server silently modifies tool descriptions -2. **Use `skip_quarantine` for internal servers**: If you control the MCP server and trust it, skip quarantine to avoid manual approval on every update (the successor key `auto_approve_tool_changes` becomes the active control in an upcoming release) +2. **Use `auto_approve_tool_changes` sparingly**: A trusted server's baseline is already auto-approved; only set `auto_approve_tool_changes: true` for servers you fully control where you also want post-baseline changes/additions approved without review (this disables rug-pull protection for that server). The deprecated `skip_quarantine: true` is migrated onto this key automatically. 3. **Monitor the doctor output**: Run `mcpproxy doctor` regularly to check for pending tools 4. **Export descriptions for audit**: Use the export API to keep records of approved tool descriptions 5. **Check activity logs**: Monitor `tool_description_changed` events for unexpected changes diff --git a/internal/runtime/tool_quarantine.go b/internal/runtime/tool_quarantine.go index cebd4ebff..097726283 100644 --- a/internal/runtime/tool_quarantine.go +++ b/internal/runtime/tool_quarantine.go @@ -103,6 +103,15 @@ const ( ReasonDescriptionMatch TransitionReason = "description_match" ReasonUserApprove TransitionReason = "user_approve" ReasonAutoApprove TransitionReason = "auto_approve" + // ReasonBaselineTrust marks promotion of a never-reviewed (pending) tool to + // approved because the server is trusted (non-quarantined) and has no prior + // approved baseline yet — the current toolset IS the baseline (MCP-2931). + ReasonBaselineTrust TransitionReason = "baseline_trust" + // ReasonAutoApproveChanges marks promotion driven by the explicit per-server + // auto_approve_tool_changes flag (MCP-2931/MCP-2930). It is the ONLY reason, + // besides explicit user action, that may clear a changed (rug-pull) record — + // because the operator opted into auto-approving changes for that server. + ReasonAutoApproveChanges TransitionReason = "auto_approve_changes" ) // assertToolApprovalInvariant checks that a state transition is valid according @@ -123,7 +132,8 @@ func assertToolApprovalInvariant(oldStatus, newStatus string, reason TransitionR case storage.ToolApprovalStatusChanged: switch reason { case ReasonHashMatch, ReasonDescriptionRevert, ReasonFormulaMigration, - ReasonContentMatch, ReasonDescriptionMatch, ReasonUserApprove: + ReasonContentMatch, ReasonDescriptionMatch, ReasonUserApprove, + ReasonAutoApproveChanges: return nil default: return fmt.Errorf("invariant violation: changed→approved with reason %q "+ @@ -131,7 +141,7 @@ func assertToolApprovalInvariant(oldStatus, newStatus string, reason TransitionR } case storage.ToolApprovalStatusPending: switch reason { - case ReasonUserApprove, ReasonAutoApprove: + case ReasonUserApprove, ReasonAutoApprove, ReasonBaselineTrust, ReasonAutoApproveChanges: return nil default: return fmt.Errorf("invariant violation: pending→approved with reason %q "+ @@ -183,10 +193,16 @@ func (r *Runtime) checkToolApprovals(serverName string, tools []*config.ToolMeta serverSkipped := false serverQuarantined := false + autoApproveChanges := false for _, sc := range cfg.Servers { if sc.Name == serverName { serverSkipped = sc.IsQuarantineSkipped() serverQuarantined = sc.Quarantined + // Per-server opt-in to auto-approve post-baseline changes AND + // additions (MCP-2931). Note: a legacy skip_quarantine:true is + // migrated onto this flag at config load (MCP-2930), so it also + // reads true for skip_quarantine servers. + autoApproveChanges = sc.IsAutoApproveToolChanges() break } } @@ -202,6 +218,25 @@ func (r *Runtime) checkToolApprovals(serverName string, tools []*config.ToolMeta enforceNewTools := globalEnabled && !serverSkipped enforceQuarantine := globalEnabled && !serverSkipped && serverQuarantined + // Trust-baseline model (MCP-2931): a trusted (non-quarantined) server whose + // tools have NEVER been approved before treats its CURRENT toolset as the + // baseline — every current tool is auto-approved instead of stranded as + // pending. A record in approved OR changed state proves a prior baseline + // exists (changed implies the tool was approved once, then mutated), so its + // presence disqualifies the baseline pass and post-baseline review resumes. + // Detection is snapshotted BEFORE the loop so promoting tools mid-pass does + // not flip the decision. + serverHasBaseline := false + if priorRecords, listErr := r.storageManager.ListToolApprovals(serverName); listErr == nil { + for _, rec := range priorRecords { + if rec.Status == storage.ToolApprovalStatusApproved || rec.Status == storage.ToolApprovalStatusChanged { + serverHasBaseline = true + break + } + } + } + isBaselinePass := enforceNewTools && !serverQuarantined && !serverHasBaseline + result := &ToolApprovalResult{ BlockedTools: make(map[string]bool), } @@ -271,9 +306,24 @@ func (r *Runtime) checkToolApprovals(serverName string, tools []*config.ToolMeta } if err != nil { - // No existing record - this is a new tool. - if !enforceNewTools { - // Quarantine disabled or server has skip_quarantine - auto-approve + // No existing record - this is a new tool. Decide whether it + // auto-approves and under which provenance label: + // - "auto" quarantine disabled globally or skip_quarantine + // - "auto-baseline" trusted server establishing its baseline (MCP-2931 #1) + // - "auto-approve-changes" trusted server, post-baseline addition, operator opted in (MCP-2931 #3) + // Otherwise the tool is pending and blocked until reviewed (MCP-2931 #2). + autoApprove := false + approvedBy := "" + switch { + case !enforceNewTools: + autoApprove, approvedBy = true, "auto" + case isBaselinePass: + autoApprove, approvedBy = true, "auto-baseline" + case autoApproveChanges: + autoApprove, approvedBy = true, "auto-approve-changes" + } + + if autoApprove { now := time.Now().UTC() record := &storage.ToolApprovalRecord{ ServerName: serverName, @@ -282,7 +332,7 @@ func (r *Runtime) checkToolApprovals(serverName string, tools []*config.ToolMeta ApprovedHash: currentHash, HashSchemaVersion: storage.OutputSchemaHashSchemaVersion, Status: storage.ToolApprovalStatusApproved, - ApprovedBy: "auto", + ApprovedBy: approvedBy, ApprovedAt: now, CurrentDescription: tool.Description, CurrentSchema: schemaJSON, @@ -293,13 +343,15 @@ func (r *Runtime) checkToolApprovals(serverName string, tools []*config.ToolMeta r.logger.Error("Failed to save auto-approved tool record", zap.String("server", serverName), zap.String("tool", toolName), + zap.String("approved_by", approvedBy), zap.Error(saveErr)) continue } - r.logger.Info("New tool discovered, auto-approved (quarantine disabled or server skipped)", + r.logger.Info("New tool discovered, auto-approved", zap.String("server", serverName), - zap.String("tool", toolName)) + zap.String("tool", toolName), + zap.String("approved_by", approvedBy)) r.emitToolQuarantineEvent(serverName, toolName, "tool_auto_approved", "", currentHash, "", tool.Description, "", schemaJSON) @@ -307,9 +359,10 @@ func (r *Runtime) checkToolApprovals(serverName string, tools []*config.ToolMeta continue } - // Quarantine enabled — new tool requires user review before use. - // This applies to ALL servers (including trusted ones) to prevent - // injection attacks via new tool additions on compromised servers. + // Quarantine enabled, no baseline exemption — new tool requires user + // review before use. This applies to trusted servers post-baseline + // (and always to quarantined ones) to prevent injection attacks via + // new tool additions on compromised servers. record := &storage.ToolApprovalRecord{ ServerName: serverName, ToolName: toolName, @@ -388,11 +441,67 @@ func (r *Runtime) checkToolApprovals(serverName string, tools []*config.ToolMeta } if existing.Status == storage.ToolApprovalStatusPending { - // Still pending - update current info + // Capture whether the stored hash still matches the live tool BEFORE + // we overwrite CurrentHash — the baseline migration (MCP-2931 #4) + // only promotes a stranded pending record that is unchanged since it + // was recorded. + priorHashMatches := existing.CurrentHash == currentHash + + // Update current info to the live snapshot. existing.CurrentHash = currentHash existing.CurrentDescription = tool.Description existing.CurrentSchema = schemaJSON existing.CurrentOutputSchema = outputSchemaJSON + + // Decide whether this pending record is promoted to approved: + // - baseline migration: trusted server, no prior baseline, hash unchanged (MCP-2931 #4) + // - auto_approve_tool_changes: operator opted in (MCP-2931 #3) + promote := false + var promoteReason TransitionReason + var promoteBy string + switch { + case isBaselinePass && priorHashMatches: + promote, promoteReason, promoteBy = true, ReasonBaselineTrust, "auto-baseline" + case autoApproveChanges: + promote, promoteReason, promoteBy = true, ReasonAutoApproveChanges, "auto-approve-changes" + } + + if promote { + if invErr := r.enforceInvariant(serverName, toolName, existing.Status, storage.ToolApprovalStatusApproved, promoteReason); invErr != nil { + // Refuse to promote on an invariant violation — keep blocked. + if saveErr := r.storageManager.SaveToolApproval(existing); saveErr != nil { + r.logger.Debug("Failed to update pending tool approval", + zap.String("server", serverName), zap.String("tool", toolName), zap.Error(saveErr)) + } + if enforceNewTools { + result.BlockedTools[toolName] = true + result.PendingCount++ + } + continue + } + existing.Status = storage.ToolApprovalStatusApproved + existing.ApprovedHash = currentHash + existing.HashSchemaVersion = storage.OutputSchemaHashSchemaVersion + existing.ApprovedAt = time.Now().UTC() + existing.ApprovedBy = promoteBy + existing.PreviousDescription = "" + existing.PreviousSchema = "" + existing.PreviousOutputSchema = "" + if saveErr := r.storageManager.SaveToolApproval(existing); saveErr != nil { + r.logger.Error("Failed to promote pending tool approval", + zap.String("server", serverName), zap.String("tool", toolName), + zap.String("approved_by", promoteBy), zap.Error(saveErr)) + continue + } + r.logger.Info("Pending tool promoted to approved", + zap.String("server", serverName), zap.String("tool", toolName), + zap.String("approved_by", promoteBy)) + r.emitToolQuarantineEvent(serverName, toolName, "tool_auto_approved", "", currentHash, + "", tool.Description, "", schemaJSON) + continue + } + + // Stays pending — persist the updated current info. if saveErr := r.storageManager.SaveToolApproval(existing); saveErr != nil { r.logger.Debug("Failed to update pending tool approval", zap.String("server", serverName), @@ -400,7 +509,14 @@ func (r *Runtime) checkToolApprovals(serverName string, tools []*config.ToolMeta zap.Error(saveErr)) } - if enforceQuarantine { + // Two-gate consistency (MCP-2931 #5): block from the index whenever + // the stored status is pending and quarantine is enforced, matching + // the call-time gate (internal/server/mcp.go) which blocks on stored + // pending status regardless of whether the SERVER is quarantined. + // Previously this only blocked when the server itself was quarantined + // (enforceQuarantine), so a post-baseline pending tool on a trusted + // server was indexed/visible but uncallable. + if enforceNewTools { result.BlockedTools[toolName] = true result.PendingCount++ } @@ -436,6 +552,36 @@ func (r *Runtime) checkToolApprovals(serverName string, tools []*config.ToolMeta } continue } + // auto_approve_tool_changes (MCP-2931 #3): the operator opted into + // auto-approving changes for this server, so a still-changed record is + // re-baselined to its current snapshot instead of staying blocked. + if autoApproveChanges { + if invErr := r.enforceInvariant(serverName, toolName, existing.Status, storage.ToolApprovalStatusApproved, ReasonAutoApproveChanges); invErr != nil { + result.BlockedTools[toolName] = true + result.ChangedCount++ + continue + } + existing.Status = storage.ToolApprovalStatusApproved + existing.ApprovedHash = currentHash + existing.CurrentHash = currentHash + existing.HashSchemaVersion = storage.OutputSchemaHashSchemaVersion + existing.ApprovedAt = time.Now().UTC() + existing.ApprovedBy = "auto-approve-changes" + existing.CurrentDescription = tool.Description + existing.CurrentSchema = schemaJSON + existing.CurrentOutputSchema = outputSchemaJSON + existing.PreviousDescription = "" + existing.PreviousSchema = "" + existing.PreviousOutputSchema = "" + if saveErr := r.storageManager.SaveToolApproval(existing); saveErr == nil { + r.logger.Info("Changed tool auto-approved (auto_approve_tool_changes enabled)", + zap.String("server", serverName), + zap.String("tool", toolName)) + r.emitToolQuarantineEvent(serverName, toolName, "tool_auto_approved", "", currentHash, + "", tool.Description, "", schemaJSON) + } + continue + } // Tool still has the changed description — keep it blocked if globalEnabled { result.BlockedTools[toolName] = true @@ -585,7 +731,43 @@ func (r *Runtime) checkToolApprovals(serverName string, tools []*config.ToolMeta continue } - // Hash differs AND description differs - genuine tool change (rug pull) + // Hash differs AND description differs - genuine tool change (rug pull). + // auto_approve_tool_changes (MCP-2931 #3): when the operator opted into + // auto-approving changes for this server, re-baseline to the current + // snapshot instead of flagging it changed — so no `changed` ever surfaces. + if autoApproveChanges { + if invErr := r.enforceInvariant(serverName, toolName, existing.Status, storage.ToolApprovalStatusApproved, ReasonAutoApproveChanges); invErr != nil { + result.BlockedTools[toolName] = true + result.ChangedCount++ + continue + } + existing.Status = storage.ToolApprovalStatusApproved + existing.ApprovedHash = currentHash + existing.CurrentHash = currentHash + existing.HashSchemaVersion = storage.OutputSchemaHashSchemaVersion + existing.ApprovedAt = time.Now().UTC() + existing.ApprovedBy = "auto-approve-changes" + existing.CurrentDescription = tool.Description + existing.CurrentSchema = schemaJSON + existing.CurrentOutputSchema = outputSchemaJSON + existing.PreviousDescription = "" + existing.PreviousSchema = "" + existing.PreviousOutputSchema = "" + if saveErr := r.storageManager.SaveToolApproval(existing); saveErr != nil { + r.logger.Error("Failed to auto-approve changed tool", + zap.String("server", serverName), + zap.String("tool", toolName), + zap.Error(saveErr)) + continue + } + r.logger.Info("Tool change auto-approved (auto_approve_tool_changes enabled)", + zap.String("server", serverName), + zap.String("tool", toolName)) + r.emitToolQuarantineEvent(serverName, toolName, "tool_auto_approved", "", currentHash, + "", tool.Description, "", schemaJSON) + continue + } + oldDesc := existing.CurrentDescription oldSchema := existing.CurrentSchema oldOutputSchema := existing.CurrentOutputSchema diff --git a/internal/runtime/tool_quarantine_invariant_test.go b/internal/runtime/tool_quarantine_invariant_test.go index 7264ddab7..1f8ad77bb 100644 --- a/internal/runtime/tool_quarantine_invariant_test.go +++ b/internal/runtime/tool_quarantine_invariant_test.go @@ -47,6 +47,7 @@ func TestAssertToolApprovalInvariant_ChangedToApproved_ValidReasons(t *testing.T ReasonContentMatch, ReasonDescriptionMatch, ReasonUserApprove, + ReasonAutoApproveChanges, // explicit per-server opt-in (MCP-2931) } for _, reason := range validReasons { t.Run(string(reason), func(t *testing.T) { @@ -64,7 +65,12 @@ func TestAssertToolApprovalInvariant_ChangedToApproved_InvalidReason(t *testing. } func TestAssertToolApprovalInvariant_PendingToApproved_ValidReasons(t *testing.T) { - validReasons := []TransitionReason{ReasonUserApprove, ReasonAutoApprove} + validReasons := []TransitionReason{ + ReasonUserApprove, + ReasonAutoApprove, + ReasonBaselineTrust, // trusted-server baseline establishment (MCP-2931) + ReasonAutoApproveChanges, // explicit per-server opt-in (MCP-2931) + } for _, reason := range validReasons { t.Run(string(reason), func(t *testing.T) { err := assertToolApprovalInvariant(storage.ToolApprovalStatusPending, storage.ToolApprovalStatusApproved, reason) @@ -123,23 +129,23 @@ func TestMultiPass_DiscoverChangeReconnectReconnect(t *testing.T) { maliciousDesc := "MALICIOUS: steal credentials" schema := `{"type":"object"}` - // Pass 1: Discover and approve the tool + // Pass 1: Discover the tool. On a trusted (non-quarantined) server with no + // prior baseline, the current toolset auto-approves as the baseline + // (MCP-2931), so it is approved — not pending. tools := []*config.ToolMetadata{{ ServerName: "github", Name: "create_issue", Description: originalDesc, ParamsJSON: schema, }} result, err := rt.checkToolApprovals("github", tools) require.NoError(t, err) - assert.Equal(t, 1, result.PendingCount) + assert.Equal(t, 0, result.PendingCount, "trusted-server baseline tool must auto-approve, not pend") + assert.Empty(t, result.BlockedTools) - // Approve the tool - err = rt.ApproveTools("github", []string{"create_issue"}, "admin") - require.NoError(t, err) - - // Verify approved + // Verify baseline-approved record, err := rt.storageManager.GetToolApproval("github", "create_issue") require.NoError(t, err) assert.Equal(t, storage.ToolApprovalStatusApproved, record.Status) + assert.Equal(t, "auto-baseline", record.ApprovedBy) // Pass 2: Description changes (rug pull) changedTools := []*config.ToolMetadata{{ @@ -243,35 +249,34 @@ func TestMultiPass_PendingStaysBlocked(t *testing.T) { } } -// TestMultiPass_PendingOnTrustedServer verifies pending tools stay pending in -// storage on non-quarantined (but quarantine-enabled) servers. Note: on trusted -// servers, pending tools are blocked on first discovery but not on subsequent -// reconnects (enforceQuarantine is false for non-quarantined servers). +// TestMultiPass_PendingOnTrustedServer verifies that a POST-baseline new tool on +// a trusted (non-quarantined, quarantine-enabled) server stays pending AND stays +// blocked from the index across every discovery pass (MCP-2931 #2 + #5 two-gate +// consistency). The critical invariant: status must NEVER become "approved" +// without user action once a baseline exists. func TestMultiPass_PendingOnTrustedServer(t *testing.T) { rt := setupQuarantineRuntime(t, nil, []*config.ServerConfig{ {Name: "github", Enabled: true}, // trusted, NOT quarantined }) - tools := []*config.ToolMetadata{{ - ServerName: "github", Name: "create_issue", - Description: "Creates a GitHub issue", ParamsJSON: `{"type":"object"}`, + // Establish the baseline first (auto-approved). + baseline := []*config.ToolMetadata{{ + ServerName: "github", Name: "baseline_tool", + Description: "Baseline tool", ParamsJSON: `{"type":"object"}`, }} - - // Pass 1: New tool discovery — blocked (enforceNewTools = true) - result, err := rt.checkToolApprovals("github", tools) + _, err := rt.checkToolApprovals("github", baseline) require.NoError(t, err) - assert.True(t, result.BlockedTools["create_issue"], "pass 1: new tool must be blocked") - record, err := rt.storageManager.GetToolApproval("github", "create_issue") - require.NoError(t, err) - assert.Equal(t, storage.ToolApprovalStatusPending, record.Status) + // A new tool appears post-baseline. + tools := []*config.ToolMetadata{ + baseline[0], + {ServerName: "github", Name: "create_issue", Description: "Creates a GitHub issue", ParamsJSON: `{"type":"object"}`}, + } - // Pass 2+: Status stays pending in storage, even though the tool may not be - // in BlockedTools (enforceQuarantine is false for non-quarantined servers). - // The critical invariant is: status must NEVER become "approved" without user action. - for pass := 2; pass <= 4; pass++ { - _, err := rt.checkToolApprovals("github", tools) + for pass := 1; pass <= 4; pass++ { + result, err := rt.checkToolApprovals("github", tools) require.NoError(t, err, "pass %d", pass) + assert.True(t, result.BlockedTools["create_issue"], "pass %d: post-baseline pending tool must stay blocked", pass) record, err := rt.storageManager.GetToolApproval("github", "create_issue") require.NoError(t, err, "pass %d", pass) diff --git a/internal/runtime/tool_quarantine_test.go b/internal/runtime/tool_quarantine_test.go index 23cba7c85..03f2ec2be 100644 --- a/internal/runtime/tool_quarantine_test.go +++ b/internal/runtime/tool_quarantine_test.go @@ -235,14 +235,25 @@ func TestCheckToolApprovals_PerServerSkip_AutoApproved(t *testing.T) { } func TestCheckToolApprovals_TrustedServer_NewToolPending(t *testing.T) { - // When quarantine is globally enabled, new tools from trusted (non-quarantined) - // servers should still require approval. This prevents injection via new tool - // additions on compromised servers. + // Trust-baseline model (MCP-2931): a trusted (non-quarantined) server's + // CURRENT toolset auto-approves as the baseline, but a NEW tool that appears + // AFTER the baseline still requires review. This keeps injection protection + // against new tool additions on a compromised server while no longer + // stranding the legitimately-trusted baseline. rt := setupQuarantineRuntime(t, nil, []*config.ServerConfig{ {Name: "github", Enabled: true}, // trusted, NOT quarantined }) + // Establish the baseline with the server's initial tool. + baseline := []*config.ToolMetadata{ + {ServerName: "github", Name: "create_issue", Description: "Creates issues", ParamsJSON: `{"type":"object"}`}, + } + _, err := rt.checkToolApprovals("github", baseline) + require.NoError(t, err) + + // A new tool appears after the baseline (server already has approved records). tools := []*config.ToolMetadata{ + baseline[0], { ServerName: "github", Name: "new_malicious_tool", @@ -254,7 +265,7 @@ func TestCheckToolApprovals_TrustedServer_NewToolPending(t *testing.T) { result, err := rt.checkToolApprovals("github", tools) require.NoError(t, err) - assert.Equal(t, 1, result.PendingCount, "New tool on trusted server should be pending when quarantine enabled") + assert.Equal(t, 1, result.PendingCount, "post-baseline new tool on trusted server should be pending") assert.True(t, result.BlockedTools["new_malicious_tool"], "New tool should be blocked until approved") // Verify storage record diff --git a/internal/runtime/tool_quarantine_trust_baseline_test.go b/internal/runtime/tool_quarantine_trust_baseline_test.go new file mode 100644 index 000000000..9c74f90f2 --- /dev/null +++ b/internal/runtime/tool_quarantine_trust_baseline_test.go @@ -0,0 +1,279 @@ +package runtime + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/storage" +) + +// TestCheckToolApprovals_TrustedServer_BaselineAutoApprove verifies MCP-2931 +// requirement #1: when a server is trusted (server-level NOT quarantined) and +// quarantine is globally enabled, its CURRENT toolset auto-approves as the +// baseline (status approved, ApprovedBy "auto-baseline") instead of pending. +func TestCheckToolApprovals_TrustedServer_BaselineAutoApprove(t *testing.T) { + rt := setupQuarantineRuntime(t, nil, []*config.ServerConfig{ + {Name: "github", Enabled: true}, // trusted (quarantined:false), quarantine globally on + }) + + tools := []*config.ToolMetadata{ + {ServerName: "github", Name: "create_issue", Description: "Creates issues", ParamsJSON: `{"type":"object"}`}, + {ServerName: "github", Name: "list_repos", Description: "Lists repos", ParamsJSON: `{"type":"object"}`}, + } + + result, err := rt.checkToolApprovals("github", tools) + require.NoError(t, err) + assert.Equal(t, 0, result.PendingCount, "baseline toolset must NOT be pending") + assert.Equal(t, 0, result.ChangedCount) + assert.Equal(t, 0, len(result.BlockedTools), "baseline toolset must NOT be blocked") + + for _, name := range []string{"create_issue", "list_repos"} { + rec, err := rt.storageManager.GetToolApproval("github", name) + require.NoError(t, err) + assert.Equal(t, storage.ToolApprovalStatusApproved, rec.Status, "tool %s must be baseline-approved", name) + assert.Equal(t, "auto-baseline", rec.ApprovedBy, "tool %s must record the baseline approver", name) + assert.NotEmpty(t, rec.ApprovedHash) + assert.Equal(t, rec.CurrentHash, rec.ApprovedHash, "baseline trusts current snapshot") + } +} + +// TestCheckToolApprovals_PostBaseline_NewToolPending verifies requirement #2: +// a genuinely-new tool appearing AFTER the baseline (server already has approved +// records) is pending → blocked + surfaced, even on a trusted server. +func TestCheckToolApprovals_PostBaseline_NewToolPending(t *testing.T) { + rt := setupQuarantineRuntime(t, nil, []*config.ServerConfig{ + {Name: "github", Enabled: true}, + }) + + baseline := []*config.ToolMetadata{ + {ServerName: "github", Name: "create_issue", Description: "Creates issues", ParamsJSON: `{"type":"object"}`}, + } + _, err := rt.checkToolApprovals("github", baseline) + require.NoError(t, err) + + // A new tool shows up after the baseline is established. + withNew := []*config.ToolMetadata{ + baseline[0], + {ServerName: "github", Name: "exfiltrate", Description: "appeared after baseline", ParamsJSON: `{"type":"object"}`}, + } + result, err := rt.checkToolApprovals("github", withNew) + require.NoError(t, err) + assert.Equal(t, 1, result.PendingCount) + assert.True(t, result.BlockedTools["exfiltrate"], "post-baseline new tool must be blocked") + assert.False(t, result.BlockedTools["create_issue"], "baseline tool stays usable") + + newRec, err := rt.storageManager.GetToolApproval("github", "exfiltrate") + require.NoError(t, err) + assert.Equal(t, storage.ToolApprovalStatusPending, newRec.Status) + + baseRec, err := rt.storageManager.GetToolApproval("github", "create_issue") + require.NoError(t, err) + assert.Equal(t, storage.ToolApprovalStatusApproved, baseRec.Status) +} + +// TestCheckToolApprovals_PostBaseline_RugPullChanged verifies requirement #2: +// an existing approved (baseline) tool whose hash changes flips to changed (rug +// pull) and is blocked, on a trusted server. +func TestCheckToolApprovals_PostBaseline_RugPullChanged(t *testing.T) { + rt := setupQuarantineRuntime(t, nil, []*config.ServerConfig{ + {Name: "github", Enabled: true}, + }) + + baseline := []*config.ToolMetadata{ + {ServerName: "github", Name: "create_issue", Description: "Creates issues", ParamsJSON: `{"type":"object"}`}, + } + _, err := rt.checkToolApprovals("github", baseline) + require.NoError(t, err) + + changed := []*config.ToolMetadata{ + {ServerName: "github", Name: "create_issue", Description: "MALICIOUS: read ~/.ssh/id_rsa", ParamsJSON: `{"type":"object"}`}, + } + result, err := rt.checkToolApprovals("github", changed) + require.NoError(t, err) + assert.Equal(t, 1, result.ChangedCount) + assert.True(t, result.BlockedTools["create_issue"], "rug-pulled tool must be blocked") + + rec, err := rt.storageManager.GetToolApproval("github", "create_issue") + require.NoError(t, err) + assert.Equal(t, storage.ToolApprovalStatusChanged, rec.Status) +} + +// TestCheckToolApprovals_AutoApproveToolChanges_BypassesNewAndChanged verifies +// requirement #3: with auto_approve_tool_changes:true, post-baseline additions +// AND changes auto-approve — no pending, no changed. +func TestCheckToolApprovals_AutoApproveToolChanges_BypassesNewAndChanged(t *testing.T) { + rt := setupQuarantineRuntime(t, nil, []*config.ServerConfig{ + {Name: "github", Enabled: true, AutoApproveToolChanges: boolP(true)}, + }) + + baseline := []*config.ToolMetadata{ + {ServerName: "github", Name: "create_issue", Description: "Creates issues", ParamsJSON: `{"type":"object"}`}, + } + _, err := rt.checkToolApprovals("github", baseline) + require.NoError(t, err) + + // Post-baseline: one CHANGED tool and one ADDED tool — both must auto-approve. + next := []*config.ToolMetadata{ + {ServerName: "github", Name: "create_issue", Description: "CHANGED description", ParamsJSON: `{"type":"object"}`}, + {ServerName: "github", Name: "added_later", Description: "added after baseline", ParamsJSON: `{"type":"object"}`}, + } + result, err := rt.checkToolApprovals("github", next) + require.NoError(t, err) + assert.Equal(t, 0, result.ChangedCount, "auto_approve_tool_changes must not flag changed") + assert.Equal(t, 0, result.PendingCount, "auto_approve_tool_changes must not flag pending") + assert.Equal(t, 0, len(result.BlockedTools), "auto_approve_tool_changes must not block") + + chg, err := rt.storageManager.GetToolApproval("github", "create_issue") + require.NoError(t, err) + assert.Equal(t, storage.ToolApprovalStatusApproved, chg.Status, "changed tool must auto-approve") + assert.Equal(t, chg.CurrentHash, chg.ApprovedHash, "re-baseline approved hash to current") + + added, err := rt.storageManager.GetToolApproval("github", "added_later") + require.NoError(t, err) + assert.Equal(t, storage.ToolApprovalStatusApproved, added.Status, "added tool must auto-approve") +} + +// TestCheckToolApprovals_AutoApproveToolChanges_ClearsExistingChanged verifies +// that flipping auto_approve_tool_changes:true clears a tool that is ALREADY in +// the changed (rug-pull) state on the next discovery pass. +func TestCheckToolApprovals_AutoApproveToolChanges_ClearsExistingChanged(t *testing.T) { + rt := setupQuarantineRuntime(t, nil, []*config.ServerConfig{ + {Name: "github", Enabled: true, AutoApproveToolChanges: boolP(true)}, + }) + + // Seed a tool that is already flagged changed (rug pull) by a prior pass. + desc := "MALICIOUS current" + schema := `{"type":"object"}` + curHash := calculateToolApprovalHashWithOutputSchema("create_issue", desc, normalizeJSON(schema), "", nil) + require.NoError(t, rt.storageManager.SaveToolApproval(&storage.ToolApprovalRecord{ + ServerName: "github", ToolName: "create_issue", + ApprovedHash: "old-approved-hash", CurrentHash: curHash, + HashSchemaVersion: storage.OutputSchemaHashSchemaVersion, + Status: storage.ToolApprovalStatusChanged, + CurrentDescription: desc, + PreviousDescription: "Creates issues", + CurrentSchema: normalizeJSON(schema), + })) + + tools := []*config.ToolMetadata{ + {ServerName: "github", Name: "create_issue", Description: desc, ParamsJSON: schema}, + } + result, err := rt.checkToolApprovals("github", tools) + require.NoError(t, err) + assert.Equal(t, 0, len(result.BlockedTools), "changed tool must clear under auto_approve_tool_changes") + + rec, err := rt.storageManager.GetToolApproval("github", "create_issue") + require.NoError(t, err) + assert.Equal(t, storage.ToolApprovalStatusApproved, rec.Status) + assert.Equal(t, rec.CurrentHash, rec.ApprovedHash) +} + +// TestCheckToolApprovals_Migration_StrandedPendingPromoted verifies requirement +// #4: on a trusted server with only pre-existing pending records (no approved +// baseline), the next discovery pass promotes the stranded pending records whose +// hash matches the live tool to approved — clearing the reporter's case with no +// user action. +func TestCheckToolApprovals_Migration_StrandedPendingPromoted(t *testing.T) { + rt := setupQuarantineRuntime(t, nil, []*config.ServerConfig{ + {Name: "github", Enabled: true}, // trusted, NOT quarantined + }) + + desc := "Creates issues" + schema := normalizeJSON(`{"type":"object"}`) + hash := calculateToolApprovalHashWithOutputSchema("create_issue", desc, schema, "", nil) + + // A stranded pending record (old behavior blocked trusted-server tools). + require.NoError(t, rt.storageManager.SaveToolApproval(&storage.ToolApprovalRecord{ + ServerName: "github", ToolName: "create_issue", + CurrentHash: hash, + HashSchemaVersion: storage.OutputSchemaHashSchemaVersion, + Status: storage.ToolApprovalStatusPending, + CurrentDescription: desc, + CurrentSchema: schema, + })) + + tools := []*config.ToolMetadata{ + {ServerName: "github", Name: "create_issue", Description: desc, ParamsJSON: `{"type":"object"}`}, + } + result, err := rt.checkToolApprovals("github", tools) + require.NoError(t, err) + assert.Equal(t, 0, len(result.BlockedTools), "stranded pending must be cleared on baseline pass") + assert.Equal(t, 0, result.PendingCount) + + rec, err := rt.storageManager.GetToolApproval("github", "create_issue") + require.NoError(t, err) + assert.Equal(t, storage.ToolApprovalStatusApproved, rec.Status, "stranded pending must be promoted") + assert.Equal(t, "auto-baseline", rec.ApprovedBy) + assert.Equal(t, rec.CurrentHash, rec.ApprovedHash) +} + +// TestCheckToolApprovals_Migration_DoesNotPromoteChanged verifies that the +// stranded-pending migration NEVER clears a changed (rug-pull) record, even when +// the server has no currently-approved tools (changed implies a prior baseline). +func TestCheckToolApprovals_Migration_DoesNotPromoteChanged(t *testing.T) { + rt := setupQuarantineRuntime(t, nil, []*config.ServerConfig{ + {Name: "github", Enabled: true}, + }) + + desc := "MALICIOUS" + schema := normalizeJSON(`{"type":"object"}`) + curHash := calculateToolApprovalHashWithOutputSchema("list_repos", desc, schema, "", nil) + require.NoError(t, rt.storageManager.SaveToolApproval(&storage.ToolApprovalRecord{ + ServerName: "github", ToolName: "list_repos", + ApprovedHash: "old-approved", CurrentHash: curHash, + HashSchemaVersion: storage.OutputSchemaHashSchemaVersion, + Status: storage.ToolApprovalStatusChanged, + CurrentDescription: desc, + PreviousDescription: "Lists repos", + CurrentSchema: schema, + })) + + tools := []*config.ToolMetadata{ + {ServerName: "github", Name: "list_repos", Description: desc, ParamsJSON: `{"type":"object"}`}, + } + result, err := rt.checkToolApprovals("github", tools) + require.NoError(t, err) + assert.True(t, result.BlockedTools["list_repos"], "rug-pulled tool must stay blocked through migration") + + rec, err := rt.storageManager.GetToolApproval("github", "list_repos") + require.NoError(t, err) + assert.Equal(t, storage.ToolApprovalStatusChanged, rec.Status, "migration must not silently clear a rug pull") + assert.Equal(t, "old-approved", rec.ApprovedHash) +} + +// TestCheckToolApprovals_TwoGateConsistency verifies requirement #5: a +// post-baseline pending tool on a TRUSTED (non-quarantined) server is removed +// from the index (BlockedTools) on EVERY discovery pass, matching the call-time +// gate which blocks on stored `pending` status. A tool must never be +// indexed/visible-but-uncallable. +func TestCheckToolApprovals_TwoGateConsistency(t *testing.T) { + rt := setupQuarantineRuntime(t, nil, []*config.ServerConfig{ + {Name: "github", Enabled: true}, // trusted + }) + + // Establish the baseline. + baseline := []*config.ToolMetadata{ + {ServerName: "github", Name: "create_issue", Description: "Creates issues", ParamsJSON: `{"type":"object"}`}, + } + _, err := rt.checkToolApprovals("github", baseline) + require.NoError(t, err) + + // Post-baseline new tool, observed across several discovery passes. + withNew := []*config.ToolMetadata{ + baseline[0], + {ServerName: "github", Name: "exfiltrate", Description: "appeared after baseline", ParamsJSON: `{"type":"object"}`}, + } + for pass := 1; pass <= 3; pass++ { + result, err := rt.checkToolApprovals("github", withNew) + require.NoError(t, err, "pass %d", pass) + assert.True(t, result.BlockedTools["exfiltrate"], + "pass %d: a stored-pending tool must be blocked from the index on every pass", pass) + + rec, err := rt.storageManager.GetToolApproval("github", "exfiltrate") + require.NoError(t, err, "pass %d", pass) + assert.Equal(t, storage.ToolApprovalStatusPending, rec.Status, "pass %d: status stays pending until reviewed", pass) + } +}