Skip to content

Gate update calls on the ICP service-discoverability protocol - #168

Draft
aterga wants to merge 13 commits into
mainfrom
arshavir/festive-edison-i03vob
Draft

Gate update calls on the ICP service-discoverability protocol#168
aterga wants to merge 13 commits into
mainfrom
arshavir/festive-edison-i03vob

Conversation

@aterga

@aterga aterga commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

canister_update_call could write to any canister id an agent could name — including ids this server had merely found behind a domain, in a response header, an /env.json, or a JS bundle. Provenance like that is evidence about bytes a frontend happened to ship, not a statement by an application about what it comprises, and it should never have authorized a write. That left the financial guard doing double duty as the primary boundary against arbitrary value-moving calls, which is not a job a curated name list can hold.

This makes the ICP service-discoverability protocol the write boundary, in two layers, and reframes #163's guard as defence in depth behind it. A state-changing call runs only when every check in both layers passes.

Layer 1 — the registration gate (crates/imcp2-core/src/authorization.rs, new):

  1. The financial guard (Layer 2) does not refuse the method.
  2. application_origin is supplied — an https origin, canonicalized.
  3. That exact origin is a registered application whose developer accepted the current ICP MCP Developer Terms.
  4. That origin serves a well-formed /.well-known/ic-architecture manifest, re-read on every call (no cache).
  5. The manifest declares the target canister.
  6. Only then does the call execute.

All six must pass, so the order decides only which refusal the caller reads. Layer 2 is evaluated first because it is offline and its refusal is the more useful one: "transfer 1 ICP" should be answered do it in a wallet you control, not that application isn't registered — the latter reads as though registering would make the transfer possible. A value-moving request therefore also never triggers an outbound fetch, and never reveals whether the named origin is registered.

derivation_origin deliberately cannot stand in for application_origin: derivation origins are shared by design — this crate's own registry maps five NNS frontends and eight Oisy hosts onto one each, and target_origin collapses gateway domains — so keying authorization on one would let any frontend in such a set write against a sibling's manifest, and the manifest is served at the application origin anyway.

Layer 2 — the financial guard (compliance.rs, behaviour unchanged) now sits inside that surface. It is origin-blind, so registration cannot launder a value-moving call through it. Per the review discussion, the earlier CMC concern resolves on its own: the CMC is not a registered application and declares no manifest, so canister_update_call cannot reach it at all — the list is now belt to Layer 1's braces rather than the boundary itself.

Registration is two facts, both required. The protocol proves composition and nothing else — serving a manifest establishes no acceptance of terms, no entitlement to expose the canisters listed, no statement about which methods are safe. So this adds the ICP MCP Developer Terms at /developer-terms, addressed to publishers rather than end users, requiring that the publisher is entitled to expose every canister its manifest lists and that its MCP-reachable operations move no value, are safe for an assistant to call, and handle personal data lawfully. REGISTERED_APPLICATIONS records who accepted which revision and when.

Related issues

Builds on #163 (merged into this branch — the diff collapses to just this change once #163 lands). Follows the review discussion on that PR.

Changes

  • New authorization.rs — the six-step gate, the RegisteredApplication registry, and DEVELOPER_TERMS_VERSION. The policy lives in authorize_with, with the registry and the manifest fetch injected, so the tests drive the production chain rather than a copy of it.
  • New architecture.rs — the protocol itself: /.well-known/ic-architecture parsing (validated version, principal-checked ids, sanitized labels, whole-manifest rejection past the entry cap rather than silent truncation, a 5s read budget) and /.well-known/ii-derivation-origin. Documents which of the five protocol layers this server implements where.
  • canister_update_call takes application_origin (required) and echoes it back with declared_as — how the application's own manifest describes the canister that was called.
  • The protocol's own files take over from /ai-connect.html, which is removed outright along with parse_meta/attr. /.well-known/ic-architecture is the authorizing composition source; /.well-known/ii-derivation-origin is the identity source, keeping the cross-origin authorization check against the declared origin's ii-alternative-origins (ICPBB-430). candid:service, getApiDoc, and OQL were already implemented and are now documented as the same protocol's interface/behaviour/data layers.
  • /.well-known/ic-app.json stays as a read-only fallback. Dropping it (as the first commit did) was a gratuitous regression: apps that shipped it lost their declared canisters from discovery, which the write gate never needed. It ranks below ic-architecture, supplies the declared derivation origin only when an app serves no ii-derivation-origin, and authorizes nothing.
  • The header, /env.json, and bundle mining stay as read-only hints, relabelled as such in code, output schema, tool descriptions, and README: they say what may be readable, never what may be written.
  • New /developer-terms page, in the same document shell as /terms, with its revision and effective date interpolated from the constant the gate enforces (pinned by a test, so the page and the enforced revision cannot drift). Linked from /terms §4 and the landing-page footer.
  • Docs in step: README gains "The ICP service-discoverability protocol", "Update-call authorization", "The Developer Terms" and "Hardened discovery fetches" sections; both directory-submission dossiers, the landing-page copy, SERVER_INSTRUCTIONS, and the local binary's README describe the two-layer posture.
  • The manifest fetch reuses the existing site-fetch guards (public-address resolution, connection pinning, redirect policy, capped body) and adds an exact-origin check, since the shared redirect policy permits same-host different-port hops. Because the registry check precedes the fetch, the authorization path only ever fetches curated origins — a caller cannot use an update call to steer the client at an origin of their choosing.

Testing

cargo test --locked --workspace --all-targets: 173 core (up from 153) + 52 + 11 + 9 + 20 local, all green. The five properties from the review, each driving the real gate with an injected registry and fetch:

  • a canister found only through bundle mining, /env.json, a header, or /ai-connect.html cannot receive updates — and the same id is writable from the application that declares it, so the refusal is about the missing declaration, not the id;
  • a canister the manifest declares, whose origin lacks a current Terms acceptance, cannot — asserted together with a fetch count of zero, which turns "the registry is checked before anything is fetched" into a pinned property;
  • a registered non-financial app can, and the echo names what authorized it;
  • a financial method stays blocked on a registered app — including when the app declares the ICP ledger in its own manifest — and the financial refusal is byte-identical registered, unregistered, and with no origin at all;
  • revocation, a Terms-revision bump, and a manifest that drops a canister each fail closed on the next call (and reverse in both directions, so nothing latches); an unreadable manifest fails closed and says it is retryable, distinguishably from a denial.

Also: an empty registry authorizes nothing and says so; membership compares parsed principals; origin canonicalization accepts equivalent spellings and refuses non-https / user-info / another port without fetching; end-to-end proofs through a real MCP client in imcp2-local; and a test asserting both the tool description and SERVER_INSTRUCTIONS teach the gate.

  • cargo build --locked --all-targets
  • cargo test --locked --all-targets (and --workspace --all-targets)
  • cargo fmt --all / cargo clippy --all-targets (tree is warning-free) — see note below
  • npm test --prefix monitoring/mcp-status (dashboard unchanged)

cargo clippy --all-targets is unchanged from the pre-change baseline: 6 warnings, all pre-existing (map_or simplifications ×5, one redundant closure), none in new code — verified by measuring the same command against the merge base. cargo fmt --all is deliberately not run: on a pristine tree it rewrites 3283 lines across 15 files it should not touch, so the repo is not currently rustfmt-clean and running it would bury this change in unrelated churn. The two new files are rustfmt-clean; touched regions match their surroundings. Worth fixing separately.

Review round already applied

The second commit is the result of an adversarial pass over the first: 32 candidate findings, 28 refuted on the code, 4 confirmed and fixed. Beyond the layer reordering and the ic-app.json restoration above, it fixes two real bugs — is_app_data_candidate compared a label to "frontend" by equality, so a manifest-declared frontend (the protocol's own example folds to "the frontend (frontend)") was handed out as an app data canister; and parse_derivation_origin accepted a bare host, so any one-token 200 body became a bogus cross-origin claim that then hard-failed app resolution. It also corrects prose that had drifted from the code: "revocation takes effect on the very next call" (true of caching, misleading about a compiled-in table), "only registry origins are ever fetched" (true of the write path, not of discovery), and two directory-submission walkthroughs that told reviewers to observe update-call outcomes the gate makes unreachable.

Two things for you to confirm

  1. The registry ships empty, so no application can receive an update call until a publisher actually accepts the Developer Terms and a reviewed change adds a row. That is the honest fail-closed default — I can't record an acceptance that hasn't happened — but it means update calls are off on merge, and the directory-submission docs now say so plainly. If some app should ship registered, that's a row plus the acceptance to point at.
  2. imcp2-local inherits the gate, so writing to your own canister through the local binary is refused unless it is registered. I chose uniform enforcement over a per-deployment escape hatch (no bypass to misuse), and said so plainly in its README — but the local binary's primary user is the canister owner, so this is a real product trade-off rather than a detail.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SzGAjX3k9utczAg2Jw8yQX

claude added 10 commits August 28, 2026 09:26
The name registry exists to route agents to well-known apps; routing
them to a staking/funds frontend sits badly with the server's
not-a-financial-tool posture, so the NNS entry is removed: no name
resolution for "nns", no repair guidance toward it, and the unknown-app
note no longer enumerates it. The KNOWN_DERIVATION_ORIGINS mappings
stay: a user who brings an NNS URL themselves still derives the correct
per-app identity for reads instead of a silently wrong (empty-looking)
one, and update calls to the NNS canisters are refused by the
finance-canister guard added alongside this change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
Adds a third group to the financial-transactions guard: a curated,
labeled disallow list of canisters whose purpose is holding, staking,
exchanging, or moving value. On these, canister_update_call refuses
EVERY update method; each entry carries what the service is and how it
is financial, and the refusal interpolates both labels verbatim.

The list (49 entries, every id verified against the IC dashboard's
canister registry and the services' own published sources on
2026-08-28):

- The network's own funds infrastructure: NNS Governance
  (manage_neuron disburses neuron-held ICP), both NNS dapp frontends,
  the Genesis Token canister, the ICP ledger, the cycles ledger.
- Chain-key minters (they move REAL assets on other chains): ckBTC,
  ckETH/ckERC20, ckDOGE.
- All 14 mainnet chain-key token ledgers.
- Five prominent SNS governance canisters (OpenChat, Gold DAO,
  WaterNeuron, KongSwap, Dragginz) — the same disburse pattern.
- Wallets: the Oisy backend, the Chain Fusion Signer (signs and
  broadcasts Bitcoin/Ethereum transactions), Oisy Trade, the Oisy
  rewards canister, and the wallet frontend.
- Exchanges: ICPSwap (SwapFactory, PasscodeManager, farm and
  staking-pool factories, frontend), KongSwap (backend, treasury
  extension, frontend), Sonic (swap, LBP registry, vesting, plus the
  XTC and WICP DIP-20 ledgers whose legacy transfer names the ICRC
  group deliberately does not match), ICDex/ICLighthouse (router,
  DexAggregator, DAO trader, trading-mining, frontend), and ICPEx
  (router, token-creation service, frontend).

Method-level refusals keep precedence, so the tailored messages
survive (a creation spend still points at the icp CLI; an ICRC hit
still names the standard method); the canister blanket catches
everything else on listed canisters. The CMC stays deliberately
unlisted (its notify_* calls are the documented recovery path), as do
read-only indexes/archives and the exchange-rate canister. The module
docs note the honest limit: exchanges create per-pair pool canisters
dynamically, so the list covers each service's central canisters and
the ICRC method group plus the stated policy cover the rest.

Tests: every entry refuses arbitrary update names with both labels and
the protective policy wording; the list is well-formed (valid, unique
principals, non-empty labels); method-level precedence is pinned. The
server instructions, README tool table, both directory docs, and the
landing page now state the canister-level refusal alongside the method
refusals.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
Per maintainer review on #163: the service no longer exists (its site
and repository are gone), so its backend, treasury extension, frontend,
and SNS governance come off the list — a dead service does not meet the
list's prominence bar. The list stands at 45 entries.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
Per maintainer review on #163: the submission docs read as if a gap had
been closed at some point, rather than describing a product that simply
does not support financial transactions. The financial sections of both
directory docs now state the posture present-tense only — the
"(resolved in code)" heading, the shipped-mitigations lead with its PR
citations, the "execution paths that once moved funds are removed from
the binary" sentences, the "per maintainer review" attributions, and
the "Status: resolved in code" gap-closure paragraph are gone. The
mcp-review email logistics stay (no reply needed; answer with the
stated posture if one arrives), and the production-gap blocker keeps
its operational fact (deploy a release from current main before
submitting) without dwelling on what the old build serves.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
Answering review on #163 ("why not list all SNS governance
canisters?"): enumerating them can't stay current — 54 SNS DAOs exist
today (a third of them aborted swaps, some renamed or folded) and new
ones launch by NNS proposal. Their value-moving surface is one
standardized method, so the guard now refuses `manage_neuron` on EVERY
canister, the way it refuses the ICRC transfer/approval names: that
covers each existing SNS DAO's governance, every future one, and the
NNS's, with no list to go stale. The four per-DAO governance entries
come off the canister list (the NNS Governance entry stays — the
network's own governance keeps its whole update surface refused), and
the standardized-methods group is renamed and re-documented to say it
carries both the ICRC token surface and the governance interface.

Also per review: the Chain Fusion Signer label had an accidental run of
spaces (interpolated verbatim into refusals) — collapsed; and the gate
function's doc now describes all three refusal scopes instead of the
stale "both groups" wording.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
Per review on #163. WaterNeuron (liquid staking): the water_neuron
protocol canister (ICP deposits, nICP minting, unstaking withdrawals),
the nICP ledger, and the frontend — ids pinned by the project's own
canister_ids.json and the dashboard registry; the nICP index/archive
(read-only) and the icrc_validator (proposal validator) stay off, as do
the two SNS-registered dapp canisters with no documented role. MULTI/DEX
(exchange): its backend, bridge, and frontend, self-declared by the
app's own /.well-known/ic-app.json manifest (the frontend id also
matches the live x-ic-canister-id header and the known-app registry).
The list stands at 47 entries.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
Review on #163: the guard refuses manage_neuron everywhere, but the
server instructions, README tool row, both directory docs, and the
landing page still enumerated only the ICRC methods, the ledger-scoped
methods, and the canister list — understating the guard and leaving SNS
neuron coverage unexplained. All four surfaces now name the governance
method alongside the ICRC names.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
…e-vpawel-7-finance-canister-disallow' into arshavir/festive-edison-i03vob
`canister_update_call` could write to any canister id an agent could
name — including ids this server had merely *found* behind a domain, in a
response header, an `/env.json`, or a JS bundle. Provenance like that is
evidence about bytes a frontend happened to ship, not a statement by an
application about what it comprises, and it should never have authorized a
write. The financial guard was doing double duty as the boundary against
arbitrary value-moving calls, which is not a job a curated name list can
hold.

