feat(mcp-use): add HOL Guard tool middleware proof - #71

• Introduce a local HOL Guard policy middleware gating tools/call at
 Middleware.on_call_tool(...).
• Fail closed on deny/review/provider errors/invalid output/oversize/non-serializable inputs without
 downstream execution.
• Add contract tests using the real mcp-use MiddlewareManager chain plus CI workflow coverage.

graph TD
  A["MCP client"] --> B["mcp-use MiddlewareManager"] --> C["MCPUseHOLGuardMiddleware"] --> D["LocalHOLGuardProvider"] --> E{{"hol-guard CLI"}}
  C -->|"allow"| F["Downstream tool handler"] --> G["Tool result"]
  C -->|"deny/review/error"| H["MCP error result"]

Loading

The following are alternative approaches to this PR:

1. Embed a Guard evaluator library (no subprocess)

• ➕ Removes subprocess management complexity (timeout/kill/I/O bounds).
• ➕ Potentially faster and easier to unit test deterministically.
• ➖ May introduce heavier dependencies or tighter coupling to Guard internals.
• ➖ Could diverge from the canonical hol-guard CLI behavior users run locally.

2. Support optional Guard Cloud / remote provider

• ➕ Centralized policy management and auditability across clients.
• ➕ No local workspace/CLI requirement for consumers.
• ➖ Introduces network dependency and new failure modes.
• ➖ Conflicts with the PR’s stated “local-only by default” proof scope.

3. Stricter decision schema (single canonical field)

• ➕ Reduces ambiguity in _classify and improves long-term maintainability.
• ➕ Makes provider contract easier to document and validate.
• ➖ Less tolerant of existing/variant HOL Guard outputs.
• ➖ May reduce compatibility with harnesses emitting nested/legacy keys.

Recommendation: The current approach is the best fit for a “local-only, minimal integration proof” because it exercises the real CLI, enforces strict size/time bounds, and fails closed at the exact mcp-use middleware boundary. If this is ever promoted beyond proof-of-concept, consider (1) a well-defined, minimal decision schema to shrink _classify surface area and (2) an optional non-subprocess provider for environments where spawning processes is undesirable.

Enhancement (2) +242 / -0

__init__.pyExport middleware public API +3/-0

Export middleware public API

• Exposes 'GuardDecision', 'LocalHOLGuardProvider', and 'MCPUseHOLGuardMiddleware' as the package’s public surface via '__all__'.

integrations/mcp-use-hol-guard/src/mcp_use_hol_guard/init.py

middleware.pyImplement HOL Guard-gated 'tools/call' middleware with bounded local provider +239/-0

Implement HOL Guard-gated 'tools/call' middleware with bounded local provider

• Implements an mcp-use Middleware that constructs a PreToolUse-style payload, enforces JSON-serializability and request/output size limits, invokes a decision provider (defaulting to a local 'hol-guard' subprocess with timeout), and only calls downstream on explicit allow. All other conditions (deny/review, provider failure, invalid output) fail closed with fixed MCP error messages that do not echo tool arguments.

integrations/mcp-use-hol-guard/src/mcp_use_hol_guard/middleware.py

Tests (1) +108 / -0

test_middleware.pyContract tests proving allow=1 downstream call, deny/review/failures=0 +108/-0

Contract tests proving allow=1 downstream call, deny/review/failures=0

• Adds async pytest coverage using the real 'MiddlewareManager' chain to assert exactly-once downstream execution on allow and zero executions on deny/review, provider exceptions, invalid provider output, oversized inputs, and non-serializable arguments (without leaking repr/exception strings).

integrations/mcp-use-hol-guard/tests/test_middleware.py

Documentation (1) +21 / -0

README.mdDocument the mcp-use middleware proof contract and usage +21/-0

Document the mcp-use middleware proof contract and usage

• Adds README describing the middleware attachment point, allow/deny/review semantics, local provider execution constraints (size, timeout), and the non-endorsement scope statement with upstream discussion link.

integrations/mcp-use-hol-guard/README.md

Other (2) +52 / -0

mcp-use-hol-guard.ymlAdd CI workflow to run the HOL Guard contract tests +32/-0

Add CI workflow to run the HOL Guard contract tests

