Skip to content

feat(mcp): plugin provider guardrail parity (schema validation, strategy reminder, executable rollback)#324

Merged
hyoshi merged 1 commit into
mainfrom
feat/plugin-guardrail-parity
Jun 23, 2026
Merged

feat(mcp): plugin provider guardrail parity (schema validation, strategy reminder, executable rollback)#324
hyoshi merged 1 commit into
mainfrom
feat/plugin-guardrail-parity

Conversation

@hyoshi

@hyoshi hyoshi commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Third-party plugin MCP tools now get the same three safety guardrails that built-in Google/Meta tools already enforce. Investigation traced every path through handle_call_tool_dispatch_tool and found policy gates / staleness / throttle / action_log were already universal, while three guardrails were built-in-only.

GAP A — server-side inputSchema validation

_build_tool_validators() no longer skips _PLUGIN_NAMES. A plugin that declares real-spend bounds (e.g. budget minimum:1, required, enum) now has them enforced server-side before dispatch, exactly like a built-in — instead of relying on the unverifiable assumption that every provider validates its own input. Permissive plugin schemas are unaffected; malformed schemas are skipped per-tool (same as built-ins).

GAP B — STRATEGY.md reminder after mutating plugin calls

maybe_build_reminder_for_plugin() appends the operator's STRATEGY.md sections after a successful mutating plugin call (classified via derive_semantics), honoring MUREO_DISABLE_STRATEGY_REMINDER. Read-only plugin tools skip it. The reminder build is now fully inside the never-raise guard.

GAP C — executable plugin rollback (bounded)

A plugin-declared meta["mureo"]["reversal"] is now executable (previously recorded-but-not-executable) when its operation:

  1. is non-destructive (destructive-verb refusal runs first),
  2. names a registered plugin tool (plugin_reversal_param_keys_PLUGIN_NAMES),
  3. carries only params within the tool's inputSchema property keys.

At apply time the existing executor re-enters handle_call_tool, which re-runs policy gates + GAP A validation and refuses recursion into rollback_*. The allow-list remains a real prompt-injection defense — the plugin path is strictly additive and bounded at both plan and execution time. The planner reaches the server via a lazy import that degrades to "not a plugin op" on failure (no import cycle).

Files

  • mureo/mcp/server.py — GAP A (validator build), GAP B (_maybe_append_plugin_strategy_reminder), GAP C (plugin_reversal_param_keys + per-tool schema keys)
  • mureo/core/strategy_reminder.pymaybe_build_reminder_for_plugin + shared _read_strategy_reminder
  • mureo/rollback/planner.py — plugin escape hatch in plan_rollback + trust-model docstring

Test plan

  • GAP A: below-minimum / missing-required rejected before dispatch; valid passes; validator built for plugin tool
  • GAP B: mutating plugin gets reminder appended (output preserved); read-only plugin gets none; env opt-out; build failure → None
  • GAP C: registered→schema-keys, unregistered→(False,None), schema-less→(True,None), extra-params rejected, destructive-before-lookup, cross-namespace (built-in name) refused, end-to-end executor dispatch
  • ruff check + black --check clean on changed mureo/ files
  • python-reviewer: APPROVE (no CRITICAL/HIGH)
  • Note: test_no_plugins_is_additive_no_op fails only on dev machines with mureo-pro plugins installed (verified identical on pristine main); passes in clean CI.

https://claude.ai/code/session_011rAu94b1o1xWYZhARk1VmL

Third-party plugin MCP tools now get the same three safety guardrails
that built-in Google/Meta tools already enforce.

- GAP A: server-side inputSchema validation. _build_tool_validators no
  longer skips _PLUGIN_NAMES, so a plugin's declared bounds (e.g. budget
  minimum:1) are enforced before dispatch, exactly like a built-in.
  Malformed plugin schemas are skipped per-tool, same as built-ins.
