feat: MCP/ToolSafetyContract security hardening (#356, #357, #358, #359, #371)#385
Merged
Conversation
Treat remote MCP server metadata as untrusted input at the adapter boundary: - #371: ingest server ToolAnnotations into a conservative ToolSafetyContract (annotation_trust trust/ignore/cap; cap default). readOnlyHint -> READ, destructiveHint -> DESTRUCTIVE, unannotated -> EXTERNAL; remote determinism pinned to NONE. Provenance recorded on tool.metadata. - #359: MetadataPolicy controls for server-provided names/descriptions (length cap, control-char stripping, whitespace normalisation, placeholder mode, name validation/sanitisation, permissive() escape hatch). Raw server description preserved on tool.metadata for audit. - #358: fingerprint each tool's raw JSON Schema at discovery (schema_dict_fingerprint); verify against supplied pins with on_drift error/warn/accept; build_pin_file/load_pins lockfile helpers. Adds Tool.metadata provenance carrier; MCPMetadataError / MCPSchemaDriftError; ApprovalDeniedError / SafetyCeilingError (for the execution-time enforcement that follows). Exports + public-API snapshot updated. Note: the chainweaver mcp lock CLI command and doctor --check-drift wiring are deferred as a documented follow-up to keep this off the CLI surface.
#356, #357) #356 — execution-time safety enforcement (all opt-in, defaults unchanged): - new chainweaver/approvals.py: ApprovalCallback/ApprovalContext/ ApprovalDecision/ApprovalRecord + coerce_approval_callback, mirroring the decision_callback seam (executor only *calls* the user callback, so the no-LLM/no-network/no-randomness invariants hold). - FlowExecutor(approval_callback=, strict_safety=, max_side_effect_level=): steps whose effective contract has requires_approval=True invoke the callback before the tool runs; DENY / raise / invalid-return / strict-without -callback abort the step with ApprovalDeniedError; max_side_effect_level refuses over-ceiling steps with SafetyCeilingError. Decisions recorded on the new StepRecord.approval, enforced on both sync and async lanes. - contracts.side_effect_exceeds() ordering helper. #357 — dry-run mode: - Tool(dry_run_fn=) + Tool.run_dry()/supports_dry_run; supports_dry_run=True now requires a dry_run_fn at construction. - execute_flow(dry_run=, dry_run_unsupported=): read-only steps run, declared dry_run_fn previews run, other side-effecting steps skip (stub) or abort; step cache and checkpointer bypassed; ExecutionResult.dry_run set. Composed sub-flows inherit the mode via per-thread run-scoped state. Exports + public-API snapshot updated; 1592 passed, 92.58% coverage.
…el (#356, #357, #358, #359, #371) - docs/security.md: consolidated sections for the approval/ceiling enforcement seam (#356), dry-run rehearsals (#357), and the MCP-imported-metadata trust model (annotations->contract / MetadataPolicy / schema pinning). - README error table: ApprovalDeniedError, SafetyCeilingError, MCPMetadataError, MCPSchemaDriftError. - AGENTS.md repo map + StepRecord.approval / ExecutionResult.dry_run field rows. - Fix mypy nits in the new safety tests.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the MCP import boundary and makes ToolSafetyContract actionable at execution time via opt-in enforcement (approvals + side-effect ceilings) and an execution-time dry-run rehearsal mode, with accompanying docs, exports, and tests.
Changes:
- Add execution-time safety gating to
FlowExecutor(approval callback seam,strict_safety,max_side_effect_level) and record approval outcomes on traces. - Add
execute_flow(dry_run=True)to run read-only steps, rundry_run_fnpreviews where available, and skip/abort other side-effecting steps while bypassing cache/checkpointing. - Harden
MCPToolAdapterby deriving conservative safety contracts from annotations, sanitizing remote metadata, and pinning/verifying remote JSON-schema fingerprints.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_mcp_adapter_trust.py | Covers MCP adapter hardening: annotation mapping, metadata policy, schema pinning/drift. |
| tests/test_execution_safety.py | Covers approval gating, side-effect ceilings, and dry-run dispatch/bypass behavior. |
| tests/fixtures/public_api.json | Updates public API snapshot for new exports and signature changes. |
| README.md | Documents newly added typed exceptions and their meanings. |
| docs/security.md | Documents approval enforcement, dry-run mode, and MCP trust/pinning model. |
| chainweaver/tools.py | Adds Tool.metadata, dry_run_fn, and dry-run execution helpers. |
| chainweaver/mcp/adapter.py | Implements annotation→contract mapping, metadata sanitization, schema pinning/drift handling. |
| chainweaver/mcp/init.py | Exposes new MCP adapter surface (policies + pin helpers). |
| chainweaver/executor.py | Enforces safety contracts at runtime and adds dry-run execution path + trace fields. |
| chainweaver/exceptions.py | Adds typed exceptions for approvals/ceilings and MCP metadata/schema drift. |
| chainweaver/contracts.py | Adds side_effect_exceeds() helper for ceiling enforcement. |
| chainweaver/compat.py | Adds schema_dict_fingerprint() for raw JSON Schema pinning. |
| chainweaver/approvals.py | Introduces approval callback protocol/models and callback coercion helper. |
| chainweaver/init.py | Exports new public symbols for approvals, exceptions, and helpers. |
| AGENTS.md | Updates repo map and trace field tables for new modules/fields. |
- Record an explicit ApprovalRecord(DENY, reason) on every denial path (callback raised / invalid return / strict_safety with no callback), so the trace distinguishes 'not gated' from 'gated but denied' (was approval=None). - Async lane now passes step_id=getattr(step, 'step_id', None) into the approval gate so DAG step ids populate ApprovalContext.step_id. - Validate annotation_trust / on_drift in MCPToolAdapter.__init__ so a typo (e.g. on_drift='erorr') fails loudly instead of silently disabling drift protection. - ApprovalDeniedError message normalised to end with exactly one period (repo exception-message convention). - Docs: dry_run_fn is synchronous (not 'same signature as fn' incl. async); MetadataPolicy.permissive() shown called, not as a bare method reference; added a dry_run_fn Args entry. - Tests: assert DENY ApprovalRecord on raise/invalid/strict paths; add adapter policy-literal validation tests. 1594 passed, 92.59% coverage.
…anitisation Address PR #385 audit findings: - Export ApprovalCallable and coerce_approval_callback from the chainweaver package __all__ so the approval seam mirrors its sibling decisions.py seam (which exports DecisionCallable / coerce_decision_callback). Users writing typed approval callbacks can now import the alias and coercion helper from the top-level package. - MetadataPolicy.apply_name now re-validates a sanitised tool name against a custom name_pattern and raises MCPMetadataError when it still does not match, instead of returning a name that silently violates the configured policy. The default pattern is unaffected (its charset matches the sanitiser). - Refresh tests/fixtures/public_api.json for the two new public exports.
This was referenced Jun 14, 2026
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
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
Implements the recommended five-issue MCP /
ToolSafetyContractsecurity hardening cluster as a single coherent PR. The spine is theToolSafetyContract:MCPToolAdapternow populates it conservatively from untrusted remote metadata (#371), the executor enforces it at run time (#356) and previews via it (#357), and the adapter import boundary is hardened against name/description abuse (#359) and silent schema drift (#358). All five are opt-in or behaviour-preserving by default.Changes
#371 — Ingest MCP
ToolAnnotations→ToolSafetyContract(mcp/adapter.py)MCPToolAdapter(annotation_trust="trust"|"ignore"|"cap")(default"cap").readOnlyHint → READ(neverNONE— a remote call still observes the world),destructiveHint → DESTRUCTIVE, unannotated →EXTERNAL; remotedeterminism_levelalwaysNONE. Contract source recorded ontool.metadata.#359 — Trust controls for server-provided metadata (
mcp/adapter.py)MetadataPolicy(control-char stripping, whitespace normalisation, length cap,description_mode="placeholder", name validation/sanitisation,permissive()escape hatch). Raw server description preserved ontool.metadata["mcp_raw_description"].#358 — Pin & verify MCP-adapted tool schemas (
mcp/adapter.py,compat.py)compat.schema_dict_fingerprint(); raw input/output schemas fingerprinted at discovery (tool.metadata["mcp_schema_hash"]);discover_tools(pins=/pins_path=)+on_drift="error"|"warn"|"accept";build_pin_file()/load_pins()lockfile helpers.#356 — Enforce
ToolSafetyContractat execution (newapprovals.py,executor.py,contracts.py)ApprovalCallback/ApprovalContext/ApprovalDecision/ApprovalRecordseam (mirrorsdecision_callback— executor only calls the user seam, so the no-LLM/no-network/no-randomness invariants hold).FlowExecutor(approval_callback=, strict_safety=, max_side_effect_level=); decisions recorded onStepRecord.approval; enforced on both sync and async lanes.contracts.side_effect_exceeds()helper.#357 — Execution-time dry-run (
executor.py,tools.py)Tool(dry_run_fn=)+Tool.run_dry()/supports_dry_run;execute_flow(dry_run=, dry_run_unsupported=): read-only steps run, declareddry_run_fnpreviews run, other side-effecting steps skip (stub) or abort; cache + checkpointer bypassed;ExecutionResult.dry_runset; composed sub-flows inherit the mode.Public API exports +
tests/fixtures/public_api.jsonsnapshot updated;docs/security.md, README error table, and AGENTS.md repo map / field tables updated.Testing
ruff check chainweaver/ tests/ examples/) — All checks passed!ruff format --check chainweaver/ tests/ examples/) — 168 files already formattedpython -m mypy chainweaver/ tests/) — Success: no issues in 168 source filespython -m pytest tests/) — 1592 passed, 1 skipped, 92.58% coveragetests/test_mcp_adapter_trust.py(25 cases),tests/test_execution_safety.py(20 cases) — annotation-mapping matrix, metadata policy, schema-pin drift, approve/deny/raise/invalid/strict/ceiling, dry-run dispatch + cache bypass + serialization round-trip.Related Issues
Closes #356, #357, #358, #359, #371.
Mode B scope notes / deltas
annotation_trustdefault"cap"andMetadataPolicydefaults on are deliberate (conservative) behaviour changes for MCP-imported tools;annotation_trust="ignore"/MetadataPolicy.permissive()restore prior behaviour. Flagged for maintainer confirmation.chainweaver mcp lockcommand anddoctor --check-driftwiring are intentionally not included to keep this PR off the CLI surface; the library-level pin/verify + lockfile helpers deliver the core protection. Suggested as a focused follow-up.dry_run_fnis"skip"(stub) for a useful wiring rehearsal;"abort"is available for high-fidelity runs.execute_flow(sync) feature per the issue framing.Checklist
AGENTS.mdanddocs/agent-context/)__all__, snapshot,docs/security.md, README, AGENTS.md)https://claude.ai/code/session_01TRXw5DESD7cYUkPiKQqBKU
Generated by Claude Code