Writes are now authorized in two layers, and a call runs only when every
check in both passes:

**Layer 1 — the registration gate** (`authorization.rs`, new).
`application_origin` is required; the origin must be a registered
application whose developer accepted the current ICP MCP Developer Terms;
that exact origin must serve a well-formed `/.well-known/ic-architecture`
manifest, re-read on every call; and the manifest must declare the target
canister. Anything else refuses, with a message that says which of those
failed and offers the read path instead.

`derivation_origin` deliberately cannot stand in for `application_origin`:
derivation origins are shared by design (this crate's own registry maps
five NNS frontends and eight Oisy hosts onto one each, and `target_origin`
collapses gateway domains), so keying authorization on one would let any
frontend in such a set write against a sibling's manifest — and the
manifest is served at the application origin anyway.

**Layer 2 — the financial guard** (`compliance.rs`, unchanged behaviour)
now sits *inside* that surface, reframed as defence in depth: it is
origin-blind, so registration cannot launder a value-moving call through
it. The CMC concern the list was carrying resolves on its own — the CMC is
not a registered application and declares no manifest, so it is
unreachable by an update call at all.

Registration is two facts, both required. The protocol proves composition;
it establishes nothing about who accepted what. So this adds the **ICP MCP
Developer Terms** at `/developer-terms` — addressed to publishers, not end
users — requiring that the publisher is entitled to expose every canister
its manifest lists, and that its MCP-reachable operations move no value,
are safe for an assistant to call, and handle personal data lawfully. The
registry records who accepted which revision, and **ships empty**: no
application can receive an update call until a publisher accepts the Terms
and a reviewed change adds a row. Revocation is removal of the row, and a
revision bump invalidates stale acceptances; both take effect on the next
call, since nothing is cached.