- GAP B: STRATEGY.md reminder after a successful *mutating* plugin call.
  maybe_build_reminder_for_plugin skips the built-in suffix classifier
  (the dispatch branch already classified the tool via derive_semantics)
  and honors MUREO_DISABLE_STRATEGY_REMINDER. Read-only plugin tools
  skip it. The reminder build is now fully inside the never-raise guard.
- GAP C: executable plugin rollback (bounded). A plugin-declared
  meta["mureo"]["reversal"] is planned when its operation names a
  *registered* plugin tool, is non-destructive, and its params fit the
  tool's inputSchema keys. The rollback allow-list stays a real
  prompt-injection defense: destructive verbs are refused first, only
  registered plugin tools are accepted, params are bounded, and the
  executor re-runs policy gates + inputSchema validation at apply time.
  planner consults server.plugin_reversal_param_keys via a lazy import
  that degrades to "not a plugin op" on failure (no import cycle).

Tests pin all three gaps incl. the GAP C trust-model boundaries
(destructive-before-lookup, cross-namespace, extra-param rejection) and
an end-to-end executor dispatch of a plugin reversal.

Claude-Session: https://claude.ai/code/session_011rAu94b1o1xWYZhARk1VmL
@hyoshi
hyoshi merged commit a67df13 into main Jun 23, 2026
9 checks passed
@hyoshi
hyoshi deleted the feat/plugin-guardrail-parity branch June 23, 2026 10:04
hyoshi added a commit that referenced this pull request Jun 24, 2026
…#325)

A plugin MCP tool that catches its own API error and RETURNS it as an
"API error: ..." TextContent (the built-in api_error_handler idiom)
instead of raising was promoted to STATE.json's action_log as a
successful mutation. That left a phantom action and — via a declared
meta["mureo"]["reversal"] — a phantom *executable* rollback, so
rollback_apply could "reverse" a change that never happened.

Built-in mutations already skip this via native_reversal._is_error_result.
This brings the plugin dispatch path to the same behavior:

- _helpers: add API_ERROR_PREFIX + public is_error_result() as the one
  source of truth (api_error_handler now uses the constant; output is
  byte-identical). native_reversal._is_error_result delegates to it.
- server plugin branch: gate record_mutation_action_log on
  not is_error_result(result). The jsonl audit still records the attempt
  (ok=True — it did not raise), and the strategy reminder is still
  appended for the mutating call, matching the built-in dispatch which
  appends it regardless of the error envelope.

Fail-safe by design: a plugin success payload literally starting with
"API error:" would only lose a STATE.json audit row (still in the jsonl
audit), never gain a phantom mutation/rollback.

Found during a post-merge parity sweep of #324.

Claude-Session: https://claude.ai/code/session_011rAu94b1o1xWYZhARk1VmL
hyoshi added a commit that referenced this pull request Jun 24, 2026
…328)

A plugin's static meta["mureo"]["reversal"] is fixed at server start, so it
cannot carry the actual entity id of a given call nor the entity's prior
state — useless for a real status toggle (set_ad_status(ad_id, status)) or
value edit. #324 made a registered plugin reversal *executable*, but only
from that static template; this adds the missing call-time capture.

- tool_provider: new optional secondary Protocol MCPReversibleToolProvider
  with `async capture_reversal(name, arguments) -> dict | None`. A provider
  opts in by also implementing it; the provider owns the platform client and
  reads prior state itself.
- server: plugin dispatch calls capture_reversal BEFORE the mutation
  (mirroring native_reversal.capture_before_state), then records the
  captured runtime-correct reversal in preference to the static meta
  reversal. Best-effort: never raises into dispatch, never blocks the
  mutation, skipped for read-only tools and when no STATE.json exists in cwd.
  Falls back to the static meta reversal when capture is absent/None/raises.

The rollback planner and executor are unchanged: the captured reversal's
operation is still gated by the registered-non-destructive-plugin-tool check,
schema-bounded params, and policy re-auth at apply time. The #325
error-envelope skip still applies — a captured reversal for a FAILED mutation
is never promoted, so no phantom executable rollback is left behind.

