fix(mcp): redact error text crossing the trust boundary — a handler exception leaked credentials

.github/workflows/ci.yml

@@ -32,18 +32,29 @@ jobs:
       - name: Install package (editable) + test deps
         run: |
           python -m pip install --upgrade pip setuptools wheel
-          # Install the package itself so `import sentinel_harness` and the
-          # `sentinel` console-script resolve (guards the flat-layout install blocker).
-          pip install -e .
-          # Test + quality deps, all pinned-enough to be reproducible:
-          #   pytest            — the suite runner.
-          #   pytest-randomly   — randomizes test order every run (prints the seed)
-          #                       so order-sensitivity / hidden inter-test state is caught.
-          #   coverage          — measures line+branch coverage for the hard gate below.
-          #   ruff              — REQUIRED lint (pinned to the version pre-commit + `make ci`
-          #                       run, so local and CI verdicts match exactly).
-          #   hypothesis        — property-based tests in tests/test_prop_*.py.
-          pip install pytest pytest-randomly coverage ruff==0.15.20 hypothesis
+          # Install the package WITH ITS `test` EXTRA, not a hand-copied dep list.
+          #
+          # This used to read `pip install -e .` followed by
+          #   pip install pytest pytest-randomly coverage ruff==... hypothesis
+          # — five of the `test` extra's nine entries. The two it omitted were `mcp` and
+          # `anyio[trio]`, so `tests/test_mcp_protocol.py`'s module-level
+          # `importorskip("mcp")` fired on every CI run and the ENTIRE MCP protocol E2E
+          # layer silently skipped. Local runs showed 6 skips, CI showed 12, and nothing
+          # compared the two numbers. Those tests exercise the one surface an untrusted MCP
+          # peer reaches — the layer most worth running in CI was the layer never running
+          # in CI, and it reported green the whole time. Skip is not pass.
+          #
+          # A hand-copied dependency list is a second source of truth that nothing
+          # reconciles; `[test]` cannot drift from pyproject.toml because it IS
+          # pyproject.toml. `tests/test_ci_installs_the_test_extra.py` now asserts this.
+          # ruff stays separate and PINNED (not in the extra) so the lint verdict is
+          # byte-identical between pre-commit, `make ci` and CI.
+          pip install -e ".[test]"
+          pip install ruff==0.15.20
+          # Fail loudly here rather than skipping quietly later: if an optional test
+          # dependency did not install, every test that needs it would report "skipped",
+          # which is indistinguishable from "there was nothing to run".
+          python -c "import mcp, anyio, trio, hypothesis, pytest_randomly, coverage; print('test extra OK')"
 
       - name: Smoke-test install (import + console script)
         env:

.github/workflows/release.yml

@@ -46,8 +46,13 @@ jobs:
       - name: Install (mirrors ci.yml toolchain)
         run: |
           python -m pip install --upgrade pip
-          pip install -e .
-          pip install pytest pytest-randomly coverage ruff==0.15.20 hypothesis
+          # `[test]` extra, matching ci.yml. Both files previously hand-copied a partial
+          # dep list that omitted `mcp` + `anyio[trio]`, so the release gate skipped the
+          # MCP protocol E2E layer too — the release gate is the LAST place a silently
+          # narrowed test run should go unnoticed. See the long comment in ci.yml.
+          pip install -e ".[test]"
+          pip install ruff==0.15.20
+          python -c "import mcp, anyio, trio, hypothesis, pytest_randomly, coverage; print('test extra OK')"
       - name: Lint (ruff)
         run: ruff check .
       - name: Test suite (coverage-gated, same 88% floor as CI)

README.md

@@ -13,7 +13,7 @@
   <img alt="python" src="https://img.shields.io/badge/python-3.10%2B-2997ff"/>
   <img alt="bedrock-agentcore" src="https://img.shields.io/badge/Amazon%20Bedrock-AgentCore%20Harness-ff9900"/>
   <img alt="version" src="https://img.shields.io/badge/version-0.4.0-2997ff"/>
