Read the II pairing from /version instead of guessing it from the hostname - #94
Merged
Merged
Conversation
…tname The production dashboard reported "II /mcp connect page served: FAIL — GET https://internetcomputer.org/mcp → 404". Production was healthy; the check was probing the wrong host. The dashboard derived the Internet Identity origin by stripping the leading `mcp.` label off the MCP host. That encodes an assumption which only holds when the MCP server is a subdomain of its II: it maps mcp.beta.id.ai to beta.id.ai correctly, but maps mcp.internetcomputer.org to internetcomputer.org — the marketing site, which legitimately 404s on /mcp. A red II section on a healthy deployment is worse than no check, because it trains operators to ignore it. Fixing the value alone would have relocated the bug: one origin cannot describe a server that mounts more than one II, so whichever mount the hostname guess did not name went unmonitored everywhere, silently. So the server now advertises the pairing and the dashboard reads it: - GET /version gains an `instances` array — one entry per SERVED mount, with its name, mcp_path, ii_origin and ii_canister. Built from the live McpServer handles, so it tracks $MCP_SERVE_BETA and honours II_URL / II_URL_PROD overrides rather than restating defaults a monitor could drift from. McpServer::instance() exposes what it is built with. - The dashboard reads that array and runs the II-health section once per instance, ids and section titles suffixed per instance name. deriveIiOrigin is gone; no hostname arithmetic remains. - Advertised origins are still validated against the probe allowlist before anything is fetched — they arrive in a remote response, and a misconfigured or compromised server must not be able to steer these probes at a third party. A rejected entry is reported as a warn rather than dropped, so the section never reads as fully monitored while covering less. - A build with no instances[] (or an unreachable /version) now fails with "cannot determine the pairing" instead of confidently probing a guess. --ii / II_ORIGIN pins an origin for that case. deploy.sh pinned the dashboard's allowlist to the PARENT domain, because the derived II origin was a sibling host. On mcp.internetcomputer.org that quietly allowlisted all of internetcomputer.org. The II origins are now covered by the built-in id.ai suffixes, so it pins the deployment's own host instead. Verified end to end, not just by unit test: run against a local server with MCP_SERVE_BETA=1, the dashboard discovers both mounts and passes every check against the real id.ai and beta.id.ai; against live production (still running 80b8586, which predates instances[]) it now reports "advertises no II instances" instead of a false II outage, and --ii https://id.ai makes it pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R8ZshwKmjD5fZ4Hs9dS6zh
There was a problem hiding this comment.
Pull request overview
This PR fixes false Internet Identity (II) outage reports in the MCP status dashboard by having the MCP server advertise its II pairing via GET /version and having the dashboard consume that authoritative mapping instead of deriving II origins from the MCP hostname.
Changes:
- Server:
GET /versionnow includes aninstances[]array describing each served mount’s{name, mcp_path, ii_origin, ii_canister}. - Dashboard: removes hostname-based II derivation, parses
instances[], and runs II health checks per instance (with suffixed IDs). - Deployment: tightens the SSRF allowlist pinning in
deploy.shand updates service commentary to reflect instance discovery via/version.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.rs | Adds instances[] to /version, built from live McpServer handles. |
| src/lib.rs | Exposes McpServer::instance() so /version can report II pairing. |
| monitoring/mcp-status/report.js | Updates CLI rendering to display multiple II origins. |
| monitoring/mcp-status/README.md | Updates docs to describe advertised-instance discovery and allowlist validation. |
| monitoring/mcp-status/config.js | Replaces derived-II logic with parseAdvertisedInstances() + allowlist enforcement. |
| monitoring/mcp-status/cli.js | Updates --ii help text to reflect “pin override” semantics. |
| monitoring/mcp-status/checks.test.js | Adds/updates tests for advertised-instance parsing and per-instance behavior. |
| monitoring/mcp-status/checks.js | Implements instance-aware linkage + II health sections and suggestions handling. |
| deploy/native/imcp-status.service | Updates service commentary to match /version discovery behavior. |
| deploy/native/deploy.sh | Pins SSRF allowlist to the deployment host (not the parent domain). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot caught a real break I missed: I updated the terminal renderer but not the hosted web UI. public/index.html read `targets.iiOrigin` and `facts.ii.canisterId`, so on the page Caddy actually serves at /status/ — the one this whole change exists to fix — the header would have read "II: (unresolved)" even with both instances discovered, and the canister line would have vanished. Rather than keep a back-compat single-instance shape alongside the new one, update the consumers. Both live in this repo and ship with the server, so there is no external reader to be compatible with, and a singular `iiOrigin` sitting next to `iiOrigins` reintroduces exactly the failure this PR removes: a consumer reads the singular field and silently reports one of N instances. - public/index.html renders every advertised II with its name and canister id. - facts.ii is renamed facts.iiInstances. The old key held ONE instance's facts, so anything still reaching for `facts.ii.canisterId` would read undefined against the new map; an absent key makes that a visible break rather than a quiet wrong answer. - report.test.js's evilReport now carries `iiOrigins`. That test had gone vacuous: it pins CWE-150 terminal-injection sanitising on a field the renderer no longer reads, so it passed while asserting nothing. Confirmed the coverage is real again by mutation — deleting the clean() call on the iiOrigins path fails both report tests, restoring it passes. Also drop the stale "/mcp → beta, /mcp-prod → production" examples from two comments I wrote before rebasing onto #92. /mcp-prod no longer exists; the repo now has no reference to it at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R8ZshwKmjD5fZ4Hs9dS6zh
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The production dashboard reports:
Production is healthy. The check was probing the wrong host.
The dashboard derived the Internet Identity origin by stripping the leading
mcp.label off the MCP host. That encodes an assumption which only holds when the MCP server is a subdomain of its II — it mapsmcp.beta.id.ai→beta.id.aicorrectly, but mapsmcp.internetcomputer.org→internetcomputer.org, the marketing site, which legitimately 404s on/mcp. A red II section on a healthy deployment is worse than no check: it trains operators to ignore the dashboard.Fixing only the value would have relocated the bug. One origin cannot describe a server that mounts more than one II, so whichever mount the hostname guess didn't name went unmonitored — silently, on every deployment. So the server now advertises its pairing and the dashboard reads it.
Related issues
None filed. Surfaced by the live production dashboard. Builds on #92, which made
/mcpserve production II and put beta behind$MCP_SERVE_BETA.Changes
GET /versiongains aninstancesarray — one entry per served mount:{name, mcp_path, ii_origin, ii_canister}. Built from the liveMcpServerhandles, so it tracks$MCP_SERVE_BETAand honoursII_URL/II_URL_PRODoverrides instead of restating defaults a monitor could drift from.McpServer::instance()exposes what the server was built with.ii-mcp-flow:prod).deriveIiOriginis deleted — no hostname arithmetic remains anywhere.warnrather than dropped, so the section never reads as fully monitored while covering less than it claims.instances[](or an unreachable/version) now fails with "the server advertises no II instances" instead of confidently probing a guess.--ii/II_ORIGINpins an origin for that case.deploy.shallowlist tightened. It pinned the dashboard's SSRF allowlist to the parent domain, because the derived II origin was a sibling host — onmcp.internetcomputer.orgthat quietly allowlisted all ofinternetcomputer.org. The II origins are now covered by the built-inid.aisuffixes, so it pins the deployment's own host instead.Testing
Verified end to end against real hosts, not only by unit test.
MCP_SERVE_BETA=1— dashboard discovers both mounts from/versionand passes every check against the realid.aiandbeta.id.ai:mcp.internetcomputer.org, still running80b8586, which predatesinstances[]) — now reports "advertises no II instances at /version (a build predating the instances[] field…)" instead of a false II outage.--ii https://id.ai— all six II checks pass, confirming the pinned-override path./mcp-prodconfirmed 404 on this build in both modes (production andMCP_SERVE_BETA=1); theinstanceslist contains only what is actually routed.MCP_STATUS_ALLOWED_HOSTSrefuses the run rather than probing.cargo build --locked --all-targetscargo test --locked --all-targets— 139 + 7 passnpm test --prefix monitoring/mcp-status— 25 pass (was 19; added coverage for advertised-instance parsing, allowlist rejection, missinginstances[], per-instance id suffixing, and instance-aware suggestions)cargo fmt --all/cargo clippy --all-targets: the tree is not clean at baseline —cargo fmt --checkflags 11 files and clippy reports 8 warnings onmain, none in code this PR touches. Confirmed every fmt hunk inmain.rs/lib.rs(lines 19, 39, 94, 101, 208, 518, 561, 569) falls outside the lines added here, and no clippy warning points at either file. Not runningcargo fmt --all, which would reformat unrelated files and swamp the diff; there is no CI fmt gate.Checklist
monitoring/mcp-status/README.mdquestion 2 rewritten, CLI--iihelp updated, and the reasoning recorded next to the code.Note on rollout
The dashboard and the binary ship from the same commit, so they move together. Until a
release-*tag carries this to production, the live dashboard will show the II section as undetermined rather than failed — accurate, and no longer a false alarm.Generated by Claude Code