Repository navigation
[Feature]: classify tool-definition changes (benign / unsure / dangerous) and apply a policy by tier instead of hold-all or trust-all #1713
Description
Problem / motivation
When an approved tool's definition changes, the only choices are all-or-nothing, per server:
manual(default) andscan: 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:
- 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 (readOnlyHintto 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).
- Policy per class, in the server's config (an extension of
trust_mode: scan, or a newchange_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
scanmode 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).
- I searched existing issues and discussions for a similar request (searched tool change auto-approve, auto_approve_tool_changes, rug pull, trust mode scan changed, tool update approval; nothing found for classifying changes. Related, all closed: security: tool/server approvals are not bound to the reviewed definitions (add expected_hashes / review fingerprint) #1566 and Tool approval hash: unescaped '|' field joins let two different contracts share a hash (rug-pull bypass) #1606 on binding approvals to hashes, security: server tool baseline is not durable; replaced inventory or empty review re-triggers auto-baseline of unreviewed tools #1567 on baselines, Tool approval: adding an outputSchema to an approved tool is accepted as a hash-formula migration (skips review and trust_mode=scan) #1591)
Activity
james-crowley commentedon Oct 20, 2026
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 thebenignandunsureclasses. 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: useheld_verdictand the existing tier annotation, add the policy setting, and keep RT-02 intact by making the release an explicit, logged, per-hash decision.kaspermuutra-dotcom commentedon Oct 20, 2026
+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 flipsreadOnlyHintfrom false to true, removesdestructiveHint, 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 abovescan.Kasper
james-crowley commentedon Oct 20, 2026
Thanks @kaspermuutra-dotcom, that is a useful input. A flip of
readOnlyHintfrom false to true, a removeddestructiveHintor a parameter that names a recipient or target is dangerous whatever the description says, so those would put a change in thedangerousclass before any text diff is considered, and a description-only change would count asbenignonly 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 abovescanrather than part of the first step. I will fold the annotation and schema delta into the classification rules when I write this up.kaspermuutra-dotcom commentedon Oct 20, 2026
Thanks @james-crowley, that ordering makes sense: annotations and schema first, text last.
Two more deltas that might belong in
dangerous: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 toscan, 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.james-crowley commentedon Oct 22, 2026
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 (
readOnlyHintfalse,destructiveHinttrue,idempotentHintfalse,openWorldHinttrue), so removing an explicit hint counts as a change to its default.dangerousis checked first, and any one of these is enough:held_verdictother thanclean);readOnlyHintfalse to true,destructiveHinttrue to false,idempotentHintfalse to true,openWorldHintfalse 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);enum,patternor length limit, or a new parameter whose name or description names a recipient, target, URL, path or command;benignneeds 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).unsureis 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_hashesbinds approvals today, so any later change drops it back toscan. 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
simulatemode, with the default policy still holding everything.kaspermuutra-dotcom commentedon Oct 22, 2026
Thanks for folding those in. The rule set reads right to me, especially checking
dangerousfirst 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
benignis 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 inexpected_hashescloses 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
Cfcharacter asdangerous, and only then measure distance.3. Some one-token edits are always semantic.
never→always, a droppednot,max 10→max 1000, a removedonly. These pass any reasonable distance threshold. A short token list (negations, modals, numerals) that lifts a change to at leastunsureis cheap and covers the most likely tricks.It's also worth stating explicitly that a tool with no prior definition starts at
unsureat minimum.On the verifier format, a minimal version could be:
What matters most:
scopeseparates "the definition looks safe" from "the server was called and does what its definition claims." Those make different promises.expires_atexists 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.
james-crowley commentedon Oct 23, 2026
Thanks @kaspermuutra-dotcom. All three edge cases belong in the rules, so I will add them:
ApprovedHashand the previous description, schema and annotations on the approval record), so this costs nothing extra.Cfcharacter (zero-width, bidi control, tag characters) makes the changedangerous. The proxy'soutput_sanitisationalready names the same classes (bidi,zero_width) for responses.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 leastunsure, whatever the edit distance.Also, a tool with no approved definition starts at
unsureat the least. A first-seen tool keeps going through the existing new-tool path.On the verification format: binding to a canonical hash, keeping
staticandbehavioralscope 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 forexpected_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.kaspermuutra-dotcom commentedon Oct 23, 2026
@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
Dumbris commentedon Oct 25, 2026
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.manualkeeps asking a person for everything.autotrusts 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 newbccparameter 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:
enumset, additions includedpattern,format, min/max, length,additionalProperties)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
openWorldHinttrue 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_modePR.auto | scan | manual, defaultscan, an unknown value falls back tomanual. Order: explicit per-server value, then the legacy flags, thendefault_trust_mode, thenscan.Upgrade. A startup line and
mcpproxy doctorwill name every server whosetrust_modeis 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
manualon the servers you do not trust, a profilemax_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
scanon 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/listhistory through them? How often they fire on real releases decides whether "any new optional parameter" should be held too.feat(quarantine): structural change gate in front of the scan-mode ap…
Dumbris commentedon Oct 26, 2026
The structural gate is on main as #1781 (766655d). What it does under
scan, before the text scan gets a say:enumset, 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 thoseNarrowing 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 doctorname servers whosetrust_modeis empty.The three PRs from James (#1717, #1745, #1746) are merged too, and #1793 makes
enable_socket: falsethe 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_modestays open for James's PR.