Resolves the call-time-capture half of #327 (the allow-list half shipped in
#324).

Closes #327

Claude-Session: https://claude.ai/code/session_011rAu94b1o1xWYZhARk1VmL
@hyoshi hyoshi mentioned this pull request Jun 24, 2026
hyoshi added a commit that referenced this pull request Jun 24, 2026
Plugin guardrail parity (schema validation / strategy reminder / executable
rollback, #324), runtime-correct plugin reversal capture (MCPReversibleToolProvider,
#327/#328), error-envelope action_log skip (#325), Reports tolerance for a
missing platform account_id + quieter tolerant-read logging (#329, supersedes
#323), and canonical STATE.json schema docs for the Code Write path (#331).

Claude-Session: https://claude.ai/code/session_011rAu94b1o1xWYZhARk1VmL
hyoshi added a commit that referenced this pull request Jul 14, 2026
…s reach them (#414) (#416)

StrategyPolicyGate (#360) enforces STRATEGY.md ## Guardrails before
dispatch, but its budget extraction is hard-wired to the built-in
Google/Meta argument keys. A plugin tool that carries its budget under any
other name was read as "no budget proposed" and sailed past every cap —
SILENTLY: no startup error, no warning. The operator who wrote
max_daily_budget_per_campaign believed their platform was protected when
it was not, on a surface where real money moves. Three external plugins
each hand-rolled the same normalize-and-delegate workaround gate.

Add the declaration seam (issue option A):

- A plugin declares its keys in standard MCP metadata — no new mureo
  Protocol surface, reusing the existing meta["mureo"] channel:

      _meta={"mureo": {"budget": {"daily": "daily_budget_micros",
                                  "current": "current_budget_micros",
                                  "unit": "micros"}}}

- derive_semantics parses it (rejecting a malformed hint WHOLE — a
  half-applied declaration would recreate the silent underenforcement this
  seam exists to remove); the server registers it at import; the ONE
  built-in gate consults it by tool name. No per-plugin gate needed.
- A declaration REPLACES the built-in key scan for that tool, for every
  channel: the plugin owns its argument vocabulary, so an unrelated field
  spelled `amount` cannot false-trip a cap (documented, with the corollary
  that a tool carrying a lifetime budget must declare `lifetime` too).
- A declared key that is PRESENT but unreadable (inf/nan, bool,
  non-numeric string, nested object) makes the gate DENY: the cap cannot be
  verified, so the call is refused with a clear reason. Treating garbage as
  "no proposal" would be the same silent bypass with extra steps — and
  weaker than the built-in scan, where a raw inf simply exceeds any finite
  cap. An absent key (or null / blank) still means "no budget on that
  channel". Fail-closed engages only once the operator wrote a rule.
- Stringified numbers are accepted (plugins hit them in the wild).

Also fix docs/plugin-authoring.md, stale since #324/#327 and misleading
the plugin authors who read it as canon:

- inputSchema validation is SERVER-SIDE enforced before dispatch (#324),
  not "advisory on the client side" — a tool whose declared types do not
  match what its handler accepts now has those calls rejected.
- A plugin-declared reversal IS executable (#324) when it names a
  registered, non-destructive tool — the docs still said plugin operations
  were recorded for audit but "not auto-reversible".
- capture_reversal / MCPReversibleToolProvider (#327/#328) were entirely
  absent from the docs; now documented with an example verified against the
  real Protocol signature.

Tests (TDD, red first): declaration parsing (full / defaults / every
malformed shape / absent), pure extraction (declared daily/current/
lifetime, micros conversion, numeric strings, declaration-replaces-scan,
undeclared-keeps-scan), deny-on-unreadable across value types and both
channels, the registry, an end-to-end gate denial against a real
STRATEGY.md, and the server wiring.

Closes #414

Claude-Session: https://claude.ai/code/session_0115NBUphwqsL1isTV8R7NHg
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant