Repository navigation
/status: fix stale "II /mcp delegation flow enabled" check - #38
Merged
Merged
Conversation
The "II /mcp delegation flow enabled" check WARNed because it asserted the /mcp CSP form-action allows https:. That was the precondition for the old delegation flow, where II form-POSTed the delegation to the server. II #4086 retired that flow, so form-action no longer allows https: (it's now 'self' http://127.0.0.1:*, tightened on purpose); the 127.0.0.1 entry is for the unrelated /cli loopback flow, not MCP. The current connect flow never form-POSTs: the key-request and completion callbacks are fetch() calls (governed by connect-src, which allows the https origin) and the hand-back to finish_url is a top-level navigation (not governed by form-action). Nothing is blocked — the check was a false positive. Fix (the ticket's preferred option): drop the form-action assertion and just verify GET <ii>/mcp returns 200. Per-user trust is on-chain and not globally inspectable, so a served connect page is the only meaningful instance-wide signal. Updates the check description/expected/detail, drops the now-dead WARN suggestion, and refreshes the dashboard README. Replaces the obsolete "warns on loopback-only form-action" test with one pinning that a served /mcp now passes regardless of form-action. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014T9N8USDfNK5yzznPGg7Ym
There was a problem hiding this comment.
Pull request overview
Updates the /status monitoring dashboard’s Internet Identity /mcp health check to align with the post-#4086 connect flow (fetch + navigation), removing the stale CSP form-action assertion that was causing false WARNs.
Changes:
- Simplifies the
ii-mcp-flowcheck to pass/fail based onGET <ii>/mcpreturning200. - Removes the
form-actionparsing/logic and the related WARN suggestion path. - Updates documentation and tests to reflect that “page served” is the meaningful instance-wide signal.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| monitoring/mcp-status/README.md | Updates the documented model of the II /mcp connect flow and what the dashboard asserts. |
| monitoring/mcp-status/checks.test.js | Replaces the obsolete CSP-based WARN test with a served-page PASS test. |
| monitoring/mcp-status/checks.js | Removes form-action-based health logic and updates the check text/details and suggestions accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Follow-up to the review: the check now only asserts GET <ii>/mcp returns 200, so "delegation flow enabled" was misleading. Rename the user-facing label to "II /mcp connect page served" (the `ii-mcp-flow` id is unchanged) and align the remaining "delegation flow enabled" phrasings in the linkage description and README (heading + question 2) with the connect-page terminology. The historical reference to the old form-POST delegation flow is left intact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014T9N8USDfNK5yzznPGg7Ym
aterga
pushed a commit
that referenced
this pull request
Jul 5, 2026
Resolve the conflict in src/main.rs: PRs #35/#37/#38 landed on main while this branch was open. The only content conflict was the Candid encode/decode layer — #37 added CWE-674 hardening (guard_candid_text + MAX_CANDID_DEPTH/size caps) to the very functions this branch had moved into the new src/calls.rs module. Resolution: keep this branch's structure (the functions live in calls.rs) and port #37's hardening into calls.rs: - guard_candid_text / MAX_CANDID_TEXT_BYTES / MAX_CANDID_DEPTH / depth_err now live in calls.rs (pub(crate)), used by encode_args + decode_bytes_with_did. - management.rs's install-arg guard now calls crate::calls::guard_candid_text. - the CWE-674 guard test moved to calls::tests alongside the function. discover.rs (SSRF #35 + discovery buffering #37), management.rs, and the monitoring/* changes auto-merged. Build clean, all 53 tests pass, no new clippy warnings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hv4cGJfD97SGxHM2myTAbA
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.
Problem
The
/statusdashboard's "II /mcp delegation flow enabled" check WARNs because it asserts II's/mcpCSPform-actionallowshttps:. That was the precondition for the old delegation flow, where II form-POSTed the delegation to the server. internet-identity#4086 removed that flow, soform-actionno longer allowshttps:(it's now'self' http://127.0.0.1:*, tightened on purpose and locked by a regression test). The127.0.0.1entry is for the unrelated/cliloopback flow, not MCP.Why it's a false positive
The current connect flow never form-POSTs:
fetch()calls (governed byconnect-src, which allows the https origin), andfinish_urlis a top-level navigation (not governed byform-action).Nothing is blocked — the end-to-end connect works today.
Fix
Took the ticket's preferred option: drop the
form-actionassertion and just checkGET <ii>/mcpreturns 200. Per-user trust is on-chain and not globally inspectable, so a served connect page is the only meaningful instance-wide signal.checks.js:ii-mcp-flownowpasson 200 /failotherwise (no morewarn); removed theform-actionparse, thefacts.mcpFormActionfact, and theallowsHttpsPostlogic. Rewrote the description,expected,target, anddetailto describe the fetch +finish_urlnavigation model.checks.js: dropped the now-deadwarnsuggestion inbuildSuggestions.README.md: refreshed both spots that described the retired form-POST model.checks.test.js: replaced the obsolete "warns when form-action forbids https posts" test with "passes on a served /mcp even with loopback-only form-action", which pins the fix.parseCspDirectiveis kept (still used for theframe-ancestorscheck).Tests
Full monitoring suite green: 17/17 (
node --test).🤖 Generated with Claude Code
https://claude.ai/code/session_014T9N8USDfNK5yzznPGg7Ym
Generated by Claude Code