fix(mcp, ticket-112): validate receipt IDs before cleanup effects

fixos/mcp_server.py

@@ -13,6 +13,7 @@
 import hashlib
 import json
 import os
+import re
 import sys
 import uuid
 from collections.abc import Mapping
@@ -26,6 +27,7 @@
 CATALOG_SCHEMA = "wellmanifest.mcp/operation-catalog/v1"
 RECEIPT_SCHEMA = "wellmanifest.mcp/task-receipt/v1"
 PLAN_SCHEMA = "wellmanifest.nl-dsl-llm/task-plan/v1"
+_REQUEST_ID_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9._-]{0,127}")
 
 OPERATIONS_CATALOG: tuple[dict[str, Any], ...] = (
     {
@@ -100,7 +102,12 @@
                 "dry_run": {"type": "boolean", "default": True},
                 "min_age_days": {"type": "number", "default": 7.0},
                 "include_projects": {"type": "boolean", "default": True},
-                "request_id": {"type": "string", "default": ""},
+                "request_id": {
+                    "type": "string",
+                    "default": "",
+                    "maxLength": 128,
+                    "description": "Opaque ID starting with a letter/digit, using letters, digits, dots, underscores or hyphens; no '..'. Empty generates an ID.",
+                },
             },
             "additionalProperties": False,
         },
@@ -390,10 +397,20 @@ def run_fixos_scan_projects(min_age_days: float = 7.0) -> dict[str, Any]:
     }
 
 
+def _validated_request_id(request_id: str) -> str:
+    """Return a bounded portable receipt name, never a filesystem path."""
+    clean_id = (request_id or "").strip()
+    if not _REQUEST_ID_RE.fullmatch(clean_id) or ".." in clean_id:
+        raise ValueError("invalid_request_id")
+    return clean_id
+
+
 def task_status(request_id: str, *, receipts_dir: str | Path | None = None) -> dict[str, Any]:
     """Read back a durable execution receipt without re-running any effect (wellmanifest/mcp)."""
     clean_id = (request_id or "").strip()
-    if not clean_id or "/" in clean_id or ".." in clean_id:
+    try:
+        clean_id = _validated_request_id(clean_id)
+    except ValueError:
         return {
             "schema": RECEIPT_SCHEMA,
             "found": False,
@@ -430,7 +447,9 @@ def run_fixos_auto_cleanup(
         _cleanup_user_cache,
     )
 
-    req_id = (request_id or "").strip() or f"fixos-mcp-{uuid.uuid4().hex[:12]}"
+    req_id = _validated_request_id(
+        (request_id or "").strip() or f"fixos-mcp-{uuid.uuid4().hex[:12]}"
+    )
     existing = task_status(req_id, receipts_dir=receipts_dir)
     if existing.get("found"):
         return existing

project/ticket-112/README.md

@@ -0,0 +1,34 @@
+# Ticket 112: Reject unsafe MCP receipt request identifiers before cleanup
+
+- **ID**: ticket-112
+- **Owner**: agent:codex
+- **Status**: IN_PROGRESS
+- **Workflow state**: PUBLICATION
+- **Created**: 2026-10-31
+
+## Goal and scope
+
+A controlled dry-run with request_id `../escaped` created a receipt outside the
+configured receipt directory. Reject traversal, absolute paths, unsafe portable
+filenames and overlong identifiers before receipt lookup, scanning or cleanup.
+Retain normal receipt replay and automatically generated identifiers.
+
+SESSION_EXECUTION_AUTHORIZATION: the user requested continuing FixOS repairs
+and protected publication on 2026-10-31 after the earlier MCP repair. This is a
+bounded correction to the same MCP cleanup interface. Prior publication actor
+mismatches remain historical evidence and do not grant approval authority.
+
+## Acceptance criteria
+
+- [x] AC-01: Invalid request IDs fail before receipt directory access or cleanup,
+  for both dry-run and real execution; JSON-RPC reports an error.
+- [x] AC-02: Valid IDs and generated IDs preserve receipt replay; checks pass.
+- [ ] AC-03: Request independent validation and protected publication of the exact head.
+
+## Validation
+
+- A controlled dry-run reproduced receipt traversal outside the receipt root.
+- New regression suite initially failed in 21 cases before the correction.
+- Focused MCP checks: 36 passed; existing receipt replay remains valid.
+- Ruff (including import ordering), diff whitespace and managed governance passed.
+- Full governed tests and independent exact-head publication remain in the delivery process.

project/ticket-112/intent.json

@@ -0,0 +1,79 @@
+{
+  "schema": "new-project.intent/v3",
+  "ticket": "ticket-112",
+  "summary": "Reject unsafe MCP receipt request identifiers before cleanup",
+  "workstream": "application",
+  "classification": {
+    "kind": "BUG",
+    "priority": "P1",
+    "origin": "requested"
+  },
+  "allowedPaths": [
+    "project/ticket-112/**",
+    "fixos/mcp_server.py",
+    "tests/unit/test_mcp_server.py"
+  ],
+  "forbiddenPaths": [
+    "project/ticket-*/user-*.md"
+  ],
+  "stacks": [
+    "python"
+  ],
+  "dependsOn": [],
+  "conflictsWith": [],
+  "integrationTicket": null,
+  "delivery": {
+    "acceptedBaseSha": "da980fac5189756b29d6c987c293c8e793014ad9",
+    "targetBranch": "main",
+    "outcome": "Publish strict receipt request-id validation before MCP cleanup scans or effects, preventing filesystem traversal and unsafe receipt filenames.",
+    "nonGoals": [
+      "No cleanup policy changes",
+      "No changes to existing receipt contents or publication authority",
+      "No runtime dependencies or new MCP operations"
+    ],
+    "complexity": "S",
+    "estimatedMinutes": 30,
+    "budgets": {
+      "maxImplementationFiles": 2,
+      "maxAffectedComponents": 1,
+      "maxPublicInterfaceChanges": 1,
+      "maxRuntimeDependencies": 0
+    },
+    "architecture": {
+      "status": "accepted",
+      "decision": "Share one portable request-id validator between receipt readback and cleanup execution, validating before directory access or scans.",
+      "components": [
+        {
+          "name": "mcp-receipts",
+          "paths": [
+            "fixos/mcp_server.py",
+            "tests/unit/test_mcp_server.py"
+          ]
+        }
+      ],
+      "responsibilityChanges": false,
+      "interfaceChanges": [
+        "Reject malformed request identifiers before any cleanup operation; keep normal and automatically generated identifiers valid."
+      ],
+      "dataChanges": [],
+      "ui": {
+        "impact": "none",
+        "states": [],
+        "evidence": []
+      },
+      "rollback": "Revert through protected publication; preserve receipt history."
+    },
+    "runtimeDependencies": [],
+    "validation": [
+      {
+        "criterion": "AC-01",
+        "commands": [
+          "python -m pytest tests/unit/test_mcp_server.py -q",
+          "python -m ruff check --select I fixos/mcp_server.py tests/unit/test_mcp_server.py",
+          "./project/governance-check.sh"
+        ],
+        "evidence": "project/ticket-112/README.md"
+      }
+    ]
+  }
+}

tests/unit/test_mcp_server.py

@@ -110,6 +110,80 @@ def test_task_execute_rejects_mismatched_digest():
         task_execute("fixos://diagnostics/quick", {}, registry_digest="0" * 64)
 
 
+@pytest.mark.parametrize("dry_run", [True, False])
+@pytest.mark.parametrize(
+    "request_id",
+    ["../escaped", "/absolute", r"..\escaped", "nested/id", "C:escaped", "bad\x00id", "..", ".hidden", "bad id", "x" * 129],
+)
+def test_cleanup_rejects_unsafe_request_ids_before_any_effect(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch, request_id: str, dry_run: bool
+):
+    def unexpected_access(*args, **kwargs):
+        pytest.fail("Invalid request ID reached receipt access or cleanup scanning")
+
+    for name in ("_receipts_dir", "scan_user_cache_dirs", "scan_additional_safe_caches", "scan_build_and_package_caches"):
+        monkeypatch.setattr(f"fixos.mcp_server.{name}", unexpected_access)
+
+    with pytest.raises(ValueError, match="invalid_request_id"):
+        task_execute(
+            "fixos://cleanup/auto",
+            {"dry_run": dry_run, "include_projects": False},
+            request_id=request_id,
+            receipts_dir=tmp_path / "receipts",
+        )
+    assert list(tmp_path.iterdir()) == []
+
+
+def test_jsonrpc_reports_unsafe_receipt_id_without_creating_files(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+):
+    monkeypatch.setattr("fixos.mcp_server.scan_user_cache_dirs", list)
+    monkeypatch.setattr("fixos.mcp_server.scan_additional_safe_caches", list)
+    monkeypatch.setattr("fixos.mcp_server.scan_build_and_package_caches", list)
+    response = handle_jsonrpc_message(
+        {
+            "jsonrpc": "2.0",
+            "id": 1,
+            "method": "tools/call",
+            "params": {
+                "name": "fixos_auto_cleanup",
+                "arguments": {"request_id": "../escaped", "include_projects": False},
+            },
+        },
+        receipts_dir=tmp_path / "receipts",
+    )
+    assert response["result"]["isError"] is True
+    assert response["result"]["structuredContent"]["error"] == "invalid_request_id"
+    assert list(tmp_path.iterdir()) == []
+
+
+@pytest.mark.parametrize("request_id", ["", "../escaped", r"nested\id", "C:escaped", "bad\x00id", "x" * 129])
+def test_status_rejects_unsafe_ids_without_accessing_receipts(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch, request_id: str
+):
+    def unexpected_access(*args, **kwargs):
+        pytest.fail("Invalid status request reached receipt directory access")
+
+    monkeypatch.setattr("fixos.mcp_server._receipts_dir", unexpected_access)
+    result = task_status(request_id, receipts_dir=tmp_path / "receipts")
+    assert result["found"] is False
+    assert result["error"] == "invalid_request_id"
+    assert list(tmp_path.iterdir()) == []
+
+
+def test_generated_receipt_id_remains_replayable(tmp_path: Path, monkeypatch: pytest.MonkeyPatch):
+    monkeypatch.setattr("fixos.mcp_server.scan_user_cache_dirs", list)
+    monkeypatch.setattr("fixos.mcp_server.scan_additional_safe_caches", list)
+    monkeypatch.setattr("fixos.mcp_server.scan_build_and_package_caches", list)
+    result = task_execute(
+        "fixos://cleanup/auto",
+        {"dry_run": True, "include_projects": False},
+        receipts_dir=tmp_path,
+    )
+    assert result["request_id"].startswith("fixos-mcp-")
+    assert task_status(result["request_id"], receipts_dir=tmp_path)["receipt_hash"] == result["receipt_hash"]
+
+
 def test_jsonrpc_protocol_roundtrip(tmp_path: Path, monkeypatch: pytest.MonkeyPatch):
     monkeypatch.setattr(
         "fixos.diagnostics.quick_snapshot.collect_quick_snapshot",