-  <img alt="tests" src="https://img.shields.io/badge/offline%20tests-3772%20passing-1D8102"/>
+  <img alt="tests" src="https://img.shields.io/badge/offline%20tests-3817%20passing-1D8102"/>
   <img alt="coverage" src="https://img.shields.io/badge/coverage-90%25-1D8102"/>
   <img alt="milestones" src="https://img.shields.io/badge/milestones-M0--M15%20delivered-1D8102"/>
   <img alt="hardening" src="https://img.shields.io/badge/adversarial%20audit-100%20defects%20fixed-8b5cf6"/>
@@ -99,7 +99,7 @@ Honest build status per capability — mirrors the self-audit.
 | **Tools** | `nvd_lookup` / `epss_kev` / `attack_lookup` / `web_search` | 🟡 **reference stubs** (offline-safe, contract-tested) | `tools/`, `tests/test_tool_handlers.py` |
 | **Tools** | `siem_query` / `asset_lookup` / `enrich_ioc` / `ops_query` — backend-pluggable | 🟢 **built + tested** (offline mock default; `*_LIVE`=1 switches to a real stdlib-HTTP client — env-driven URL + bearer, timeouts, all failures→`upstream_error` with no silent fallback — proven end-to-end against an in-process 127.0.0.1 mock server, zero external network) | `tools/{siem_query,asset_lookup,enrich_ioc,ops_query}/`, `tests/test_*_live.py` |
 
-🟢 built & validated · 🟡 built, partial · 🟠 designed with loadable config · ⚪ design narrative only. **3772 offline tests pass** (+6 skipped when optional deps absent).
+🟢 built & validated · 🟡 built, partial · 🟠 designed with loadable config · ⚪ design narrative only. **3817 offline tests pass** (+6 skipped when optional deps absent).
 
 ## 🚀 Quickstart
 
@@ -247,7 +247,7 @@ Borrowed patterns (see [`docs/BLUEPRINT.md`](docs/BLUEPRINT.md)): supervisor→s
 | [`docs/GOVERNANCE.md`](docs/GOVERNANCE.md) | Registry dual-gate, HITL, sandbox hooks, and tag-guard controls |
 | [`docs/COMPLIANCE.md`](docs/COMPLIANCE.md) | Capability → SOC 2 / ISO 27001 / NIST CSF 2.0 control mapping (anchors machine-verified) |
 | [`docs/OBSERVABILITY.md`](docs/OBSERVABILITY.md) | Logging (`logutil`), metrics (token/latency/tool-call/error/eval), the OTEL/Transaction-Search path |
-| [`docs/TESTING.md`](docs/TESTING.md) | The 3772-test offline suite: layout, determinism, how to run |
+| [`docs/TESTING.md`](docs/TESTING.md) | The 3817-test offline suite: layout, determinism, how to run |
 | [`docs/FIDELITY-REPORT.md`](docs/FIDELITY-REPORT.md) | The self-audit — real vs. built vs. designed, with limits stated |
 | [`docs/ROADMAP.md`](docs/ROADMAP.md) | Delivered milestones (M0–M12) and what's next |
 | [**API reference (live)**](https://aws-samples.github.io/sample-sentinel-harness/) | Rendered `sentinel_harness` API docs (pdoc → GitHub Pages) |
@@ -285,7 +285,7 @@ sentinel-harness/
 ├── iac-cdk/              L3 CDK stacks (9; guardrail/identity/obs/vpc live) 🟢
 ├── iac-terraform/        deployable Terraform mirror (validate-clean)  🟢
 ├── docs/                 QUICKSTART · ARCHITECTURE · BLUEPRINT · SETUP · HARNESSES · GOVERNANCE · TESTING · FIDELITY-REPORT · ROADMAP
-├── tests/                offline unit + config tests (3772)     🟢
+├── tests/                offline unit + config tests (3817)     🟢
 └── .github/workflows/    CI incl. a customer-name / secret gate
 ```
 

docs/FIDELITY-REPORT.md

@@ -29,7 +29,7 @@ This is real, not narration:
 - **Layer 3 foundation ships runnable, tested code.** The dual-gate tool/skill registry, PreToolUse sandbox hook, and Agent Factory (fleet provision, dry-run, cross-env tag-guard) are built and unit-tested.
 - **Native IaC, not hand-rolled resources.** The gateway / registry / memory / harness stacks use the native `AWS::BedrockAgentCore::*` CloudFormation types. Per the README status matrix, the **Gateway and Memory CFN types are registered**; the Registry type is not yet in CFN (see limitations).
 - **Config path works.** `pip install -e .` succeeds (`[tool.setuptools] packages = ["sentinel_harness", "intake"]`), the `sentinel` console script works, and `sentinel create <harness.yaml>` loads real config via `sentinel_harness/loader.py` (systemPrompt file read, `bedrockModelConfig` / `agentCoreGateway` / `managedMemoryConfiguration` mapping, `${ENV}` expansion, `@gateway/tool` allowedTools grammar).
-- **Scale.** 3772 offline tests pass (+6 skipped when optional deps absent) across 137 test files, with 37 evidence JSON artifacts, 22 scenarios, 20 tools (incl. a 7-tool deterministic detection-engineering suite), an `iac-cdk` project (9 stacks synth-green) and an `iac-terraform` mirror (`validate`-clean).
+- **Scale.** 3817 offline tests pass (+6 skipped when optional deps absent) across 158 test files, with 37 evidence JSON artifacts, 22 scenarios, 20 tools (incl. a 7-tool deterministic detection-engineering suite), an `iac-cdk` project (9 stacks synth-green) and an `iac-terraform` mirror (`validate`-clean).
 - **Clean anonymization.** No real account IDs (only the `000000000000` placeholder), no customer or company names, no secrets. The CI secret-and-name scan is self-non-matching and fails the build on any hit.
 
 ## 4. Live controls retained for demos (us-east-1)

docs/INVARIANTS.md

@@ -848,6 +848,10 @@ the packaged install path.
 | **INV-MCP-1** | An unreadable registry is REFUSED, not treated as "no filtering". `_load_approved_set` swallowed every exception and returned an empty set, and the gate read `if approved and tool not in approved` — so an empty set made the condition falsy and every tool in `tools/` was exposed, pending ones included. Reproduced: the broken path served 18 tools vs 17, the extra being `web_search`, the only non-approved entry and the one tool that fetches attacker-influenceable content. Now raises `GovernanceUnavailable`; only the explicit `SENTINEL_MCP_ALLOW_PENDING=1` escape hatch may proceed ungoverned, and it warns. An empty-but-READ approved set correctly excludes everything. | `mcp_server._load_approved_set` / `_discover_tools` | `test_mcp_server.py::TestGovernanceGate` |
 | **INV-MCP-2** | The registry is found regardless of CWD. `DEFAULT_REGISTRY_PATH` is CWD-relative and the wheel shipped no `registry/` at all (verified by building one), so `pip install && sentinel-mcp` from any non-checkout dir could not read it — and combined with the pre-fix INV-MCP-1, failed open on EVERY packaged install. The registry is now packaged as `sentinel_harness/data/tools.yaml` AND `load_yaml` falls back to that installed copy. Two independent fixes because data-only packaging is easy to drop again (it was dropped once, for `connectors`) and a path fallback with nothing to find is useless. | `registry._resolve_registry_path` + `[tool.setuptools.package-data]` | `test_registry.py::test_packaged_registry_resolves`, `test_packaging.py::test_registry_yaml_is_declared_as_package_data` |
 | **INV-MCP-3** | The PyYAML-fallback parser does not flatten a nested key onto a tool item. `_mini_yaml` wrote `current[key]=value` for every `k:v` line regardless of depth, so a nested `status: approved` (in a documented example, a sub-config) overwrote a top-level `status: pending` — a fail-OPEN promotion, and PyYAML resolves the same file correctly, so the two parsers disagreed on an approval decision in the permissive direction. Now tracks the item's key indent and skips deeper sub-maps. | `registry._mini_yaml` | `test_registry_minyaml.py::test_mini_yaml_does_not_flatten_a_nested_status`, `::test_mini_yaml_agrees_with_pyyaml_on_the_nested_case` |
+| **INV-MCP-4** | Exception text crossing the MCP boundary is REDACTED. This module is the one surface an untrusted peer reaches, and two paths handed it `str(exc)` verbatim: `_invoke_tool`'s response `message`, and — quieter — `_discover_tools`' `[LOAD ERROR: {exc}]`, which `list_tools` SERVES as a tool description. Reproduced: a handler raising `postgresql://svc:SUPERSECRET_PW@db.internal/soc (token=ABSK_...)` delivered the password and the token to the peer. This is INV-TICKET-1's shape a second time — round 20 fixed it in `create_ticket` at that one call site while recording that a one-site fix is not an invariant — so the redaction now lives once and both paths use it. A DENYLIST by design: the exception TYPE and ordinary diagnostics survive, because a peer told only "an error occurred" cannot tell a bad argument from an outage. Hostnames are deliberately NOT redacted (stdio transport, operator-configured peer, and a hostname is not replayable the way a credential is) — asserted, so the trade-off is visible. | `mcp_server._safe_error_text` | `test_mcp_error_redaction.py` |
+| **INV-CI-1** | CI installs the WHOLE `test` extra, so no test layer can skip silently. Both `ci.yml` and `release.yml` installed a hand-copied dep list — `pytest pytest-randomly coverage ruff hypothesis`, five of the extra's nine entries. The two omitted were `mcp` and `anyio[trio]`, and `test_mcp_protocol.py` opens with `importorskip("mcp")`: so the ENTIRE MCP protocol E2E layer skipped on every CI run — the 7 tests covering the one surface an untrusted peer reaches — while CI reported green. Reproduced in a uv project pinned to CI's exact dep list (`mcp: ABSENT`, layer collapses to `1 skipped`). Local showed 6 skips, CI showed 12, and **nothing compared the two numbers**; a skip looks identical whether the code is fine, the test is broken, or the test never existed. Fixed at the source, not the symptom: `-e ".[test]"` cannot drift from `pyproject.toml` because it IS `pyproject.toml`, and the install step ends with an explicit `python -c "import mcp, anyio, ..."` so a resolver hiccup FAILS instead of degrading to a skip. `ruff` stays pinned outside the extra so the lint verdict is byte-identical local vs CI. | `.github/workflows/{ci,release}.yml` | `test_ci_installs_the_test_extra.py` |
+| **INV-MCP-5** | The `mcp` dependency is UPPER-BOUNDED, because the code needs the 1.x decorator API. `pyproject.toml` declared `mcp>=1.0` unbounded, and mcp 2.0.0 removed `Server.list_tools()` / `Server.call_tool()` — the two decorators `mcp_server.create_server` registers its handlers with. Verified against a real 2.0.0 install: `create_server()` raises `AttributeError: 'Server' object has no attribute 'list_tools'`, so `pip install sentinel-harness[mcp] && sentinel mcp serve` **could not start at all** on the current PyPI release. A user-facing install-time break, not a test artifact. What hid it: CI never installed `mcp` (INV-CI-1), so every test that would have caught it skipped on every run — the silent skip was concealing a broken published dependency contract, not a stale test. Also recorded: I first concluded 2.0 compatibility from `from mcp.server import Server` still resolving. It does resolve, and the API behind it is gone — **an import check is not a compatibility check**; compatibility must be probed by calling the surface. Now `mcp>=1.0,<2` in both the `mcp` and `test` extras (they must AGREE, or CI would test 1.x while users got 2.x), and the guard asserts the bound's PREMISE by calling the decorator surface, so the pin gets lifted deliberately when the code is ported rather than lingering as a constraint nobody dares touch. | `pyproject.toml` extras · `mcp_server.create_server` | `test_mcp_version_bound.py` |
+| **INV-CI-2** | Every test file using `pytest.mark.anyio` defines its own `anyio_backend` fixture. Omitting it does not produce "fixture not found" — pytest reports `async def functions are not natively supported` and FAILS, and an `importorskip` inside the test body never runs. It must be LOCAL rather than inherited from anyio's plugin, whose `anyio_backend` is parametrised over every installed backend and would silently run each async test twice. `test_mcp_protocol.py` had the convention (module importorskip + local fixture); `test_mcp_error_redaction.py` was written beside it and carried neither, passing locally and failing on all four CI Pythons. "A fix applied to one call site is not an invariant" — this time landing on a TESTING CONVENTION, which needs a check precisely because nobody greps for conventions. Guard carries a positive control: it fails if it finds zero async files. | tests using `pytest.mark.anyio` | `test_ci_installs_the_test_extra.py::test_every_async_test_file_pins_a_backend` |
 
 ---
 

docs/ROADMAP.md

@@ -51,7 +51,7 @@ live) · 🟡 skeleton / partial · 🔴 gap.
 | `specialists/` | `cve-intel` (docker-build + live-validated on AgentCore Runtime) + `attack-mapper` / `threat-hunt` (real graph/plan builders) + `adversarial-reviewer` (agent_a2a + local_a2a + two-stage Dockerfile + contract test) | ✅ | all four specialists shipped |
 | `longrunning/` | `bas-runner` (BAS case-gen + detection-replay) + `detonation` (full simulated microVM lifecycle + orchestrator) | 🟩 | both built + tested; detonation stays an honest SIMULATED no-op |
 | `iac-cdk/lib/` | 9 synth-green stacks — `gateway` / `registry` / `memory` / `network` / `identity` / `guardrail` / `observability` / `harness` / `runtime` (+ `iam`); `iac-terraform/` mirror is `terraform validate`-clean | ✅ | `guardrail` / `identity` / `observability` LIVE-deployed (us-east-1); the Registry + `runtime` custom-resource/raw-CfnResource stacks synth clean but fail on deploy until their CFN types are GA (both control-plane APIs are separately live-verified — Registry via `registry_live.py`, `CreateAgentRuntime` via a real arm64 microVM that served a live A2A call, HTTP 200, real Bedrock model, on a non-prod test account, then torn down — `evidence/live_a2a_runtime_result.json`) |
-| `tests/` | 137 files, **3772 offline passing** (+6 skipped) | ✅ | add tests with each new module |
+| `tests/` | 158 files, **3817 offline passing** (+6 skipped) | ✅ | add tests with each new module |
 | `evidence/` | 37 evidence sets | ✅ | add one per milestone |
 
 ### 0.3 Fit score (vs. a full three-layer SecOps agent program)
@@ -181,8 +181,8 @@ Each milestone gives: **goal / files / reused APIs / acceptance (live evidence)
 Suggest one feature branch per milestone.
 
 ### M0 — Environment & baseline reproduction (half a day)
-**Goal:** on a fresh machine, get all 3772 offline tests green and reproduce ≥1 live scenario.
-- [ ] `uv sync` + `uv run pytest -q` → 3772 passing (+6 skipped) (offline).
+**Goal:** on a fresh machine, get all 3817 offline tests green and reproduce ≥1 live scenario.
+- [ ] `uv sync` + `uv run pytest -q` → 3817 passing (+6 skipped) (offline).
 - [ ] Configure `SENTINEL_EXECUTION_ROLE_ARN` / `SENTINEL_REGION` / `AWS_PROFILE` (non-prod) — see `docs/SETUP.md`.
 - [ ] Run `scenarios/scenario_cve_triage.py`; compare `evidence/cve_triage_result.json` shape.
 - [ ] Run `scenarios/scenario_hitl_resume.py`; reproduce pause→approve→resume.
@@ -419,7 +419,7 @@ hand-off reuses the live-capable M1/M2 engine (driven offline here, labeled a wi
       (`make deploy`, cost note, `make destroy`) + the no-lock-in export. — `docs/QUICKSTART.md`
 - [x] `tests/smoke/`: offline acceptance suite (default offline; `SENTINEL_SMOKE_LIVE=1` opt-in for live). — `tests/smoke/`
 
-**Acceptance:** `make test` → 3772 offline tests green; `make seed-registry` → dual-gate `ok`;
+**Acceptance:** `make test` → 3817 offline tests green; `make seed-registry` → dual-gate `ok`;
 `make create-harnesses` (DRY_RUN=1) → 8 harnesses validate offline with zero AWS; `sentinel export` → valid
 compilable Strands Python; `make smoke` → the offline acceptance suite green. A fresh non-prod account can then
 run `make deploy` (free-tier foundation) and the live scenarios; `make destroy` tears it all down.