From 18f04790dc8a825ed249c31b7257d6a9a1c57393 Mon Sep 17 00:00:00 2001 From: konrad tjaden Date: Tue, 20 Oct 2026 07:55:15 +0200 Subject: [PATCH 01/11] test: add reusable HTTP MCP backend read campaign harness --- .github/workflows/pull-request-validation.yml | 14 + docs/mcp-http/README.md | 2 + docs/mcp-http/campaign-testing.md | 96 +++++++ tests/http_mcp/campaign_reads.py | 270 ++++++++++++++++++ tests/http_mcp/read-plan.example.json | 13 + tests/http_mcp/requirements.txt | 1 + tests/http_mcp/test_campaign_reads.py | 107 +++++++ 7 files changed, 503 insertions(+) create mode 100644 docs/mcp-http/campaign-testing.md create mode 100644 tests/http_mcp/campaign_reads.py create mode 100644 tests/http_mcp/read-plan.example.json create mode 100644 tests/http_mcp/requirements.txt create mode 100644 tests/http_mcp/test_campaign_reads.py diff --git a/.github/workflows/pull-request-validation.yml b/.github/workflows/pull-request-validation.yml index 24b7f6f7..613a83fc 100644 --- a/.github/workflows/pull-request-validation.yml +++ b/.github/workflows/pull-request-validation.yml @@ -27,6 +27,20 @@ concurrency: cancel-in-progress: true jobs: + campaign-read-harness: + name: Offline HTTP MCP campaign harness + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Check out triggering commit + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Verify generic harness failure boundaries without live credentials + run: | + python3 -m venv .venv-campaign + .venv-campaign/bin/python -m pip install -r tests/http_mcp/requirements.txt + .venv-campaign/bin/python -m unittest discover -s tests/http_mcp -p 'test_*.py' + public-disclosure: name: Public disclosure gate runs-on: ubuntu-latest diff --git a/docs/mcp-http/README.md b/docs/mcp-http/README.md index d87f97b1..78f19be2 100644 --- a/docs/mcp-http/README.md +++ b/docs/mcp-http/README.md @@ -21,5 +21,7 @@ share the Web public origin or use an explicitly configured standalone origin. [browser-approved pairing](../integrations/browser-pairing.md). - [Operation contracts, previews and transport authority](../contracts/mcp-operation-contracts.md): executable schemas, fixed raw reads, identifiers and required prerequisites. +- [Repeatable campaign checks](campaign-testing.md): private live MCP/backend read + captures, generic repository test collateral and future capability coverage. Keep runnable recipes in the [Docker HTTP MCP example](../../docker/examples/mcp-http/README.md). diff --git a/docs/mcp-http/campaign-testing.md b/docs/mcp-http/campaign-testing.md new file mode 100644 index 00000000..854fe8c5 --- /dev/null +++ b/docs/mcp-http/campaign-testing.md @@ -0,0 +1,96 @@ +# Repeatable HTTP MCP campaign checks + +Test a business capability through HTTP MCP, then independently observe its backend +state, UI or actual provider effect. API-only calls cannot establish an MCP pass. +Python here is external test tooling; the application and MCP runtime remain C#. + +The complete beta.8 requirements and acceptance gate remain in the +[canonical campaign guide](../implementation/0.1.2-beta.8-campaign.md), +[operation catalog](../contracts/mcp-operation-catalog.json) and +[offline validator](../../tools/ci/validate-beta8-campaign.py). Retain their fixed +denominators, mode exclusions, independent evidence, provider receipts, soak and +cleanup requirements. Source CI and a subset of live reads cannot satisfy that gate. + +## Run selected live read checks + +The [read harness](../../tests/http_mcp/campaign_reads.py) is a reusable starting +point for checking HTTP MCP/backend communication on owned fixtures. It initializes +the actual endpoint, compares its complete tools/schema with the supplied catalog, +executes selected advertised reads, then reads the corresponding API with a fresh +request-scoped execution token. Both observations and fixture-specific typed JSON +pointer assertions must agree. Empty results do not count as positive fixture coverage. + +Use a private directory outside the checkout with access restricted to the campaign +operator, including its parent directory on Windows. Place a profile there: + +```json +{ + "resourceUri": "https://helpdesk.example.invalid/mcp", + "credentialFile": "approved-mcp-credential.key" +} +``` + +Store the explicitly approved, account-bound MCP-purpose credential in that separate +file. Use an ordinary scoped credential for ordinary access checks and a separately +approved resource-bound owner grant where the case requires owner authority. The +harness never discovers ambient CLI, browser, NetRatel or another client's credentials. + +Copy the [example plan](../../tests/http_mcp/read-plan.example.json) into the private +directory and replace its placeholders with actual owned fixture values. Verify the +deployment source/version/image pins and released catalog hashes using the full +campaign preflight before running a plan. Install dependencies into a dedicated +environment, then run from the repository root: + +```sh +python -m pip install -r tests/http_mcp/requirements.txt +python tests/http_mcp/campaign_reads.py \ + --catalog docs/contracts/mcp-operation-catalog.json \ + --plan /private/campaign/read-plan.json \ + --profile /private/campaign/profile.json \ + --output /private/campaign/new-read-captures +``` + +PowerShell users can run the same command on one line with absolute Windows paths. +The output directory must be new, its parent must exist, and the profile, credential +and output must be outside the checkout. Each run uses fresh RPC IDs and hashed +sanitized JSON captures. Tokens stay in memory and known confidential fields, signed +URL tokens and duplicate MCP JSON text are redacted before capture persistence. +Other business records still make these captures private. Do not commit them. + +The runner accepts only catalog-advertised GET/read-only operations. It refuses +schema drift, redirects, plaintext HTTP, missing assertions, mismatched responses +and automatic retries. Its summary describes selected supporting reads, not a full +campaign receipt. Volatile responses may legitimately differ between reads; keep +the failed captures and use a dedicated case with precise durable assertions rather +than broadly ignoring changed fields. + +## Expand coverage with application capabilities + +When adding application behavior, update the typed API and MCP operation schemas, +regenerate the operation catalog through the existing source workflow, and add the +operation to the campaign contract. Add owned fixture plans and specific independent +postconditions for the new behavior. Preserve ordinary-access and foreign-tenant +denials alongside owner success. Record public provider contract regressions in the +owning C# test project; record deployed executions separately. + +For mutations, extend a dedicated campaign lane with an unchanged preview, one +confirmed MCP execution and an independent persisted effect. Record fixture ownership +before dependent work. Reconcile uncertain outcomes before considering a retry; +never resend an external effect solely because the client timed out. Read-only +parity is insufficient for provider dispatch, approval commits, mail delivery, +idempotency, revocation, concurrency and cleanup. + +Keep generic code and synthetic plans in the repository. Keep credentials, passwords, +one-time grants, private hosts, live customer data, raw captures, evidence archives +and deployment-specific fixture registries in the private campaign directory. Publish +reviewed findings without those identifiers. Never copy a laptop campaign folder +wholesale into this public repository. + +Run the offline harness regressions with: + +```sh +python -m unittest discover -s tests/http_mcp -p 'test_*.py' +``` + +These synthetic offline tests verify the harness's failure boundaries. They do not +claim deployed MCP operation coverage or change the immutable beta.8 gate. diff --git a/tests/http_mcp/campaign_reads.py b/tests/http_mcp/campaign_reads.py new file mode 100644 index 00000000..0c849204 --- /dev/null +++ b/tests/http_mcp/campaign_reads.py @@ -0,0 +1,270 @@ +#!/usr/bin/env python3 +"""Opt-in deployed HTTP MCP reads, independently checked against the backend. + +This produces private supporting captures, not a full campaign acceptance receipt. +No mutation, credential issuance, target fallback or ambient profile discovery. +""" + +import argparse +import hashlib +import json +from pathlib import Path +import re +import sys +import time +from datetime import datetime, timezone +from urllib.error import HTTPError +from urllib.parse import quote, urlencode, urlsplit +from urllib.request import build_opener, HTTPRedirectHandler, Request +import uuid + +from jsonschema import Draft202012Validator, FormatChecker + + +class CampaignError(Exception): + """Safe diagnostic: never includes a response body or credential.""" + + +class NoRedirect(HTTPRedirectHandler): + def redirect_request(self, req, fp, code, msg, headers, newurl): + raise CampaignError("Redirect refused; verify the explicitly selected endpoint") + + +def utc(): + return datetime.now(timezone.utc).isoformat() + + +def pointer(value, path): + if path == "": + return value + if not path.startswith("/"): + raise CampaignError("Assertion must use a JSON pointer") + for part in path[1:].split("/"): + part = part.replace("~1", "/").replace("~0", "~") + value = value[int(part)] if isinstance(value, list) else value[part] + return value + + +def read_operation(catalog, case): + rows = {x["tool"] + "." + x["operation"]: x for x in catalog["operations"]} + op = rows[case["operation"]] + if op["method"] != "GET" or op["readOnly"] is not True: + raise CampaignError("This runner accepts only advertised GET/read-only operations") + if op.get("sensitiveResultFields"): + raise CampaignError("Declared secret-returning operations need a dedicated private lane") + schema = next(x["inputSchema"] for x in catalog["toolSchemas"] if x["tool"] == op["tool"]) + args = {"operation": op["operation"]} + if "request" in case: + args["request"] = case["request"] + if not Draft202012Validator(schema, format_checker=FormatChecker()).is_valid(args): + raise CampaignError("Plan arguments do not satisfy the selected catalog schema") + if not case.get("assertions"): + raise CampaignError("Every read needs fixture-specific typed assertions") + path = op["pathTemplate"] + query = {} + for name, value in case.get("request", {}).items(): + selector = "{" + name + "}" + if selector in path: + path = path.replace(selector, quote(str(value), safe="")) + else: + if isinstance(value, bool): + value = str(value).lower() + query[name] = value + if "{" in path or not path.startswith("/api/v1/"): + raise CampaignError("Unresolved or unsupported independent API path") + if any(part in (".", "..") for part in path.split("/")): + raise CampaignError("Dot path selectors are not supported") + return op, args, path + (("?" + urlencode(query, doseq=True)) if query else "") + + +def compare(mcp, api, assertions): + if mcp.get("success") is not True or "data" not in mcp: + raise CampaignError("MCP read did not succeed with structured data") + if mcp["data"] in (None, [], {}): + raise CampaignError("Empty observations do not establish positive fixture coverage") + if mcp["data"] != api: + raise CampaignError("MCP data differs from the independent API observation") + for check in assertions: + observed = pointer(api, check["pointer"]) + if type(observed) is not type(check["expected"]) or observed != check["expected"]: + raise CampaignError("Typed fixture assertion failed") + + +class Captures: + def __init__(self, directory, secrets=()): + self.directory = directory + self.secrets = list(secrets) + self.sequence = 0 + + def redact(self, value): + protected = {"password", "newpassword", "currentpassword", "smtppassword", + "secret", "signingsecret", "clientsecret", "accesstoken", + "refreshtoken", "plaintextkey", "apikey", "pairingcode", "pairingsecret", + "activationtoken", "integrationcredential", "authorization"} + if isinstance(value, dict): + return {k: "[REDACTED]" if k.casefold() in protected else self.redact(v) + for k, v in value.items()} + if isinstance(value, list): + return [self.redact(v) for v in value] + if isinstance(value, str): + # MCP duplicates structured content in JSON text blocks. + if value.lstrip().startswith(("{", "[")): + try: + return json.dumps(self.redact(json.loads(value))) + except ValueError: + pass + for secret in self.secrets: + if secret: + value = value.replace(secret, "[REDACTED]") + value = re.sub(r"(?i)([?&](?:token|key|code)=)[^&\s]+", r"\1[REDACTED]", value) + return value + + def write(self, kind, value): + self.sequence += 1 + path = self.directory / f"{self.sequence:05d}-{kind}.json" + raw = (json.dumps(self.redact(value), indent=2) + "\n").encode() + path.write_bytes(raw) + return {"path": path.name, "sha256": hashlib.sha256(raw).hexdigest()} + + +class Transport: + def __init__(self, resource, credential, captures): + uri = urlsplit(resource) + if (uri.scheme != "https" or not uri.hostname or uri.username or uri.password + or uri.path != "/mcp" or uri.query or uri.fragment): + raise CampaignError("Select an explicit HTTPS origin with the exact /mcp resource") + self.resource = resource + self.origin = uri.scheme + "://" + uri.netloc + self.credential = credential + self.captures = captures + self.opener = build_opener(NoRedirect()) + self.session = None + self.last_call = 0 + + def request(self, url, method, body, headers): + started = utc() + tick = time.monotonic() + request = Request(url, body, headers, method=method) + try: + with self.opener.open(request, timeout=30) as response: + raw = response.read(8 * 1024 * 1024 + 1) + if len(raw) > 8 * 1024 * 1024: + raise CampaignError("Response exceeded the capture bound") + return raw, response.headers, started, (time.monotonic() - tick) * 1000 + except HTTPError as error: + raise CampaignError(f"HTTP {error.code}; no automatic retry or target fallback") from None + + def rpc(self, method, params, notification=False): + delay = 0.5 - (time.monotonic() - self.last_call) + if delay > 0: + time.sleep(delay) + message = {"jsonrpc": "2.0", "method": method, "params": params} + if not notification: + message["id"] = str(uuid.uuid4()) + request_ref = self.captures.write("mcp-request", message) + headers = {"Authorization": "Bearer " + self.credential, + "Content-Type": "application/json", "Accept": "application/json, text/event-stream", + "MCP-Protocol-Version": "2025-03-26"} + if self.session: + headers["Mcp-Session-Id"] = self.session + raw, response_headers, started, elapsed = self.request( + self.resource, "POST", json.dumps(message).encode(), headers) + self.last_call = time.monotonic() + self.session = response_headers.get("Mcp-Session-Id", self.session) + if notification: + result = {} + elif raw.lstrip().startswith(b"{"): + result = json.loads(raw) + else: + events = [json.loads(line[5:].strip()) for line in raw.decode().splitlines() + if line.startswith("data:")] + result = next(x for x in events if x.get("id") == message["id"]) + if not notification and result.get("id") != message["id"]: + raise CampaignError("MCP response ID did not match its request") + return result, {"request": request_ref, + "response": self.captures.write("mcp-response", result), + "startedAt": started, "latencyMs": elapsed} + + def independent_api(self, path): + raw, _, _, _ = self.request(self.origin + "/api/v1/mcp/execution-token", "POST", b"", + {"Authorization": "Bearer " + self.credential, + "X-RatelDesk-Mcp-Resource": self.resource}) + exchange = json.loads(raw) + token = exchange["accessToken"] + self.captures.secrets.append(token) + self.captures.write("api-execution-exchange", exchange) + raw, _, at, _ = self.request(self.origin + path, "GET", None, + {"Authorization": "Bearer " + token, "Accept": "application/json"}) + body = json.loads(raw) + return body, self.captures.write("api-independent", {"observedAt": at, "path": path, "body": body}) + + +def run(args): + repo = Path(__file__).resolve().parents[2] + output = args.output.resolve() + profile_path = args.profile.resolve() + if output.is_relative_to(repo) or profile_path.is_relative_to(repo): + raise CampaignError("Profile and evidence must remain outside the source checkout") + catalog_bytes = args.catalog.read_bytes() + catalog = json.loads(catalog_bytes) + plan = json.loads(args.plan.read_text(encoding="utf-8")) + # Validate the whole plan before issuing even an initialize call. + prepared = [read_operation(catalog, case) for case in plan["cases"]] + ids = [case["id"] for case in plan["cases"]] + if not ids or len(ids) != len(set(ids)): + raise CampaignError("Use at least one case with unique case IDs") + profile = json.loads(profile_path.read_text(encoding="utf-8-sig")) + keypath = Path(profile["credentialFile"]) + keypath = keypath if keypath.is_absolute() else profile_path.parent / keypath + if keypath.resolve().is_relative_to(repo) or keypath.stat().st_size > 16384: + raise CampaignError("Credential file must be bounded and outside the checkout") + credential = keypath.read_text(encoding="utf-8-sig").strip() + if not credential or "\n" in credential or "\r" in credential: + raise CampaignError("Credential file must contain one nonempty credential") + # Refuse existing output rather than silently mixing candidate runs. + output.mkdir(mode=0o700, parents=False, exist_ok=False) + captures = Captures(output, [credential]) + transport = Transport(profile["resourceUri"], credential, captures) + summary = {"startedAt": utc(), "catalogSha256": hashlib.sha256(catalog_bytes).hexdigest(), + "planSha256": hashlib.sha256(args.plan.read_bytes()).hexdigest(), + "scope": "Selected read-only supporting evidence; not full campaign acceptance", + "cases": [], "completed": False} + try: + initialized, _ = transport.rpc("initialize", {"protocolVersion": "2025-03-26", "capabilities": {}, + "clientInfo": {"name": "rateldesk-campaign-reads", "version": "1"}}) + if initialized.get("result", {}).get("protocolVersion") != "2025-03-26": + raise CampaignError("MCP initialize did not negotiate the supported protocol") + transport.rpc("notifications/initialized", {}, notification=True) + live, _ = transport.rpc("tools/list", {}) + expected = {x["tool"]: x["inputSchema"] for x in catalog["toolSchemas"]} + actual = {x["name"]: x["inputSchema"] for x in live.get("result", {}).get("tools", [])} + if live.get("result", {}).get("nextCursor") or actual != expected: + raise CampaignError("Live tools/schema differ from the supplied released catalog") + for case, (op, arguments, path) in zip(plan["cases"], prepared): + wire, mcp_ref = transport.rpc("tools/call", {"name": op["tool"], "arguments": arguments}) + result = wire.get("result", {}).get("structuredContent", {}) + api, api_ref = transport.independent_api(path) + compare(result, api, case["assertions"]) + summary["cases"].append({"id": case["id"], "operation": case["operation"], + "outcome": "PASS", "mcp": mcp_ref, "independentApi": api_ref, + "assertions": case["assertions"]}) + summary["completed"] = True + finally: + summary["completedAt"] = utc() + captures.write("read-summary", summary) + print(f"Verified {len(summary['cases'])} selected MCP/API reads; supporting evidence only.") + + +if __name__ == "__main__": + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--catalog", type=Path, required=True) + parser.add_argument("--plan", type=Path, required=True) + parser.add_argument("--profile", type=Path, required=True) + parser.add_argument("--output", type=Path, required=True) + try: + run(parser.parse_args()) + except Exception as error: + # Untrusted response/error text may contain secrets. Do not dump it. + print(str(error) if isinstance(error, CampaignError) else + "Read campaign stopped (" + type(error).__name__ + "); inspect private captures.", file=sys.stderr) + sys.exit(1) diff --git a/tests/http_mcp/read-plan.example.json b/tests/http_mcp/read-plan.example.json new file mode 100644 index 00000000..faaac14e --- /dev/null +++ b/tests/http_mcp/read-plan.example.json @@ -0,0 +1,13 @@ +{ + "cases": [ + { + "id": "owned-incident-read", + "operation": "helpdesk_incidents.get", + "request": {"incidentId": "REPLACE_WITH_OWNED_INCIDENT_ID"}, + "assertions": [ + {"pointer": "/id", "expected": "REPLACE_WITH_OWNED_INCIDENT_ID"}, + {"pointer": "/trackingId", "expected": "REPLACE_WITH_OBSERVED_TRACKING_ID"} + ] + } + ] +} diff --git a/tests/http_mcp/requirements.txt b/tests/http_mcp/requirements.txt new file mode 100644 index 00000000..426346b5 --- /dev/null +++ b/tests/http_mcp/requirements.txt @@ -0,0 +1 @@ +jsonschema>=4.21,<5 diff --git a/tests/http_mcp/test_campaign_reads.py b/tests/http_mcp/test_campaign_reads.py new file mode 100644 index 00000000..2925d6d6 --- /dev/null +++ b/tests/http_mcp/test_campaign_reads.py @@ -0,0 +1,107 @@ +"""Offline harness regression tests; these are not deployed acceptance results.""" + +import json +from pathlib import Path +import tempfile +import unittest +from unittest.mock import patch +from types import SimpleNamespace + +from campaign_reads import CampaignError, Captures, NoRedirect, Transport, compare, read_operation, run + + +CATALOG = json.loads((Path(__file__).resolve().parents[2] / + "docs/contracts/mcp-operation-catalog.json").read_text()) + + +class ReadCampaignTests(unittest.TestCase): + def case(self): + return {"id": "owned-incident", "operation": "helpdesk_incidents.get", + "request": {"incidentId": "owned-fixture"}, + "assertions": [{"pointer": "/id", "expected": "owned-fixture"}]} + + def test_catalog_builds_exact_read_route(self): + op, args, path = read_operation(CATALOG, self.case()) + self.assertEqual(op["tool"], "helpdesk_incidents") + self.assertEqual(args, {"operation": "get", "request": {"incidentId": "owned-fixture"}}) + self.assertEqual(path, "/api/v1/incidents/owned-fixture") + + def test_mutation_is_rejected_before_transport(self): + case = {**self.case(), "operation": "helpdesk_incidents.delete"} + with self.assertRaises(CampaignError): + read_operation(CATALOG, case) + + def test_unknown_request_fields_are_rejected(self): + case = self.case() + case["request"]["unexpected"] = True + with self.assertRaises(CampaignError): + read_operation(CATALOG, case) + + def test_fixture_assertions_are_required(self): + case = self.case() + case["assertions"] = [] + with self.assertRaises(CampaignError): + read_operation(CATALOG, case) + + def test_dot_selector_cannot_escape_independent_api_route(self): + case = self.case() + case["request"]["incidentId"] = ".." + with self.assertRaises(CampaignError): + read_operation(CATALOG, case) + + def test_profile_inside_checkout_is_rejected_before_network(self): + with tempfile.TemporaryDirectory() as temporary: + with patch("campaign_reads.Transport") as transport: + with self.assertRaises(CampaignError): + run(SimpleNamespace(output=Path(temporary) / "new", + profile=Path(__file__).resolve())) + transport.assert_not_called() + + def test_api_success_cannot_replace_failed_mcp(self): + with self.assertRaises(CampaignError): + compare({"success": False, "data": {"id": "owned-fixture"}}, + {"id": "owned-fixture"}, self.case()["assertions"]) + + def test_different_mcp_projection_fails(self): + with self.assertRaises(CampaignError): + compare({"success": True, "data": {"id": "wrong"}}, + {"id": "owned-fixture"}, self.case()["assertions"]) + + def test_empty_reads_do_not_count_as_fixture_coverage(self): + for value in (None, [], {}): + with self.assertRaises(CampaignError): + compare({"success": True, "data": value}, value, [{"pointer": "", "expected": value}]) + + def test_boolean_does_not_match_integer_assertion(self): + with self.assertRaises(CampaignError): + compare({"success": True, "data": {"n": 1}}, {"n": 1}, + [{"pointer": "/n", "expected": True}]) + + def test_matching_mcp_api_and_typed_fixture(self): + compare({"success": True, "data": {"id": "owned-fixture"}}, + {"id": "owned-fixture"}, self.case()["assertions"]) + + def test_redaction_covers_mcp_duplicate_json_and_signed_url(self): + with tempfile.TemporaryDirectory() as temporary: + captures = Captures(Path(temporary), ["synthetic-sensitive-value"]) + ref = captures.write("test", {"accessToken": "synthetic-sensitive-value", + "content": [{"text": json.dumps({"signingSecret": "other-synthetic-secret"})}], + "url": "https://example.invalid/image?token=synthetic-token&size=1", + "message": "synthetic-sensitive-value"}) + raw = (Path(temporary) / ref["path"]).read_text() + for value in ("synthetic-sensitive-value", "other-synthetic-secret", "synthetic-token"): + self.assertNotIn(value, raw) + + def test_plain_http_and_noncanonical_resources_are_refused(self): + for uri in ("http://example.invalid/mcp", "https://example.invalid/other/mcp", + "https://example.invalid/mcp?token=x", "https://user@example.invalid/mcp"): + with self.assertRaises(CampaignError): + Transport(uri, "synthetic", None) + + def test_redirect_cannot_forward_credential_to_other_origin(self): + with self.assertRaises(CampaignError): + NoRedirect().redirect_request(None, None, 302, "", {}, "https://other.invalid/mcp") + + +if __name__ == "__main__": + unittest.main() From e060887b7c86bac0dcaa8c01fd72dcaec30a39bd Mon Sep 17 00:00:00 2001 From: konrad tjaden Date: Tue, 20 Oct 2026 08:20:59 +0200 Subject: [PATCH 02/11] test: bound credential reads and retain sanitized HTTP failures --- docs/mcp-http/campaign-testing.md | 4 ++++ tests/http_mcp/campaign_reads.py | 25 +++++++++++++++++++++---- tests/http_mcp/test_campaign_reads.py | 24 ++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/docs/mcp-http/campaign-testing.md b/docs/mcp-http/campaign-testing.md index 854fe8c5..af491b06 100644 --- a/docs/mcp-http/campaign-testing.md +++ b/docs/mcp-http/campaign-testing.md @@ -80,6 +80,10 @@ never resend an external effect solely because the client timed out. Read-only parity is insufficient for provider dispatch, approval commits, mail delivery, idempotency, revocation, concurrency and cleanup. +Plans use flat route/query selectors and scalar query lists. Null optional query +values are omitted; nested query objects need a dedicated lane. HTTP denials retain +a bounded, sanitized structured error capture and stop the run without a retry. + Keep generic code and synthetic plans in the repository. Keep credentials, passwords, one-time grants, private hosts, live customer data, raw captures, evidence archives and deployment-specific fixture registries in the private campaign directory. Publish diff --git a/tests/http_mcp/campaign_reads.py b/tests/http_mcp/campaign_reads.py index 0c849204..b36282e8 100644 --- a/tests/http_mcp/campaign_reads.py +++ b/tests/http_mcp/campaign_reads.py @@ -67,6 +67,11 @@ def read_operation(catalog, case): if selector in path: path = path.replace(selector, quote(str(value), safe="")) else: + if value is None: + continue + if isinstance(value, dict) or (isinstance(value, list) and + any(isinstance(item, (dict, list)) for item in value)): + raise CampaignError("Nested query arguments require a dedicated read lane") if isinstance(value, bool): value = str(value).lower() query[name] = value @@ -97,7 +102,7 @@ def __init__(self, directory, secrets=()): self.sequence = 0 def redact(self, value): - protected = {"password", "newpassword", "currentpassword", "smtppassword", + protected = {"password", "newpassword", "currentpassword", "smtppassword", "token", "secret", "signingsecret", "clientsecret", "accesstoken", "refreshtoken", "plaintextkey", "apikey", "pairingcode", "pairingsecret", "activationtoken", "integrationcredential", "authorization"} @@ -152,6 +157,14 @@ def request(self, url, method, body, headers): raise CampaignError("Response exceeded the capture bound") return raw, response.headers, started, (time.monotonic() - tick) * 1000 except HTTPError as error: + raw = error.read(8 * 1024 * 1024 + 1) + try: + error_body = json.loads(raw[:8 * 1024 * 1024]) + except (ValueError, UnicodeError): + error_body = {"nonJsonOrTruncated": True} + self.captures.write("http-error", {"observedAt": utc(), "method": method, + "httpStatus": error.code, "body": error_body, + "responseExceededBound": len(raw) > 8 * 1024 * 1024}) raise CampaignError(f"HTTP {error.code}; no automatic retry or target fallback") from None def rpc(self, method, params, notification=False): @@ -216,9 +229,13 @@ def run(args): profile = json.loads(profile_path.read_text(encoding="utf-8-sig")) keypath = Path(profile["credentialFile"]) keypath = keypath if keypath.is_absolute() else profile_path.parent / keypath - if keypath.resolve().is_relative_to(repo) or keypath.stat().st_size > 16384: - raise CampaignError("Credential file must be bounded and outside the checkout") - credential = keypath.read_text(encoding="utf-8-sig").strip() + if keypath.resolve().is_relative_to(repo): + raise CampaignError("Credential file must be outside the checkout") + with keypath.open("rb") as source: + credential_bytes = source.read(16385) + if len(credential_bytes) > 16384: + raise CampaignError("Credential file exceeds the supported bound") + credential = credential_bytes.decode("utf-8-sig").strip() if not credential or "\n" in credential or "\r" in credential: raise CampaignError("Credential file must contain one nonempty credential") # Refuse existing output rather than silently mixing candidate runs. diff --git a/tests/http_mcp/test_campaign_reads.py b/tests/http_mcp/test_campaign_reads.py index 2925d6d6..73849203 100644 --- a/tests/http_mcp/test_campaign_reads.py +++ b/tests/http_mcp/test_campaign_reads.py @@ -1,11 +1,13 @@ """Offline harness regression tests; these are not deployed acceptance results.""" import json +import io from pathlib import Path import tempfile import unittest from unittest.mock import patch from types import SimpleNamespace +from urllib.error import HTTPError from campaign_reads import CampaignError, Captures, NoRedirect, Transport, compare, read_operation, run @@ -43,6 +45,13 @@ def test_fixture_assertions_are_required(self): with self.assertRaises(CampaignError): read_operation(CATALOG, case) + def test_optional_null_query_is_omitted(self): + case = {"id": "overdue", "operation": "helpdesk_automation.overdue_tasks", + "request": {"page": None, "pageSize": 5}, + "assertions": [{"pointer": "/total", "expected": 1}]} + _, _, path = read_operation(CATALOG, case) + self.assertEqual(path, "/api/v1/ops/tasks/overdue?pageSize=5") + def test_dot_selector_cannot_escape_independent_api_route(self): case = self.case() case["request"]["incidentId"] = ".." @@ -102,6 +111,21 @@ def test_redirect_cannot_forward_credential_to_other_origin(self): with self.assertRaises(CampaignError): NoRedirect().redirect_request(None, None, 302, "", {}, "https://other.invalid/mcp") + def test_http_denial_is_captured_with_secret_redacted(self): + with tempfile.TemporaryDirectory() as temporary: + captures = Captures(Path(temporary), ["synthetic-credential"]) + transport = Transport("https://example.invalid/mcp", "synthetic-credential", captures) + error = HTTPError("https://example.invalid/mcp", 403, "denied", {}, + io.BytesIO(b'{"accessToken":"synthetic-response-secret"}')) + with patch.object(transport.opener, "open", side_effect=error): + with self.assertRaises(CampaignError): + transport.request(transport.resource, "POST", b"{}", {}) + files = list(Path(temporary).glob("*-http-error.json")) + self.assertEqual(len(files), 1) + captured = files[0].read_text() + self.assertNotIn("synthetic-response-secret", captured) + self.assertEqual(json.loads(captured)["httpStatus"], 403) + if __name__ == "__main__": unittest.main() From d0a9d3a80eaaa40a6c3a27d711701c11549e844c Mon Sep 17 00:00:00 2001 From: konrad tjaden Date: Tue, 20 Oct 2026 09:51:49 +0200 Subject: [PATCH 03/11] Cover escaped secrets in campaign capture regressions --- docs/mcp-http/campaign-testing.md | 2 ++ tests/http_mcp/campaign_reads.py | 3 ++- tests/http_mcp/test_campaign_reads.py | 22 ++++++++++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/docs/mcp-http/campaign-testing.md b/docs/mcp-http/campaign-testing.md index af491b06..b3e671eb 100644 --- a/docs/mcp-http/campaign-testing.md +++ b/docs/mcp-http/campaign-testing.md @@ -55,6 +55,8 @@ The output directory must be new, its parent must exist, and the profile, creden and output must be outside the checkout. Each run uses fresh RPC IDs and hashed sanitized JSON captures. Tokens stay in memory and known confidential fields, signed URL tokens and duplicate MCP JSON text are redacted before capture persistence. +Regression checks cover Unicode-escaped secrets in duplicate JSON text and signed +URL tokens inside HTML-escaped links. Other business records still make these captures private. Do not commit them. The runner accepts only catalog-advertised GET/read-only operations. It refuses diff --git a/tests/http_mcp/campaign_reads.py b/tests/http_mcp/campaign_reads.py index b36282e8..41193402 100644 --- a/tests/http_mcp/campaign_reads.py +++ b/tests/http_mcp/campaign_reads.py @@ -121,7 +121,8 @@ def redact(self, value): for secret in self.secrets: if secret: value = value.replace(secret, "[REDACTED]") - value = re.sub(r"(?i)([?&](?:token|key|code)=)[^&\s]+", r"\1[REDACTED]", value) + value = re.sub(r"(?i)([?&](?:amp;)?(?:token|key|code)=)[^&\s<>\"']+", + r"\1[REDACTED]", value) return value def write(self, kind, value): diff --git a/tests/http_mcp/test_campaign_reads.py b/tests/http_mcp/test_campaign_reads.py index 73849203..7698d956 100644 --- a/tests/http_mcp/test_campaign_reads.py +++ b/tests/http_mcp/test_campaign_reads.py @@ -107,6 +107,28 @@ def test_plain_http_and_noncanonical_resources_are_refused(self): with self.assertRaises(CampaignError): Transport(uri, "synthetic", None) + def test_duplicate_json_text_decodes_unicode_escaped_secret_before_redaction(self): + with tempfile.TemporaryDirectory() as temporary: + captures = Captures(Path(temporary), []) + duplicate = json.dumps({"activationToken": "synthetic+activation/value="}) + duplicate = duplicate.replace("+", "\\u002B").replace("/", "\\u002F") + ref = captures.write("test", {"id": "synthetic-rpc-id", + "content": [{"type": "text", "text": duplicate}]}) + captured = json.loads((Path(temporary) / ref["path"]).read_text()) + self.assertEqual(captured["id"], "synthetic-rpc-id") + nested = json.loads(captured["content"][0]["text"]) + self.assertEqual(nested["activationToken"], "[REDACTED]") + + def test_html_escaped_signed_url_is_redacted_without_losing_link_markup(self): + with tempfile.TemporaryDirectory() as temporary: + captures = Captures(Path(temporary), []) + ref = captures.write("test", {"html": + 'View'}) + captured = json.loads((Path(temporary) / ref["path"]).read_text()) + self.assertNotIn("synthetic-private-link", captured["html"]) + self.assertIn('&token=[REDACTED]">View', captured["html"]) + def test_redirect_cannot_forward_credential_to_other_origin(self): with self.assertRaises(CampaignError): NoRedirect().redirect_request(None, None, 302, "", {}, "https://other.invalid/mcp") From 1112286e7e3e1248d110c6aa21a328b7ef99f2f4 Mon Sep 17 00:00:00 2001 From: Jeremi Avenant Date: Tue, 20 Oct 2026 11:10:08 +0000 Subject: [PATCH 04/11] Repair beta.8 campaign defects for beta.9 with focused regressions --- Directory.Build.props | 2 +- docs/contracts/mcp-operation-catalog.json | 23 ++-- .../implementation/0.1.2-beta.9-baseline.json | 106 ++++++++++++++++++ .../0.1.2-beta.9-mcp-regressions.md | 72 ++++++++++++ docs/implementation/README.md | 1 + docs/mcp-http/campaign-testing.md | 7 ++ docs/release-notes/0.1.2-beta.9.md | 13 +++ docs/release-notes/README.md | 6 + docs/releases/README.md | 8 ++ .../Categories/TicketCategoryEndpoints.cs | 7 +- .../Organizations/TenantBrandingEndpoints.cs | 2 + .../Endpoints/Services/ServiceEndpoints.cs | 1 + .../Tickets/PublicTicketEndpoints.cs | 2 +- .../HelpdeskAgentClient.cs | 3 + .../Resources/ResourceDatasetAbstractions.cs | 1 + .../OrchestrationInternalClient.cs | 10 +- .../RequestTaskApprovalService.cs | 9 +- .../Resources/DataManagementService.cs | 16 ++- .../Tools/HelpdeskToolDefinitions.cs | 46 +++++++- .../Tools/McpOperationCatalog.cs | 2 +- .../Tools/OperatorOperationCatalog.cs | 2 +- src/Helpdesk.ServiceDefaults/Extensions.cs | 2 +- .../Api/AiAgentOpsLogBufferTests.cs | 23 ++++ .../Api/Beta9WorkflowAndBrandingHttpTests.cs | 103 +++++++++++++++++ .../Api/DatasetEndpointsAuthorizationTests.cs | 2 + .../Api/PublicTicketEndpointsTests.cs | 52 ++++++++- .../Api/ServiceItemsEndpointsTests.cs | 16 +++ .../Api/TicketCategoryEndpointsTests.cs | 26 +++++ .../Api/WorkflowOpsEndpointsTests.cs | 17 +++ .../OrchestrationInternalClientTests.cs | 20 ++++ ...RequestTaskApprovalStartupProviderTests.cs | 52 +++++++++ .../Resources/OperatorDatasetWriterTests.cs | 34 ++++++ .../Mcp/AgentClientHttpTests.cs | 15 +++ .../Mcp/McpHttpBeta9RegressionTests.cs | 78 +++++++++++++ tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs | 24 +++- tests/http_mcp/campaign_reads.py | 14 ++- tests/http_mcp/test_campaign_reads.py | 19 +++- 37 files changed, 801 insertions(+), 35 deletions(-) create mode 100644 docs/implementation/0.1.2-beta.9-baseline.json create mode 100644 docs/implementation/0.1.2-beta.9-mcp-regressions.md create mode 100644 docs/release-notes/0.1.2-beta.9.md create mode 100644 tests/Helpdesk.Tests/Api/Beta9WorkflowAndBrandingHttpTests.cs create mode 100644 tests/Helpdesk.Tests/Infrastructure/RequestTasks/RequestTaskApprovalStartupProviderTests.cs create mode 100644 tests/Helpdesk.Tests/Mcp/McpHttpBeta9RegressionTests.cs diff --git a/Directory.Build.props b/Directory.Build.props index 5b4bd54e..dfa59aa1 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -1,7 +1,7 @@ 0.1.2 - beta.8 + beta.9 $(VersionPrefix) $(VersionPrefix)-$(VersionSuffix) diff --git a/docs/contracts/mcp-operation-catalog.json b/docs/contracts/mcp-operation-catalog.json index 4e77d14f..dd6f7e1b 100644 --- a/docs/contracts/mcp-operation-catalog.json +++ b/docs/contracts/mcp-operation-catalog.json @@ -1,7 +1,7 @@ { - "schemaHash": "dd1d6d04490d740851d4d7e20bae92ca15ba259dec95994c8b0cda4d328d3c14", - "catalogRevision": "2026-10-19.mcp-owner-operations-v23", - "catalogHash": "f8aa2de86bf2cf46d123ba06a8bde1235ca0156758c4d1aead4febe8179ff243", + "schemaHash": "fc3603d87ad56fc994cd77385474874a5bc5c9df79a7dc825f5785460463a6f7", + "catalogRevision": "2026-10-20.beta9-contracts-v24", + "catalogHash": "44d767508d35628a4a9c562e842834ad97053435133890646b417edd5b189f74", "operationCount": 389, "operations": [ { @@ -15761,7 +15761,7 @@ "readOnly": true, "confirmationRequired": false, "previewSupported": false, - "description": "Read workflow operational counters.", + "description": "Read organization-filtered workflow operational counters as Prometheus text.", "sensitiveResultFields": [], "transports": [ "stdio", @@ -15769,16 +15769,17 @@ ], "transportSupport": { "stdio": "Available with explicitly configured API identity; authority and prerequisites apply", - "http": "requires_explicit_administrator_identity: ordinary bounded integration credential is denied" + "http": "available_subject_to_identity_permission_scope_and_prerequisites" }, "credentialModes": [ - "explicit mcp-owner resource-bound grant from a current instance administrator (HTTP gateway)", - "explicit external Authentik identity satisfying HelpdeskAdmin" + "api-purpose account-bound credential (stdio)", + "mcp-purpose resource-bound credential with request-scoped API execution token (HTTP gateway)", + "explicit external Authentik identity" ], - "authority": "HelpdeskAdmin; existing API identity, tenant and record-specific checks apply", + "authority": "Request.Read, Request.Write or Request.Manager in the selected organizations; existing API identity, tenant and record-specific checks apply", "boundedIntegrationCredential": { - "support": "denied_by_policy", - "reason": "HelpdeskAdmin is instance-wide authority. Assignable scoped grants do not confer it; use an explicitly supported administrator identity." + "support": "permission_and_organization_intersection", + "reason": "API checks account state, expiry, revocation, credential purpose/resource, granted permissions and organization scope." }, "ownerAdministrationCredential": { "purpose": "mcp-owner", @@ -50236,7 +50237,7 @@ "properties": { "operation": { "const": "workflow_metrics", - "description": "Read workflow operational counters. Select this operation in the outer operation field." + "description": "Read organization-filtered workflow operational counters as Prometheus text. Select this operation in the outer operation field." }, "confirm": { "type": "boolean", diff --git a/docs/implementation/0.1.2-beta.9-baseline.json b/docs/implementation/0.1.2-beta.9-baseline.json new file mode 100644 index 00000000..3961df12 --- /dev/null +++ b/docs/implementation/0.1.2-beta.9-baseline.json @@ -0,0 +1,106 @@ +{ + "baseline": { + "version": "0.1.2-beta.8", + "sourceRevision": "548c94c482d70d5db1620cb1fd77dd65245e8fb7", + "readiness": "NOT_READY", + "operationCounts": { + "FAIL": 16, + "PASS": 350, + "EXCLUDED": 7, + "NOT_RUN": 16 + }, + "archiveSha256": "3d5d96c18664c4a91335cc5f1332d180fdf621c5f816076d08a46e8c84873acb", + "finalResultsSha256": "5792e3d4d6bf1670255a04918850aa51ddce0832dd76c7328566d55b6ef8500f" + }, + "findings": [ + { + "id": "MCP-B8-WORKFLOW-METRICS", + "evidence": { + "path": "workflow-metrics-diagnosis/finding.json", + "sha256": "277c3e20e3070351e7252179fae8d42d44891a78245532d837dfd6b72c46ca99" + } + }, + { + "id": "MCP-B8-UNKNOWN-OUTER-ARGUMENT", + "evidence": { + "path": "schema-boundaries/unknown-envelope-finding.json", + "sha256": "6c0271f68ad3af65e14baf505f7a670be73fd650f69b3bb245f776686e6cff0b" + } + }, + { + "id": "API-B8-SERVICE-SCOPE-PROJECTION", + "evidence": { + "path": "service-projection/finding.json", + "sha256": "746023f0dad04522f6ccd9b10833a3d73c395a44d065a43e38154490bd60e4ad" + } + }, + { + "id": "API-B8-CATEGORY-TENANT-ID-FORMAT", + "evidence": { + "path": "category-tenant-diagnosis/finding.json", + "sha256": "4ea9662ac5b18a195ab84aeeb6b69bfda03b5cba2b929733364189b659fb968b" + } + }, + { + "id": "API-B8-SELF-SERVICE-POSTCOMMIT-500", + "evidence": { + "path": "self-service-postcommit-diagnosis/finding-with-manual-completion-trigger-v3.json", + "sha256": "9b7bd1f7c65605e922e8cd14a4a2a650ce4562e8a6f82a7d56c72ced09b5b704" + } + }, + { + "id": "API-B8-NUMERIC-LOGO-ANTIFORGERY", + "evidence": { + "path": "numeric-logo-diagnosis/finding.json", + "sha256": "1148e670601b7104396c2536adab7583d0997ead7e88d448508ad479cfaeca7f" + } + }, + { + "id": "B8-ORCHESTRATION-MANAGED-IDENTITY-CONTRACT", + "evidence": { + "path": "provider-contract-mismatch/finding.json", + "sha256": "f344daa34e653313c0b5ccc3b004c4a8fe024a5fa0abb4f7c3a01423e634fa86" + } + }, + { + "id": "B8-OPS-LOG-OBSERVABILITY-GAP", + "evidence": { + "path": "native-log-failure-observability/finding.json", + "sha256": "7c344872d2b394fccc3ac0d220c6b9c2e607a71fe3af2c62dc7374c977c1daff" + } + }, + { + "id": "B8-SELF-SERVICE-DATASET-KEY-LOOKUP", + "evidence": { + "path": "self-service-dataset-binding/valid-key-rejection-finding-with-verified-workaround-v2.json", + "sha256": "b71a9a11c7a03bc180d324347d14e40a699c5b6d2f28993d935afdab6768441c" + } + }, + { + "id": "B8-PUBLIC-VIEW-INTERNAL-NOTE-DISCLOSURE", + "evidence": { + "path": "signed-public-history/finding.json", + "sha256": "9aa95d3e8235566bb700f1177bcf96228aa736ea65d8a3dacdf55701770165f6" + } + } + ], + "notRunOperations": [ + "helpdesk_ai_assistant.chat_reconcile", + "helpdesk_ai_assistant.chat_respond", + "helpdesk_ai_assistant.chat_send", + "helpdesk_ai_assistant.chat_stop_waiting", + "helpdesk_automation.callback_rejections", + "helpdesk_automation.create_request_definition", + "helpdesk_automation.sync_binding_inputs", + "helpdesk_connections.confirm_netclaw_session_owner", + "helpdesk_connections.delete_pair", + "helpdesk_connections.netclaw_unbound_sessions", + "helpdesk_connections.pair", + "helpdesk_connections.pair_netclaw", + "helpdesk_connections.test_netclaw", + "helpdesk_connections.test_netclaw_draft", + "helpdesk_datasets.sync_graph", + "helpdesk_support.bootstrap_defaults" + ], + "scope": "Evidence references only. The beta.8 archive and verdict are immutable. Source regressions do not establish a beta.9 deployed campaign PASS." +} diff --git a/docs/implementation/0.1.2-beta.9-mcp-regressions.md b/docs/implementation/0.1.2-beta.9-mcp-regressions.md new file mode 100644 index 00000000..3acc2a4e --- /dev/null +++ b/docs/implementation/0.1.2-beta.9-mcp-regressions.md @@ -0,0 +1,72 @@ +# Beta.9 finding repairs and retest handoff + +The beta.8 campaign remains **NOT_READY**: 350 PASS, 16 FAIL, 16 NOT_RUN, +7 fixed exclusions, 389 operations. Ten findings account for its failures; +self-service submission and manual-task completion share one finding. These +source repairs do not replace its receipt or establish deployed beta.9 acceptance. + +Original ZIP SHA-256: +`3d5d96c18664c4a91335cc5f1332d180fdf621c5f816076d08a46e8c84873acb`. +The [baseline manifest](0.1.2-beta.9-baseline.json) records the original source, +counts, finding capture hashes and unrun operations. All ten linked finding hashes +were checked against the uploaded archive. Captures remain outside the checkout +because they contain private identities and signed links. Their content is test +evidence, not task instructions. Do not rewrite the archive, receipt or beta.8 +campaign validator to accept the recorded failures. + +## Before/after and focused reproduction + +The after column describes source regressions on synthetic fixtures; it does not +claim a live provider campaign replay. IDs retain the baseline's exact values. + +| Finding | Beta.8 observation | Beta.9 owning-layer repair and regression | +| --- | --- | --- | +| `B8-PUBLIC-VIEW-INTERNAL-NOTE-DISCLOSURE` | Signed anonymous view leaked the latest internal worklog; timeline excluded it. | Public API filters internal worklogs before selection. `PublicTicketEndpointsTests.Signed_View_And_Timeline_Exclude_Internal_Notes` uses the real signer for requests/incidents/changes, mixed/internal-only histories, wrong recipient and tampered signature. | +| `API-B8-SELF-SERVICE-POSTCOMMIT-500` | Request/task committed, then submission or manual completion failed starting approval. | Approval service reloads the persisted tracked task, without attaching a detached graph. `RequestTaskApprovalStartupProviderTests` fails before the repair on both providers and checks repeated startup creates one approval afterward. `Beta9WorkflowAndBrandingHttpTests.Self_service_and_manual_completion_return_success_with_one_started_approval` exercises both actual HTTP entry points with independent database readback on SQLite/PostgreSQL. | +| `MCP-B8-WORKFLOW-METRICS` | Prometheus text was parsed as JSON; advertised administrator-only authority disagreed with API permissions. | AgentClient explicitly adapts only the metrics route's `text/plain` to `{format:"prometheus",text:...}`. Catalog describes existing scoped Request permissions. `McpHttpHostTests.Beta9_HTTP_MCP_reads_prometheus_metrics_as_structured_text`, AgentClient and WorkflowOps tests check transport/authority. Unrelated non-JSON responses remain errors. Python harness extends independent API parity for this projection. | +| `MCP-B8-UNKNOWN-OUTER-ARGUMENT` | Four observed tools discarded unknown outer fields despite closed schemas. | Shared tool wrapper validates operation envelopes before SDK reflection binding, including raw duplicate fields. `McpHttpHostTests.Beta9_HTTP_rejects_unknown_envelopes_for_every_catalog_operation_before_upstream_IO` covers all 389 operations and zero backend/token requests; companion stdio regression covers both unknown and duplicate fields. | +| `API-B8-SERVICE-SCOPE-PROJECTION` | Persisted organization audience missing from create/update/get/list DTOs. | API projection copies `AllowedOrganizationIds`; `ServiceItemsEndpointsTests.Service_create_update_get_and_list_preserve_organization_audience` checks all four responses. Existing audience tests preserve foreign customer denial. | +| `API-B8-CATEGORY-TENANT-ID-FORMAT` | D-format GUID lookup could not find N-format stored organization. | API accepts either persisted format and compares permission scopes as GUIDs. `TicketCategoryEndpointsTests.Compact_organization_ID_supports_scoped_category_CRUD` covers create/update/list/delete and foreign read denial on SQLite/PostgreSQL. | +| `API-B8-NUMERIC-LOGO-ANTIFORGERY` | Numeric-tenant multipart logo failed because endpoint antiforgery middleware was absent. | API explicitly disables antiforgery for the authenticated multipart route, preserving administrator authorization. `Beta9WorkflowAndBrandingHttpTests.Numeric_branding_accepts_authenticated_multipart_and_serves_exact_signed_bytes` checks upload, stored URL, exact signed binary readback and tampered-token denial on both providers. | +| `B8-ORCHESTRATION-MANAGED-IDENTITY-CONTRACT` | Managed NetRatel identity projection lacked legacy `claims` and was rejected. | Provider parser accepts the bounded managed projection and legacy claims. `OrchestrationInternalClientTests.Identity_accepts_managed_and_legacy_contracts_and_rejects_malformed_success` checks managed/global-null-tenant/legacy shapes and malformed/HTML/partial 200s. No live provider success is inferred. | +| `B8-OPS-LOG-OBSERVABILITY-GAP` | Actual native transport warning reached stdout but API/MCP logs were empty. | Service-default Serilog forwards to registered ILogger providers and the ops buffer. `AiAgentOpsLogBufferTests.Service_defaults_forward_real_transport_warning_to_searchable_redacted_ops_buffer` emits a real configured application warning, checks conversation/trace search, redaction and wrong-trace exclusion. | +| `B8-SELF-SERVICE-DATASET-KEY-LOOKUP` | Valid option key rejected by label-only search lookup. | Data service resolves the current scoped row by key independently of search columns, cache and paging. `OperatorDatasetWriterTests.Bound_selection_resolves_exact_current_key_without_label_search_or_page_limits` checks 61 rows, disjoint key/label, case canonicalization, frozen label and missing/foreign/inactive rejection. | + +## Reproduce source verification + +Use the pinned SDK, Docker for PostgreSQL cases, and an isolated checkout: + +```sh +dotnet build Helpdesk.sln +dotnet test Helpdesk.sln --no-build --logger "trx;LogFileName=helpdesk-tests.trx" --results-directory TestResults +tools/ci/validate-layout.sh +python3 tools/ci/validate-docs.py +python3 -m unittest discover -s tests/http_mcp -p 'test_*.py' +``` + +The HTTP/stdio tests use real hosts with synthetic backends or isolated application +databases. Relational HTTP tests use real routes and fresh readback scopes. +TRX output is retained by the normal `.NET validation` CI job. This branch includes +the unchanged [PR #248](https://github.com/BostonTechnologies/RatelDesk/pull/248) +harness foundation at `d0a9d3a80eaaa40a6c3a27d711701c11549e844c` and extends it. +Use [campaign testing](../mcp-http/campaign-testing.md) for separately authorized +live reads with a fresh profile, candidate catalog and evidence directory. The +exact deployed beta.9 must receive a fresh campaign before any readiness verdict. + +## Sixteen operations still need live evidence + +All remain NOT_RUN in the immutable baseline; source tests do not promote them. + +| Operations | Fresh live prerequisite | +| --- | --- | +| `helpdesk_ai_assistant.chat_send`, `.chat_respond`, `.chat_reconcile`, `.chat_stop_waiting` | Healthy native provider authentication, owned paired conversation and actual turn/reconciliation state. Do not repeat delivery-uncertain sends. | +| `helpdesk_automation.create_request_definition`, `.sync_binding_inputs` | Provider authoring/input-sync permissions and owned definitions/bindings. | +| `helpdesk_automation.callback_rejections` | Populated owned rejection fixture and authorized readback. | +| `helpdesk_connections.pair`, `.delete_pair` | Disposable owned pairing lifecycle on both selected systems. | +| `helpdesk_connections.pair_netclaw`, `.test_netclaw`, `.test_netclaw_draft`, `.netclaw_unbound_sessions`, `.confirm_netclaw_session_owner` | Explicit pairing credentials/permissions, healthy connectivity and owned unbound-session fixtures with reviewed ownership counts. | +| `helpdesk_datasets.sync_graph` | Explicit Graph credential and directory permissions for the owned organization. | +| `helpdesk_support.bootstrap_defaults` | Isolated instance or authority over every affected organization. The original capture records a global operation with no selector and unrelated routing data that would be changed. This is an isolation prerequisite, not a missing credential or a demonstrated product defect. | + +The baseline's SMTP partially accepted deliveries remain ReviewRequired. This +work does not retry them or alter provider sessions, mail controls or retained +campaign fixtures. diff --git a/docs/implementation/README.md b/docs/implementation/README.md index 0c1a9676..3e876f51 100644 --- a/docs/implementation/README.md +++ b/docs/implementation/README.md @@ -15,3 +15,4 @@ guidance is linked from the [documentation index](../README.md). - [0.1.2-beta.5 MCP regression and next-campaign handoff](0.1.2-beta.5-mcp-regressions.md) - [0.1.2-beta.6 individual acceptance register and executable closure gate](0.1.2-beta.6-acceptance/README.md) - [0.1.2-beta.8 owner HTTP-MCP campaign contract and receipt guide](0.1.2-beta.8-campaign.md) +- [0.1.2-beta.9 finding repairs and live retest handoff](0.1.2-beta.9-mcp-regressions.md) diff --git a/docs/mcp-http/campaign-testing.md b/docs/mcp-http/campaign-testing.md index b3e671eb..b7a46e88 100644 --- a/docs/mcp-http/campaign-testing.md +++ b/docs/mcp-http/campaign-testing.md @@ -1,5 +1,12 @@ # Repeatable HTTP MCP campaign checks +For beta.9, the runner independently adapts the workflow metrics API's explicit +Prometheus `text/plain` contract to `{format:"prometheus",text:...}`, matching MCP. +Other API reads still require JSON. Include a fixture-specific `/text` assertion; +for logs, assert the actual known warning in `/items/0/message` and its trace, +so an empty search cannot establish a positive result. See the +[beta.9 finding/retest map](../implementation/0.1.2-beta.9-mcp-regressions.md). + Test a business capability through HTTP MCP, then independently observe its backend state, UI or actual provider effect. API-only calls cannot establish an MCP pass. Python here is external test tooling; the application and MCP runtime remain C#. diff --git a/docs/release-notes/0.1.2-beta.9.md b/docs/release-notes/0.1.2-beta.9.md new file mode 100644 index 00000000..1a5726b1 --- /dev/null +++ b/docs/release-notes/0.1.2-beta.9.md @@ -0,0 +1,13 @@ +# 0.1.2-beta.9 candidate + +Repairs ten beta.8 findings, prioritizing internal-note exclusion from signed +public views and tracked approval startup after self-service/manual-task commits. +Also repairs organization audience DTOs, GUID category lookup, numeric-tenant +logo upload, exact dataset key binding, managed NetRatel identity diagnostics, +searchable application logging, metrics transport and MCP argument enforcement. + +See the [finding-by-finding regression and live handoff](../implementation/0.1.2-beta.9-mcp-regressions.md). +Beta.8 remains NOT_READY and immutable. Sixteen operations still need live +provider, pairing, populated-state or isolation evidence. Source/CI tests do not +establish deployed beta.9 readiness. This candidate does not publish artifacts, +deploy an owner instance or move `latest`. diff --git a/docs/release-notes/README.md b/docs/release-notes/README.md index 65797c70..ff3e586c 100644 --- a/docs/release-notes/README.md +++ b/docs/release-notes/README.md @@ -1,5 +1,11 @@ # Release notes +## 0.1.2-beta.9 candidate + +[Beta.8 campaign finding repairs](0.1.2-beta.9.md) cover signed public views, +approval startup, API/provider contracts, logging and strict MCP boundaries. +Deployed beta.9 acceptance requires a separate fresh campaign. + ## 0.1.2-beta.8 [Delegated owner operations over HTTP MCP](0.1.2-beta.8.md) add explicit diff --git a/docs/releases/README.md b/docs/releases/README.md index add03df2..2f41fae2 100644 --- a/docs/releases/README.md +++ b/docs/releases/README.md @@ -1,5 +1,13 @@ # Releases +## 0.1.2-beta.9 candidate — campaign finding repairs + +[Candidate notes](../release-notes/0.1.2-beta.9.md) and the +[finding-to-regression map](../implementation/0.1.2-beta.9-mcp-regressions.md) +separate source verification from fresh deployed acceptance. Beta.8 evidence is +immutable; sixteen live operations remain unproven. Publication follows the +normal reviewed merge and protected release workflow. + RatelDesk has one repository-owned release line. The root `Directory.Build.props` is the source of truth for `VersionPrefix` and `VersionSuffix`. Applications and release outputs inherit those values. Prereleases remain immutable. ## 0.1.2-beta.8 — delegated owner operations diff --git a/src/Helpdesk.API/Endpoints/Categories/TicketCategoryEndpoints.cs b/src/Helpdesk.API/Endpoints/Categories/TicketCategoryEndpoints.cs index 14479c2c..e95a83cf 100644 --- a/src/Helpdesk.API/Endpoints/Categories/TicketCategoryEndpoints.cs +++ b/src/Helpdesk.API/Endpoints/Categories/TicketCategoryEndpoints.cs @@ -48,12 +48,11 @@ private static async Task GetCategories( if (tenantId.HasValue && !tenant.IsHelpdeskAdmin && tenantId != currentTenantId) { var access = CurrentUserAccessProfile.FromClaims(user); - var organization = tenantId.Value.ToString(); var permitted = access.OrganizationIdsForAny( HelpdeskPermissions.IncidentRead, HelpdeskPermissions.IncidentWrite, HelpdeskPermissions.IncidentManager, HelpdeskPermissions.IncidentUser, HelpdeskPermissions.RequestRead, HelpdeskPermissions.RequestWrite, HelpdeskPermissions.RequestManager, HelpdeskPermissions.RequestUser, HelpdeskPermissions.ChangeRead, HelpdeskPermissions.ChangeWrite, HelpdeskPermissions.ChangeManager, HelpdeskPermissions.ChangeUser); - if (!permitted.Contains(organization)) return Results.Forbid(); + if (!permitted.Any(id => ParseTenantId(id) == tenantId)) return Results.Forbid(); } tenantId ??= currentTenantId; var selectedIds = new List(); @@ -144,7 +143,9 @@ private static async Task CreateCategory( } else { - var exists = await db.Organizations.AnyAsync(x => x.Id == dto.TenantId!.Value.ToString()); + var dashedId = dto.TenantId!.Value.ToString("D"); + var compactId = dto.TenantId.Value.ToString("N"); + var exists = await db.Organizations.AnyAsync(x => x.Id == dashedId || x.Id == compactId, cancellationToken); if (!exists) { return Results.BadRequest("Tenant does not exist."); diff --git a/src/Helpdesk.API/Endpoints/Organizations/TenantBrandingEndpoints.cs b/src/Helpdesk.API/Endpoints/Organizations/TenantBrandingEndpoints.cs index 506583f3..061269ce 100644 --- a/src/Helpdesk.API/Endpoints/Organizations/TenantBrandingEndpoints.cs +++ b/src/Helpdesk.API/Endpoints/Organizations/TenantBrandingEndpoints.cs @@ -120,6 +120,8 @@ public static void MapTenantBrandingEndpoints(this IEndpointRouteBuilder app) return Results.Ok(new { url = logoUrl }); }) + // This API authenticates bearer clients, including MCP multipart uploads. + .DisableAntiforgery() .Accepts("multipart/form-data"); app.MapGet("/api/tenants/{tenantId:int}/branding/logo/{filename}", ( diff --git a/src/Helpdesk.API/Endpoints/Services/ServiceEndpoints.cs b/src/Helpdesk.API/Endpoints/Services/ServiceEndpoints.cs index ee01d8a5..bf81f776 100644 --- a/src/Helpdesk.API/Endpoints/Services/ServiceEndpoints.cs +++ b/src/Helpdesk.API/Endpoints/Services/ServiceEndpoints.cs @@ -738,6 +738,7 @@ private static ServiceDto ToDtoWithDepth(Service s, IDictionary Description = s.Description, ParentServiceId = s.ParentServiceId, AllowedCustomerIds = s.AllowedCustomerIds, + AllowedOrganizationIds = s.AllowedOrganizationIds, Depth = depth }; } diff --git a/src/Helpdesk.API/Endpoints/Tickets/PublicTicketEndpoints.cs b/src/Helpdesk.API/Endpoints/Tickets/PublicTicketEndpoints.cs index 0b5747a0..f528582a 100644 --- a/src/Helpdesk.API/Endpoints/Tickets/PublicTicketEndpoints.cs +++ b/src/Helpdesk.API/Endpoints/Tickets/PublicTicketEndpoints.cs @@ -55,7 +55,7 @@ private static async Task GetPublicTicketView( } var lastNote = await db.WorkLogs.AsNoTracking() - .Where(l => l.TicketId == ticket.Id) + .Where(l => l.TicketId == ticket.Id && !l.IsInternalNote) .OrderByDescending(l => l.LoggedAt) .Select(l => l.NotesText) .FirstOrDefaultAsync(); diff --git a/src/Helpdesk.AgentClient/HelpdeskAgentClient.cs b/src/Helpdesk.AgentClient/HelpdeskAgentClient.cs index ecb5f524..75a87c4b 100644 --- a/src/Helpdesk.AgentClient/HelpdeskAgentClient.cs +++ b/src/Helpdesk.AgentClient/HelpdeskAgentClient.cs @@ -109,6 +109,9 @@ public HelpdeskAgentClient( throw new AgentClientRemoteException("api_request_failed", $"Helpdesk API request failed with HTTP {(int)response.StatusCode}.", (int)response.StatusCode, null) { CorrelationId = correlationId }; + if (method == HttpMethod.Get && path == "/api/v1/metrics/workflows" && + response.Content.Headers.ContentType?.MediaType == "text/plain") + return new JsonObject { ["format"] = "prometheus", ["text"] = text }; return string.IsNullOrWhiteSpace(text) ? null : JsonNode.Parse(text); } catch (AgentClientRemoteException exception) when (exception.CorrelationId is null && correlationId is not null) diff --git a/src/Helpdesk.Application/Resources/ResourceDatasetAbstractions.cs b/src/Helpdesk.Application/Resources/ResourceDatasetAbstractions.cs index 625438a6..1768e9d3 100644 --- a/src/Helpdesk.Application/Resources/ResourceDatasetAbstractions.cs +++ b/src/Helpdesk.Application/Resources/ResourceDatasetAbstractions.cs @@ -13,6 +13,7 @@ public interface IDataManagementService Task DeleteDatasetAsync(string datasetId, CancellationToken cancellationToken = default); Task> GetRowsAsync(string datasetId, string? query, int page, int pageSize, CancellationToken cancellationToken = default); Task> GetOptionsAsync(string datasetId, string organizationId, string? query, int page, int pageSize, CancellationToken cancellationToken = default); + Task GetOptionByKeyAsync(string datasetId, string organizationId, string key, CancellationToken cancellationToken = default); Task CreateCredentialAsync(string datasetId, CreateDatasetIngestCredentialDto dto, CancellationToken cancellationToken = default); Task RevokeCredentialAsync(string datasetId, string credentialId, CancellationToken cancellationToken = default); Task> ListCredentialsAsync(string datasetId, CancellationToken cancellationToken = default); diff --git a/src/Helpdesk.Infrastructure/Orchestration/OrchestrationInternalClient.cs b/src/Helpdesk.Infrastructure/Orchestration/OrchestrationInternalClient.cs index 3af518d2..edcbd734 100644 --- a/src/Helpdesk.Infrastructure/Orchestration/OrchestrationInternalClient.cs +++ b/src/Helpdesk.Infrastructure/Orchestration/OrchestrationInternalClient.cs @@ -221,7 +221,15 @@ private static bool HasIdentityContract(string content) { using var document = JsonDocument.Parse(content); if (document.RootElement.ValueKind != JsonValueKind.Object) return false; - return TryGetProperty(document.RootElement, "claims", out var claims) && claims.ValueKind == JsonValueKind.Object; + var root = document.RootElement; + if (TryGetProperty(root, "claims", out var claims) && claims.ValueKind == JsonValueKind.Object) return true; + // Managed NetRatel principals expose a bounded identity projection. + // A 200 or an arbitrary JSON object alone does not establish the contract. + return new[] { "issuer", "audience", "client_id" }.All(name => + TryGetProperty(root, name, out var value) && value.ValueKind == JsonValueKind.String && + !string.IsNullOrWhiteSpace(value.GetString())) && + TryGetProperty(root, "tenant_id", out var tenant) && + (tenant.ValueKind == JsonValueKind.Null || tenant.ValueKind == JsonValueKind.String && !string.IsNullOrWhiteSpace(tenant.GetString())); } catch (JsonException) { diff --git a/src/Helpdesk.Infrastructure/RequestTasks/RequestTaskApprovalService.cs b/src/Helpdesk.Infrastructure/RequestTasks/RequestTaskApprovalService.cs index 6dc8c607..b26f6d26 100644 --- a/src/Helpdesk.Infrastructure/RequestTasks/RequestTaskApprovalService.cs +++ b/src/Helpdesk.Infrastructure/RequestTasks/RequestTaskApprovalService.cs @@ -36,6 +36,12 @@ public RequestTaskApprovalService(HelpdeskDbContext db, IRequestFormSchemaParser public async Task StartApprovalAsync(RequestTask task, CancellationToken ct) { + // Workflow task enumeration returns detached snapshots while this scope + // can still track the request/task graph from creation or completion. + // Always mutate the context's persisted instance; attaching the snapshot + // a second time corrupts/conflicts with that graph in relational saves. + task = await db.RequestTasks.FirstOrDefaultAsync(x => x.Id == task.Id && x.RequestId == task.RequestId, ct) + ?? throw new KeyNotFoundException("Request task not found."); if (task.Type != RequestTaskType.Approval) { throw new InvalidOperationException("Task is not an approval task."); @@ -72,9 +78,6 @@ public async Task StartApprovalAsync(RequestTask task, Cancellation request.WorkflowUpdatedAt = now; request.UpdatedAt = DateTime.UtcNow; - db.RequestTasks.Update(task); - db.Requests.Update(request); - var existingEmails = await db.RequestTaskApprovals .Where(x => x.RequestTaskId == task.Id) .Select(x => x.ApproverEmail) diff --git a/src/Helpdesk.Infrastructure/Resources/DataManagementService.cs b/src/Helpdesk.Infrastructure/Resources/DataManagementService.cs index 427ccee3..12814e96 100644 --- a/src/Helpdesk.Infrastructure/Resources/DataManagementService.cs +++ b/src/Helpdesk.Infrastructure/Resources/DataManagementService.cs @@ -279,6 +279,19 @@ public async Task> GetOptionsAsync(string datasetI return result; } + public async Task GetOptionByKeyAsync(string datasetId, string organizationId, string key, CancellationToken cancellationToken = default) + { + // Resolve current persisted membership, independently of search columns, + // page limits and the five-minute option search cache. + var definition = await _db.DatasetDefinitions.AsNoTracking().FirstOrDefaultAsync( + x => x.Id == datasetId && x.OrganizationId == organizationId && x.IsActive, cancellationToken); + if (definition is null) return null; + var normalizedKey = key.Trim().ToLowerInvariant(); + var row = await _db.DatasetRows.AsNoTracking().FirstOrDefaultAsync( + x => x.DatasetId == datasetId && x.OrganizationId == organizationId && x.ExternalKey.ToLower() == normalizedKey, cancellationToken); + return row is null ? null : MapOptionDto(row, definition); + } + public async Task CreateCredentialAsync(string datasetId, CreateDatasetIngestCredentialDto dto, CancellationToken cancellationToken = default) { var dataset = await _db.DatasetDefinitions.FirstOrDefaultAsync(x => x.Id == datasetId, cancellationToken) @@ -1158,8 +1171,7 @@ public sealed class SelfServiceDatasetBindingResolver( continue; } - var options = await _dataManagementService.GetOptionsAsync(binding.DatasetId, organizationId, selectedKey, 1, 25, cancellationToken); - var match = options.Items.FirstOrDefault(x => string.Equals(x.Key, selectedKey, StringComparison.OrdinalIgnoreCase)); + var match = await _dataManagementService.GetOptionByKeyAsync(binding.DatasetId, organizationId, selectedKey, cancellationToken); if (match is null) { if (binding.AllowFreeText) diff --git a/src/Helpdesk.Mcp.Core/Tools/HelpdeskToolDefinitions.cs b/src/Helpdesk.Mcp.Core/Tools/HelpdeskToolDefinitions.cs index ee6e488c..f45ab2c0 100644 --- a/src/Helpdesk.Mcp.Core/Tools/HelpdeskToolDefinitions.cs +++ b/src/Helpdesk.Mcp.Core/Tools/HelpdeskToolDefinitions.cs @@ -2,6 +2,7 @@ using System.Reflection; using System.Text.Json; using System.Text.Json.Nodes; +using ModelContextProtocol; using Microsoft.Extensions.DependencyInjection; using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; @@ -49,7 +50,50 @@ private static McpServerTool Create(MethodInfo method) var tool = McpServerTool.Create(method, context => (context.Services ?? throw new InvalidOperationException("MCP request context has no service provider.")).GetRequiredService(), options); tool.ProtocolTool.InputSchema = JsonSerializer.SerializeToElement(InputSchema(definitions)); - return tool; + return new ValidatedTool(tool); + } + + // Validate before reflection binding can discard unrecognized arguments, in + // both HTTP and stdio transports. Discovery and runtime share the catalog. + private sealed class ValidatedTool(McpServerTool inner) : McpServerTool + { + public override Tool ProtocolTool => inner.ProtocolTool; + public override IReadOnlyList Metadata => inner.Metadata; + + public override ValueTask InvokeAsync(RequestContext request, CancellationToken cancellationToken = default) + { + if (HasDuplicateFields(JsonSerializer.SerializeToElement(request.JsonRpcRequest.Params))) + throw new McpException("Duplicate tool argument fields are not supported."); + var args = request.Params.Arguments; + if (args is null || args.Keys.Any(key => key is not ("operation" or "request" or "confirm")) || + !args.TryGetValue("operation", out var op) || op.ValueKind != JsonValueKind.String || + args.TryGetValue("confirm", out var confirm) && confirm.ValueKind is not (JsonValueKind.True or JsonValueKind.False)) + throw new McpException("Invalid tool argument envelope. Use operation, request and confirm only."); + var definition = McpOperationCatalog.All.SingleOrDefault(item => item.Tool == ProtocolTool.Name && item.Operation == op.GetString()); + if (definition is null) throw new McpException("Unknown tool operation."); + var hasRequest = args.TryGetValue("request", out var payload); + if (definition.RequestContract == "None" && hasRequest || + definition.RequiresRequest && (!hasRequest || payload.ValueKind != JsonValueKind.Object) || + hasRequest && payload.ValueKind is not (JsonValueKind.Object or JsonValueKind.Null)) + throw new McpException("Invalid request for the selected tool operation."); + if (hasRequest && ValidateSupportedFields(definition.Tool, definition.Operation, payload) is { } error) + throw new McpException(error); + return inner.InvokeAsync(request, cancellationToken); + } + + private static bool HasDuplicateFields(JsonElement value) + { + if (value.ValueKind == JsonValueKind.Object) + { + var names = new HashSet(StringComparer.Ordinal); + foreach (var property in value.EnumerateObject()) + if (!names.Add(property.Name) || HasDuplicateFields(property.Value)) return true; + } + else if (value.ValueKind == JsonValueKind.Array) + foreach (var item in value.EnumerateArray()) + if (HasDuplicateFields(item)) return true; + return false; + } } private static JsonObject InputSchema(IReadOnlyList definitions) diff --git a/src/Helpdesk.Mcp.Core/Tools/McpOperationCatalog.cs b/src/Helpdesk.Mcp.Core/Tools/McpOperationCatalog.cs index 772bc1b7..c0431aeb 100644 --- a/src/Helpdesk.Mcp.Core/Tools/McpOperationCatalog.cs +++ b/src/Helpdesk.Mcp.Core/Tools/McpOperationCatalog.cs @@ -27,7 +27,7 @@ public sealed record McpOperationDefinition( /// public static class McpOperationCatalog { - public const string Revision = "2026-10-19.mcp-owner-operations-v23"; + public const string Revision = "2026-10-20.beta9-contracts-v24"; public static IReadOnlyList All { get; } = [ diff --git a/src/Helpdesk.Mcp.Core/Tools/OperatorOperationCatalog.cs b/src/Helpdesk.Mcp.Core/Tools/OperatorOperationCatalog.cs index 888dff42..a658b5e3 100644 --- a/src/Helpdesk.Mcp.Core/Tools/OperatorOperationCatalog.cs +++ b/src/Helpdesk.Mcp.Core/Tools/OperatorOperationCatalog.cs @@ -73,7 +73,7 @@ private static IEnumerable Automation() yield return new("helpdesk_automation", "workflow_timeline", "GET", "/api/v1/requests/{requestId}/workflow-timeline", QueryType: typeof(OperatorWorkflowTimelineFilter), Authority: "Request.Manager in the request organization or HelpdeskAdmin", Description: "Read independently persisted request workflow events, including provider execution identities and effects."); foreach (var (name, path) in new[] { ("overdue_tasks", "overdue"), ("escalated_tasks", "escalated"), ("pending_retries", "retries-pending"), ("critical_failures", "critical-failures"), ("callback_rejections", "orchestration/callback-rejections"), ("binding_issues", "orchestration/binding-issues") }) yield return new("helpdesk_automation", name, "GET", "/api/v1/ops/tasks/" + path, QueryType: name == "critical_failures" ? typeof(OperatorCriticalFailureFilter) : name == "callback_rejections" ? typeof(OperatorCallbackFilter) : typeof(OperatorPageFilter), Authority: "Request.Manager in an authorized organization or HelpdeskAdmin", Description: "Read bounded workflow operational evidence and pending recovery work."); - yield return new("helpdesk_automation", "workflow_metrics", "GET", "/api/v1/metrics/workflows", Description: "Read workflow operational counters."); + yield return new("helpdesk_automation", "workflow_metrics", "GET", "/api/v1/metrics/workflows", Authority: "Request.Read, Request.Write or Request.Manager in the selected organizations", Description: "Read organization-filtered workflow operational counters as Prometheus text."); yield return new("helpdesk_attachments", "list", "GET", "/api/v1/tickets/{ticketId}/attachments/", Authority: "Parent ticket visibility", Description: "List authorized attachment metadata for an incident, request or change."); yield return new("helpdesk_attachments", "download", "GET", "/api/v1/attachments/{attachmentId}", Authority: "Parent ticket visibility", Description: "Read a bounded attachment as base64 with its media type and filename; maximum file size is 1 MiB."); diff --git a/src/Helpdesk.ServiceDefaults/Extensions.cs b/src/Helpdesk.ServiceDefaults/Extensions.cs index b5ef9305..d7ee6f52 100644 --- a/src/Helpdesk.ServiceDefaults/Extensions.cs +++ b/src/Helpdesk.ServiceDefaults/Extensions.cs @@ -90,7 +90,7 @@ public static TBuilder ConfigureOpenTelemetry(this TBuilder builder) w { loggerConfiguration.WriteTo.Console(new CompactJsonFormatter()); } - }); + }, writeToProviders: true); if (!verboseDependencyLogs) { diff --git a/tests/Helpdesk.Tests/Api/AiAgentOpsLogBufferTests.cs b/tests/Helpdesk.Tests/Api/AiAgentOpsLogBufferTests.cs index d003bf04..f19ba40e 100644 --- a/tests/Helpdesk.Tests/Api/AiAgentOpsLogBufferTests.cs +++ b/tests/Helpdesk.Tests/Api/AiAgentOpsLogBufferTests.cs @@ -1,11 +1,34 @@ using Helpdesk.API.Ops; using Microsoft.Extensions.Logging; using Xunit; +using System.Diagnostics; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Configuration; namespace Helpdesk.Tests.Api; public sealed class AiAgentOpsLogBufferTests { + [Fact] + public void Service_defaults_forward_real_transport_warning_to_searchable_redacted_ops_buffer() + { + var builder = Host.CreateApplicationBuilder(); + builder.Configuration.AddInMemoryCollection(new Dictionary { ["Observability:ConsoleLogsEnabled"] = "false" }); + builder.ConfigureOpenTelemetry(); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(); + using var host = builder.Build(); + using var activity = new Activity("native-transport-regression").Start(); + host.Services.GetRequiredService().CreateLogger("Helpdesk.NativeTransport") + .LogWarning("Transport failed for conversation {ConversationId}; token=synthetic-secret", "conversation-synthetic"); + var buffer = host.Services.GetRequiredService(); + var record = Assert.Single(buffer.Query(null, "Warning", "conversation-synthetic", activity.TraceId.ToString(), 10).Items); + Assert.DoesNotContain("synthetic-secret", record.Message); + Assert.Equal(activity.TraceId.ToString(), record.TraceId); + Assert.Empty(buffer.Query(null, null, "conversation-synthetic", "different-trace", 10).Items); + } + [Fact] public void Query_Redacts_Common_Secret_Patterns() { diff --git a/tests/Helpdesk.Tests/Api/Beta9WorkflowAndBrandingHttpTests.cs b/tests/Helpdesk.Tests/Api/Beta9WorkflowAndBrandingHttpTests.cs new file mode 100644 index 00000000..b0339df6 --- /dev/null +++ b/tests/Helpdesk.Tests/Api/Beta9WorkflowAndBrandingHttpTests.cs @@ -0,0 +1,103 @@ +using System.Net; +using System.Net.Http.Json; +using System.Text.Json; +using Helpdesk.API.Endpoints.Requests; +using Helpdesk.API.Endpoints.RequestTasks; +using Helpdesk.API.Endpoints.Organization; +using Helpdesk.API.Services; +using Helpdesk.Infrastructure.Persistence; +using Helpdesk.Shared.DTOs.Request; +using Helpdesk.Shared.Models; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using TicketServices = Helpdesk.Application.Services.Tickets; + +namespace Helpdesk.Tests.Api; + +public sealed class Beta9WorkflowAndBrandingHttpTests +{ + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task Self_service_and_manual_completion_return_success_with_one_started_approval(bool postgres, bool manualPredecessor) + { + await using var h = await IncidentReceiverTests.Harness.CreateAsync(postgres, serviceIdentity: true, + configureServices: services => + { + services.AddScoped(); + services.AddSingleton(); + services.AddAuthorization(options => options.AddPolicy("SelfService.User", p => p.RequireAuthenticatedUser())); + }, configureApp: app => { app.MapSelfServiceRequestEndpoints(); app.MapRequestTaskEndpoints(); }); + var manualTemplate = Guid.NewGuid(); var approvalTemplate = Guid.NewGuid(); + var tasks = new List(); + if (manualPredecessor) tasks.Add(new { id = manualTemplate, name = "Manual predecessor", type = "manual", order = 0 }); + tasks.Add(new { id = approvalTemplate, name = "Review", type = "approval", order = 1, + dependsOn = manualPredecessor ? new[] { manualTemplate } : Array.Empty(), + approvalApprovers = new[] { new { source = "Customer", id = h.CustomerId, name = "Approver", email = "approver@example.test" } } }); + await h.EditAsync(async db => + { + db.RequestForms.Add(new RequestForm { Id = "beta9-form", Title = "Approval fixture", OrganizationId = h.OrganizationId, + JsonSchema = JsonSerializer.SerializeToDocument(new { title = "Approval fixture", fields = Array.Empty(), tasks }) }); + }); + using var submitted = await h.AdminAsync(HttpMethod.Post, "/api/v1/self-service/requests", new SubmitSelfServiceRequestDto + { + RequestFormId = "beta9-form", PayloadJson = "{}", Title = "Synthetic beta.9 request", + RequestedForPersonId = h.CustomerId, RequestedForPersonSource = SelfServiceRequestPersonSource.Customer + }); + Assert.Equal(HttpStatusCode.OK, submitted.StatusCode); + var receipt = (await submitted.Content.ReadFromJsonAsync())!; + if (manualPredecessor) + { + string? manualId = null; + await h.EditAsync(async db => + { + var manual = await db.RequestTasks.SingleAsync(x => x.RequestId == receipt.RequestId && x.Type == RequestTaskType.Manual); + manualId = manual.Id; + Assert.Empty(await db.RequestTaskApprovals.ToListAsync()); + }); + using var started = await h.AdminAsync(HttpMethod.Post, $"/api/v1/request-tasks/{manualId}/start"); + started.EnsureSuccessStatusCode(); + using var completed = await h.AdminAsync(HttpMethod.Post, $"/api/v1/request-tasks/{manualId}/complete"); + Assert.Equal(HttpStatusCode.OK, completed.StatusCode); + } + await h.EditAsync(async db => + { + Assert.Single(await db.Requests.AsNoTracking().ToListAsync()); + var approval = await db.RequestTasks.AsNoTracking().SingleAsync(x => x.Type == RequestTaskType.Approval); + Assert.Equal(RequestTaskStatus.PendingApproval, approval.Status); + Assert.Single(await db.RequestTaskApprovals.AsNoTracking().Where(x => x.RequestTaskId == approval.Id).ToListAsync()); + Assert.Equal("PendingApproval", (await db.Requests.AsNoTracking().SingleAsync()).WorkflowStatus); + }); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Numeric_branding_accepts_authenticated_multipart_and_serves_exact_signed_bytes(bool postgres) + { + await using var h = await IncidentReceiverTests.Harness.CreateAsync(postgres, serviceIdentity: true, + configureServices: services => services.AddAuthorization(options => options.AddPolicy("HelpdeskAdmin", p => p.RequireRole("HelpdeskAdmin"))), + configureApp: app => app.MapTenantBrandingEndpoints()); + var bytes = Convert.FromBase64String("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+aXioAAAAASUVORK5CYII="); + using var content = new MultipartFormDataContent(); + var file = new ByteArrayContent(bytes); + file.Headers.ContentType = new("image/png"); + content.Add(file, "file", "synthetic-logo.png"); + using var request = new HttpRequestMessage(HttpMethod.Post, "/api/v1/tenants/17/branding/logo") { Content = content }; + request.Headers.Add("X-Synthetic-Admin", "true"); + using var uploaded = await h.Client.SendAsync(request); + Assert.Equal(HttpStatusCode.OK, uploaded.StatusCode); + var result = await uploaded.Content.ReadFromJsonAsync(); + var url = result.GetProperty("url").GetString()!; + using var brandingResponse = await h.AdminAsync(HttpMethod.Get, "/api/v1/tenants/17/branding"); + Assert.Equal(url, (await brandingResponse.Content.ReadFromJsonAsync())!.LogoUrl); + var path = new Uri(url).PathAndQuery.Replace("/prefix/api/", "/api/", StringComparison.Ordinal); + using var image = await h.Client.GetAsync(path); + Assert.Equal(HttpStatusCode.OK, image.StatusCode); + Assert.Equal(bytes, await image.Content.ReadAsByteArrayAsync()); + using var invalidToken = await h.Client.GetAsync(path + "tampered"); + Assert.Equal(HttpStatusCode.Unauthorized, invalidToken.StatusCode); + } +} diff --git a/tests/Helpdesk.Tests/Api/DatasetEndpointsAuthorizationTests.cs b/tests/Helpdesk.Tests/Api/DatasetEndpointsAuthorizationTests.cs index 6e6f7c87..354e122f 100644 --- a/tests/Helpdesk.Tests/Api/DatasetEndpointsAuthorizationTests.cs +++ b/tests/Helpdesk.Tests/Api/DatasetEndpointsAuthorizationTests.cs @@ -408,6 +408,8 @@ public Task> GetRowsAsync(string datasetId, string? q public Task> GetOptionsAsync(string datasetId, string organizationId, string? query, int page, int pageSize, CancellationToken cancellationToken = default) => Task.FromResult(new PagedResult { Page = page, PageSize = pageSize, Total = 0 }); + public Task GetOptionByKeyAsync(string datasetId, string organizationId, string key, CancellationToken cancellationToken = default) => Task.FromResult(null); + public Task CreateCredentialAsync(string datasetId, CreateDatasetIngestCredentialDto dto, CancellationToken cancellationToken = default) => Task.FromResult(new DatasetIngestCredentialCreatedDto { diff --git a/tests/Helpdesk.Tests/Api/PublicTicketEndpointsTests.cs b/tests/Helpdesk.Tests/Api/PublicTicketEndpointsTests.cs index 1e92597d..66fbd1b9 100644 --- a/tests/Helpdesk.Tests/Api/PublicTicketEndpointsTests.cs +++ b/tests/Helpdesk.Tests/Api/PublicTicketEndpointsTests.cs @@ -3,6 +3,8 @@ using Helpdesk.API.Endpoints.Tickets; using Helpdesk.Application.Tickets; using Helpdesk.Infrastructure.Persistence; +using Helpdesk.Infrastructure.Security; +using Helpdesk.Infrastructure.Storage; using Helpdesk.Shared.DTOs; using Helpdesk.Shared.DTOs.Worklog; using Helpdesk.Shared.Enums; @@ -14,11 +16,48 @@ using Microsoft.Data.Sqlite; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; namespace Helpdesk.Tests.Api; public sealed class PublicTicketEndpointsTests { + [Theory] + [InlineData("request", false)] + [InlineData("request", true)] + [InlineData("incident", false)] + [InlineData("change", false)] + public async Task Signed_View_And_Timeline_Exclude_Internal_Notes(string kind, bool internalOnly) + { + var signer = new PublicTicketLinkSigner(Options.Create(new StorageOptions + { + ImageSigningSecret = "synthetic-public-ticket-regression-signing-key" + }), NullLogger.Instance); + await using var harness = await PublicTicketTestHarness.CreateAsync(signer); + Ticket ticket = kind switch { "request" => new Request(), "change" => new Change(), _ => new Incident() }; + ticket.Id = "signed-ticket"; + ticket.TrackingId = "SIGNED-REGRESSION"; + await harness.SeedTicketAsync(ticket); + await harness.SeedWorkLogsAsync(internalOnly); + const string email = "customer@example.com"; + var token = signer.GenerateToken(ticket.TrackingId, email, DateTimeOffset.UtcNow.AddMinutes(5)); + var query = $"?trackingId={ticket.TrackingId}&email={Uri.EscapeDataString(email)}&token={token}"; + + var view = await harness.Client.GetFromJsonAsync("/api/v1/tickets/public/view" + query); + Assert.Equal(internalOnly ? null : "Public update", view!.LastPublicNote); + var timeline = await harness.Client.GetFromJsonAsync>("/api/v1/tickets/public/timeline" + query); + Assert.DoesNotContain(timeline!, x => x.MessageText == "Private diagnostic"); + Assert.Equal(internalOnly ? 0 : 1, timeline!.Count); + foreach (var endpoint in new[] { "view", "timeline" }) + { + var wrongRecipient = await harness.Client.GetAsync($"/api/v1/tickets/public/{endpoint}" + query.Replace("customer%40", "other%40")); + Assert.Equal(HttpStatusCode.Unauthorized, wrongRecipient.StatusCode); + var alteredSignature = await harness.Client.GetAsync($"/api/v1/tickets/public/{endpoint}" + query + "tampered"); + Assert.Equal(HttpStatusCode.Unauthorized, alteredSignature.StatusCode); + } + } + [Fact] public async Task View_Returns_Incident_For_Valid_Signed_Tracking_Link() { @@ -127,7 +166,7 @@ private PublicTicketTestHarness(SqliteConnection connection, WebApplication app, public HttpClient Client { get; } - public static async Task CreateAsync() + public static async Task CreateAsync(IPublicTicketLinkSigner? signer = null) { var connection = new SqliteConnection("Data Source=:memory:"); await connection.OpenAsync(); @@ -140,7 +179,7 @@ public static async Task CreateAsync() builder.Services.AddHttpContextAccessor(); builder.Services.AddDbContext(options => options.UseSqlite(connection, sqlite => sqlite.MigrationsAssembly("Helpdesk.Infrastructure.SqliteMigrations"))); builder.Services.AddScoped(_ => new TestTenantContext("org-1", "admin-1", isHelpdeskAdmin: true)); - builder.Services.AddSingleton(); + builder.Services.AddSingleton(signer ?? new TestPublicTicketLinkSigner()); var app = builder.Build(); app.MapPublicTicketEndpoints(); @@ -171,6 +210,15 @@ public async Task SeedTimelineAsync(params TicketTimelineEvent[] events) await db.SaveChangesAsync(); } + public async Task SeedWorkLogsAsync(bool internalOnly) + { + using var scope = app.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + if (!internalOnly) db.WorkLogs.Add(new WorkLog { Id = "public-note", TicketId = "signed-ticket", NotesText = "Public update", LoggedAt = DateTime.UtcNow.AddMinutes(-1) }); + db.WorkLogs.Add(new WorkLog { Id = "internal-note", TicketId = "signed-ticket", NotesText = "Private diagnostic", IsInternalNote = true, LoggedAt = DateTime.UtcNow }); + await db.SaveChangesAsync(); + } + public async ValueTask DisposeAsync() { Client.Dispose(); diff --git a/tests/Helpdesk.Tests/Api/ServiceItemsEndpointsTests.cs b/tests/Helpdesk.Tests/Api/ServiceItemsEndpointsTests.cs index d9c11454..2b86dbb6 100644 --- a/tests/Helpdesk.Tests/Api/ServiceItemsEndpointsTests.cs +++ b/tests/Helpdesk.Tests/Api/ServiceItemsEndpointsTests.cs @@ -34,6 +34,22 @@ namespace Helpdesk.Tests.Api; public sealed class ServiceItemsEndpointsTests { + [Fact] + public async Task Service_create_update_get_and_list_preserve_organization_audience() + { + await using var harness = await ServiceItemsTestHarness.CreateAsync(isHelpdeskAdmin: true); + var created = await harness.Client.PostAsJsonAsync("/api/v1/services", new CreateServiceDto { Name = "Scoped", AllowedOrganizationIds = ["tenant-1"] }); + created.EnsureSuccessStatusCode(); + var service = (await created.Content.ReadFromJsonAsync())!; + Assert.Equal(["tenant-1"], service.AllowedOrganizationIds); + var update = await harness.Client.PutAsJsonAsync($"/api/v1/services/{service.Id}", new { name = "Updated", allowedOrganizationIds = new[] { "tenant-2" } }); + update.EnsureSuccessStatusCode(); + Assert.Equal(["tenant-2"], (await update.Content.ReadFromJsonAsync())!.AllowedOrganizationIds); + Assert.Equal(["tenant-2"], (await harness.Client.GetFromJsonAsync($"/api/v1/services/{service.Id}"))!.AllowedOrganizationIds); + var list = await harness.Client.GetFromJsonAsync>("/api/v1/services"); + Assert.Equal(["tenant-2"], list!.Single(x => x.Id == service.Id).AllowedOrganizationIds); + } + [Fact] public async Task Request_form_edit_preserves_omitted_audience_and_accepts_explicit_clear() { diff --git a/tests/Helpdesk.Tests/Api/TicketCategoryEndpointsTests.cs b/tests/Helpdesk.Tests/Api/TicketCategoryEndpointsTests.cs index 58d8415d..a82cd49b 100644 --- a/tests/Helpdesk.Tests/Api/TicketCategoryEndpointsTests.cs +++ b/tests/Helpdesk.Tests/Api/TicketCategoryEndpointsTests.cs @@ -27,6 +27,32 @@ public sealed class TicketCategoryEndpointsTests private static readonly Guid Tenant = Guid.Parse("78000000-0000-0000-0000-000000000001"); private static readonly Guid OtherTenant = Guid.Parse("78000000-0000-0000-0000-000000000002"); + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Compact_organization_ID_supports_scoped_category_CRUD(bool postgres) + { + await using var h = await Harness.Create(postgres); + var organizationId = Guid.NewGuid(); + await h.WithDb(async db => { db.Organizations.Add(new Organization { Id = organizationId.ToString("N"), Name = "Compact organization" }); await db.SaveChangesAsync(); }); + var create = await h.Client.PostAsJsonAsync("/api/v1/categories", new CreateTicketCategoryDto { Name = "Scoped", Type = TicketCategoryType.Request, TenantId = organizationId }); + Assert.Equal(HttpStatusCode.Created, create.StatusCode); + var category = (await create.Content.ReadFromJsonAsync())!; + Assert.Equal(organizationId, category.TenantId); + var update = await h.Client.PutAsJsonAsync($"/api/v1/categories/{category.Id}", new UpdateTicketCategoryDto { Name = "Updated", Type = TicketCategoryType.Request, IsActive = true }); + update.EnsureSuccessStatusCode(); + var scoped = await h.Client.GetFromJsonAsync>($"/api/v1/categories?tenantId={organizationId}"); + Assert.Equal("Updated", Assert.Single(scoped!).Name); + Assert.Equal(HttpStatusCode.Forbidden, await DeniedAsync()); + h.Client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Test", "admin"); + (await h.Client.DeleteAsync($"/api/v1/categories/{category.Id}")).EnsureSuccessStatusCode(); + async Task DeniedAsync() + { + h.Client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Test", "reader"); + return (await h.Client.GetAsync($"/api/v1/categories?tenantId={organizationId}")).StatusCode; + } + } + [Theory] [InlineData(false)] [InlineData(true)] diff --git a/tests/Helpdesk.Tests/Api/WorkflowOpsEndpointsTests.cs b/tests/Helpdesk.Tests/Api/WorkflowOpsEndpointsTests.cs index a2dfc185..bc174350 100644 --- a/tests/Helpdesk.Tests/Api/WorkflowOpsEndpointsTests.cs +++ b/tests/Helpdesk.Tests/Api/WorkflowOpsEndpointsTests.cs @@ -26,6 +26,22 @@ namespace Helpdesk.Tests.Api; public sealed class WorkflowOpsEndpointsTests { + [Theory] + [InlineData("Admin", true)] + [InlineData("RequestManager", true)] + [InlineData("Customer", false)] + public async Task Workflow_metrics_preserve_API_authority_and_prometheus_contract(string role, bool allowed) + { + await using var harness = await WorkflowOpsHarness.CreateAsync(role, useSqlite: true); + var response = await harness.Client.GetAsync("/api/v1/metrics/workflows"); + Assert.Equal(allowed ? HttpStatusCode.OK : HttpStatusCode.Forbidden, response.StatusCode); + if (allowed) + { + Assert.Equal("text/plain", response.Content.Headers.ContentType!.MediaType); + Assert.Contains("# TYPE", await response.Content.ReadAsStringAsync()); + } + } + [Fact] public async Task RequestManager_CanReadTenantScopedWorkflowOps_ButNotRetriesPending() { @@ -275,6 +291,7 @@ protected override Task HandleAuthenticateAsync() new Claim(ClaimTypes.NameIdentifier, "admin-1"), new Claim(ClaimTypes.Role, HelpdeskPermissions.HelpdeskAdmin) } + : actor == "Customer" ? new[] { new Claim(ClaimTypes.NameIdentifier, "customer-only"), new Claim(ClaimTypes.Role, HelpdeskPermissions.SelfServiceUser) } : new[] { new Claim(ClaimTypes.NameIdentifier, "manager-1"), diff --git a/tests/Helpdesk.Tests/Infrastructure/Orchestration/OrchestrationInternalClientTests.cs b/tests/Helpdesk.Tests/Infrastructure/Orchestration/OrchestrationInternalClientTests.cs index eb9d83d0..1ea5c9dd 100644 --- a/tests/Helpdesk.Tests/Infrastructure/Orchestration/OrchestrationInternalClientTests.cs +++ b/tests/Helpdesk.Tests/Infrastructure/Orchestration/OrchestrationInternalClientTests.cs @@ -12,6 +12,26 @@ namespace Helpdesk.Tests.Infrastructure.Orchestration; public sealed class OrchestrationInternalClientTests { + [Theory] + [InlineData("{\"issuer\":\"netratel\",\"audience\":\"orchestration\",\"client_id\":\"managed\",\"tenant_id\":\"tenant-a\"}", true)] + [InlineData("{\"claims\":{\"sub\":[\"legacy\"]}}", true)] + [InlineData("{\"issuer\":\"netratel\",\"audience\":\"orchestration\",\"client_id\":\"managed\",\"tenant_id\":null}", true)] + [InlineData("{\"issuer\":\"netratel\",\"audience\":\"orchestration\",\"client_id\":\"managed\"}", false)] + [InlineData("{\"issuer\":\"netratel\",\"audience\":\"orchestration\",\"client_id\":\"\",\"tenant_id\":\"tenant-a\"}", false)] + [InlineData("{\"claims\":[]}", false)] + [InlineData("{}", false)] + [InlineData("login", false)] + public async Task Identity_accepts_managed_and_legacy_contracts_and_rejects_malformed_success(string body, bool expected) + { + var handler = new RecordingHandler(_ => new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(body) }); + var factory = Substitute.For(); + factory.CreateClient(PairingTransport.ClientName).Returns(new HttpClient(handler)); + var tokens = Substitute.For(); + tokens.GetAccessTokenAsync(Arg.Any(), Arg.Any()).Returns("synthetic-token"); + var result = await new OrchestrationInternalClient(factory, tokens, NullLogger.Instance).IdentityAsync(Settings()); + Assert.Equal(expected, result.Success); + } + [Fact] public async Task Successful_empty_response_is_not_invented_as_a_submission() { diff --git a/tests/Helpdesk.Tests/Infrastructure/RequestTasks/RequestTaskApprovalStartupProviderTests.cs b/tests/Helpdesk.Tests/Infrastructure/RequestTasks/RequestTaskApprovalStartupProviderTests.cs new file mode 100644 index 00000000..104f7804 --- /dev/null +++ b/tests/Helpdesk.Tests/Infrastructure/RequestTasks/RequestTaskApprovalStartupProviderTests.cs @@ -0,0 +1,52 @@ +using System.Text.Json; +using Helpdesk.Application.RequestTasks; +using Helpdesk.Application.Services.Notifications; +using Helpdesk.Application.Tickets; +using Helpdesk.Infrastructure.RequestTasks; +using Helpdesk.Shared.Models; +using Helpdesk.Tests.Infrastructure.Persistence; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; + +namespace Helpdesk.Tests.Infrastructure.RequestTasks; + +public sealed class RequestTaskApprovalStartupProviderTests +{ + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task Approval_starts_from_detached_workflow_snapshot_after_request_or_predecessor_commit(bool postgres, bool predecessor) + { + await using var database = await ServiceIdentityMigrationTests.DatabaseFixture.CreateAsync(postgres); + await using var db = database.Open(); + await db.Database.MigrateAsync(); + var templateId = Guid.NewGuid(); + db.RequestForms.Add(new RequestForm { Id = "form", JsonSchema = JsonDocument.Parse($$""" + {"fields":[],"tasks":[{"id":"{{templateId}}","name":"Review","type":"approval","order":1, + "approvalApprovers":[{"source":"Customer","id":"customer","name":"Approver","email":"approver@example.test"}]}]} + """) }); + var request = new Request { Id = "request", TrackingId = "REQ-STARTUP", RequestFormId = "form", Title = "Approval regression" }; + var task = new RequestTask { Id = "approval", RequestId = request.Id, TemplateId = templateId.ToString(), Type = RequestTaskType.Approval, Title = "Review" }; + db.Requests.Add(request); + db.RequestTasks.Add(task); + if (predecessor) db.RequestTasks.Add(new RequestTask { Id = "manual", RequestId = request.Id, Title = "Manual predecessor", Status = RequestTaskStatus.Completed }); + await db.SaveChangesAsync(); + // WorkflowEngine reads task snapshots through IRepository.GetAllAsync; + // the creating/completing HTTP scope still tracks the committed graph. + var snapshot = await db.RequestTasks.AsNoTracking().SingleAsync(x => x.Id == task.Id); + var notifications = Substitute.For(); + var service = new RequestTaskApprovalService(db, new RequestFormSchemaParser(), notifications, + Substitute.For(), NullLogger.Instance); + await service.StartApprovalAsync(snapshot, CancellationToken.None); + await service.StartApprovalAsync(snapshot, CancellationToken.None); + await using var readback = database.Open(); + Assert.Equal(RequestTaskStatus.PendingApproval, (await readback.RequestTasks.SingleAsync(x => x.Id == "approval")).Status); + Assert.Equal("PendingApproval", (await readback.Requests.SingleAsync()).WorkflowStatus); + Assert.Single(await readback.RequestTaskApprovals.ToListAsync()); + Assert.Equal(predecessor ? 2 : 1, await readback.RequestTasks.CountAsync()); + Assert.Single(await readback.Requests.ToListAsync()); + } +} diff --git a/tests/Helpdesk.Tests/Infrastructure/Resources/OperatorDatasetWriterTests.cs b/tests/Helpdesk.Tests/Infrastructure/Resources/OperatorDatasetWriterTests.cs index 6f7b8cbc..b4e41173 100644 --- a/tests/Helpdesk.Tests/Infrastructure/Resources/OperatorDatasetWriterTests.cs +++ b/tests/Helpdesk.Tests/Infrastructure/Resources/OperatorDatasetWriterTests.cs @@ -3,6 +3,7 @@ using Helpdesk.Infrastructure.Persistence; using Helpdesk.Infrastructure.Resources; using Helpdesk.Shared.DTOs.Resources; +using Helpdesk.Shared.DTOs.RequestForm; using Helpdesk.Shared.Enums; using Helpdesk.Shared.Models; using Helpdesk.Shared.Services; @@ -17,6 +18,39 @@ namespace Helpdesk.Tests.Infrastructure.Resources; public sealed class OperatorDatasetWriterTests { + [Fact] + public async Task Bound_selection_resolves_exact_current_key_without_label_search_or_page_limits() + { + await using var fixture = await Fixture.CreateAsync(); + await using (var edit = fixture.Readback()) + { + var definition = await edit.DatasetDefinitions.SingleAsync(x => x.Id == "custom"); + definition.DisplayColumn = "name"; + definition.SearchColumns = ["name"]; + await edit.SaveChangesAsync(); + } + await fixture.Service.UpsertRowsAsOperatorAsync("custom", "org-a", false, new UpsertDatasetRowsDto + { + Rows = Enumerable.Range(0, 60).Select(i => Row($"decoy-{i:D3}", "Common label")).Append(Row("zz-Key-061", "Visible label")).ToList() + }); + Assert.Empty((await fixture.Service.GetOptionsAsync("custom", "org-a", "zz-Key-061", 1, 25)).Items); + var resolver = new SelfServiceDatasetBindingResolver(fixture.Service, NullLogger.Instance); + FormField[] fields = [new() { Key = "choice", Label = "Choice", DataBinding = new() { DatasetId = "custom" } }]; + var result = await resolver.NormalizePayloadAsync("org-a", fields, "{\"choice\":\"ZZ-key-061\"}"); + Assert.True(result.Success); + var payload = JsonNode.Parse(result.PayloadJson)!; + Assert.Equal("zz-Key-061", payload["choice"]!.GetValue()); + Assert.Equal("Visible label", payload["_boundLabels"]!["choice"]!.GetValue()); + Assert.False((await resolver.NormalizePayloadAsync("org-b", fields, "{\"choice\":\"zz-Key-061\"}")).Success); + Assert.False((await resolver.NormalizePayloadAsync("org-a", fields, "{\"choice\":\"missing\"}")).Success); + await using (var edit = fixture.Readback()) + { + (await edit.DatasetDefinitions.SingleAsync(x => x.Id == "custom")).IsActive = false; + await edit.SaveChangesAsync(); + } + Assert.False((await resolver.NormalizePayloadAsync("org-a", fields, "{\"choice\":\"zz-Key-061\"}")).Success); + } + [Fact] public async Task Scoped_operator_write_persists_rows_and_delete_missing_is_explicit() { diff --git a/tests/Helpdesk.Tests/Mcp/AgentClientHttpTests.cs b/tests/Helpdesk.Tests/Mcp/AgentClientHttpTests.cs index 1befd3cc..020fafdc 100644 --- a/tests/Helpdesk.Tests/Mcp/AgentClientHttpTests.cs +++ b/tests/Helpdesk.Tests/Mcp/AgentClientHttpTests.cs @@ -7,6 +7,21 @@ namespace Helpdesk.Tests.Mcp; public sealed class AgentClientHttpTests { + [Fact] + public async Task Prometheus_workflow_metrics_have_an_explicit_typed_text_adapter_only_for_that_route() + { + var client = new HelpdeskAgentClient(new AgentClientConfiguration("https://api.example", "https://auth.example/token", "client", "agent", "secret", "scope", null), + () => new TestHandler(_ => new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("# TYPE helpdesk_tasks_in_progress gauge\nhelpdesk_tasks_in_progress 2\n", Encoding.UTF8, "text/plain") + })); + var metrics = await client.GetAsync("/api/v1/metrics/workflows", authenticated: false); + Assert.Equal("prometheus", metrics!["format"]!.GetValue()); + Assert.Contains("helpdesk_tasks_in_progress 2", metrics["text"]!.GetValue()); + var invalid = await Assert.ThrowsAsync(() => client.GetAsync("/api/v1/system/version", authenticated: false)); + Assert.Equal("api_response_invalid", invalid.Code); + } + [Fact] public async Task Health_uses_one_deadline_and_preserves_completed_checks() { diff --git a/tests/Helpdesk.Tests/Mcp/McpHttpBeta9RegressionTests.cs b/tests/Helpdesk.Tests/Mcp/McpHttpBeta9RegressionTests.cs new file mode 100644 index 00000000..bf2251c9 --- /dev/null +++ b/tests/Helpdesk.Tests/Mcp/McpHttpBeta9RegressionTests.cs @@ -0,0 +1,78 @@ +using System.Text.Json; +using Helpdesk.Mcp.Tools; +using Xunit; + +namespace Helpdesk.Tests.Mcp; + +public sealed partial class McpHttpHostTests +{ + [Fact] + public async Task Beta9_HTTP_rejects_unknown_envelopes_for_every_catalog_operation_before_upstream_IO() + { + using var environment = new McpHostEnvironment(); + var outbound = new AgentClientBoundaryProbe(); + using var factory = CreateFactory(environment.SigningKey, boundaryProbe: outbound); + using var http = CreateClient(factory); + var bearer = CreateToken(environment.SigningKey, ["helpdesk.mcp"], ["ai-assistant"]); + var tools = HelpdeskToolDefinitions.Create(); + var id = 1; + foreach (var operation in McpOperationCatalog.All) + { + var arguments = McpExecutableSchemaTests.ArgumentsFromDiscovery(tools.Single(x => x.ProtocolTool.Name == operation.Tool).ProtocolTool.InputSchema, operation.Operation); + arguments["unexpectedOuterArgument"] = "must fail before binding"; + using var response = await HttpCallAsync(http, bearer, id++, "tools/call", ToolCallParameters(operation.Tool, arguments)); + AssertRejected(response); + } + foreach (var raw in new[] + { + "{\"operation\":\"status\",\"operation\":\"status\"}", + "{\"operation\":\"status\",\"confirm\":false,\"confirm\":true}", + "{\"operation\":\"status\",\"request\":{}}", + "{\"operation\":\"status\",\"confirm\":\"true\"}", + "{}" + }) + { + using var args = JsonDocument.Parse(raw); + using var response = await HttpCallAsync(http, bearer, id++, "tools/call", ToolCallParameters("helpdesk_auth", args.RootElement)); + AssertRejected(response); + } + Assert.Equal(0, outbound.ApiRequests); + Assert.Equal(0, outbound.TokenRequests); + } + + [Fact] + public async Task Beta9_stdio_rejects_unknown_and_duplicate_outer_fields() + { + await using var stdio = StdioMcpClient.Start(); + using var initialize = await stdio.CallAsync(1, "initialize", InitializeParameters()); + await stdio.NotifyAsync("notifications/initialized", new { }); + foreach (var raw in new[] { "{\"operation\":\"get\",\"extra\":true}", "{\"operation\":\"get\",\"operation\":\"get\"}" }) + { + using var args = JsonDocument.Parse(raw); + using var response = await stdio.CallAsync(2, "tools/call", ToolCallParameters("helpdesk_capabilities", args.RootElement)); + AssertRejected(response); + } + } + + [Fact] + public async Task Beta9_HTTP_MCP_reads_prometheus_metrics_as_structured_text() + { + using var environment = new McpHostEnvironment(); + var outbound = new AgentClientBoundaryProbe(); + using var factory = CreateFactory(environment.SigningKey, boundaryProbe: outbound); + using var http = CreateClient(factory); + using var response = await HttpCallAsync(http, CreateToken(environment.SigningKey, ["helpdesk.mcp"], ["ai-assistant"]), 1, + "tools/call", ToolCallParameters("helpdesk_automation", new { operation = "workflow_metrics" })); + var result = StructuredContent(response.RootElement); + Assert.True(result.GetProperty("success").GetBoolean()); + Assert.Equal("prometheus", result.GetProperty("data").GetProperty("format").GetString()); + Assert.Contains("helpdesk_tasks_in_progress 2", result.GetProperty("data").GetProperty("text").GetString()); + Assert.Equal(1, outbound.ApiRequests); + } + + private static void AssertRejected(JsonDocument response) + { + Assert.True(response.RootElement.TryGetProperty("error", out _) || + response.RootElement.GetProperty("result").TryGetProperty("isError", out var isError) && isError.GetBoolean(), response.RootElement.GetRawText()); + } +} diff --git a/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs b/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs index 08bd5ff1..f84b20cd 100644 --- a/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs +++ b/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs @@ -313,7 +313,13 @@ public async Task External_http_AI_status_and_bounded_logs_require_the_real_exac options.ConfigurationManager = new Microsoft.IdentityModel.Protocols.StaticConfigurationManager(discovery); })); }); - api.Services.GetRequiredService().Add(LogLevel.Information, "Synthetic HTTP identity", new EventId(1), marker, null); + string traceId; + using (var activity = new Activity("owned-native-transport-regression").Start()) + { + traceId = activity.TraceId.ToString(); + api.Services.GetRequiredService().CreateLogger("Helpdesk.NativeTransport") + .LogWarning("{Marker} token=synthetic-transport-secret", marker); + } using var environment = new McpHostEnvironment(); var ingressToken = CreateToken(environment.SigningKey, ["helpdesk.mcp"], ["ai-assistant"]); var validToken = provider.Token(issuer: aiIssuer, audience: aiAudience, roleClaim: "groups", roles: [aiGroup]); @@ -341,14 +347,22 @@ public async Task External_http_AI_status_and_bounded_logs_require_the_real_exac } else Assert.Equal(401, statusResult.GetProperty("failure").GetProperty("upstreamStatus").GetInt32()); - using var logs = await HttpCallAsync(client, ingressToken, 2, "tools/call", ToolCallParameters("helpdesk_logs", new { operation = "search", request = new { contains = marker, limit = 1 } })); + using var logs = await HttpCallAsync(client, ingressToken, 2, "tools/call", ToolCallParameters("helpdesk_logs", new { operation = "search", request = new { contains = marker, correlationId = traceId, limit = 1 } })); var logResult = StructuredContent(logs.RootElement); Assert.Equal(permitted, logResult.GetProperty("success").GetBoolean()); if (permitted) { var data = logResult.GetProperty("data"); Assert.True(data.GetProperty("redacted").GetBoolean()); - Assert.Equal(marker, Assert.Single(data.GetProperty("items").EnumerateArray()).GetProperty("message").GetString()); + var item = Assert.Single(data.GetProperty("items").EnumerateArray()); + Assert.Equal(marker + " token=[REDACTED]", item.GetProperty("message").GetString()); + Assert.Equal(traceId, item.GetProperty("traceId").GetString()); + using var direct = api.CreateClient(); + direct.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", validToken); + using var independent = await direct.GetAsync("/api/v1/ops/ai-agent/logs?contains=" + Uri.EscapeDataString(marker) + "&correlationId=" + traceId); + independent.EnsureSuccessStatusCode(); + using var apiLogs = JsonDocument.Parse(await independent.Content.ReadAsStringAsync()); + Assert.Equal(data.GetProperty("items").GetRawText(), apiLogs.RootElement.GetProperty("items").GetRawText()); } else Assert.Equal(401, logResult.GetProperty("failure").GetProperty("upstreamStatus").GetInt32()); @@ -1783,7 +1797,9 @@ protected override Task SendAsync(HttpRequestMessage reques probe._apiAuthorizations.Enqueue(request.Headers.Authorization.ToString()); return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { - Content = new StringContent("{\"correlationId\":\"corr-123\"}", Encoding.UTF8, "application/json") + Content = request.RequestUri!.AbsolutePath == "/api/v1/metrics/workflows" + ? new StringContent("# TYPE helpdesk_tasks_in_progress gauge\nhelpdesk_tasks_in_progress 2\n", Encoding.UTF8, "text/plain") + : new StringContent("{\"correlationId\":\"corr-123\"}", Encoding.UTF8, "application/json") }); } } diff --git a/tests/http_mcp/campaign_reads.py b/tests/http_mcp/campaign_reads.py index 41193402..811e088f 100644 --- a/tests/http_mcp/campaign_reads.py +++ b/tests/http_mcp/campaign_reads.py @@ -95,6 +95,16 @@ def compare(mcp, api, assertions): raise CampaignError("Typed fixture assertion failed") +def decode_api_read(path, raw, headers): + # The workflow metrics API deliberately serves Prometheus; match the MCP + # adapter's explicit representation without accepting arbitrary non-JSON. + if path == "/api/v1/metrics/workflows": + if headers.get("Content-Type", "").split(";", 1)[0].lower() != "text/plain": + raise CampaignError("Workflow metrics did not return Prometheus text") + return {"format": "prometheus", "text": raw.decode("utf-8")} + return json.loads(raw) + + class Captures: def __init__(self, directory, secrets=()): self.directory = directory @@ -207,9 +217,9 @@ def independent_api(self, path): token = exchange["accessToken"] self.captures.secrets.append(token) self.captures.write("api-execution-exchange", exchange) - raw, _, at, _ = self.request(self.origin + path, "GET", None, + raw, headers, at, _ = self.request(self.origin + path, "GET", None, {"Authorization": "Bearer " + token, "Accept": "application/json"}) - body = json.loads(raw) + body = decode_api_read(path, raw, headers) return body, self.captures.write("api-independent", {"observedAt": at, "path": path, "body": body}) diff --git a/tests/http_mcp/test_campaign_reads.py b/tests/http_mcp/test_campaign_reads.py index 7698d956..95a6ba06 100644 --- a/tests/http_mcp/test_campaign_reads.py +++ b/tests/http_mcp/test_campaign_reads.py @@ -9,7 +9,7 @@ from types import SimpleNamespace from urllib.error import HTTPError -from campaign_reads import CampaignError, Captures, NoRedirect, Transport, compare, read_operation, run +from campaign_reads import CampaignError, Captures, NoRedirect, Transport, compare, decode_api_read, read_operation, run CATALOG = json.loads((Path(__file__).resolve().parents[2] / @@ -17,6 +17,23 @@ class ReadCampaignTests(unittest.TestCase): + def test_metrics_text_matches_explicit_mcp_projection(self): + text = b"# TYPE helpdesk_tasks_in_progress gauge\nhelpdesk_tasks_in_progress 2\n" + api = decode_api_read("/api/v1/metrics/workflows", text, {"Content-Type": "text/plain; version=0.0.4"}) + compare({"success": True, "data": api}, api, [{"pointer": "/format", "expected": "prometheus"}, + {"pointer": "/text", "expected": text.decode()}]) + + def test_non_metrics_text_and_metrics_html_are_rejected(self): + with self.assertRaises(json.JSONDecodeError): + decode_api_read("/api/v1/system/version", b"not JSON", {"Content-Type": "text/plain"}) + with self.assertRaises(CampaignError): + decode_api_read("/api/v1/metrics/workflows", b"login", {"Content-Type": "text/html"}) + + def test_empty_log_search_cannot_satisfy_known_warning_assertion(self): + with self.assertRaises((CampaignError, IndexError)): + compare({"success": True, "data": {"items": []}}, {"items": []}, + [{"pointer": "/items/0/message", "expected": "Synthetic transport failure"}]) + def case(self): return {"id": "owned-incident", "operation": "helpdesk_incidents.get", "request": {"incidentId": "owned-fixture"}, From a8573095a7955c7cb155d9433bef8e62fa4a7ae0 Mon Sep 17 00:00:00 2001 From: Jeremi Avenant Date: Tue, 20 Oct 2026 11:10:36 +0000 Subject: [PATCH 05/11] Use the existing task routes in the beta.9 HTTP fixture --- tests/Helpdesk.Tests/Api/Beta9WorkflowAndBrandingHttpTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/Helpdesk.Tests/Api/Beta9WorkflowAndBrandingHttpTests.cs b/tests/Helpdesk.Tests/Api/Beta9WorkflowAndBrandingHttpTests.cs index b0339df6..344ce66d 100644 --- a/tests/Helpdesk.Tests/Api/Beta9WorkflowAndBrandingHttpTests.cs +++ b/tests/Helpdesk.Tests/Api/Beta9WorkflowAndBrandingHttpTests.cs @@ -2,7 +2,6 @@ using System.Net.Http.Json; using System.Text.Json; using Helpdesk.API.Endpoints.Requests; -using Helpdesk.API.Endpoints.RequestTasks; using Helpdesk.API.Endpoints.Organization; using Helpdesk.API.Services; using Helpdesk.Infrastructure.Persistence; @@ -29,7 +28,7 @@ public async Task Self_service_and_manual_completion_return_success_with_one_sta services.AddScoped(); services.AddSingleton(); services.AddAuthorization(options => options.AddPolicy("SelfService.User", p => p.RequireAuthenticatedUser())); - }, configureApp: app => { app.MapSelfServiceRequestEndpoints(); app.MapRequestTaskEndpoints(); }); + }, configureApp: app => app.MapSelfServiceRequestEndpoints()); var manualTemplate = Guid.NewGuid(); var approvalTemplate = Guid.NewGuid(); var tasks = new List(); if (manualPredecessor) tasks.Add(new { id = manualTemplate, name = "Manual predecessor", type = "manual", order = 0 }); From a6fcfd05fe2eaa4807f237fbec673895d232190a Mon Sep 17 00:00:00 2001 From: Jeremi Avenant Date: Tue, 20 Oct 2026 11:12:35 +0000 Subject: [PATCH 06/11] Verify emitted log content and document beta.9 wire contracts --- docs/contracts/mcp-operation-contracts.md | 11 +++++++++++ docs/implementation/0.1.2-beta.9-mcp-regressions.md | 2 +- tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs | 4 +++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/contracts/mcp-operation-contracts.md b/docs/contracts/mcp-operation-contracts.md index bbb855a3..70c594a5 100644 --- a/docs/contracts/mcp-operation-contracts.md +++ b/docs/contracts/mcp-operation-contracts.md @@ -14,6 +14,11 @@ identifies semantic changes separately from runtime acceptance evidence. ## Requests, previews and effects +Beta.9 enforces the complete operation-specific argument envelope before SDK +binding in both transports. Unknown outer properties and duplicate JSON fields +are rejected before backend I/O; a no-request operation does not accept a +`request` property. The advertised closed schemas remain authoritative. + Keep the `operation`, `request`, `confirm` envelope. Required fields belong inside `request`; field names, enum values and nested extension objects come from the operation's schema. For a confirmation-controlled operation, omitted or false @@ -50,6 +55,12 @@ configuration surface. ## Typed operator adapters +`helpdesk_automation.workflow_metrics` returns structured data with +`format: "prometheus"` and `text` containing the API's Prometheus metrics. It +preserves the API's Request.Read/Request.Write/Request.Manager organization scope; +it does not require instance-administrator authority. Other API reads retain +their JSON contracts. + Beta.8 adds named adapters to canonical application endpoints. Discovery binds each operation to one method, route and typed request; agents cannot provide an arbitrary API method, host or path. Existing operation envelopes remain diff --git a/docs/implementation/0.1.2-beta.9-mcp-regressions.md b/docs/implementation/0.1.2-beta.9-mcp-regressions.md index 3acc2a4e..190df175 100644 --- a/docs/implementation/0.1.2-beta.9-mcp-regressions.md +++ b/docs/implementation/0.1.2-beta.9-mcp-regressions.md @@ -29,7 +29,7 @@ claim a live provider campaign replay. IDs retain the baseline's exact values. | `API-B8-CATEGORY-TENANT-ID-FORMAT` | D-format GUID lookup could not find N-format stored organization. | API accepts either persisted format and compares permission scopes as GUIDs. `TicketCategoryEndpointsTests.Compact_organization_ID_supports_scoped_category_CRUD` covers create/update/list/delete and foreign read denial on SQLite/PostgreSQL. | | `API-B8-NUMERIC-LOGO-ANTIFORGERY` | Numeric-tenant multipart logo failed because endpoint antiforgery middleware was absent. | API explicitly disables antiforgery for the authenticated multipart route, preserving administrator authorization. `Beta9WorkflowAndBrandingHttpTests.Numeric_branding_accepts_authenticated_multipart_and_serves_exact_signed_bytes` checks upload, stored URL, exact signed binary readback and tampered-token denial on both providers. | | `B8-ORCHESTRATION-MANAGED-IDENTITY-CONTRACT` | Managed NetRatel identity projection lacked legacy `claims` and was rejected. | Provider parser accepts the bounded managed projection and legacy claims. `OrchestrationInternalClientTests.Identity_accepts_managed_and_legacy_contracts_and_rejects_malformed_success` checks managed/global-null-tenant/legacy shapes and malformed/HTML/partial 200s. No live provider success is inferred. | -| `B8-OPS-LOG-OBSERVABILITY-GAP` | Actual native transport warning reached stdout but API/MCP logs were empty. | Service-default Serilog forwards to registered ILogger providers and the ops buffer. `AiAgentOpsLogBufferTests.Service_defaults_forward_real_transport_warning_to_searchable_redacted_ops_buffer` emits a real configured application warning, checks conversation/trace search, redaction and wrong-trace exclusion. | +| `B8-OPS-LOG-OBSERVABILITY-GAP` | Actual native transport warning reached stdout but API/MCP logs were empty. | Service-default Serilog forwards to registered ILogger providers and the ops buffer. `AiAgentOpsLogBufferTests.Service_defaults_forward_real_transport_warning_to_searchable_redacted_ops_buffer` checks conversation/trace search, redaction and wrong-trace exclusion. `McpHttpHostTests.External_http_AI_status_and_bounded_logs_require_the_real_exact_API_scheme_and_separate_token` emits a real configured warning and proves populated HTTP MCP/independent API parity through actual API authentication. | | `B8-SELF-SERVICE-DATASET-KEY-LOOKUP` | Valid option key rejected by label-only search lookup. | Data service resolves the current scoped row by key independently of search columns, cache and paging. `OperatorDatasetWriterTests.Bound_selection_resolves_exact_current_key_without_label_search_or_page_limits` checks 61 rows, disjoint key/label, case canonicalization, frozen label and missing/foreign/inactive rejection. | ## Reproduce source verification diff --git a/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs b/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs index f84b20cd..166aedca 100644 --- a/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs +++ b/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs @@ -355,7 +355,9 @@ public async Task External_http_AI_status_and_bounded_logs_require_the_real_exac var data = logResult.GetProperty("data"); Assert.True(data.GetProperty("redacted").GetBoolean()); var item = Assert.Single(data.GetProperty("items").EnumerateArray()); - Assert.Equal(marker + " token=[REDACTED]", item.GetProperty("message").GetString()); + Assert.Contains(marker, item.GetProperty("message").GetString()); + Assert.Contains("token=[REDACTED]", item.GetProperty("message").GetString()); + Assert.DoesNotContain("synthetic-transport-secret", item.GetProperty("message").GetString()); Assert.Equal(traceId, item.GetProperty("traceId").GetString()); using var direct = api.CreateClient(); direct.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", validToken); From e5f53620ba9ad3a83e110ac7bd4b3745309f0f12 Mon Sep 17 00:00:00 2001 From: Jeremi Avenant Date: Tue, 20 Oct 2026 11:14:28 +0000 Subject: [PATCH 07/11] Retain synthetic finding reproduction receipts in CI artifacts --- .github/workflows/pull-request-validation.yml | 2 ++ .../0.1.2-beta.9-mcp-regressions.md | 8 ++++++ .../Api/Beta9WorkflowAndBrandingHttpTests.cs | 9 +++++++ .../Api/PublicTicketEndpointsTests.cs | 2 ++ .../Api/ServiceItemsEndpointsTests.cs | 6 +++++ .../Api/TicketCategoryEndpointsTests.cs | 3 +++ tests/Helpdesk.Tests/Beta9Evidence.cs | 25 +++++++++++++++++++ .../OrchestrationInternalClientTests.cs | 3 +++ .../Resources/OperatorDatasetWriterTests.cs | 5 ++++ .../Mcp/McpHttpBeta9RegressionTests.cs | 6 +++++ tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs | 2 ++ 11 files changed, 71 insertions(+) create mode 100644 tests/Helpdesk.Tests/Beta9Evidence.cs diff --git a/.github/workflows/pull-request-validation.yml b/.github/workflows/pull-request-validation.yml index 613a83fc..ea59a4e7 100644 --- a/.github/workflows/pull-request-validation.yml +++ b/.github/workflows/pull-request-validation.yml @@ -100,6 +100,8 @@ jobs: run: dotnet build Helpdesk.sln --configuration Release --no-restore - name: Run .NET tests + env: + RATELDESK_BETA9_EVIDENCE_DIR: ${{ github.workspace }}/TestResults/beta9-source-evidence run: >- dotnet test Helpdesk.sln --configuration Release --no-build --logger "trx;LogFileName=helpdesk-tests.trx" diff --git a/docs/implementation/0.1.2-beta.9-mcp-regressions.md b/docs/implementation/0.1.2-beta.9-mcp-regressions.md index 190df175..4b76cd31 100644 --- a/docs/implementation/0.1.2-beta.9-mcp-regressions.md +++ b/docs/implementation/0.1.2-beta.9-mcp-regressions.md @@ -53,6 +53,14 @@ Use [campaign testing](../mcp-http/campaign-testing.md) for separately authorize live reads with a fresh profile, candidate catalog and evidence directory. The exact deployed beta.9 must receive a fresh campaign before any readiness verdict. +The same CI artifact also includes `beta9-source-evidence/*.json`: opt-in receipts +written after focused assertions, with selected synthetic HTTP/API observations, +binary hashes, rejected-operation counts and populated redacted log readbacks. +They identify their assembly version/CI revision and explicitly state that they +are source regressions. To produce them locally, set +`RATELDESK_BETA9_EVIDENCE_DIR` to a fresh directory outside the checkout before +running the tests. Raw credential headers and signed URLs are not recorded. + ## Sixteen operations still need live evidence All remain NOT_RUN in the immutable baseline; source tests do not promote them. diff --git a/tests/Helpdesk.Tests/Api/Beta9WorkflowAndBrandingHttpTests.cs b/tests/Helpdesk.Tests/Api/Beta9WorkflowAndBrandingHttpTests.cs index 344ce66d..62aa8fca 100644 --- a/tests/Helpdesk.Tests/Api/Beta9WorkflowAndBrandingHttpTests.cs +++ b/tests/Helpdesk.Tests/Api/Beta9WorkflowAndBrandingHttpTests.cs @@ -69,6 +69,9 @@ await h.EditAsync(async db => Assert.Single(await db.RequestTaskApprovals.AsNoTracking().Where(x => x.RequestTaskId == approval.Id).ToListAsync()); Assert.Equal("PendingApproval", (await db.Requests.AsNoTracking().SingleAsync()).WorkflowStatus); }); + Beta9Evidence.Record("API-B8-SELF-SERVICE-POSTCOMMIT-500", $"http-{postgres}-{manualPredecessor}", + new { submissionStatus = (int)submitted.StatusCode, manualCompletionStatus = manualPredecessor ? 200 : (int?)null, + requestCount = 1, approvalCount = 1, approvalStatus = "PendingApproval" }); } [Theory] @@ -98,5 +101,11 @@ public async Task Numeric_branding_accepts_authenticated_multipart_and_serves_ex Assert.Equal(bytes, await image.Content.ReadAsByteArrayAsync()); using var invalidToken = await h.Client.GetAsync(path + "tampered"); Assert.Equal(HttpStatusCode.Unauthorized, invalidToken.StatusCode); + Beta9Evidence.Record("API-B8-NUMERIC-LOGO-ANTIFORGERY", $"http-{postgres}", new + { + uploadStatus = (int)uploaded.StatusCode, binaryReadStatus = (int)image.StatusCode, + contentSha256 = Convert.ToHexString(System.Security.Cryptography.SHA256.HashData(bytes)).ToLowerInvariant(), + storedUrlMatchesUpload = true, alteredSignatureStatus = (int)invalidToken.StatusCode + }); } } diff --git a/tests/Helpdesk.Tests/Api/PublicTicketEndpointsTests.cs b/tests/Helpdesk.Tests/Api/PublicTicketEndpointsTests.cs index 66fbd1b9..302bd95e 100644 --- a/tests/Helpdesk.Tests/Api/PublicTicketEndpointsTests.cs +++ b/tests/Helpdesk.Tests/Api/PublicTicketEndpointsTests.cs @@ -56,6 +56,8 @@ public async Task Signed_View_And_Timeline_Exclude_Internal_Notes(string kind, b var alteredSignature = await harness.Client.GetAsync($"/api/v1/tickets/public/{endpoint}" + query + "tampered"); Assert.Equal(HttpStatusCode.Unauthorized, alteredSignature.StatusCode); } + Beta9Evidence.Record("B8-PUBLIC-VIEW-INTERNAL-NOTE-DISCLOSURE", $"{kind}-{internalOnly}", + new { view.LastPublicNote, publicTimeline = timeline.Select(x => x.MessageText), wrongRecipientStatus = 401, alteredSignatureStatus = 401 }); } [Fact] diff --git a/tests/Helpdesk.Tests/Api/ServiceItemsEndpointsTests.cs b/tests/Helpdesk.Tests/Api/ServiceItemsEndpointsTests.cs index 2b86dbb6..fbfad632 100644 --- a/tests/Helpdesk.Tests/Api/ServiceItemsEndpointsTests.cs +++ b/tests/Helpdesk.Tests/Api/ServiceItemsEndpointsTests.cs @@ -48,6 +48,12 @@ public async Task Service_create_update_get_and_list_preserve_organization_audie Assert.Equal(["tenant-2"], (await harness.Client.GetFromJsonAsync($"/api/v1/services/{service.Id}"))!.AllowedOrganizationIds); var list = await harness.Client.GetFromJsonAsync>("/api/v1/services"); Assert.Equal(["tenant-2"], list!.Single(x => x.Id == service.Id).AllowedOrganizationIds); + Beta9Evidence.Record("API-B8-SERVICE-SCOPE-PROJECTION", "http-crud", new + { + createdAudience = service.AllowedOrganizationIds, + updatedAudience = list.Single(x => x.Id == service.Id).AllowedOrganizationIds, + checkedResponses = new[] { "create", "update", "get", "list" } + }); } [Fact] diff --git a/tests/Helpdesk.Tests/Api/TicketCategoryEndpointsTests.cs b/tests/Helpdesk.Tests/Api/TicketCategoryEndpointsTests.cs index a82cd49b..e6582e7a 100644 --- a/tests/Helpdesk.Tests/Api/TicketCategoryEndpointsTests.cs +++ b/tests/Helpdesk.Tests/Api/TicketCategoryEndpointsTests.cs @@ -46,6 +46,9 @@ public async Task Compact_organization_ID_supports_scoped_category_CRUD(bool pos Assert.Equal(HttpStatusCode.Forbidden, await DeniedAsync()); h.Client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Test", "admin"); (await h.Client.DeleteAsync($"/api/v1/categories/{category.Id}")).EnsureSuccessStatusCode(); + Beta9Evidence.Record("API-B8-CATEGORY-TENANT-ID-FORMAT", $"http-{postgres}", + new { storedOrganizationFormat = "N", typedTenantFormat = "D", createStatus = (int)create.StatusCode, + roundTripTenantMatches = category.TenantId == organizationId, checkedOperations = new[] { "create", "update", "list", "delete" }, foreignScopeStatus = 403 }); async Task DeniedAsync() { h.Client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Test", "reader"); diff --git a/tests/Helpdesk.Tests/Beta9Evidence.cs b/tests/Helpdesk.Tests/Beta9Evidence.cs new file mode 100644 index 00000000..eecf2975 --- /dev/null +++ b/tests/Helpdesk.Tests/Beta9Evidence.cs @@ -0,0 +1,25 @@ +using System.Reflection; +using System.Text.Json; + +namespace Helpdesk.Tests; + +// Opt-in source-regression receipts. Call only after assertions pass, and pass +// selected synthetic observations rather than credentials, requests or signed URLs. +internal static class Beta9Evidence +{ + public static void Record(string findingId, string scenario, object observations) + { + var directory = Environment.GetEnvironmentVariable("RATELDESK_BETA9_EVIDENCE_DIR"); + if (string.IsNullOrWhiteSpace(directory)) return; + Directory.CreateDirectory(directory); + var receipt = new + { + findingId, scenario, observedAt = DateTimeOffset.UtcNow, outcome = "PASS", + scope = "Synthetic source regression; does not establish a deployed campaign pass", + productVersion = typeof(Beta9Evidence).Assembly.GetCustomAttribute()?.InformationalVersion, + ciRevision = Environment.GetEnvironmentVariable("GITHUB_SHA"), observations + }; + File.WriteAllText(Path.Combine(directory, findingId + "-" + scenario + ".json"), + JsonSerializer.Serialize(receipt, new JsonSerializerOptions { WriteIndented = true })); + } +} diff --git a/tests/Helpdesk.Tests/Infrastructure/Orchestration/OrchestrationInternalClientTests.cs b/tests/Helpdesk.Tests/Infrastructure/Orchestration/OrchestrationInternalClientTests.cs index 1ea5c9dd..4ba8e3ae 100644 --- a/tests/Helpdesk.Tests/Infrastructure/Orchestration/OrchestrationInternalClientTests.cs +++ b/tests/Helpdesk.Tests/Infrastructure/Orchestration/OrchestrationInternalClientTests.cs @@ -30,6 +30,9 @@ public async Task Identity_accepts_managed_and_legacy_contracts_and_rejects_malf tokens.GetAccessTokenAsync(Arg.Any(), Arg.Any()).Returns("synthetic-token"); var result = await new OrchestrationInternalClient(factory, tokens, NullLogger.Instance).IdentityAsync(Settings()); Assert.Equal(expected, result.Success); + Beta9Evidence.Record("B8-ORCHESTRATION-MANAGED-IDENTITY-CONTRACT", + Convert.ToHexString(System.Security.Cryptography.SHA256.HashData(System.Text.Encoding.UTF8.GetBytes(body))).ToLowerInvariant(), + new { syntheticProviderBody = body, expectedSuccess = expected, actualSuccess = result.Success }); } [Fact] diff --git a/tests/Helpdesk.Tests/Infrastructure/Resources/OperatorDatasetWriterTests.cs b/tests/Helpdesk.Tests/Infrastructure/Resources/OperatorDatasetWriterTests.cs index b4e41173..527f91e0 100644 --- a/tests/Helpdesk.Tests/Infrastructure/Resources/OperatorDatasetWriterTests.cs +++ b/tests/Helpdesk.Tests/Infrastructure/Resources/OperatorDatasetWriterTests.cs @@ -49,6 +49,11 @@ public async Task Bound_selection_resolves_exact_current_key_without_label_searc await edit.SaveChangesAsync(); } Assert.False((await resolver.NormalizePayloadAsync("org-a", fields, "{\"choice\":\"zz-Key-061\"}")).Success); + Beta9Evidence.Record("B8-SELF-SERVICE-DATASET-KEY-LOOKUP", "scoped-exact-key", new + { + normalizedPayload = payload, rowCount = 61, keySearchReturnedNoOptions = true, + foreignScopeRejected = true, missingKeyRejected = true, inactiveDatasetRejected = true + }); } [Fact] diff --git a/tests/Helpdesk.Tests/Mcp/McpHttpBeta9RegressionTests.cs b/tests/Helpdesk.Tests/Mcp/McpHttpBeta9RegressionTests.cs index bf2251c9..aa631ddb 100644 --- a/tests/Helpdesk.Tests/Mcp/McpHttpBeta9RegressionTests.cs +++ b/tests/Helpdesk.Tests/Mcp/McpHttpBeta9RegressionTests.cs @@ -38,6 +38,11 @@ public async Task Beta9_HTTP_rejects_unknown_envelopes_for_every_catalog_operati } Assert.Equal(0, outbound.ApiRequests); Assert.Equal(0, outbound.TokenRequests); + Beta9Evidence.Record("MCP-B8-UNKNOWN-OUTER-ARGUMENT", "http-all-operations", new + { + rejectedCatalogOperations = McpOperationCatalog.All.Count, rejectedAdditionalEnvelopes = 5, + upstreamApiRequests = outbound.ApiRequests, upstreamTokenRequests = outbound.TokenRequests + }); } [Fact] @@ -68,6 +73,7 @@ public async Task Beta9_HTTP_MCP_reads_prometheus_metrics_as_structured_text() Assert.Equal("prometheus", result.GetProperty("data").GetProperty("format").GetString()); Assert.Contains("helpdesk_tasks_in_progress 2", result.GetProperty("data").GetProperty("text").GetString()); Assert.Equal(1, outbound.ApiRequests); + Beta9Evidence.Record("MCP-B8-WORKFLOW-METRICS", "http-mcp", new { structuredContent = result.Clone(), upstreamRequests = outbound.ApiRequests }); } private static void AssertRejected(JsonDocument response) diff --git a/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs b/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs index 166aedca..764dd3a6 100644 --- a/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs +++ b/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs @@ -365,6 +365,8 @@ public async Task External_http_AI_status_and_bounded_logs_require_the_real_exac independent.EnsureSuccessStatusCode(); using var apiLogs = JsonDocument.Parse(await independent.Content.ReadAsStringAsync()); Assert.Equal(data.GetProperty("items").GetRawText(), apiLogs.RootElement.GetProperty("items").GetRawText()); + Beta9Evidence.Record("B8-OPS-LOG-OBSERVABILITY-GAP", "http-mcp-and-api", + new { mcpItems = data.GetProperty("items").Clone(), independentApiItems = apiLogs.RootElement.GetProperty("items").Clone() }); } else Assert.Equal(401, logResult.GetProperty("failure").GetProperty("upstreamStatus").GetInt32()); From af11fb3bb201f7c9cbadad1cfec3f84fd8ca9a9f Mon Sep 17 00:00:00 2001 From: Jeremi Avenant Date: Tue, 20 Oct 2026 11:16:06 +0000 Subject: [PATCH 08/11] Preserve non-admin audience privacy in service projections --- src/Helpdesk.API/Endpoints/Services/ServiceEndpoints.cs | 1 + tests/Helpdesk.Tests/Api/ServiceItemsEndpointsTests.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/Helpdesk.API/Endpoints/Services/ServiceEndpoints.cs b/src/Helpdesk.API/Endpoints/Services/ServiceEndpoints.cs index bf81f776..14f94c96 100644 --- a/src/Helpdesk.API/Endpoints/Services/ServiceEndpoints.cs +++ b/src/Helpdesk.API/Endpoints/Services/ServiceEndpoints.cs @@ -310,6 +310,7 @@ public static void MapServiceEndpoints(this IEndpointRouteBuilder app) if (!tenant.IsHelpdeskAdmin) { dto.AllowedCustomerIds = []; + dto.AllowedOrganizationIds = []; } return Results.Ok(dto); }) diff --git a/tests/Helpdesk.Tests/Api/ServiceItemsEndpointsTests.cs b/tests/Helpdesk.Tests/Api/ServiceItemsEndpointsTests.cs index fbfad632..8149394f 100644 --- a/tests/Helpdesk.Tests/Api/ServiceItemsEndpointsTests.cs +++ b/tests/Helpdesk.Tests/Api/ServiceItemsEndpointsTests.cs @@ -184,6 +184,7 @@ public async Task ServiceVisibility_EnforcesCustomerAndOrganizationAllowLists() var customerService = await customerServiceResponse.Content.ReadFromJsonAsync(); Assert.NotNull(customerService); Assert.Empty(customerService!.AllowedCustomerIds); + Assert.Empty(customerService.AllowedOrganizationIds); Assert.Equal(System.Net.HttpStatusCode.Forbidden, (await harness.Client.GetAsync("/api/v1/services/customer-two")).StatusCode); Assert.Equal(System.Net.HttpStatusCode.Forbidden, From 9e40a62b92376c193176c2c41615c53af8a9eaa5 Mon Sep 17 00:00:00 2001 From: Jeremi Avenant Date: Tue, 20 Oct 2026 11:19:17 +0000 Subject: [PATCH 09/11] Compare log API parity as JSON values across serializer encoders --- tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs b/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs index 764dd3a6..99827af2 100644 --- a/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs +++ b/tests/Helpdesk.Tests/Mcp/McpHttpHostTests.cs @@ -364,7 +364,7 @@ public async Task External_http_AI_status_and_bounded_logs_require_the_real_exac using var independent = await direct.GetAsync("/api/v1/ops/ai-agent/logs?contains=" + Uri.EscapeDataString(marker) + "&correlationId=" + traceId); independent.EnsureSuccessStatusCode(); using var apiLogs = JsonDocument.Parse(await independent.Content.ReadAsStringAsync()); - Assert.Equal(data.GetProperty("items").GetRawText(), apiLogs.RootElement.GetProperty("items").GetRawText()); + Assert.True(JsonElement.DeepEquals(data.GetProperty("items"), apiLogs.RootElement.GetProperty("items"))); Beta9Evidence.Record("B8-OPS-LOG-OBSERVABILITY-GAP", "http-mcp-and-api", new { mcpItems = data.GetProperty("items").Clone(), independentApiItems = apiLogs.RootElement.GetProperty("items").Clone() }); } From de849460302ed40a47956e094c95c6ae16a2edf4 Mon Sep 17 00:00:00 2001 From: Jeremi Avenant Date: Tue, 20 Oct 2026 11:34:37 +0000 Subject: [PATCH 10/11] Preserve structured MCP validation failures before argument binding --- .../Tools/HelpdeskToolDefinitions.cs | 48 ++++++++++++------- src/Helpdesk.Mcp.Core/Tools/HelpdeskTools.cs | 2 +- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/Helpdesk.Mcp.Core/Tools/HelpdeskToolDefinitions.cs b/src/Helpdesk.Mcp.Core/Tools/HelpdeskToolDefinitions.cs index f45ab2c0..789225f4 100644 --- a/src/Helpdesk.Mcp.Core/Tools/HelpdeskToolDefinitions.cs +++ b/src/Helpdesk.Mcp.Core/Tools/HelpdeskToolDefinitions.cs @@ -2,7 +2,7 @@ using System.Reflection; using System.Text.Json; using System.Text.Json.Nodes; -using ModelContextProtocol; +using System.Text.Json.Serialization; using Microsoft.Extensions.DependencyInjection; using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; @@ -57,41 +57,57 @@ private static McpServerTool Create(MethodInfo method) // both HTTP and stdio transports. Discovery and runtime share the catalog. private sealed class ValidatedTool(McpServerTool inner) : McpServerTool { + private static readonly JsonSerializerOptions ResponseOptions = new(JsonSerializerDefaults.Web) + { + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull + }; public override Tool ProtocolTool => inner.ProtocolTool; public override IReadOnlyList Metadata => inner.Metadata; public override ValueTask InvokeAsync(RequestContext request, CancellationToken cancellationToken = default) { - if (HasDuplicateFields(JsonSerializer.SerializeToElement(request.JsonRpcRequest.Params))) - throw new McpException("Duplicate tool argument fields are not supported."); + if (HasDuplicateEnvelopeFields(JsonSerializer.SerializeToElement(request.JsonRpcRequest.Params))) + return Reject(request, "Duplicate tool argument envelope fields are not supported."); var args = request.Params.Arguments; if (args is null || args.Keys.Any(key => key is not ("operation" or "request" or "confirm")) || !args.TryGetValue("operation", out var op) || op.ValueKind != JsonValueKind.String || args.TryGetValue("confirm", out var confirm) && confirm.ValueKind is not (JsonValueKind.True or JsonValueKind.False)) - throw new McpException("Invalid tool argument envelope. Use operation, request and confirm only."); + return Reject(request, "Invalid tool argument envelope. Use operation, request and confirm only."); var definition = McpOperationCatalog.All.SingleOrDefault(item => item.Tool == ProtocolTool.Name && item.Operation == op.GetString()); - if (definition is null) throw new McpException("Unknown tool operation."); + if (definition is null) return Reject(request, "Unknown tool operation."); var hasRequest = args.TryGetValue("request", out var payload); if (definition.RequestContract == "None" && hasRequest || definition.RequiresRequest && (!hasRequest || payload.ValueKind != JsonValueKind.Object) || hasRequest && payload.ValueKind is not (JsonValueKind.Object or JsonValueKind.Null)) - throw new McpException("Invalid request for the selected tool operation."); + return Reject(request, "Invalid request for the selected tool operation."); if (hasRequest && ValidateSupportedFields(definition.Tool, definition.Operation, payload) is { } error) - throw new McpException(error); + return Reject(request, error); return inner.InvokeAsync(request, cancellationToken); } - private static bool HasDuplicateFields(JsonElement value) + private static ValueTask Reject(RequestContext request, string message) { - if (value.ValueKind == JsonValueKind.Object) + var tools = (request.Services ?? throw new InvalidOperationException("MCP request context has no service provider.")).GetRequiredService(); + var content = JsonSerializer.SerializeToElement(tools.Validation(message), ResponseOptions); + return ValueTask.FromResult(new CallToolResult { - var names = new HashSet(StringComparer.Ordinal); - foreach (var property in value.EnumerateObject()) - if (!names.Add(property.Name) || HasDuplicateFields(property.Value)) return true; - } - else if (value.ValueKind == JsonValueKind.Array) - foreach (var item in value.EnumerateArray()) - if (HasDuplicateFields(item)) return true; + IsError = true, + StructuredContent = content, + Content = [new TextContentBlock { Text = content.GetRawText() }] + }); + } + + private static bool HasDuplicateEnvelopeFields(JsonElement parameters) + { + if (parameters.ValueKind != JsonValueKind.Object) return false; + var names = new HashSet(StringComparer.Ordinal); + foreach (var property in parameters.EnumerateObject()) + if (!names.Add(property.Name)) return true; + if (!parameters.TryGetProperty("arguments", out var arguments) || arguments.ValueKind != JsonValueKind.Object) return false; + names.Clear(); + foreach (var property in arguments.EnumerateObject()) + if (!names.Add(property.Name)) return true; + // Nested request validation keeps its existing field-specific errors. return false; } } diff --git a/src/Helpdesk.Mcp.Core/Tools/HelpdeskTools.cs b/src/Helpdesk.Mcp.Core/Tools/HelpdeskTools.cs index db9ae5b8..e9f1419a 100644 --- a/src/Helpdesk.Mcp.Core/Tools/HelpdeskTools.cs +++ b/src/Helpdesk.Mcp.Core/Tools/HelpdeskTools.cs @@ -684,7 +684,7 @@ private async Task Mutation(string operation, IReadOnlyLis private HelpdeskToolResponse Completed(string summary, JsonNode? data) => Response(true, "completed", summary, data, AffectedIds: [], CorrelationId: (data as JsonObject)?["correlationId"]?.GetValue()); private HelpdeskToolResponse Invalid(string tool, string operation, IReadOnlyList allowed) => Response(false, "validation_failed", $"Unsupported {tool} operation '{operation}'.", Failure: new HelpdeskToolFailure("unsupported_operation", false, AllowedOperations: allowed)); - private HelpdeskToolResponse Validation(string summary) => Response(false, "validation_failed", summary, Failure: new HelpdeskToolFailure("validation_error", false)); + internal HelpdeskToolResponse Validation(string summary) => Response(false, "validation_failed", summary, Failure: new HelpdeskToolFailure("validation_error", false)); private static string? TicketCreateValidation(string tool, JsonObject? request, string subject) { if (string.IsNullOrWhiteSpace(request?["title"]?.ToString()) || string.IsNullOrWhiteSpace(request?["description"]?.ToString())) From 3fe71d70d88b310f71566c9c8dabd9fe36db03af Mon Sep 17 00:00:00 2001 From: Jeremi Avenant Date: Tue, 20 Oct 2026 11:36:21 +0000 Subject: [PATCH 11/11] Keep nested argument diagnostics in their owning operation validators --- src/Helpdesk.Mcp.Core/Tools/HelpdeskToolDefinitions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Helpdesk.Mcp.Core/Tools/HelpdeskToolDefinitions.cs b/src/Helpdesk.Mcp.Core/Tools/HelpdeskToolDefinitions.cs index 789225f4..27568aae 100644 --- a/src/Helpdesk.Mcp.Core/Tools/HelpdeskToolDefinitions.cs +++ b/src/Helpdesk.Mcp.Core/Tools/HelpdeskToolDefinitions.cs @@ -80,8 +80,8 @@ public override ValueTask InvokeAsync(RequestContext