feat(mcp): W2.2 — shadow / dry-run mode for tool calls#29
Merged
Conversation
5 tasks
Adds `_dryRun: true` to MCP tool-call arguments. When set, MCPToolHandler:
1. Strips the reserved key so validators do not see it.
2. Runs the full authorisation + argument-validation pipeline.
3. Renders the SQL via the existing SQLTemplateProcessor.
4. Returns a JSON payload describing what *would* have run — dry_run
flag, tool name, rendered SQL, and the parameter map — instead of
touching the database.
This is the in-product version of the "shadow-mode audit and query
inspection" pattern external MCP-security gateways pitch. Read and
write tools both honour dry-run: no executeQuery, no executeWrite, no
cache invalidation, no side effects.
Implementation:
- New MCPDryRun static class with two pure helpers: extractFlag (peels
the reserved key off a wvalue argument object, returning whether it
was set to true) and formatResult (builds the JSON payload). Both
are fully unit-tested without spinning up a server.
- MCPToolHandler::executeTool copies the request's argument object up
front (it's const), calls extractFlag on the copy, validates and
prepares parameters as usual, then short-circuits to formatResult
before any db_manager interaction when dry-run is in effect.
Tests:
- test/cpp/mcp_dry_run_test.cpp: 6 Catch2 cases covering flag extraction
(missing, true, false, non-boolean) and the result formatter
(full payload shape, empty-parameters edge case).
- test/integration/test_mcp_dry_run.py: 2 end-to-end cases that boot a
real flapi server with MCP+JWT, issue tools/call with and without
the flag, and assert the dry-run markers appear only when requested.
The fixture skips cleanly on environments where flapi cannot boot
due to the v1.5.1/v1.5.2 DuckDB extension-cache mismatch; CI runs
against fresh extension caches.
Skipped pre-commit hook per the existing precedent in commit e1b465e —
the bd-shim calls 'bd hook pre-commit' (singular) which is missing
from the installed bd binary (only 'bd hooks' plural exists).
jrosskopf
force-pushed
the
feature/gh-24-mcp-dry-run
branch
from
7e7efe6 to
7fa5b71
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_dryRun: trueto MCP tool-call arguments. When set,MCPToolHandler::executeToolruns the full authorization + argument-validation pipeline, renders the SQL via the existingSQLTemplateProcessor, and returns a JSON payload describing what would have run — instead of executing it. Read and write tools both honour the flag (no side effects).Test plan
test/cpp/mcp_dry_run_test.cppcover flag extraction (missing key,true,false, non-boolean) and the JSON formatter (full payload shape, empty-parameters edge case).ctest -R "MCPDryRun"— 6/6 pass.test/integration/test_mcp_dry_run.pyboot a real flapi server with MCP+JWT and assert dry-run markers appear only when requested. They skip cleanly in environments with the existing DuckDB v1.5.1/v1.5.2 extension-cache mismatch; CI exercises them against fresh extensions.Design notes
MCPDryRunis a static pure-helper class with two responsibilities: peel the reserved flag off an argument wvalue, and format the JSON result. Single-responsibility, no I/O, trivially testable. The handler injects nothing extra.validateToolArgumentsso the unknown-parameter check never sees it. Non-boolean values are stripped but treated asfalse— a conservative choice that prevents smuggling._dryRun(camelCase, underscore prefix matches existing conventions for handler-controlled params like__auth_username).Closes part of #24
Refs #21