• Introduces a GitHub Actions workflow that installs the integration package with test extras and runs pytest for the contract suite on PRs and main pushes affecting the integration.

.github/workflows/mcp-use-hol-guard.yml

pyproject.tomlDefine packaging and test dependencies for mcp-use-hol-guard +20/-0

Define packaging and test dependencies for mcp-use-hol-guard

• Adds a standalone Python package definition pinned to 'mcp-use==1.7.0', with pytest/pytest-asyncio test extras and wheel build configuration for the integration source package.

integrations/mcp-use-hol-guard/pyproject.toml

Description

The new mcp-use integration copies the same HOL Guard payload encoding/parsing/classification and
bounded subprocess I/O helpers that already exist in other integrations, which increases the chance
of future security/bugfix drift across adapters. A fix to parsing/bounds behavior in one integration
will not automatically propagate to this one, creating inconsistent enforcement over time.

Code

integrations/mcp-use-hol-guard/src/mcp_use_hol_guard/middleware.py[R36-39]

+def _encode_payload(payload: dict[str, Any]) -> bytes | None:
+    try:
+        return json.dumps(payload, separators=(",", ":"), ensure_ascii=False).encode("utf-8")
+    except (TypeError, ValueError, UnicodeEncodeError):

Evidence

The mcp-use integration adds helper implementations for JSON encoding, JSON extraction from stdout,
decision classification, and bounded subprocess communication; the BeeAI integration already
contains substantially the same helper code. This demonstrates the PR is introducing another
independently maintained copy of the same security-sensitive logic.

integrations/mcp-use-hol-guard/src/mcp_use_hol_guard/middleware.py[15-150]
integrations/beeai-hol-guard/src/beeai_hol_guard/middleware.py[19-175]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`mcp_use_hol_guard.middleware` introduces near-identical helper implementations (`_encode_payload`, `_parse_json_output`, `_classify`, `_terminate_process`, `_communicate_bounded`) that are already maintained in other `integrations/*-hol-guard` packages. This duplication makes it likely that future fixes (e.g., output parsing edge cases or I/O bounding behavior) will be applied to one adapter but missed in others.
## Issue Context
There is already an established pattern of these helpers living inside each integration package. Adding another copy increases the maintenance surface and the risk of inconsistent enforcement behavior.
## Fix Focus Areas
- Create a small shared internal module (e.g., `integrations/_hol_guard_common/` or a top-level `hol_guard_common/` package) containing the shared helper functions and constants.
- Update this new integration to import those helpers instead of redefining them.
- Optionally migrate existing integrations (at least the ones already using the same helper set) to the shared module to eliminate drift.
### References
- integrations/mcp-use-hol-guard/src/mcp_use_hol_guard/middleware.py[15-150]
- integrations/beeai-hol-guard/src/beeai_hol_guard/middleware.py[19-175]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The contract test mutates MiddlewareManager._record_telemetry, which is a private attribute and
may stop working (or raise) if mcp-use changes the implementation (e.g., adds __slots__ or renames
internals), breaking CI without any functional change to the middleware.

Code

integrations/mcp-use-hol-guard/tests/test_middleware.py[R29-31]

+    manager = MiddlewareManager()
+    manager._record_telemetry = False
+    manager.add_middleware(MCPUseHOLGuardMiddleware(provider))

Relevance

●● Moderate

Private dependency internals are a valid maintainability concern, but history lacks a close
precedent for this exact test setup.

PR-#62

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new test directly writes a private underscore-prefixed field on the external
MiddlewareManager, coupling the test suite to mcp-use internals.

integrations/mcp-use-hol-guard/tests/test_middleware.py[29-32]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The tests rely on `MiddlewareManager._record_telemetry` (a private/internal attribute). If mcp-use refactors this internal field, the tests can fail even though the middleware contract still holds.
### Issue Context
This integration pins `mcp-use==1.7.0` today, but the proof (and its CI workflow) will likely be re-run/maintained over time, and brittle coupling to private API increases maintenance cost.
### Fix Focus Areas
- integrations/mcp-use-hol-guard/tests/test_middleware.py[29-32]
### Suggested fix
- Prefer a documented/public toggle in mcp-use (if available) to disable telemetry.
- If no public toggle exists, guard the assignment:
- `if hasattr(manager, "_record_telemetry"): manager._record_telemetry = False`
- or wrap in `try/except AttributeError`.
- Add a short comment explaining why telemetry is being disabled (avoid noisy side effects in tests).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The contract test mutates MiddlewareManager._record_telemetry, which is a private attribute and
may stop working (or raise) if mcp-use changes the implementation (e.g., adds __slots__ or renames
internals), breaking CI without any functional change to the middleware.

Code

integrations/mcp-use-hol-guard/tests/test_middleware.py[R29-31]

+    manager = MiddlewareManager()
+    manager._record_telemetry = False
+    manager.add_middleware(MCPUseHOLGuardMiddleware(provider))

Relevance

●● Moderate

Private dependency internals are a valid maintainability concern, but history lacks a close
precedent for this exact test setup.

PR-#62

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new test directly writes a private underscore-prefixed field on the external
MiddlewareManager, coupling the test suite to mcp-use internals.

integrations/mcp-use-hol-guard/tests/test_middleware.py[29-32]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The tests rely on `MiddlewareManager._record_telemetry` (a private/internal attribute). If mcp-use refactors this internal field, the tests can fail even though the middleware contract still holds.
### Issue Context
This integration pins `mcp-use==1.7.0` today, but the proof (and its CI workflow) will likely be re-run/maintained over time, and brittle coupling to private API increases maintenance cost.
### Fix Focus Areas
- integrations/mcp-use-hol-guard/tests/test_middleware.py[29-32]
### Suggested fix
- Prefer a documented/public toggle in mcp-use (if available) to disable telemetry.
- If no public toggle exists, guard the assignment:
- `if hasattr(manager, "_record_telemetry"): manager._record_telemetry = False`
- or wrap in `try/except AttributeError`.
- Add a short comment explaining why telemetry is being disabled (avoid noisy side effects in tests).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

MCPUseHOLGuardMiddleware.on_call_tool catches all provider exceptions and fails closed without any
logging/telemetry, so a provider crash/misconfiguration can silently block all tool calls and be
hard to diagnose.

Code

integrations/mcp-use-hol-guard/src/mcp_use_hol_guard/middleware.py[R228-234]

+        try:
+            value = self._provider(payload)
+            decision = await value if inspect.isawaitable(value) else value
+        except Exception:
+            decision = GuardDecision("deny")
+        if not isinstance(decision, GuardDecision) or decision.kind not in {"allow", "deny", "review"}:
+            decision = GuardDecision("deny")

Relevance

● Weak

Recent observability logging requests were explicitly rejected; fail-closed behavior and non-leakage
are the stated contract.

PR-#57
PR-#53

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new middleware converts any provider exception into a deny decision with no side-channel
indicating that the provider errored, only that the call was blocked.

integrations/mcp-use-hol-guard/src/mcp_use_hol_guard/middleware.py[228-239]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The middleware intentionally fails closed on provider errors, but currently does so silently. This makes operational debugging difficult when the provider is broken/unavailable.
### Issue Context
The PR’s contract explicitly requires diagnostics to be fixed strings and not echo tool arguments; logging can still be safe if it is fixed and does not include request payloads.
### Fix Focus Areas
- integrations/mcp-use-hol-guard/src/mcp_use_hol_guard/middleware.py[228-239]
### Suggested fix
- Add an optional logger/callback (or use `logging.getLogger(__name__)`) and emit a fixed warning/error message when the provider raises.
- Ensure the log message does not include tool name/arguments/context IDs if those are considered sensitive for this proof (keep it constant/fixed).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

MCPUseHOLGuardMiddleware.on_call_tool catches all provider exceptions and fails closed without any
logging/telemetry, so a provider crash/misconfiguration can silently block all tool calls and be
hard to diagnose.

Code

integrations/mcp-use-hol-guard/src/mcp_use_hol_guard/middleware.py[R228-234]

+        try:
+            value = self._provider(payload)
+            decision = await value if inspect.isawaitable(value) else value
+        except Exception:
+            decision = GuardDecision("deny")
+        if not isinstance(decision, GuardDecision) or decision.kind not in {"allow", "deny", "review"}:
+            decision = GuardDecision("deny")

Relevance

● Weak

Recent observability logging requests were explicitly rejected; fail-closed behavior and non-leakage
are the stated contract.

PR-#57
PR-#53

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new middleware converts any provider exception into a deny decision with no side-channel
indicating that the provider errored, only that the call was blocked.

integrations/mcp-use-hol-guard/src/mcp_use_hol_guard/middleware.py[228-239]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The middleware intentionally fails closed on provider errors, but currently does so silently. This makes operational debugging difficult when the provider is broken/unavailable.
### Issue Context
The PR’s contract explicitly requires diagnostics to be fixed strings and not echo tool arguments; logging can still be safe if it is fixed and does not include request payloads.
### Fix Focus Areas
- integrations/mcp-use-hol-guard/src/mcp_use_hol_guard/middleware.py[228-239]
### Suggested fix
- Add an optional logger/callback (or use `logging.getLogger(__name__)`) and emit a fixed warning/error message when the provider raises.
- Ensure the log message does not include tool name/arguments/context IDs if those are considered sensitive for this proof (keep it constant/fixed).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Description

The contract test mutates MiddlewareManager._record_telemetry, which is a private attribute and
may stop working (or raise) if mcp-use changes the implementation (e.g., adds __slots__ or renames
internals), breaking CI without any functional change to the middleware.

Code

integrations/mcp-use-hol-guard/tests/test_middleware.py[R29-31]

+    manager = MiddlewareManager()
+    manager._record_telemetry = False
+    manager.add_middleware(MCPUseHOLGuardMiddleware(provider))

Relevance

●● Moderate

Private dependency internals are a valid maintainability concern, but history lacks a close
precedent for this exact test setup.

PR-#62

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new test directly writes a private underscore-prefixed field on the external
MiddlewareManager, coupling the test suite to mcp-use internals.

integrations/mcp-use-hol-guard/tests/test_middleware.py[29-32]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The tests rely on `MiddlewareManager._record_telemetry` (a private/internal attribute). If mcp-use refactors this internal field, the tests can fail even though the middleware contract still holds.

### Issue Context
This integration pins `mcp-use==1.7.0` today, but the proof (and its CI workflow) will likely be re-run/maintained over time, and brittle coupling to private API increases maintenance cost.

### Fix Focus Areas
- integrations/mcp-use-hol-guard/tests/test_middleware.py[29-32]

### Suggested fix
- Prefer a documented/public toggle in mcp-use (if available) to disable telemetry.
- If no public toggle exists, guard the assignment:
 - `if hasattr(manager, "_record_telemetry"): manager._record_telemetry = False`
 - or wrap in `try/except AttributeError`.
- Add a short comment explaining why telemetry is being disabled (avoid noisy side effects in tests).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

MCPUseHOLGuardMiddleware.on_call_tool catches all provider exceptions and fails closed without any
logging/telemetry, so a provider crash/misconfiguration can silently block all tool calls and be
hard to diagnose.

Code

integrations/mcp-use-hol-guard/src/mcp_use_hol_guard/middleware.py[R228-234]

+        try:
+            value = self._provider(payload)
+            decision = await value if inspect.isawaitable(value) else value
+        except Exception:
+            decision = GuardDecision("deny")
+        if not isinstance(decision, GuardDecision) or decision.kind not in {"allow", "deny", "review"}:
+            decision = GuardDecision("deny")

Relevance

● Weak

Recent observability logging requests were explicitly rejected; fail-closed behavior and non-leakage
are the stated contract.

PR-#57
PR-#53

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new middleware converts any provider exception into a deny decision with no side-channel
indicating that the provider errored, only that the call was blocked.

integrations/mcp-use-hol-guard/src/mcp_use_hol_guard/middleware.py[228-239]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The middleware intentionally fails closed on provider errors, but currently does so silently. This makes operational debugging difficult when the provider is broken/unavailable.

### Issue Context
The PR’s contract explicitly requires diagnostics to be fixed strings and not echo tool arguments; logging can still be safe if it is fixed and does not include request payloads.

### Fix Focus Areas
- integrations/mcp-use-hol-guard/src/mcp_use_hol_guard/middleware.py[228-239]

### Suggested fix
- Add an optional logger/callback (or use `logging.getLogger(__name__)`) and emit a fixed warning/error message when the provider raises.
- Ensure the log message does not include tool name/arguments/context IDs if those are considered sensitive for this proof (keep it constant/fixed).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