The protocol is now spoken in full, replacing the two app-declared sources
it supersedes:

  * `/.well-known/ic-architecture` (composition) replaces `/ai-connect.html`
    and the proposed `/.well-known/ic-app.json` as the app-declared canister
    source — `parse_meta`/`attr` and the `AppManifest` structs go with them;
  * `/.well-known/ii-derivation-origin` (identity) replaces `ic-app.json`'s
    `derivation_origin` key, keeping the cross-origin authorization check
    against the declared origin's `ii-alternative-origins` (ICPBB-430);
  * `candid:service`, `getApiDoc`, and OQL were already implemented and are
    now documented as the same protocol's interface/behaviour/data layers.

The header, `/env.json`, and bundle mining stay as read-only discovery
hints, relabelled as such: they say what may be readable, never what may be
written.

The manifest fetch reuses the existing site-fetch guards (public-address
resolution, connection pinning, redirect policy, capped body) and adds an
exact-origin check, since the shared redirect policy permits same-host
different-port hops. Because the registry check precedes the fetch, the
only origins ever fetched are curated ones — a caller cannot steer the
client at an origin of its choosing.

Tests (171 core, up from 153; 20 local): a canister found only via bundle
mining / env.json / a header / ai-connect cannot receive updates; a
manifest-declared canister whose origin lacks current Terms acceptance
cannot; a registered non-financial app can; a financial method stays
blocked on a registered app; revocation, a Terms bump, and a manifest that
drops a canister each fail closed on the next call; an unreadable manifest
fails closed and says it is retryable, distinguishably from a denial. Plus
end-to-end proofs through a real MCP client, and a test pinning the served
Terms revision to the constant the gate enforces.

Note for review: `imcp2-local` inherits the gate — writing to your own
canister through the local binary is refused unless it is registered. That
is deliberate (no bypass), stated in its README, and worth confirming.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SzGAjX3k9utczAg2Jw8yQX
Fixes from an adversarial pass over the previous commit. Two behaviour
changes, several honesty fixes in prose that had drifted from the code, and
a test restructuring so the tests drive the real gate.

**The financial guard is now evaluated first.** All six checks still have to
pass, so the order only decides which refusal the caller reads — and with
the registry empty, "transfer 1 ICP" was answered "that application isn't
registered", which reads as though registering could make the transfer
possible. Layer 2 is offline, so putting it first also means a value-moving
request costs no fetch and reveals nothing about the registry. Pinned by a
test that the financial refusal is identical registered, unregistered, and
with no origin at all.

**`/.well-known/ic-app.json` comes back as a read-only fallback.** Dropping
it outright was a gratuitous regression: apps that shipped it (MULTI/DEX
among them) lost their declared canisters from *discovery*, which the write
gate never needed. It is read after `ic-architecture`, ranks below it,
supplies the declared derivation origin only when an app serves no
`ii-derivation-origin`, and authorizes nothing. `/ai-connect.html` stays
removed.

Bugs found and fixed:

  * `is_app_data_candidate` compared the label to `"frontend"` by equality,
    so a manifest-declared frontend — the protocol's own example labels one
    `"the frontend (frontend)"` once name and role are folded — was handed
    out as an app *data* canister and probed as one. Now a whole-word match
    on the app's prose, split on whitespace only: an app naming a backend
    `frontend-orders-api` still gets its capability probe.
  * `parse_derivation_origin` accepted a bare host, so an SPA catch-all
    answering `/.well-known/ii-derivation-origin` with any single token
    became a bogus cross-origin claim — which the alternative-origins check
    then refused, turning a missing file into a hard failure to resolve the
    app at all. The `https://` scheme is now required (via `get`, not
    slicing, so a multi-byte first character cannot panic), and a BOM is
    tolerated.
  * The manifest read now has its own 5s budget rather than inheriting the
    15s site-fetch timeout: it sits in front of every state-changing call,
    so a slow origin must cost a prompt retry, not a long stall.
  * A refusal at a *registered* application's manifest is now logged — the
    operator's signal that a registered app's manifest has stopped
    declaring a canister its users are calling. Previously only the ALLOW
    path logged, which with an empty registry meant nothing ever did.
  * When the registry is empty the refusal says so, and says not to retry —
    otherwise an agent reads a per-origin refusal as "try another origin".

Prose that overclaimed, now matching the code:

  * "Revocation takes effect on the very next call" — true of caching,
    misleading about deployment: the table is compiled in, so revocation is
    a release. Both the README and the Developer Terms now say that, and
    what "nothing is cached" actually buys.
  * "The only origins ever fetched are the curated registry" — true of the
    authorization path, false of discovery, which reads the same path from
    any origin a caller names. Scoped accordingly.
  * The Developer Terms page had an interpolated revision beside a
    hardcoded effective date, so the "cannot drift apart" claim was false
    for half the line. The date is now the revision.
  * Both directory-submission walkthroughs told reviewers to observe
    update-call outcomes the gate makes unreachable (a Questions-only
    network rejection, a canister rejecting the caller). Rewritten to the
    outcomes a reviewer will actually see, and to say plainly that with an
    empty registry every update call is refused by design.
  * `SERVER_INSTRUCTIONS` now distinguishes a gate refusal from a
    Questions-only network rejection, so an agent does not send the user
    through a re-consent that cannot help.
  * The local binary's lead paragraph advertised writes without the gate;
    the landing page promised actions "across your apps".
  * Registered scope is now documented honestly: one row is one origin (no
    inheritance across an app's gateway origins), and a row is reviewed
    once against a manifest the app can later rewrite — an obligation under
    the Terms rather than something the code enforces.

Tests: the five proofs now drive `authorize_with`, the very function
`authorize_update_call` binds, with the registry and the manifest fetch
injected — no test re-implements the chain any more, so a step added to or
reordered in the gate cannot slip past them. They also assert the fetch
COUNT, which turns "the registry is checked before anything is fetched"
into a pinned property; that an empty registry authorizes nothing; that a
canister refused as undeclared IS writable from the application that
declares it (so the refusal is about the declaration, not the id); and that
a manifest change reverses in both directions rather than latching.

173 core tests (was 171), 20 local, clippy unchanged at the pre-existing
6-warning baseline.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SzGAjX3k9utczAg2Jw8yQX

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds protocol-based authorization for update calls, backed by application registration, live architecture manifests, and developer terms.

Changes:

  • Gates writes by registered origin and declared canister.
  • Adds service-discoverability parsing and identity resolution.
  • Updates documentation, terms, and financial safeguards.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/main.rs Serves Developer Terms.
src/assets/terms.html Links registration terms.
src/assets/developer-terms.html Defines publisher obligations.
README.md Documents discovery and authorization.
docs/openai-directory-submission.md Updates compliance guidance.
docs/anthropic-directory-submission.md Updates compliance guidance.
crates/imcp2-local/src/server.rs Tests local gate behavior.
crates/imcp2-local/README.md Documents local enforcement.
crates/imcp2-core/src/tools.rs Integrates the write gate.
crates/imcp2-core/src/lib.rs Exposes authorization constants.
crates/imcp2-core/src/identities.rs Updates identity documentation.
crates/imcp2-core/src/discover.rs Adds protocol discovery and identity files.
crates/imcp2-core/src/compliance.rs Expands financial-call refusals.
crates/imcp2-core/src/calls.rs Adds authorization arguments and output.
crates/imcp2-core/src/authorization.rs Implements registration authorization.
crates/imcp2-core/src/architecture.rs Parses and fetches architecture manifests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread crates/imcp2-core/src/authorization.rs
Comment thread crates/imcp2-core/src/architecture.rs Outdated
Comment thread crates/imcp2-core/src/discover.rs
Comment thread crates/imcp2-core/src/calls.rs Outdated
Comment thread crates/imcp2-core/src/authorization.rs Outdated
claude added 3 commits August 28, 2026 14:45
Five findings from Copilot's review, all verified against the code first.
Two were real gaps in the gate, two were real defects in it, one was a real
prose bug; the fixes are below in that order.

**A registration now pins what was reviewed.** The row gained two fields —
the canister ids and the derivation origins recorded at review — and the
gate requires both. Previously a row was reviewed once against a manifest
the registrant can rewrite at will, so two things were possible that
should not be:

  * a publisher (or whoever compromised it) could add a canister to its own
    manifest and gain writes to it with nobody having reviewed it. The pin
    means the live manifest can only ever NARROW the reviewed set — dropping
    a canister still stops writes at once, on the app's own signal — and
    never widen it.
  * nothing tied `application_origin` to `derivation_origin`: they are
    separate arguments resolved independently, so a registered application
    could have the server sign a call, to a canister it had listed, as the
    user's principal AT AN UNRELATED APP — an identity that app's canisters
    may well trust. The gate now requires the pair to match what
    registration recorded, offline and before any fetch.

Both checks are offline, add no tool argument and no round trip, and the
registry is empty, so nothing shipped changes shape. `resolve_identity_target`
moves above the gate (pure string work) so the comparison is on the
canonical effective origin the identity path will actually use. What was
documented as "review cannot pin this down, so the Developer Terms carry
it" is now enforced in code, with the Terms as the matching promise rather
than the only line of defence.

**The authorization fetch reads strictly.** It shared `read_capped` with
discovery, which returns accumulated bytes on a stream error or at the cap —
opportunistic, correct for a read hint, wrong for a fail-closed gate. New
`read_strict` errors on both. Truncation could only ever DENY a canister (a
prefix cannot add an entry), so this was not exploitable, but a gate should
not rule on a document it did not fully receive, and "the body is capped"
should mean refused rather than quietly shortened.

**Identity declarations are attributed to the exact origin.** The manifest
fetch already required the response to come from the origin asked;
`read_declared_origin_file` checked that only for the IC-hosting evidence
and parsed the body regardless — so under the shared redirect policy (which
permits a same-host different-PORT hop) a neighbouring origin's
`/.well-known/ii-derivation-origin` could be read as this application's
declaration, contradicting the function's own doc comment. Now it is
ignored, and logged.

**`application_origin` is required in the JSON schema, not just in prose.**
`#[schemars(required)]` alone did nothing: schemars_derive treats a
defaulted field as optional regardless, so `#[serde(default)]` had to go —
serde already deserializes a missing `Option` field to `None` without it.
Both halves are now pinned by a test: the schema's `required` array names
the argument, and a client that omits it anyway still deserializes, so it
reaches the gate's instructive refusal rather than rmcp's opaque
invalid-params error.

**A refusal carried a run of spaces.** rustfmt had joined a `\`-continued
string literal onto one line and kept the continuation's indentation as
literal text, so users read "the answer for          every application".
Fixed with `concat!`, and pinned: a new test renders every refusal this
module can produce — nine paths, including Layer 2's — and asserts none
carries a double space, newline or tab. The bug is invisible in source and
no existing assertion straddled the continuation, so only a rendering test
could have caught it.

Docs updated in step: the README's six-step chain, its `derivation_origin`
section (separate arguments, but the pair must match), the registry
paragraph (the pin replaces the "review cannot pin this down" caveat), and
the hardened-fetch section (strict read, identity-file attribution); plus
the tool description, which now states all five conditions an agent must
satisfy.

176 core tests (was 174), 20 local. Clippy back at the pre-existing
6-warning baseline — the pin check tripped `manual_contains`, now fixed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SzGAjX3k9utczAg2Jw8yQX
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.

3 participants