Serve skill:// resources from a reviewed static bundle by aterga · Pull Request #164 · dfinity/imcp2

aterga · 2026-08-28T10:31:40Z

Summary

The served skill:// resources are now compiled into the binary from a reviewed, versioned bundle instead of being fetched live from skills.internetcomputer.org at request time. Directory review treats dynamically retrieved behavioral instructions as a rejection trigger; a static bundle means what an agent reads is exactly what was reviewed at release, and the served binary no longer retrieves any instructions over the network.

Related issues

None — follow-up in the directory-submission series (after #158/#161).

Changes

crates/imcp2-core/static/skills/ — 23 official IC skills (SKILL.md each) bundled 2026-08-28 from the public skills registry, plus 18 companion documents under references/<skill>/<file>, served as skill://<skill>/references/<file>. The bundle README documents provenance, the curation (what was excluded and why), the local edits applied to the mirrored text, and the refresh procedure.
skills.rs — BUNDLED_SKILLS and BUNDLED_SKILL_REFERENCES tables (include_str!), with bundled_skill_document() resolving a skill:// path for both list_resources and read_resource, so the served set cannot drift from the advertised set. Two tests: the bundle is well-formed, and it is closed under its own references — no document points at the registry, every skill:// link resolves, no companion is named by a bare relative path, and every listed URI is readable.
tools.rs — IcTools (the served composition) drops its SkillsCatalog; resources come from the bundle. The live SkillsCatalog remains library code for the unserved IcProtocolTools, so an embedder who wants live skills still has the machinery.
src/lib.rs, crates/imcp2-local/ — neither binary constructs the live catalogue any more, so neither reads SKILLS_URL nor contacts skills.internetcomputer.org.
Docs follow the binary: README skills section describes the bundle and its companions; the privacy policy (draft + served page) drops the skills host from the disclosed recipients and bumps the effective date to August 28, 2026; the Anthropic submission doc's third-party list and prompt-injection acknowledgment now state that no instructions are retrieved over the network.

Local edits to the mirrored text

The files are verbatim apart from three mechanical edits, recorded in the bundle README and re-applied on refresh: companion references rewritten to their skill:// URIs; three lines of caffeine-app that told the agent to load another skill's SKILL.md over HTTP pointed at skill://writing-motoko; and handoffs to skills this bundle does not carry (wallet-integration, canhelp) neutralized. Content itself is not edited — corrections belong upstream in the registry, and four defects found during review were forwarded there.

Testing

cargo build --locked --all-targets
cargo test --locked --all-targets — green except discover::tests::search_finds_ckusdc_and_lookup_identifies_it, a live-network test failing because the IC dashboard API is down (see the comment below); unrelated to this diff and failing identically on other branches
cargo clippy --all-targets — unchanged 10-warning baseline (this repo is not rustfmt-clean; no repo-wide cargo fmt, matching prior PRs)
npm test --prefix monitoring/mcp-status (dashboard unchanged)

Checklist

I have read the Contributing guidelines.
Docs (README / comments) updated for any user-visible change.
No secrets, credentials, or internal-only information are included.

The served resource surface no longer retrieves anything over the network: the 24 official IC skills are compiled into the binary from crates/imcp2-core/static/skills/ (bundled 2026-08-28 from the public skills registry; provenance, curation notes, and the refresh procedure are in the bundle's README), and resources/list + resources/read serve exactly that bundle. The live skills catalogue stays behind as library code for the unserved protocol tools, so the hosted and local binaries no longer construct it — and no longer read SKILLS_URL or contact skills.internetcomputer.org at all. Docs follow the binary: the README's skills section describes the reviewed, versioned bundle; the privacy policy (draft and served page) drops the skills host from the disclosed recipients and bumps the effective date; the submission doc's third-party list and prompt-injection acknowledgment now state that no instructions are retrieved over the network. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk

Copilot

Pull request overview

Moves served skill:// resources from live registry fetches to a reviewed bundle compiled into both binaries.

Changes:

Adds 24 bundled IC skill documents.
Serves bundled resources without runtime registry access.
Updates constructors, privacy disclosures, and submission documentation.

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
`README.md`	Documents static skill delivery.
`src/lib.rs`	Removes hosted live-catalog construction.
`src/assets/privacy-policy.html`	Updates effective date and recipients.
`docs/icp-mcp-privacy-policy-draft.md`	Updates draft privacy disclosures.
`docs/anthropic-directory-submission.md`	Updates directory attestations.
`crates/imcp2-local/src/main.rs`	Removes local live-catalog construction.
`crates/imcp2-local/src/server.rs`	Updates local test construction.
`crates/imcp2-core/src/tools.rs`	Serves bundled skill resources.
`crates/imcp2-core/src/skills.rs`	Defines bundle lookup and validation.
`crates/imcp2-core/static/skills/README.md`	Records bundle provenance and curation.
`static/skills/agent-web-identity.md`	Bundles web identity guidance.
`static/skills/caffeine-app.md`	Bundles Caffeine app guidance.
`static/skills/canhelp.md`	Bundles canister help guidance.
`static/skills/canister-security.md`	Bundles security guidance.
`static/skills/certified-variables.md`	Bundles certification guidance.
`static/skills/cloud-engine-canisters.md`	Bundles cloud-engine guidance.
`static/skills/custom-domains.md`	Bundles custom-domain guidance.
`static/skills/cycles-management.md`	Bundles cycles guidance.
`static/skills/deploy-to-cloud-engine.md`	Bundles cloud deployment guidance.
`static/skills/encrypted-maps.md`	Bundles encrypted storage guidance.
`static/skills/evm-rpc.md`	Bundles EVM RPC guidance.
`static/skills/https-outcalls.md`	Bundles HTTPS outcall guidance.
`static/skills/ic-dashboard.md`	Bundles dashboard API guidance.
`static/skills/icp-cli.md`	Bundles CLI guidance.
`static/skills/internet-identity.md`	Bundles authentication guidance.
`static/skills/migrating-motoko-actors.md`	Bundles migration guidance.
`static/skills/mops-cli.md`	Bundles Mops guidance.
`static/skills/multi-canister.md`	Bundles multi-canister guidance.
`static/skills/service-discoverability.md`	Bundles discoverability guidance.
`static/skills/stable-memory.md`	Bundles stable-memory guidance.
`static/skills/static-site.md`	Bundles static-site guidance.
`static/skills/troubleshooting-motoko-migrations.md`	Bundles migration troubleshooting.
`static/skills/vetkeys.md`	Bundles vetKeys guidance.
`static/skills/writing-motoko.md`	Bundles Motoko guidance.

Suppressed comments (3)

crates/imcp2-core/static/skills/caffeine-app.md:434

This second live-registry instruction reintroduces dynamic behavioral content despite the PR's static-review guarantee. Direct the client to the bundled skill://writing-motoko resource.

8. **Writing Motoko without the `writing-motoko` skill.** Modern Caffeine Motoko uses
   `mo:core`, persistent actors, and specific patterns. Guessing produces compiler
   errors. Load <https://skills.internetcomputer.org/skills/writing-motoko/SKILL.md> first.

crates/imcp2-core/static/skills/caffeine-app.md:455

This third live URL again tells the agent to bypass the reviewed bundle. Keep the required companion reference entirely within skill://.

- **`writing-motoko`** — REQUIRED companion. Authoritative for all backend Motoko code:
  <https://skills.internetcomputer.org/skills/writing-motoko/SKILL.md>. Always load it before
  editing `src/backend/main.mo`.

crates/imcp2-core/static/skills/stable-memory.md:340

The summary repeats the incorrect claim that Bound::Unbounded prevents schema-compatibility breakage. It prevents size-bound failures, but existing CBOR still fails to deserialize after adding a required field without a default or migration.

- Implement `Storable` for custom types: define `BOUND`, `to_bytes`, `into_bytes`, and `from_bytes`. Use `ciborium::into_writer`/`ciborium::from_reader` for CBOR serialization (compact, fast). Prefer `Bound::Unbounded` -- it avoids backwards compatibility breakage when adding new fields. `Bound::Bounded` exists but is not recommended because exceeding `max_size` after a schema change breaks deserialization

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

Review found the bundle was not self-contained, which left the retrieval it removes still reachable from inside a served document: - Three lines of caffeine-app told the agent to load writing-motoko's SKILL.md from the registry over HTTP. A bundled instruction to fetch instructions defeats the bundle; they now point at the skill://writing-motoko resource. - Eleven companion documents that skills link to (the Motoko API reference, the asset-canister migration notes, and the rest) were not bundled, so every "see references/x.md" link was a dead end. They are bundled now, served as skill://<name>/references/<file>, and the links inside the bundled documents point at those URIs. - canhelp is dropped. It is a slash-command skill whose steps run ./scripts/*.sh from a local checkout, which no MCP client has, and its job is what get_canister_candid already does here. read_resource and list_resources now share one lookup (skills::bundled_skill_document), so what is served cannot drift from what is advertised. A test walks every bundled document and asserts the bundle is closed under its own links: nothing points at the registry, every skill:// link resolves, and every listed URI is readable. The bundle README records the two mechanical edits to the mirrored text (the link rewrite and the caffeine-app loads) so a refresh re-applies them, and states that content corrections belong upstream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk

Copilot

Pull request overview

Copilot reviewed 44 out of 44 changed files in this pull request and generated 4 comments.

aterga · 2026-08-28T11:19:44Z

CI is red on 8cdbf30, and it is not this PR's failure. Recording it here rather than leaving it unexplained.

The failing check is CI / test, on one test:

discover::tests::search_finds_ckusdc_and_lookup_identifies_it
lookup: "dashboard request failed: error sending request for url
(https://ic-api.internetcomputer.org/api/v3/canisters/xevnm-gaaaa-aaaar-qafnq-cai)"

That is a live-network test against the IC dashboard API, which is down: a direct request to the same URL returns HTTP 524 and then times out outright on three consecutive attempts from here. The token-ledgers endpoint the search half uses is equally unreachable. This PR touches the skills bundle and the resource handlers, not discover's network path, and the same test failed on the same commit-minute in #167, whose diff is unrelated to both. CI passed on this branch's previous head (5557429, 10:31Z) and on #167's (30850a0, 10:56Z); the failures start after 11:08Z.

There is no fix to port — the service is the fix — and I am not touching the test: making a live-network assertion pass while the network is down would mean weakening it, which is not something to do for an outage, and certainly not in this PR. The re-run is held until the dashboard answers again; a check-in is scheduled to do that and report back here only if the result is anything other than green.

Generated by Claude Code

The first pass extracted companion references by markdown-link syntax, which missed the ones written in prose — nine documents that skills tell the agent to read (icp-cli's four guides, vetkeys' ibe and bls-signing, encrypted-maps' metadata, and the binding-generation guide two other skills cite) were named but not bundled, so following that guidance still ended in resource_not_found. All of them are bundled now, and every `references/<file>.md` mention is rewritten to its `skill://` URI in prose as well as in links, inside the companions as well as the skills. The two cross-skill citations point at icp-cli's copy, which is where they said to look. The closure test now flags a bare relative mention as well as an unresolvable `skill://` link, which is what would have caught this: it was the mention form, not the link target, that the first pass got wrong. Three handoffs sent the agent to skills this bundle does not carry — cycles-management and internet-identity to wallet-integration, evm-rpc to canhelp. They now say the topic is outside the bundle, or name the tool that does the job. The README's closure claim is narrowed to what the bundle actually guarantees: references between bundled documents resolve to served resources. Ordinary external links (crates.io, GitHub, the docs site) stay as they are, and saying otherwise overstated the property. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk

Copilot

Pull request overview

Copilot reviewed 51 out of 51 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

crates/imcp2-core/src/skills.rs:36

The current PR description says this table bundles 24 official skills, but it contains 23 entries (with canhelp deliberately excluded). Update the PR description to 23 skills plus 18 companion documents so the reviewed release inventory is accurate.

pub const BUNDLED_SKILLS: &[(&str, &str, &str)] = &[

The nested link in static-site's legacy-asset-canister companion was rewritten in the first pass and then silently reverted: the second pass re-fetched that file from the registry to pick up the companions the first pass had missed, overwriting the edit. The closure test did not notice, because it scanned for `references/` mentions and this link is a plain sibling target with no such segment. Both halves are fixed: the link points at its bundled URI again, and the test now also requires every markdown target ending in `.md` to be an absolute URI, which is the check that would have caught the revert. The bundle README no longer offers icp_lookup_canister_info_by_id as something available here — it lives on the unserved protocol router, so naming it alongside get_canister_candid described a tool no client of this bundle can call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk

Copilot

Pull request overview

Copilot reviewed 51 out of 51 changed files in this pull request and generated 1 comment.

The bundle test rebuilds the skill:// URIs from the same tables the handler serves them from, so it stays green if list_resources omits an entry or read_resource is wired wrong — the central path of this PR was the one thing not covered. The local crate already round-trips a real client over an in-process duplex pipe for the tool surface; the resource surface now goes through the same harness. It lists resources, reads every advertised skill:// URI and asserts each arrives non-empty, checks that a companion is reachable at the child URI its parent's links point at, and asserts that an unknown skill and an unknown companion of a real skill both come back as resource_not_found rather than a panic, a disconnect, or an empty document. No network: every resource is compiled in. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk

Copilot

Pull request overview

Copilot reviewed 51 out of 51 changed files in this pull request and generated no new comments.

Suppressed comments (1)

crates/imcp2-core/src/skills.rs:455

The closure check still misses bare references to a companion's parent SKILL.md, even though that filename is not a readable MCP URI. Current examples include references/icp-cli/canister-env-vars.md:3, binding-generation.md:91, references/vetkeys/ibe.md:158, and both static-site companions. Rewrite these to their parent skill://<name> URI and extend this check to reject bare SKILL.md mentions; otherwise the documented guarantee that internal references resolve to served resources is not true.

            // The same holds for a markdown link straight at a sibling file,
            // which carries no `references/` segment to catch it by: inside a
            // companion, `](migrating-from-asset-canister.md)` addresses a
            // document the client cannot open either. Every markdown target
            // ending in `.md` must be an absolute URI.

Two conflicts, both from #164 touching the privacy policy this branch deletes: - src/assets/privacy-policy.html: modified on main (the third-party recipients list drops skills.internetcomputer.org — skills are now served from the reviewed bundle compiled into the binary, so no network fetch to disclose), deleted here. The deletion stands; the content change is ported to the page's one home in dfinity/internetcomputer-org (the #77 re-sync branch), where the policy now lives. - docs/anthropic-directory-submission.md: both sides rewrote the "Publication venue" paragraph. Kept this branch's new-home framing, folding in main's note about what the refreshed text carries (the identifier-linkability wording and the updated third-party list). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xpg83AWtkQo34jbMxpX8WB

main now ships a service-discoverability skill (#164's static bundle): the deploy-time recipe for generating /.well-known/ic-architecture with real per-environment canister IDs. That is exactly what someone who has just hit this refusal needs, and the person reading it is sometimes the one who can ship the manifest. So the refusal names skill://service-discoverability alongside the published guide, mirroring how compliance.rs points at skill://icp-cli and skill://cycles-management for the operations it declines. "Publish a manifest" is a much weaker handoff than the recipe for generating one. Also merges main, which moved under this branch while it was open. No textual conflict, but main's series touched tools.rs, IcTools::new and the README, so the merge was verified rather than assumed: it builds and the full suite passes (264 tests), and clippy is unchanged at 11. The skill and this gate agree, which is worth stating: the skill tells an app how to publish the manifest, and the gate makes publishing it the act that permits a write. Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB Co-authored-by: Claude <noreply@anthropic.com>

…tions The base branch was rebased onto main (picking up #164, #169 and #170), so this branch's base commit no longer existed and the PR went un-mergeable. Merged the new base head rather than rewriting this branch's history. Five conflicts in tools.rs, resolved deliberately: - `discover_app_canisters`'s description: kept this branch's, which documents the empty-result behaviour for a domain with no Internet-Computer evidence. - SERVER_INSTRUCTIONS and its doc comment: kept this branch's rewrite, and adopted the base branch's corrected financial paragraph into it. That paragraph no longer says canister_update_call "refuses" financial transactions as a class — the guard is a safeguard, not a complete filter, so the policy rather than the absence of a refusal is stated as the limit. - The financial-policy test: took the base branch's corrected comment and its new assertion pinning "a safeguard, not a complete filter", so the hedge cannot be dropped by a later rewrite of this text. - The model-readable-metadata policy test block: kept this branch's; the base branch has none. Both submission documents merged cleanly: the base branch adopted the wording already reviewed here, so the two no longer diverge on the paragraph that described these instructions. Workspace green (52 + 10 + 10 + 149 + 21 + 1), clippy at the 10-warning baseline. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk

…bility) (#166) * Restrict update calls to canisters an app declares (service discoverability) Reading the Internet Computer is open; writing is not. A state-changing call runs against someone's live application, and a canister being publicly callable is not a statement by its operators that they want an agent driving it. `canister_update_call` is now made ONLY to a canister the owning app DECLARES in its service-discoverability manifest at /.well-known/ic-architecture — publishing that manifest is, per the published guide, exactly how an app's operators opt in. The manifest lives at the app's origin, not on chain, so the tool takes a new `app_url` argument (open_app already returns it) naming the app whose manifest authorizes the write; it falls back to `derivation_origin` when the app serves its manifest at that same origin. A successful call echoes `declared_by`/`declared_at`, so a write's provenance is visible in the reply, not only to the gate. The new `discoverability` module holds the policy and every refusal text, alongside `compliance` (the financial-methods gate). Refusals distinguish their four causes — no origin given, origin unreachable, no manifest published, published but this canister is not in it (that one lists what the app does declare) — and each one names the standard path, links the guide so an app owner can act on a relayed refusal, and states that reads are unaffected, so an agent answers what it can rather than concluding the whole app is off limits. The module doc is explicit about what the gate is not: it is a consent-and-provenance gate, not proof of ownership (whoever controls a domain controls what its manifest says), and not a substitute for the canister's own authorization. Supporting protocol work in `discover`, needed because the gate keys on the protocol rather than on this server's earlier proposal: - /.well-known/ic-architecture is now read during discovery, ranked just under the App Connect main-backend meta and above the legacy path. The legacy /.well-known/ic-app.json is still read, at lower authority and still gate-eligible, so the apps that adopted the proposal are not cut off the day the standard path lands — the one known adopter today publishes only that path. - /.well-known/ii-derivation-origin (Layer 5) now takes precedence over the legacy manifest's `derivation_origin` field. Without it, an app that follows the published guide and pins a custom derivation origin would resolve to the wrong principal — a bug the gate would otherwise have created, since it makes the protocol the thing writes key on. - The three well-known probes on each path run concurrently, so honouring both the protocol and the legacy path costs no extra round trip. - Manifest labels fall back to the protocol's `name` when no `role` is given, so a spec-shaped manifest yields a label, not a bare principal. Everything fails closed on the misconfiguration the guide calls out as the most common one: an SPA catch-all answering /.well-known/* with index.html, which returns 200. A manifest body that is not a manifest document reads as "publishes none" rather than "declares nothing", and an HTML first line is not an origin, so a mis-served Layer 5 file leaves the app deriving against its visible origin instead of garbage. Verified against the live reference app, which today serves exactly that shape at both protocol paths. Reads are untouched: canister_query, get_canister_candid, get_canister_api_doc, the OQL surface and every discovery tool work on any canister, as before. Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB Co-authored-by: Claude <noreply@anthropic.com> * Harden the discoverability gate: redirect pinning, safe echoes, honest refusals An independent design review of the gate confirmed the shape but found five things worth fixing. All are small and none changes which canisters authorize a write. Redirect pinning. A manifest is now honoured only when the origin that ANSWERED is the origin we PROBED. The shared redirect policy already refuses a cross-domain hop, but it permits same-host different-port hops and hops to global IP literals, so without this an origin could 3xx and have another origin's declaration attributed to it -- making the `declared_by` provenance the caller is shown a true-looking but wrong statement. A skipped redirect is logged. Sanitized, capped error echo. `unreachable_refusal` interpolated the raw transport error, which for a hostile origin can carry an attacker-chosen TLS certificate subject or redirect URL straight into the model's context. It now goes through the same control-char scrub the manifest labels use, capped at 200 chars (CWE-150). Truncation no longer masquerades as absence. `manifest_canister_ids` silently dropped entries past the 100-entry cap, so declared entry 101 drew a "not declared" refusal -- a false statement about the app. The overflow is carried and reported, and the refusal blames the manifest's length rather than the app's declaration. The SPA catch-all is named, not just detected. When the protocol path answers 2xx with something that is not a manifest, the refusal now says what it answered with and that the usual cause is an SPA catch-all returning index.html, with the fix (exempt /.well-known/* from the rewrite). The guide calls this the most common failure; an operator told only "absent" would go looking for a file that is already there. This changes the message, not the verdict -- the shape check already rejected the HTML. The no-manifest refusal stops the retry loop. It previously read as "try a different app_url", which invites an agent that already has the right origin to burn turns re-resolving it. It now says outright that re-running open_app will not change anything and to stop. Also: legacy-path authorizations are logged with the origin, so the fallback can be retired on evidence rather than on a guess (today it is the only path that authorizes anything in the known-app registry); a note on `mgmt_call` records why the deferred management writes sit outside this gate and what to revisit if that group is ever served; and the README's SSRF section now says `canister_update_call` makes an untrusted outbound fetch, which was true the moment the gate landed and was not stated. New tests pin the negative space too: no gate refusal may borrow the vocabulary of the read-only-session rejection ("reconnect", "Actions & questions") or of the financial-methods refusal ("oisy.com"), since an agent sent down either path would ask the user to fix something that is not broken. Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB Co-authored-by: Claude <noreply@anthropic.com> * Hold a declared id to the protocol's canister-principal rule The guide says an `id` "is required and must be a canister principal". The gate only checked that it parsed as A principal, which is a weaker rule than the one the protocol states: user principals (29 bytes, 0x02), the anonymous principal (1 byte, 0x04) and the management canister aaaaa-aa (an EMPTY blob) all parse happily. The management canister is the one that matters: without this, a manifest declaring aaaaa-aa would authorize canister_update_call against it. Opaque canister ids are 10 bytes with the 0x01 type tag, verified against real ids rather than re-derived from memory, and every real app canister is one. Enforced on the gate's read of a manifest only -- the display path that feeds discovery listings is unchanged, since showing an odd declared id is informative while acting on one is not. Deliberately the spec's own type rule and not a policy list of ours: WHICH canisters an app may declare is the app's business, but WHAT KIND of principal an `id` may be is the protocol's. Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB Co-authored-by: Claude <noreply@anthropic.com> * Point the no-manifest refusal at the served how-to skill main now ships a service-discoverability skill (#164's static bundle): the deploy-time recipe for generating /.well-known/ic-architecture with real per-environment canister IDs. That is exactly what someone who has just hit this refusal needs, and the person reading it is sometimes the one who can ship the manifest. So the refusal names skill://service-discoverability alongside the published guide, mirroring how compliance.rs points at skill://icp-cli and skill://cycles-management for the operations it declines. "Publish a manifest" is a much weaker handoff than the recipe for generating one. Also merges main, which moved under this branch while it was open. No textual conflict, but main's series touched tools.rs, IcTools::new and the README, so the merge was verified rather than assumed: it builds and the full suite passes (264 tests), and clippy is unchanged at 11. The skill and this gate agree, which is worth stating: the skill tells an app how to publish the manifest, and the gate makes publishing it the act that permits a write. Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB Co-authored-by: Claude <noreply@anthropic.com> * Drop the ai-connect.html discovery source The App Connect bridge page is not part of the service-discoverability protocol, so discovery no longer probes /ai-connect.html or reads its ic:canister-id meta. App-declared metadata is now exactly the two manifest documents: /.well-known/ic-architecture (Layer 1, the only source that authorizes an update call) and the legacy /.well-known/ic-app.json. The HTML meta parser and its attribute tokenizer went with it: nothing else called them. The authority ranking closes the gap the removed tier left, so the protocol manifest now leads; docs and tool descriptions follow. No discovery coverage is lost in practice. Of the apps we probe, only multidex.ai actually serves the meta, and it declares that same backend id in its ic-app.json manifest; the others answer /ai-connect.html with their SPA catch-all, which never carried the meta. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB * Stop the legacy manifest path from authorizing writes Only /.well-known/ic-architecture authorizes an update call now. Publishing that manifest is the act by which an app's operators opt in, under this connector's terms; the operators who adopted this server's pre-protocol /.well-known/ic-app.json proposal published it against different terms and never agreed to those. Consent that was never given cannot be inherited from a path this server invented, so an early adopter is refused. Discovery still READS the legacy document — reads were never gated — and the gate still fetches it, but only as evidence for the refusal: an origin that serves it gets its own message naming the document it DOES publish, saying that serving the same JSON at the standard path is the whole fix, and, when the older document lists the target, making clear the refusal is about where the declaration lives rather than about an unknown canister. Reporting these apps as "publishes no manifest" would send their operators hunting for a file that is already there. ManifestProbe::Absent carries that document; only ARCHITECTURE_PATH can produce Declared. The log line changes with it: the origins still on the old path are now recorded as refused rather than as authorized, which is the same adoption signal read the other way round. Verified live: multidex.ai, the one origin that authorized anything through the legacy path, is now refused — a new live test pins exactly that, against the id its own legacy document declares. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB * Document that only the standard path authorizes a write Follows the gate change: the README's write-gate section gains the reason the legacy document cannot stand in (its publishers adopted a pre-protocol proposal under different terms), the refusal-cause list gains that case, and the app-declared-metadata section no longer reads as though the legacy path keeps writes working. The discover module's own docs say the same: the legacy manifest is read for discovery, not for authorization. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB * Bind the app manifest to the identity the write is signed as The manifest gate establishes that someone published a document at the origin the caller named. It said nothing about whose identity the call goes out under, and those were separable inputs: app_url picked the manifest, derivation_origin picked the principal. Publishing a manifest is free and the gate deliberately does not prove ownership, so an attacker's origin could declare someone else's canister while the call was signed with the principal the user holds at an app they actually trust. Where a call carries both, the app at app_url is now resolved to the derivation origin Internet Identity derives its users from, and a call naming a different one is refused. An attacker's manifest can then only ever authorize writes made as the attacker's own app identity, which is worth nothing to them. The comparison is against what the app RESOLVES to — its declared Layer 5 origin, else a known-app value, else its own origin — not against the app URL literally, so the many apps whose derivation origin differs from their website (13 of 17 in the built-in registry) still pass. resolve_app_identity also enforces Internet Identity's own rule on the way: a cross-origin declaration counts only if the declared origin authorizes this app in its ii-alternative-origins, so an app cannot claim another's identity to satisfy the check. It fails closed — an origin that cannot be resolved refuses rather than being treated as a match — and the two probes run concurrently, so the binding costs no extra round trip. The binding applies only when the caller named an app AND is acting as someone: with no app_url the manifest is read at the identity's own origin, so the two are the same app by construction, and an anonymous call carries no app identity for a foreign manifest to misuse. Also in this commit, the review findings on the same path — the binding rests on the first of them, and the rest are one-line corrections it would have been artificial to separate: - A transport failure on the identity path no longer collapses into "not declared". fetch_well_known now distinguishes Served / Absent / Unreachable, and an unreachable probe is an error rather than a fallback: a Layer 5 file we failed to fetch is not "no Layer 5 file", and defaulting past it would derive — and sign as — a principal the app does not pin. Refusing is recoverable; acting as the wrong identity is not. - app_url is reduced to its origin before it is checked, fetched or echoed, so a refusal cannot be flooded through the unbounded path of an otherwise valid URL, and every echoed origin goes through the module's own cap. - The gate's decision is split from the fetch that feeds it. With nothing published on the standard path yet, every live test of the YES branch can only skip, and a local fixture is unreachable behind the SSRF guard — so the decision is now pinned on constructed input, including that the legacy document does not authorize. - Three resolution messages and a README paragraph still named only the legacy /.well-known/ic-app.json, which stopped being the only source of a declared derivation origin when this branch gave Layer 5 precedence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB * Fail closed on the Layer 5 format, transient statuses and declared_at Three findings from the second review pass, all on the same theme: a value we could salvage is not the same as a value the app declared. - The Layer 5 file's documented format is now ENFORCED rather than coerced. It was read through the general origin normalizer, which accepts a bare host and quietly drops a path — so `example.com` or `https://example.com/path` became an authoritative identity declaration. This file decides which principal the user acts as, and a wrong one fails silently, as a call that simply isn't them. Anything that is not exactly one non-empty line carrying an explicit https origin now reads as "declares nothing", which lands the app on its own visible origin: the default the protocol already specifies for an absent file. The legacy manifest's field keeps the looser parse it shipped with; only the protocol's own document has a published format to hold to. - A non-success status no longer collapses into "publishes no manifest". That refusal tells the app's operators to publish one and the agent to stop retrying, so only a definitive absence may produce it: 404 and 410 do, while 429 and the 5xx family are a failure to find out, and a 403 is the origin declining to say. The rule is a small pure function so it can be pinned without a network, which the SSRF guard would otherwise make impossible. The gate's probe follows: only the standard path's failure is fatal now, since a legacy probe can only enrich a refusal that is happening anyway. - `declared_at` documented a value a successful call can no longer carry. It is always the standard path, and says so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB * Keep a failed read from reading as an absent declaration Third review pass, all the same shape as the second: a value we could not read is not a value the app does not have. - fetch_well_known applied the "absent" verdict to every non-success status, so a 429 or 5xx during an outage could fall through to the legacy field or the application-origin default and sign as a different principal. It now uses the same rule as the gate's probe: only 404 and 410 mean absent. - read_capped turns a mid-body transport failure into partial text, which suited the discovery crawl but let both probes classify a dropped connection as a completed non-manifest response — the permanent "publishes no manifest; stop retrying" verdict for an app that may serve a perfectly good manifest. The callers that draw a conclusion from a body now use a strict reader that propagates the failure; the opportunistic crawl keeps the fail-soft one. Hitting the size cap stays a success: that is a bounded read this server chose, not a failed one. - The over-long-manifest refusal led with "does not declare X" and admitted two sentences later that entries past the read cap were never checked. If the target is one of them, the opening clause is the false statement that handling exists to avoid, so the verdict is now indeterminate from the start. - The identity refusal told every caller to retry, including the one failure retrying cannot fix: a cross-origin derivation origin the declared origin does not authorize back. That case now gets the configuration fix instead. - Two tool descriptions still promised the old fail-soft resolution. They now distinguish a probe that answered without a usable declaration (assume) from one that did not complete (refuse). Also rewraps two refusal strings an earlier edit had joined into single 800-character source lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB * State the two limits of the ownership gap outright The module doc and README already said the gate is not proof of ownership. Review pointed at the two cases a reader is most likely to assume away, so they are now named rather than left to be inferred: an anonymous write skips the identity binding entirely, since there is no app identity to protect, and an authenticated one binds to the caller's own app while still reaching any victim method that accepts an arbitrary principal. Neither grants a capability an attacker did not already have — the IC accepts ingress from anywhere, so both calls can be sent with an ordinary agent — so what the gate withholds is this connector's willingness to make them on a user's behalf, and the binding is what keeps the user's own app principals out of it. That distinction is the honest claim, and it is now the one the docs make. Closing the remaining gap needs an association the TARGET attests to, and nothing a canister publishes today names its app's origin, so it is a protocol question rather than something to invent here. Raised on the pull request. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB * Detect a truncated read, and say what overflow means per document Fifth review pass. The strict reader still passed a body that hit the size cap off as a whole one, so a conclusion could be drawn from bytes this server chose not to read: a manifest over the cap parses as "not JSON" and becomes the permanent "publishes no manifest" verdict, and a Layer 5 file could hide a second non-empty line past the cap and slip the one-line rule. It now reads one byte beyond the cap so overflow is detectable. What overflow MEANS turned out to differ by document, which the first attempt at this got wrong and a live test caught: oisy.com answers the Layer 5 path with its SPA shell, which is larger than the 4 KiB cap, so refusing on overflow stopped resolving a perfectly healthy app. For a document whose FORM bounds its size — one canonical origin on one line — an over-cap body is positive evidence that what was served is not that document, so it reads as "declares nothing" and the app lands on its own origin. For an open-ended JSON manifest there is no such inference, so overflow stays a read we could not finish. The two are now separate outcomes rather than one rule. Also from the same pass: - The `open_app` argument schema and the `resolve_app` output schema carried the claims their descriptions had already dropped: that any failed fetch counts as no declaration, and that a "declared" derivation origin comes only from the legacy path. Both are model-readable metadata and were contradicting the code. - The manifest-check refusal told every caller to retry, including the ones retrying cannot help: an origin answering 401/403 on the path is denying it deliberately, and a blocked redirect is a configuration this server will not follow. Those now get the fix instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB * Cover the gate against a real adopter, and fix the failure classifier svault.tech (built with caffeine.ai) publishes the Layer 1 manifest at /.well-known/ic-architecture and pins a CROSS-ORIGIN derivation origin at /.well-known/ii-derivation-origin. Verified live end to end: the manifest parses and declares two canisters, the identity resolves to the declared cross-origin value (so Internet Identity's own ii-alternative-origins authorization passed — that origin lists svault.tech back), the gate authorizes a declared canister, and the binding accepts the pair. That correction matters beyond the tests: the YES branch now has live coverage, and the cross-origin case is exactly the one a literal app_url-equals-derivation_origin comparison would have false-refused, which is why the binding resolves the app instead of comparing strings. The constructed-input tests stay, since the live ones depend on someone else's deploy remaining up. Also from the sixth review pass, both in the failure classifier added one commit ago: - It looked for the word "redirect", which a stopped redirect never contains — the redirect policy surfaces it as its 3xx status — so the case the check was written for fell through to "likely transient, retry". It now classifies on the status shapes the fetch helpers actually produce, with 408 and 429 kept retryable, and the identity refusal shares the same rule instead of only knowing about an unauthorized cross-origin claim. - The over-long-manifest refusal quoted the number of canister principals RETAINED as the number of entries read, which understates when a manifest carries malformed entries. It no longer quotes a number it cannot support. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB * Correct two doc comments the fail-closed change left behind Both describe code this branch changed, and both would mislead a maintainer about exactly the semantics the change exists to establish: - WellKnown::Absent still said "any non-2xx". Only 404 and 410 reach it now; every other non-success status is Unreachable, because an origin declining or failing to serve the path has not told us the app declares nothing. Both variants now say what they mean and what they exclude. - resolve_declared_origin still said a "missing/unsuccessful/undeclared" declaration yields the application-origin default. Only a declaration the app ANSWERED without providing does; a probe that did not complete is an error, since defaulting past it would derive, and sign as, a principal the app does not pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JarhapgjytMcVSCW7d8sqB * Key the known-app fallback on the origin, not the bare host A non-default port is a different origin everywhere else in this codebase — Internet Identity derives a different principal for it, and identities::target_origin keeps it rather than stripping it the way it strips :443 — but KNOWN_DERIVATION_ORIGINS is keyed by host alone, so https://oisy.com:8443 inherited https://oisy.com. At the write gate that is the whole comparison: a manifest served on that port would authorize a write signed as the user's principal at the real app. The registry stays host-keyed; the lookup for anything derived from a URL now refuses a non-default port and drops to app_url_default, which being a distinct origin then fails the binding on its own. Reaching the case needs control of the registered host, so this closes the gap rather than a live break. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Pin the identity probes to the origin that was probed fetch_declared_manifest honours a manifest only when the origin that answered is the origin that was probed. The identity path did not: only the IC-evidence capture compared the final URL, while the BODY was parsed whatever origin produced it. The shared redirect policy refuses a cross-domain hop but permits same-host different-port hops and hops to global IP literals, so a 3xx could hand another origin's bytes to Layer 5 — the document that decides which principal every call is signed as. Same rule, in the one place fetch_well_known already funnels every identity probe through. A redirected answer is Unreachable rather than Absent: Absent means "the app declares nothing, derive against the default", and a document we deliberately ignored is not one the app does not have. Neither pin can be driven from a local server (both need a final URL that differs from the probed one, and the SSRF guard refuses loopback), so both stay on the live tests, as the existing comment now records. The full suite, live tests included, is unchanged at 266. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Pin the origin before reading the status, and on the discovery probes Two gaps the previous commit left, both raised in review. The origin pin in fetch_well_known ran AFTER the status classification, so a redirect target's 404 still became Absent — "the app declares nothing, derive against the default" — which is the bug the pin exists to prevent, reached by a different route. Nothing a foreign origin says about this path is an answer about this app, so the pin now comes first and the status is only read once the probed origin is the one answering. fetch_success_body dropped served_from entirely, so discovery attributed a redirect target's manifest to the app that was probed, at the protocol path — the top authority tier, which is what the model picks a canister on. Discovery authorizes nothing, but it does make a provenance claim, and that claim was capable of being wrong in exactly the way fetch_declared_manifest already pins against. Suite unchanged at 266, live tests included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Pin the authorization list to the origin that was probed ii-alternative-origins is the list that decides whether a cross-origin derivation-origin claim is honoured, and it was the one fetch on this path still parsing whatever a successful response carried. A redirect target's answer is not the declared origin's statement, so it cannot grant the claim; it now returns the empty list every other failure here already returns, which is the fail-closed value. The rule the last two commits applied in two places is now one predicate, answered_by, shared by all three (ic_evidence_from already had it inline). Suite unchanged at 266, live tests included — a_cross_origin_declared_ identity_binds still passes, so the real cross-origin case is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: sea-snake <thomas.gladdines@dfinity.org>

aterga requested a balanced review from Copilot August 28, 2026 10:38

aterga marked this pull request as ready for review August 28, 2026 10:38

aterga requested a review from a team August 28, 2026 10:38

Copilot started reviewing on behalf of aterga August 28, 2026 10:38 View session

sea-snake previously approved these changes Aug 28, 2026

View reviewed changes