Skip to content

[Feature]: classify tool-definition changes (benign / unsure / dangerous) and apply a policy by tier instead of hold-all or trust-all #1713

Open

Description

@james-crowley

Problem / motivation

When an approved tool's definition changes, the only choices are all-or-nothing, per server:

  • manual (default) and scan: every change is held until a person approves it. The docs say it plainly: "a changed contract is always held, whatever the verdict" (docs/features/security-quarantine.md, trust modes; also QP-10 in the quarantine pilot).
  • auto / auto_approve_tool_changes: true: every change is trusted, so a real rug pull passes.

For an operator running unattended agents this makes the safe setting costly. Upstream servers fix a typo, reword a description, or add an optional parameter all the time, and each one takes that tool offline for every agent until someone clicks Approve. The practical response is to move the whole server to auto, which removes the protection exactly where it matters (a description that gains new instructions, a read tool that becomes a write tool). In scan mode the scanner already runs on the changed definition and produces a verdict, but the verdict is ignored for changes, although it is used for new tools ("admits a new tool automatically when the offline scan is clean").

I searched for an existing request or setting and found none (see the checklist line).

Proposed solution

Classify the change when the hash differs, then apply a policy per class. I would leave the exact classifier to you; this is the shape I have in mind:

  1. Classify the delta between the approved definition and the new one:
    • benign: description changes that are small and non-semantic (whitespace, punctuation, case, an edit-distance threshold), the scanner is clean on the new text, no annotation or tier change, no new or changed required parameters, no new URLs or instruction-like phrases.
    • dangerous: a scanner finding on the new text, a tier raised by annotation (readOnlyHint to write/destructive), a new required parameter, a schema type change on an existing parameter, new URLs, or text that addresses the model (the patterns the TPA scanner already detects).
    • unsure: everything else (rewritten description, new optional parameter, output schema added or changed).
  2. Policy per class, in the server's config (an extension of trust_mode: scan, or a new change_policy), for example:
"change_policy": {
  "benign": "approve",
  "unsure": { "read": "approve", "write": "hold", "destructive": "hold", "unannotated": "hold" },
  "dangerous": "hold"
}

The tier for unsure is the higher of the old and new definition's tier, and unannotated counts as the strictest, in line with how profiles already treat unannotated. The default stays "hold everything", so nothing changes unless an operator opts in.
3. Make auto-approvals reviewable. Record the decision (class, tier, the changed fields) in the activity log and as an audit event, keep the diff available on the review screen after the fact, and bind the approval to the exact new hash, as expected_hashes does today (a second change is classified again).
4. Dry run first. A mcpproxy review simulate <server> or a log-only mode, which reports how past and pending changes would have been classified, lets an operator tune the policy before enabling it.

Alternatives considered

  • Use scan mode and approve by hand: what exists today, and the source of the outage-on-typo problem for unattended use.
  • Allow-list fields that may change without review (for example "description only"): simpler, but a description change is the classic rug-pull channel, so the scanner verdict and the tier should take part.
  • Per-tool opt-out of review: coarse, and it does not distinguish a typo from a new instruction.

Which edition?

Both (personal and server).

Activity

  1. james-crowley commented on Oct 20, 2026

    @james-crowley
    ContributorAuthor

    Update after #1689 (Spec 118): a changed tool now gets an in-process scan result stored as held_reason: awaiting_review / held_verdict: clean, which is exactly the evidence this request needs for the benign and unsure classes. What #1689 deliberately does not do, and what this issue asks for, is let a clean verdict plus a small, non-semantic diff and a low tier release the hold (opt-in, default unchanged). So the first step could be small: use held_verdict and the existing tier annotation, add the policy setting, and keep RT-02 intact by making the release an explicit, logged, per-hash decision.

  2. kaspermuutra-dotcom commented on Oct 20, 2026

    @kaspermuutra-dotcom

    +1, all-or-nothing pushes everyone to auto, which removes the protection exactly where it matters. One input that would make the classifier much more reliable than diffing text alone: the annotations and schema delta. A change that flips readOnlyHint from false to true, removes destructiveHint, or adds a parameter that names a recipient/target is dangerous by construction, whatever the description says. And a description change is only benign if behaviour didn't change too. That last part is only knowable by re-running the tool against a fixture, which could be an optional "verify" tier above scan.

    Kasper

  3. james-crowley commented on Oct 20, 2026

    @james-crowley
    ContributorAuthor

    Thanks @kaspermuutra-dotcom, that is a useful input. A flip of readOnlyHint from false to true, a removed destructiveHint or a parameter that names a recipient or target is dangerous whatever the description says, so those would put a change in the dangerous class before any text diff is considered, and a description-only change would count as benign only when the annotations and the input schema are unchanged. I agree that proving behaviour did not change needs a fixture run, which fits as an optional tier above scan rather than part of the first step. I will fold the annotation and schema delta into the classification rules when I write this up.

  4. kaspermuutra-dotcom commented on Oct 20, 2026

    @kaspermuutra-dotcom

    Thanks @james-crowley, that ordering makes sense: annotations and schema first, text last.

    Two more deltas that might belong in dangerous:

    • openWorldHint flipping from false to true. The tool can now reach outside the system it described.
    • idempotentHint flipping from false to true. Clients that trust it may start retrying calls that aren't safe to repeat.

    On the optional tier above scan: one way to keep it out of mcpproxy's core is to accept an external result keyed to the hash of the exact tool definitions it covers. If the definitions change, the hash no longer matches and the result drops back to scan, so nothing stale is trusted. We produce exactly that kind of signed, definition-bound result at Suncly, so I'd be glad to help sketch the format when you get to that tier.

  5. james-crowley commented on Oct 22, 2026

    @james-crowley
    ContributorAuthor

    Folding the annotation and schema points in, this is the rule set I would write up. Hints are compared by their effective value with the MCP defaults applied (readOnlyHint false, destructiveHint true, idempotentHint false, openWorldHint true), so removing an explicit hint counts as a change to its default.

    dangerous is checked first, and any one of these is enough:

    • the scanner is not clean on the new definition (held_verdict other than clean);
    • a hint becomes less cautious: readOnlyHint false to true, destructiveHint true to false, idempotentHint false to true, openWorldHint false to true. The first two lower the tier a profile sees; the last two change what a client assumes is safe to retry, or how far the tool reaches (thanks @kaspermuutra-dotcom for both);
    • the tier goes up (read to write or destructive, or annotated to unannotated);
    • in the input schema: a new required parameter, a type change on an existing one, a removed enum, pattern or length limit, or a new parameter whose name or description names a recipient, target, URL, path or command;
    • a new URL, or text that addresses the model, in the description or a parameter description.

    benign needs all of these: annotations, input schema and output schema unchanged, a clean scan, and a description diff that is small and not semantic (whitespace, punctuation, case, edit distance under a threshold).

    unsure is everything else.

    A verification tier stays out of the first step. If it comes later, an external result should count only when it is bound to the hash of the exact definition it covers, the way expected_hashes binds approvals today, so any later change drops it back to scan. I would keep that format generic so that any verifier can produce it.

    The first step is unchanged: the classifier plus a log-only or simulate mode, with the default policy still holding everything.

  6. kaspermuutra-dotcom commented on Oct 22, 2026

    @kaspermuutra-dotcom

    Thanks for folding those in. The rule set reads right to me, especially checking dangerous first and comparing hints by their effective values. I'd close three edge cases before it ships, and I have a concrete shape for the verification format.

    1. Diff against the last approved definition, not the last seen one. If benign is judged one change at a time, a series of small edits can each stay under the edit-distance threshold while together changing what the tool does. Measuring every change from the hash in expected_hashes closes that drift path.

    2. Normalize before the whitespace/case/punctuation check. A diff that only adds zero-width, bidi-control or tag characters looks like whitespace, but it can carry hidden text addressed to the model. I'd normalize to NFC, classify any newly added Unicode Cf character as dangerous, and only then measure distance.

    3. Some one-token edits are always semantic. never → always, a dropped not, max 10 → max 1000, a removed only. These pass any reasonable distance threshold. A short token list (negations, modals, numerals) that lifts a change to at least unsure is cheap and covers the most likely tricks.

    It's also worth stating explicitly that a tool with no prior definition starts at unsure at minimum.

    On the verifier format, a minimal version could be:

    {
      "format": "mcp-tool-verification/v0",
      "subject": {
        "server": "…",
        "tool": "…",
        "definition_sha256": "…",
        "canonicalization": "rfc8785"
      },
      "verdict": "pass | fail | inconclusive",
      "scope": "static | behavioral",
      "verifier": { "id": "…", "key_id": "…" },
      "issued_at": "…",
      "expires_at": "…",
      "signature": "…"
    }
    

    What matters most:

    • The hash is computed over an RFC 8785 canonical form of name, description, input/output schema and annotations, so the client and the verifier always agree on exactly what was covered.
    • scope separates "the definition looks safe" from "the server was called and does what its definition claims." Those make different promises.
    • expires_at exists because a server's behavior can change without its definition changing.

    Policy then decides, per verifier, how far a result is allowed to move a tier. The verifier never decides that itself.

    Disclosure: I build Suncly, which runs behavioral tests on MCP servers. When the verification tier comes up, I'd be glad to be the first producer of this format and test the schema against real output.

  7. james-crowley commented on Oct 23, 2026

    @james-crowley
    ContributorAuthor

    Thanks @kaspermuutra-dotcom. All three edge cases belong in the rules, so I will add them:

    1. Measure against the approved definition. Every change is classified against the last approved hash, never against the last seen definition. Otherwise a series of small edits could drift past the threshold one step at a time. mcpproxy already keeps the approved contract next to the current one (ApprovedHash and the previous description, schema and annotations on the approval record), so this costs nothing extra.
    2. Normalize first. Apply NFC normalization before the whitespace, case and punctuation check. Any newly added Unicode Cf character (zero-width, bidi control, tag characters) makes the change dangerous. The proxy's output_sanitisation already names the same classes (bidi, zero_width) for responses.
    3. Semantic tokens. Some edits always change the meaning: adding or dropping a negation (not, never, no), a change in a modal or quantifier (only, always, must, may), or a changed number. Any of these lifts a change to at least unsure, whatever the edit distance.

    Also, a tool with no approved definition starts at unsure at the least. A first-seen tool keeps going through the existing new-tool path.

    On the verification format: binding to a canonical hash, keeping static and behavioral scope apart, an expiry, and policy (not the verifier) deciding how far a result may move a tier are the right properties. The canonical form should be the one mcpproxy already hashes for expected_hashes, so that there is one definition of "the same tool". That tier stays out of the first step here; when it comes up, the format and the trust model for verifiers belong to the maintainers.

  8. kaspermuutra-dotcom commented on Oct 23, 2026

    @kaspermuutra-dotcom

    @james-crowley agreed — the format living with the maintainers is the right call; a classifier like this should own its contract. The three edge cases are where it earns its keep, especially the "unsure" tier — the honest bucket most tools skip.

    If an independent point-in-time read on the dangerous-class set is ever useful — a signed Suncly data point, not a format to adopt — the offer stands. Either way, good to see this land.

    Kasper — Suncly

  9. Dumbris commented on Oct 25, 2026

    @Dumbris
    Member

    Thank you both. Sorry for the slow reply, this thread deserved a faster one. It is the most useful input the quarantine work has had.

    Here is where I landed.

    The default stays scan. A change whose text scans clean gets approved. manual keeps asking a person for everything. auto trusts everything. What I am changing is what the gate looks at before it trusts a clean scan.

    Schema changes. James, you are right, and you found a real hole. The scanner reads a schema as text. The only structural check looks at parameter names, types and descriptions. It never looks at required, enum, pattern, defaults or the output schema. So a new bcc parameter with a clean description sails through today.

    The fix is a plain, deterministic check that runs before the scan and holds the change whatever the scan says:

    • a new required parameter
    • a type change on an existing parameter
    • any change to an enum set, additions included
    • a constraint that gets looser or disappears (pattern, format, min/max, length, additionalProperties)
    • a default or const that is added or changed
    • any change to the output schema

    Edits that only make a schema narrower pass. I am not adding a "names a recipient / URL / path" word list. Synonyms beat it, and the structural rules above cover the cases that matter.

    Hints. Kasper and James both raised the other two hints, and you disagreed on direction. I am sidestepping that: any change to any of the four hints is held, in either direction, with the MCP defaults applied. For mcpproxy the dangerous direction is actually openWorldHint true to false. That marks the tool's output as trusted and turns off stripping and spotlighting.

    The global knob. James, yes please, send the default_trust_mode PR. auto | scan | manual, default scan, an unknown value falls back to manual. Order: explicit per-server value, then the legacy flags, then default_trust_mode, then scan.

    Upgrade. A startup line and mcpproxy doctor will name every server whose trust_mode is empty. The auto-approval event will keep the before-state (old hash, text, schema, hints, the signals it checked, the verdict), so you can see afterwards what changed. The CHANGELOG will say that old clean holds get released on the first pass.

    The classifier. I am taking the goal but not the mechanism. In practice "unsure" turns into "approve if clean", an edit-distance number sits in the source for anyone to read, and a list of negation words is an English blacklist. So there will be no benign / unsure / dangerous classes and no policy matrix. The "replay" idea I like. It will be an operator-only CLI over stored history, like access suggest. It will not be reachable over REST or MCP, where an injected agent could use it to test candidates.

    Kasper's three edge cases. Drift: every change is compared with the last approved contract, and structural steps are held one at a time, so splitting a change does not help. Hidden Unicode: already a hard scanner check over the description and both schemas, nothing new needed. One-word edits: no rule for that, I am listing it as accepted risk instead.

    What this does not catch, said plainly. A well-written paraphrase, a one-word edit, slow text drift over several releases, and a server that changes behaviour without changing its definition. No offline gate catches those. The controls are manual on the servers you do not trust, a profile max_tier, the response pipeline, and the per-token rate limit we agreed on in #1699.

    The verification tier. Deferred, on the terms you both described: bound to the exact definition hash, falls back to scan on a mismatch, evidence only, and the operator decides what it may move. Kasper, thanks for the disclosure. If you write the format up as a vendor-neutral document, I will read it with interest, but I will not build or endorse a producer right now.

    Timing. No rush on my side. The structural gate, the audit event and the upgrade warning will land together with the scan default in one release, so nobody runs the new default without them.

    One ask. James, once the rules are on main, could you run your real tools/list history through them? How often they fire on real releases decides whether "any new optional parameter" should be held too.

  10. Dumbris commented on Oct 26, 2026

    @Dumbris
    Member

    The structural gate is on main as #1781 (766655d). What it does under scan, before the text scan gets a say:

    • any change to any of the four hints, in either direction, with the MCP defaults applied
    • in the input schema: a new required parameter, a type change, any change to an enum set, a loosened or removed constraint (pattern, format, min/max, length, additionalProperties), a default or const added or changed, and a new subschema that carries any of those
    • any structural change to the output schema

    Narrowing edits pass. Each hold names the rule and the path, so the review screen says why. The automatic-approval event now keeps the before-state, and a startup line plus mcpproxy doctor name servers whose trust_mode is empty.

    The three PRs from James (#1717, #1745, #1746) are merged too, and #1793 makes enable_socket: false the headline control for untrusted stdio servers, per #1704. All of this goes out together with the scan default in v0.72.0; the release candidate is being cut now. default_trust_mode stays open for James's PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions