From 326d90a9db941f5300a0f079676ce5062562e545 Mon Sep 17 00:00:00 2001 From: George Lambert Date: Tue, 22 Sep 2026 11:11:26 -0400 Subject: [PATCH] fix: reject content/bytes on MCP tools Harden stamp/verify/hash_report so unknown and content-shaped args fail instead of being ignored; publish additionalProperties=false and cover with schema/runtime tests. Also keep streamable-http host allowlist. --- .../Done-and-not-done.md | 4 + .../How-to-implement-the-changes.md | 6 ++ .../2026-09-22-reject-content-bytes/README.md | 3 + .../What-to-Change-To.md | 3 + .../What-to-Change.md | 3 + .../git-versions.json | 4 + .../git-versions.md | 4 + src/verae_mcp_timestamping_server/guard.py | 73 ++++++++++++++++ src/verae_mcp_timestamping_server/server.py | 59 ++++++++++++- tests/test_ledger.py | 23 +++++ tests/test_server.py | 85 +++++++++++++++++-- 11 files changed, 258 insertions(+), 9 deletions(-) create mode 100644 recommended-changes/2026-09-22-reject-content-bytes/Done-and-not-done.md create mode 100644 recommended-changes/2026-09-22-reject-content-bytes/How-to-implement-the-changes.md create mode 100644 recommended-changes/2026-09-22-reject-content-bytes/README.md create mode 100644 recommended-changes/2026-09-22-reject-content-bytes/What-to-Change-To.md create mode 100644 recommended-changes/2026-09-22-reject-content-bytes/What-to-Change.md create mode 100644 recommended-changes/2026-09-22-reject-content-bytes/git-versions.json create mode 100644 recommended-changes/2026-09-22-reject-content-bytes/git-versions.md create mode 100644 src/verae_mcp_timestamping_server/guard.py diff --git a/recommended-changes/2026-09-22-reject-content-bytes/Done-and-not-done.md b/recommended-changes/2026-09-22-reject-content-bytes/Done-and-not-done.md new file mode 100644 index 0000000..0484efe --- /dev/null +++ b/recommended-changes/2026-09-22-reject-content-bytes/Done-and-not-done.md @@ -0,0 +1,4 @@ +# Done and not done + +Done: guard.py, server harden, tests, pytest 19 passed. +Not done: WASM client, git commit. diff --git a/recommended-changes/2026-09-22-reject-content-bytes/How-to-implement-the-changes.md b/recommended-changes/2026-09-22-reject-content-bytes/How-to-implement-the-changes.md new file mode 100644 index 0000000..f030a5f --- /dev/null +++ b/recommended-changes/2026-09-22-reject-content-bytes/How-to-implement-the-changes.md @@ -0,0 +1,6 @@ +# How to implement + +1. Add guard.py +2. Harden in build_mcp +3. Expand tests +4. uv run pytest -q diff --git a/recommended-changes/2026-09-22-reject-content-bytes/README.md b/recommended-changes/2026-09-22-reject-content-bytes/README.md new file mode 100644 index 0000000..187541a --- /dev/null +++ b/recommended-changes/2026-09-22-reject-content-bytes/README.md @@ -0,0 +1,3 @@ +# Recommended changes: reject content/bytes + +Review 2026-09-22. See What-to-Change.md. Run: uv run pytest -q diff --git a/recommended-changes/2026-09-22-reject-content-bytes/What-to-Change-To.md b/recommended-changes/2026-09-22-reject-content-bytes/What-to-Change-To.md new file mode 100644 index 0000000..a0967ed --- /dev/null +++ b/recommended-changes/2026-09-22-reject-content-bytes/What-to-Change-To.md @@ -0,0 +1,3 @@ +# What to Change To + +Reject forbidden content keys; forbid unknown args; additionalProperties false; tests pass. diff --git a/recommended-changes/2026-09-22-reject-content-bytes/What-to-Change.md b/recommended-changes/2026-09-22-reject-content-bytes/What-to-Change.md new file mode 100644 index 0000000..e7afa38 --- /dev/null +++ b/recommended-changes/2026-09-22-reject-content-bytes/What-to-Change.md @@ -0,0 +1,3 @@ +# What to Change + +Tool arg models ignored unknown keys so content/bytes/file could be sent and silently dropped. Schema lacked additionalProperties=false. diff --git a/recommended-changes/2026-09-22-reject-content-bytes/git-versions.json b/recommended-changes/2026-09-22-reject-content-bytes/git-versions.json new file mode 100644 index 0000000..4ad4d57 --- /dev/null +++ b/recommended-changes/2026-09-22-reject-content-bytes/git-versions.json @@ -0,0 +1,4 @@ +{ + "head": "e96fa13eddc34c0ad368990189f6c10f36b3ac86", + "dirty": true +} diff --git a/recommended-changes/2026-09-22-reject-content-bytes/git-versions.md b/recommended-changes/2026-09-22-reject-content-bytes/git-versions.md new file mode 100644 index 0000000..0c0763e --- /dev/null +++ b/recommended-changes/2026-09-22-reject-content-bytes/git-versions.md @@ -0,0 +1,4 @@ +# git-versions + +- verae-mcp-timestamping-server main e96fa13-dirty +- HEAD: e96fa13eddc34c0ad368990189f6c10f36b3ac86 diff --git a/src/verae_mcp_timestamping_server/guard.py b/src/verae_mcp_timestamping_server/guard.py new file mode 100644 index 0000000..33140b1 --- /dev/null +++ b/src/verae_mcp_timestamping_server/guard.py @@ -0,0 +1,73 @@ +"""Hash-only contract: reject content/bytes-style tool arguments.""" + +from __future__ import annotations + +from typing import Any, Mapping + +from pydantic import ConfigDict, Field, ValidationError, create_model + +from mcp.server.mcpserver.utilities.func_metadata import ArgModelBase + +FORBIDDEN_CONTENT_KEYS = frozenset( + { + "content", + "bytes", + "file", + "data", + "path", + "body", + "payload", + "blob", + "archive", + "filename", + "filepath", + "file_path", + "raw", + "plaintext", + "document", + } +) + +_REJECT = "content/bytes paths are rejected; send sha256 hex only" + + +class StrictArgModel(ArgModelBase): + """Tool args: unknown keys (including content/bytes) are errors.""" + + model_config = ConfigDict(arbitrary_types_allowed=True, extra="forbid") + + +def reject_content_keys(arguments: Mapping[str, Any] | None) -> None: + """Raise ValidationError if arguments include any content/bytes-style key.""" + if not arguments: + return + hit = sorted({k for k in arguments if k.lower() in FORBIDDEN_CONTENT_KEYS}) + if not hit: + return + raise ValidationError.from_exception_data( + "HashOnlyArguments", + [ + { + "type": "value_error", + "loc": (key,), + "input": arguments.get(key), + "ctx": {"error": _REJECT}, + } + for key in hit + ], + ) + + +def strict_arg_model(existing: type[ArgModelBase]) -> type[ArgModelBase]: + """Rebuild a tool arg model with extra='forbid'.""" + fields: dict[str, Any] = {} + for name, finfo in existing.model_fields.items(): + annotation = finfo.annotation + kwargs: dict[str, Any] = {} + if finfo.alias: + kwargs["alias"] = finfo.alias + if finfo.is_required(): + fields[name] = (annotation, Field(**kwargs)) + else: + fields[name] = (annotation, Field(default=finfo.default, **kwargs)) + return create_model(f"{existing.__name__}Strict", __base__=StrictArgModel, **fields) diff --git a/src/verae_mcp_timestamping_server/server.py b/src/verae_mcp_timestamping_server/server.py index d4fc540..16d078b 100644 --- a/src/verae_mcp_timestamping_server/server.py +++ b/src/verae_mcp_timestamping_server/server.py @@ -8,6 +8,7 @@ from mcp.server import MCPServer from mcp.server.mcpserver.exceptions import ToolError from verae_mcp_timestamping_server.global_client import GlobalClient, GlobalLedger +from verae_mcp_timestamping_server.guard import reject_content_keys, strict_arg_model from verae_mcp_timestamping_server.ledger import ( FileLedger, Ledger, @@ -28,9 +29,38 @@ def default_ledger() -> Ledger: ) +_HARDENED_TOOLS: set[int] = set() + + +def _harden_hash_only_tools(server: MCPServer) -> None: + """Forbid unknown args (content/bytes included); advertise additionalProperties=false.""" + for tool in server._tool_manager.list_tools(): + tid = id(tool) + if tid in _HARDENED_TOOLS: + continue + meta = tool.fn_metadata + object.__setattr__(meta, "arg_model", strict_arg_model(meta.arg_model)) + + def _make_validate(m): + def validate_arguments(arguments_to_validate): + reject_content_keys(arguments_to_validate) + arguments_pre_parsed = m.pre_parse_json(arguments_to_validate) + arguments_parsed_model = m.arg_model.model_validate(arguments_pre_parsed) + return arguments_parsed_model.model_dump_one_level() + + return validate_arguments + + object.__setattr__(meta, "validate_arguments", _make_validate(meta)) + params = meta.arg_model.model_json_schema(by_alias=True) + params["additionalProperties"] = False + object.__setattr__(tool, "parameters", params) + _HARDENED_TOOLS.add(tid) + + def build_mcp(ledger: Ledger | None = None) -> MCPServer: global _ledger _ledger = ledger or default_ledger() + _harden_hash_only_tools(mcp) return mcp @@ -78,11 +108,31 @@ def main() -> None: if transport == "streamable-http": host = os.environ.get("VERAE_MCP_HOST", "127.0.0.1") port = int(os.environ.get("VERAE_MCP_PORT", "8000")) + from mcp.server.transport_security import TransportSecuritySettings + + # Behind Caddy on ns1; allow public hostname + local probes. + allowed = [ + h.strip() + for h in os.environ.get( + "VERAE_MCP_ALLOWED_HOSTS", + "mcp.georgelambert.org,localhost,localhost:8000,127.0.0.1,127.0.0.1:8000,10.1.10.128,10.1.10.128:8000", + ).split(",") + if h.strip() + ] + transport_security = TransportSecuritySettings( + enable_dns_rebinding_protection=True, + allowed_hosts=allowed, + allowed_origins=os.environ.get("VERAE_MCP_ALLOWED_ORIGINS", "*").split(","), + ) if os.environ.get("VERAE_MCP_CORS") == "1": import uvicorn from starlette.middleware.cors import CORSMiddleware - app = mcp.streamable_http_app(json_response=True) + app = mcp.streamable_http_app( + json_response=True, + transport_security=transport_security, + host=host, + ) app.add_middleware( CORSMiddleware, allow_origins=["*"], @@ -92,7 +142,12 @@ def main() -> None: ) uvicorn.run(app, host=host, port=port) return - mcp.run(transport="streamable-http", host=host, port=port) + mcp.run( + transport="streamable-http", + host=host, + port=port, + transport_security=transport_security, + ) return mcp.run() diff --git a/tests/test_ledger.py b/tests/test_ledger.py index baefba5..2d2111b 100644 --- a/tests/test_ledger.py +++ b/tests/test_ledger.py @@ -52,3 +52,26 @@ async def test_hash_report_is_json_array(): report = await ledger.hash_report(SHA256_EMPTY) assert isinstance(report, list) assert report[0].hash == SHA256_EMPTY + + +def test_validate_sha256_rejects_content_shaped_strings(): + from verae_mcp_timestamping_server.ledger import validate_sha256 + import base64 + import pytest + + with pytest.raises(ValueError, match="sha256"): + validate_sha256("") + with pytest.raises(ValueError, match="sha256"): + validate_sha256(base64.b64encode(b"hello world file bytes").decode()) + with pytest.raises(ValueError, match="sha256"): + validate_sha256("line1\nline2") + + +def test_reject_content_keys_lists_forbidden(): + import pytest + from pydantic import ValidationError + from verae_mcp_timestamping_server.guard import reject_content_keys + + with pytest.raises(ValidationError, match="content/bytes paths are rejected"): + reject_content_keys({"sha256": "a" * 64, "content": "nope"}) + reject_content_keys({"sha256": "a" * 64, "scope": "local"}) # ok diff --git a/tests/test_server.py b/tests/test_server.py index 37d64db..09e788e 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -1,14 +1,26 @@ """In-memory MCP Client against hash-only tools.""" +import base64 + import pytest from mcp import Client +from verae_mcp_timestamping_server.guard import FORBIDDEN_CONTENT_KEYS from verae_mcp_timestamping_server.ledger import SHA256_EMPTY, MemoryLedger from verae_mcp_timestamping_server.server import build_mcp pytestmark = pytest.mark.anyio +def _error_text(result) -> str: + parts = [] + for block in result.content or []: + text = getattr(block, "text", None) + if text: + parts.append(text) + return "\n".join(parts) + + async def test_stamp_tool_returns_receipts(): mcp = build_mcp(MemoryLedger()) async with Client(mcp, raise_exceptions=True) as client: @@ -23,16 +35,75 @@ async def test_stamp_tool_returns_receipts(): assert receipts[0]["scope"] == "local" -async def test_stamp_does_not_accept_content_bytes(): +async def test_stamp_and_hash_report_schemas_forbid_content_bytes(): mcp = build_mcp(MemoryLedger()) async with Client(mcp, raise_exceptions=True) as client: listed = await client.list_tools() - stamp = next(t for t in listed.tools if t.name == "stamp") - props = stamp.input_schema["properties"] - assert "sha256" in props - assert "content" not in props - assert "bytes" not in props - assert "file" not in props + by_name = {t.name: t for t in listed.tools} + for name in ("stamp", "hash_report", "verify"): + tool = by_name[name] + props = tool.input_schema["properties"] + for key in FORBIDDEN_CONTENT_KEYS: + assert key not in props, f"{name} schema still lists {key}" + assert tool.input_schema.get("additionalProperties") is False + assert "sha256" in by_name["stamp"].input_schema["properties"] + assert "sha256" in by_name["hash_report"].input_schema["properties"] + + +async def test_stamp_rejects_content_bytes_at_runtime(): + mcp = build_mcp(MemoryLedger()) + async with Client(mcp) as client: + r1 = await client.call_tool( + "stamp", + { + "sha256": SHA256_EMPTY, + "scope": "local", + "content": "this is file bytes", + }, + ) + assert r1.is_error is True + assert "content/bytes paths are rejected" in _error_text(r1) + + r2 = await client.call_tool( + "stamp", + { + "sha256": SHA256_EMPTY, + "bytes": base64.b64encode(b"payload").decode(), + }, + ) + assert r2.is_error is True + assert "content/bytes paths are rejected" in _error_text(r2) + + r3 = await client.call_tool( + "hash_report", + {"sha256": SHA256_EMPTY, "file": "/tmp/secret.bin"}, + ) + assert r3.is_error is True + assert "content/bytes paths are rejected" in _error_text(r3) + + +async def test_stamp_rejects_unknown_extra_keys(): + mcp = build_mcp(MemoryLedger()) + async with Client(mcp) as client: + result = await client.call_tool( + "stamp", + {"sha256": SHA256_EMPTY, "scope": "local", "not_a_real_field": 1}, + ) + assert result.is_error is True + assert "Extra inputs are not permitted" in _error_text(result) + + +async def test_stamp_rejects_non_digest_masquerading_as_sha256(): + mcp = build_mcp(MemoryLedger()) + async with Client(mcp) as client: + long_b64 = base64.b64encode(b"x" * 200).decode() + multiline = "line1\nline2\nline3" + for bad in ("not-a-hash", "", long_b64, multiline, "a" * 128): + result = await client.call_tool( + "stamp", {"sha256": bad, "scope": "local"} + ) + assert result.is_error is True, bad + assert "sha256" in _error_text(result).lower() async def test_verify_and_hash_report_tools():