From efb203d9e5fc0eefd6b4761ff6c7e5150cf6d6aa Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 10 Nov 2026 15:58:22 +0000 Subject: [PATCH 1/3] Fail a sign-in whose session vanished mid-call; drop the mint-time TTL cutoff `redeem_registration_delegation` called `ensure_session` before the `mcp_register_v2` call, and `set_grant_expiration`/`set_permissions` after it, so a session reaped (15 minutes after its connect began) or evicted at capacity while the call was in flight was silently re-created with a fresh key that II never registered. A code minted for it led to a session that could not sign anything. #234 avoided that by refusing every code past the 10-minute CONNECT_TTL, at the cost of throwing away registrations that started in time and had succeeded. `imcp2-core` now looks the session up instead of creating it, and records the outcome through `record_registration`, which requires the session to still hold the exact key II registered. Otherwise the redemption fails with `SIGN_IN_DROPPED_MSG` and nothing is re-created. With that, the hosted mint step no longer refuses a connect past its TTL: the TTL bounds when a redemption may start (the gate, and the claim under the lock), and one that started in time gets its code. That matters because II keeps one MCP session per user; a successful registration has already replaced the user's previous one, so refusing the code left them with neither. `reserve_code` still refuses, and discards the granted session, when the connect itself is gone. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VP2vTJm9ipTPD18xW9HyVG --- crates/imcp2-core/src/identities.rs | 227 ++++++++++++++++++++++------ src/auth.rs | 123 ++++++++------- 2 files changed, 248 insertions(+), 102 deletions(-) diff --git a/crates/imcp2-core/src/identities.rs b/crates/imcp2-core/src/identities.rs index 2864451..519a65d 100644 --- a/crates/imcp2-core/src/identities.rs +++ b/crates/imcp2-core/src/identities.rs @@ -127,11 +127,20 @@ pub const MAX_PENDING_CONNECTS: usize = 1_024; const MAX_SESSIONS: usize = 20_000; /// How long a session that never redeemed a grant is kept before the reaper -/// treats the connect as abandoned. Comfortably past `crate::auth`'s 10-minute -/// `CONNECT_TTL`, after which the connect can no longer be redeemed anyway, so -/// this only ever drops sessions that are already dead weight. +/// treats the connect as abandoned. Comfortably past the hosted server's +/// 10-minute `CONNECT_TTL`, after which no new redemption can start, so the +/// redemptions this can cut short are ones that started in time and then ran +/// past the 15 minutes. Those fail with [`SIGN_IN_DROPPED_MSG`] (see +/// [`Identities::redeem_registration_delegation`]) rather than re-create the +/// session with a key II never registered. const PENDING_CONNECT_TTL_NS: u64 = 15 * 60 * 1_000_000_000; +/// What a redemption answers when its connect's session is gone: reaped (see +/// [`PENDING_CONNECT_TTL_NS`]) or evicted at capacity, before or while the +/// `mcp_register_v2` call ran. Shown to the user on the callback page. +pub const SIGN_IN_DROPPED_MSG: &str = "This sign-in was dropped before it could finish: it took \ + too long, or the server was busy. Start the sign-in again."; + /// Per-session ceiling on cached per-app delegations. `store` inserts one per /// distinct `(derivation origin, account)` a session asks for, so a single /// authenticated session could otherwise inflate its own map without limit by @@ -563,12 +572,14 @@ impl Identities { Some(Principal::self_authenticating(&der).to_text()) } - /// Record the grant's expiration from the `mcp_register_v2` reply, at connect - /// redemption. The value is nanoseconds since the epoch. + /// Record a grant expiring at `expiration_ns` (nanoseconds since the epoch) + /// on `session_id`, creating the session if it doesn't exist. Connect + /// redemption does NOT use this: it records through `record_registration`, + /// which refuses rather than re-creates a session reaped mid-call, since a + /// re-created session's fresh key is not the one II registered. pub async fn set_grant_expiration(&self, session_id: &str, expiration_ns: u64) { - // Best effort: the redeem path ensured this session already, so a - // capacity refusal here means it was reaped mid-flight — nothing to - // record, and the client's next call gets the reconnect message. + // Best effort: a capacity refusal leaves nothing to record, and the + // client's next call gets the reconnect message. if let Err(e) = self.ensure_session(session_id).await { tracing::warn!(session_id = %session_id, "could not record the grant expiration: {e}"); return; @@ -582,32 +593,25 @@ impl Identities { // readers/writers of the session map. let opened = { let mut sessions = self.sessions.write().await; - match sessions.get_mut(session_id) { - Some(s) => { - let was_live = s.grant_expiration_ns.is_some_and(|e| e > now); - s.grant_expiration_ns = Some(expiration_ns); - // Redeeming a grant is itself activity, so a freshly-opened - // session counts as active immediately (before its first tool - // call) on the `active_sessions` gauge. - s.last_seen_ns.store(now, Ordering::Relaxed); - !was_live && expiration_ns > now - } - None => false, - } + sessions.get_mut(session_id).is_some_and(|s| record_grant(s, expiration_ns, now)) }; - // "session opened" pairs 1:1 with the reaper's "session closed", so the - // two reconcile in the journal. A re-bind of an already-live grant (a - // redeem retry within the delegation's lifetime) is not a new open. if opened { - tracing::info!( - instance = self.instance.name, - session_id = %session_id, - expiration_ns, - "session opened" - ); + self.log_session_opened(session_id, expiration_ns); } } + /// The "session opened" log. It pairs 1:1 with the reaper's "session + /// closed", so the two reconcile in the journal; logged after the session + /// lock is released, so a slow log sink can't stall the session map. + fn log_session_opened(&self, session_id: &str, expiration_ns: u64) { + tracing::info!( + instance = self.instance.name, + session_id = %session_id, + expiration_ns, + "session opened" + ); + } + /// Record the session's access level from the `mcp_register_v2` reply's /// `permissions` field: `"queries"` = read-only, `"all"` = full access (both /// case-insensitive). Any UNRECOGNIZED value leaves the level `None` (unknown) @@ -615,16 +619,12 @@ impl Identities { /// through to the ingress rejection fallback instead of the server wrongly /// considering the session writable. pub async fn set_permissions(&self, session_id: &str, permissions: &str) { - let level = match permissions.trim().to_ascii_lowercase().as_str() { - "queries" => Some(true), - "all" => Some(false), - other => { - tracing::warn!("mcp_register_v2 reply had unrecognized permissions {other:?}; access level left unknown"); - return; - } + let Some(level) = access_level(permissions) else { + return; }; - // Same best-effort posture as `set_grant_expiration`: recorded on the - // session the redeem path already created, or dropped with a warning. + let level = Some(level); + // Same best-effort posture as `set_grant_expiration` (and, like it, not + // what connect redemption uses). if let Err(e) = self.ensure_session(session_id).await { tracing::warn!(session_id = %session_id, "could not record the access level: {e}"); return; @@ -911,12 +911,12 @@ impl Identities { reg_user_key: Vec, chain: Vec, ) -> Result { - self.ensure_session(session_id).await?; - - // priv(X) to sign the ingress as, and pub(S) to register. + // priv(X) to sign the ingress as, and pub(S) to register. The connect's + // session must still be here: one reaped or evicted since the connect + // began is not re-created, which would only mint a key II never sees. let (reg_seed, reg_der, session_der) = { let sessions = self.sessions.read().await; - let s = sessions.get(session_id).ok_or("no such session")?; + let s = sessions.get(session_id).ok_or(SIGN_IN_DROPPED_MSG)?; let reg_seed = s.reg_key_seed.ok_or("no registration key was minted for this connect")?; let reg_der = s @@ -962,13 +962,48 @@ impl Identities { .map_err(|e| format!("Internet Identity rejected registration: {e}"))?; // Record expiry + access level so the signer's expiry check and the - // read-only guard have what they need. + // read-only guard have what they need — on the session II just + // registered, or not at all (see `record_registration`). let permissions = outcome.permissions.as_text(); - self.set_grant_expiration(session_id, outcome.expiration).await; - self.set_permissions(session_id, permissions).await; + self.record_registration(session_id, &session_der, outcome.expiration, permissions).await?; Ok(RegistrationOutcome { expiration_ns: outcome.expiration, permissions }) } + /// Record a successful `mcp_register_v2` — the grant's expiration and access + /// level — on the session it registered, in one step under the lock. + /// + /// Refused, with nothing recorded, when that session is gone or no longer + /// holds the key `session_der` that II just registered: it was reaped (15 + /// minutes after its connect began, see [`PENDING_CONNECT_TTL_NS`]) or + /// evicted at capacity while the call was in flight. Re-creating it, as + /// [`Self::set_grant_expiration`] would, mints a fresh key II never + /// registered, so the client would get a session that cannot sign anything. + async fn record_registration( + &self, + session_id: &str, + session_der: &[u8], + expiration_ns: u64, + permissions: &str, + ) -> Result<(), String> { + let level = access_level(permissions); + let now = now_ns(); + let opened = { + let mut sessions = self.sessions.write().await; + let s = sessions + .get_mut(session_id) + .filter(|s| s.pubkey_der == session_der) + .ok_or(SIGN_IN_DROPPED_MSG)?; + if level.is_some() { + s.read_only = level; + } + record_grant(s, expiration_ns, now) + }; + if opened { + self.log_session_opened(session_id, expiration_ns); + } + Ok(()) + } + /// Resolve an optional account `name` at `domain` to its account number /// (`None` = the default account, used when `name` is `None`). Looks the name /// up via [`Self::list_accounts`]; errors if no account (or more than one) at @@ -1212,6 +1247,36 @@ impl Identities { } } +/// Record a grant expiring at `expiration_ns` on `s`, returning whether this +/// opened the session: a genuine not-live -> live transition, so a re-bind of an +/// already-live grant (a redeem retry within the delegation's lifetime) is not a +/// new open. Redeeming a grant is itself activity, so a freshly-opened session +/// counts as active at once (before its first tool call) on the +/// `active_sessions` gauge. +fn record_grant(s: &mut Session, expiration_ns: u64, now: u64) -> bool { + let was_live = s.grant_expiration_ns.is_some_and(|e| e > now); + s.grant_expiration_ns = Some(expiration_ns); + s.last_seen_ns.store(now, Ordering::Relaxed); + !was_live && expiration_ns > now +} + +/// The access level an `mcp_register_v2` reply's `permissions` names: +/// `Some(true)` = read-only (`"queries"`), `Some(false)` = full access (`"all"`), +/// both case-insensitive. Any UNRECOGNIZED value is `None`, with a warning, so +/// the level stays unknown rather than assumed to be full access. +fn access_level(permissions: &str) -> Option { + match permissions.trim().to_ascii_lowercase().as_str() { + "queries" => Some(true), + "all" => Some(false), + other => { + tracing::warn!( + "mcp_register_v2 reply had unrecognized permissions {other:?}; access level left unknown" + ); + None + } + } +} + /// Drop every session that no longer serves anyone, returning the ids of each /// kind so the caller can log them: sessions whose grant has EXPIRED (paired /// with the "session opened" log by a "session closed" one), and ABANDONED @@ -1682,6 +1747,74 @@ mod tests { assert!(ids.require_write("sess2").await.is_ok()); } + // The session key a pending connect's session holds, as `authorize` minted it. + async fn session_der(ids: &Identities, session_id: &str) -> Vec { + ids.sessions.read().await[session_id].pubkey_der.clone() + } + + // A successful registration is recorded on the session it registered: the + // grant's expiration and the access level, in one step. + #[tokio::test] + async fn record_registration_records_on_the_session_it_registered() { + let ids = test_ids(); + ids.ensure_session("sess").await.expect("room for a session"); + let der = session_der(&ids, "sess").await; + let future = now_ns() + 3_600_000_000_000; + + ids.record_registration("sess", &der, future, "queries").await.expect("recorded"); + + assert_eq!(ids.grant_expiration_ns("sess").await, Some(future)); + assert_eq!(ids.is_read_only("sess").await, Some(true)); + } + + // A registration whose session was reaped or evicted while `mcp_register_v2` + // ran is refused, and the session is NOT re-created: a re-created session's + // fresh key is not the one II registered, so it could never sign anything. + #[tokio::test] + async fn record_registration_refuses_a_session_gone_mid_call() { + let ids = test_ids(); + ids.ensure_session("sess").await.expect("room for a session"); + let der = session_der(&ids, "sess").await; + ids.discard_session("sess").await; // reaped while the call ran + + let future = now_ns() + 3_600_000_000_000; + let refused = ids.record_registration("sess", &der, future, "all").await; + + assert_eq!(refused, Err(SIGN_IN_DROPPED_MSG.to_owned())); + assert!(ids.sessions.read().await.is_empty(), "the session is not re-created"); + } + + // Nor is a registration recorded on a session that has since been re-created + // under the same id with another key: the key II registered is gone with the + // old one. + #[tokio::test] + async fn record_registration_refuses_a_session_re_created_with_another_key() { + let ids = test_ids(); + ids.ensure_session("sess").await.expect("room for a session"); + let registered = session_der(&ids, "sess").await; + ids.discard_session("sess").await; + ids.ensure_session("sess").await.expect("room for a session"); + assert_ne!(session_der(&ids, "sess").await, registered); + + let future = now_ns() + 3_600_000_000_000; + let refused = ids.record_registration("sess", ®istered, future, "all").await; + + assert_eq!(refused, Err(SIGN_IN_DROPPED_MSG.to_owned())); + assert_eq!(ids.grant_expiration_ns("sess").await, None, "no grant on the new key"); + } + + // A redemption whose connect's session is already gone is refused up front, + // before any network call, and does not re-create the session. + #[tokio::test] + async fn redeem_refuses_a_connect_whose_session_is_gone() { + let ids = test_ids(); + + let refused = ids.redeem_registration_delegation("gone", vec![], vec![]).await; + + assert_eq!(refused.map(|_| ()), Err(SIGN_IN_DROPPED_MSG.to_owned())); + assert!(ids.sessions.read().await.is_empty(), "the session is not re-created"); + } + // `discard_session` drops a session whatever it holds, a live grant included, // which the reaper and the admission bound would otherwise keep until the // grant expires; an unknown id is a no-op. @@ -2736,7 +2869,9 @@ mod tests { #[tokio::test] async fn redeem_guards_missing_key_and_empty_chain() { let ids = test_ids(); - // No X minted yet: even a plausible user_key can't be redeemed. + // A session with no X minted yet: even a plausible user_key can't be + // redeemed. (No session at all is `redeem_refuses_a_connect_whose_session_is_gone`.) + ids.ensure_session("sess").await.expect("room for a session"); let err = ids .redeem_registration_delegation("sess", vec![1], vec![]) .await diff --git a/src/auth.rs b/src/auth.rs index 3fe1616..d80838a 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -2621,42 +2621,37 @@ const CONNECT_EXPIRED_MSG: &str = "This connect request has expired. Restart fro /// stay claimed until its own claim lets go, or a second registration could /// start before this one's guard clears the mark out from under it. /// -/// Refused for a connect past [`CONNECT_TTL`], even one whose redemption passed -/// the gate in time. `imcp2-core` reaps an unredeemed connect's session 15 -/// minutes in, on the promise that no connect is redeemed past its TTL, and -/// recording the grant on a reaped session re-creates it with a fresh key that -/// II never registered: a code minted then would hand the client a session -/// whose every signed call fails. Until [`EXPIRED_CONNECT_RETENTION`], the sweep -/// had dropped the entry long before that, so this check is what keeps the -/// promise now that expired connects are retained. +/// Not refused for being past [`CONNECT_TTL`]: the TTL bounds when a redemption +/// may START (the gate, and [`claim_redemption`] under the lock), and one that +/// started in time has done its work by now. It consented in time, and II has +/// just registered its session key, replacing the user's previous MCP session. +/// Refusing the code here would leave the user with neither. Nor can it bind +/// the code to a dead session: `imcp2-core` records the registration only on +/// the session holding the key II registered, and fails the redemption, before +/// it gets here, when that session was reaped or evicted mid-call. /// -/// A refusal also discards the session the registration just granted -/// ([`Identities::discard_session`]): no code will ever reach it, and with a -/// grant recorded nothing else would drop it before the grant expired. +/// Refused only when the connect itself is gone (evicted at the cap, or kept +/// past [`EXPIRED_CONNECT_RETENTION`]). That refusal also discards the session +/// the registration just granted ([`Identities::discard_session`]): no code will +/// ever reach it, and with a grant recorded nothing else would drop it before +/// the grant expired. async fn reserve_code( store: &AuthStore, state: &str, fresh: String, ) -> Result<(String, bool), &'static str> { - let refused = { - let mut authz = store.authz.write().await; - match authz.get_mut(state) { - None => "This connect request is no longer available. Restart from your client.", - Some(a) if a.remaining().is_zero() => CONNECT_EXPIRED_MSG, - Some(a) => { - return Ok(match &a.code { - Some(existing) => (existing.clone(), false), - None => { - a.code = Some(fresh.clone()); - (fresh, true) - } - }) + if let Some(a) = store.authz.write().await.get_mut(state) { + return Ok(match &a.code { + Some(existing) => (existing.clone(), false), + None => { + a.code = Some(fresh.clone()); + (fresh, true) } - } - }; + }); + } // Outside the `authz` lock: the session map has its own. store.identities.discard_session(state).await; - Err(refused) + Err("This connect request is no longer available. Restart from your client.") } /// A JSON error the pinned page reads and displays. @@ -2698,9 +2693,9 @@ async fn claim_redemption(store: &AuthStore, state: &str) -> RedeemClaim { if a.redeeming { return RedeemClaim::InProgress; } - // Rechecked under the lock: the gate's snapshot can predate the TTL, and a - // registration started on an expired connect could only be refused at the - // mint, after II had already granted it. + // Rechecked under the lock: the gate's snapshot can predate the TTL, and the + // TTL is what bounds when a redemption may start. One that starts here runs + // to completion, code included (see `reserve_code`). if a.remaining().is_zero() { return RedeemClaim::Expired; } @@ -5040,53 +5035,69 @@ mod tests { assert!(error.contains("unknown or already used"), "{error}"); } - // A redemption that passed the gate in time but finished after the TTL gets - // no code. The connect's session is reaped 15 minutes in, and recording the - // grant re-creates it with a key II never registered, so a code minted then - // would be bound to a session that cannot sign; retaining expired connects - // must not open that up. Within the TTL the code is minted, idempotently. - // - // The refusal also discards the session the registration granted (else it - // would hold its slot until the grant expired), and leaves the connect - // claimed until the attempt's own claim lets go, so no second registration - // can start in between. + // A redemption that started in time gets its code even if it finishes past + // the TTL: the TTL bounds when a redemption may start, and by now II has + // registered the session (replacing the user's previous one), so refusing + // would leave them with neither. A session reaped mid-call never gets this + // far: `imcp2-core` fails the redemption instead of re-creating it. Within + // the TTL the code is minted, idempotently. #[tokio::test(start_paused = true)] - async fn no_code_is_minted_for_a_connect_past_its_ttl() { + async fn a_redemption_started_in_time_gets_its_code_even_past_the_ttl() { let store = test_store(); seed_pending(&store, "sess-in-time", "bind").await; - seed_pending(&store, "sess-late", "bind").await; + seed_pending(&store, "sess-slow", "bind").await; let minted = super::reserve_code(&store, "sess-in-time", "code-1".into()).await; assert_eq!(minted, Ok(("code-1".to_owned(), true))); let again = super::reserve_code(&store, "sess-in-time", "code-2".into()).await; assert_eq!(again, Ok(("code-1".to_owned(), false)), "the first code, idempotently"); - // A late attempt: claimed in time, its registration granted, done after the TTL. - let claim = super::claim_redemption(&store, "sess-late").await; + // A slow attempt: claimed in time, its registration granted, done past the TTL. + let claim = super::claim_redemption(&store, "sess-slow").await; assert!(matches!(claim, super::RedeemClaim::Claimed(_))); - store.identities.set_grant_expiration("sess-late", u64::MAX).await; + store.identities.set_grant_expiration("sess-slow", u64::MAX).await; tokio::time::advance(super::CONNECT_TTL).await; - let late = super::reserve_code(&store, "sess-late", "code-3".into()).await; - assert_eq!(late, Err(super::CONNECT_EXPIRED_MSG)); + let slow = super::reserve_code(&store, "sess-slow", "code-3".into()).await; + assert_eq!(slow, Ok(("code-3".to_owned(), true))); assert_eq!( - store.identities.grant_expiration_ns("sess-late").await, + store.identities.grant_expiration_ns("sess-slow").await, + Some(u64::MAX), + "its session keeps the grant the code will reach" + ); + drop(claim); + } + + // A redemption whose connect vanished while it ran (evicted at the cap, or + // forgotten past its retention) gets no code, and the session its + // registration granted is discarded: no code will reach it, and with a grant + // recorded nothing else would drop it before the grant expired. + #[tokio::test] + async fn a_redemption_whose_connect_is_gone_gets_no_code() { + let store = test_store(); + seed_pending(&store, "sess-gone", "bind").await; + let claim = super::claim_redemption(&store, "sess-gone").await; + assert!(matches!(claim, super::RedeemClaim::Claimed(_))); + store.identities.set_grant_expiration("sess-gone", u64::MAX).await; + store.authz.write().await.remove("sess-gone"); + + let refused = super::reserve_code(&store, "sess-gone", "code-1".into()).await; + + assert_eq!( + refused, + Err("This connect request is no longer available. Restart from your client.") + ); + assert_eq!( + store.identities.grant_expiration_ns("sess-gone").await, None, "the granted session no code will reach is discarded" ); - { - let authz = store.authz.read().await; - assert!(authz["sess-late"].code.is_none()); - assert!(authz["sess-late"].redeeming, "still claimed by the late attempt"); - } drop(claim); - assert!(!store.authz.read().await["sess-late"].redeeming, "released by its own claim"); } // No attempt starts on a connect past its TTL, even for a request that passed // the gate just before it: the claim rechecks the TTL under the lock, so such - // a request answers "expired" instead of starting a registration that could - // only be refused at the mint. + // a request answers "expired" instead of starting a registration past it. #[tokio::test(start_paused = true)] async fn no_attempt_is_claimed_on_a_connect_past_its_ttl() { let store = test_store(); From f8bcdd3759f2eed62b1d28cf6c2a202628217bb2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 10 Nov 2026 16:10:32 +0000 Subject: [PATCH 2/3] Stop pinning the known-app discover test to oisy.com's live content `resolve_app_identity_skips_probe_for_known_apps` resolved oisy.com and expected `DerivationSource::Known`. oisy.com now publishes its own /.well-known/ii-derivation-origin (`https://oisy.com`), and the app's own declaration deliberately wins over the registry, so it resolves as `Declared` and the test failed on main and on every PR. The test also pinned the over-cap Layer 5 regression on oisy.com serving its SPA shell there, which it no longer does. - The live test now checks three registry apps that still declare nothing (nns.internetcomputer.org, multidex.ai, app.icpswap.com). Each must resolve to its registry derivation origin, unprobed, either via the registry (`Known`) or via its own agreeing declaration (`Declared`), and at least one must still go through the registry. One app adopting the declaration no longer breaks the test. - The over-cap regression gets an offline test, `an_over_cap_layer5_body_reads_as_no_declaration`: a loopback listener serves an oversized page, and `fetch_well_known` must read it as `Absent` under `Overflow::NotTheDocument` and `Unreachable` under `Overflow::Unknown`. - `Overflow::NotTheDocument`'s doc no longer says OISY serves that page. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VP2vTJm9ipTPD18xW9HyVG --- crates/imcp2-core/src/discover.rs | 110 ++++++++++++++++++++++++++---- 1 file changed, 97 insertions(+), 13 deletions(-) diff --git a/crates/imcp2-core/src/discover.rs b/crates/imcp2-core/src/discover.rs index 8aa4fc6..224ea46 100644 --- a/crates/imcp2-core/src/discover.rs +++ b/crates/imcp2-core/src/discover.rs @@ -1586,8 +1586,9 @@ enum StrictRead { enum Overflow { /// The document's form bounds its size, so an over-cap body is NOT it: the /// app declares nothing here. An SPA catch-all's index.html at the Layer 5 - /// path is the live case — OISY serves exactly that, and treating it as a - /// failed check would refuse to resolve a perfectly healthy app. + /// path is the common live case, and treating it as a failed check would + /// refuse to resolve a perfectly healthy app + /// (`an_over_cap_layer5_body_reads_as_no_declaration` pins it). NotTheDocument, /// The document has no size bound of its own, so an over-cap body is a read /// we could not finish, not a verdict. @@ -3881,19 +3882,102 @@ mod tests { assert_eq!(r.application_is_ic, Some(false), "example.com must show no IC evidence"); } - // Live network: a KNOWN app skips the IC probe entirely (the registry answers). + // Live network: an app in the known-app registry resolves to the registry's + // derivation origin, and the IC probe never runs for it. When the app + // declares nothing, the registry fills the gap (`Known`). When it publishes + // its own declaration, that wins (`Declared`) and must name the same origin. + // + // Apps adopt the declaration over time (oisy.com and nns.ic0.app already + // have), so this checks several and needs only ONE still answered by the + // registry to keep the `Known` path covered. While they answer the Layer 5 + // path with their SPA shell (over the cap), this also exercises + // `Overflow::NotTheDocument` live; `an_over_cap_layer5_body_reads_as_no_declaration` + // pins that offline. #[tokio::test] async fn resolve_app_identity_skips_probe_for_known_apps() { - // Also the regression pin for over-cap Layer 5 bodies: oisy.com answers - // /.well-known/ii-derivation-origin with its SPA shell, which is larger - // than MAX_DERIVATION_ORIGIN_BYTES. That must read as "declares nothing" - // (Overflow::NotTheDocument — a one-line origin cannot be that big, so - // what was served is not the Layer 5 document), NOT as a failed check. - // Treating it as a failure refused to resolve a perfectly healthy app, - // which is how this test caught it. - let r = resolve_app_identity("oisy.com", false).await.expect("resolve"); - assert_eq!(r.derivation_origin_source, DerivationSource::Known); - assert_eq!(r.application_is_ic, None, "known apps are not probed"); + const APPS: &[&str] = &["nns.internetcomputer.org", "multidex.ai", "app.icpswap.com"]; + let mut via_registry = 0; + for app in APPS { + let r = resolve_app_identity(app, false).await.expect("resolve"); + assert_eq!( + Some(r.derivation_origin.as_str()), + known_derivation_origin(app), + "{app}: resolves to its registry derivation origin" + ); + assert_eq!(r.application_is_ic, None, "{app}: known apps are not probed"); + match r.derivation_origin_source { + DerivationSource::Known => via_registry += 1, + DerivationSource::Declared => {} // its own declaration, agreeing + other => panic!("{app}: resolved as {other:?}"), + } + } + assert!( + via_registry > 0, + "every app in {APPS:?} now publishes its own derivation origin: add a registry app \ + that still declares nothing, so the registry path stays covered" + ); + } + + // Serve ONE HTTP response carrying `body` on a loopback port, returning the + // origin to fetch it from: an offline stand-in for an app's web server. + fn serve_once(body: String) -> String { + use std::io::{Read, Write}; + let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind a loopback port"); + let origin = format!("http://{}", listener.local_addr().expect("local address")); + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept the probe"); + let (mut request, mut buf) = (Vec::new(), [0u8; 1024]); + while !request.windows(4).any(|w| w == b"\r\n\r\n") { + match stream.read(&mut buf) { + Ok(0) | Err(_) => break, + Ok(n) => request.extend_from_slice(&buf[..n]), + } + } + let head = format!( + "HTTP/1.1 200 OK\r\ncontent-type: text/html\r\ncontent-length: {}\r\n\ + connection: close\r\n\r\n", + body.len() + ); + // The reader may hang up once it has seen enough; that's expected. + let _ = + stream.write_all(head.as_bytes()).and_then(|()| stream.write_all(body.as_bytes())); + }); + origin + } + + // An over-cap body at the Layer 5 path is an SPA catch-all's page, not the + // one-line declaration, so it reads as "declares nothing" (`Absent`), NOT as + // a failed check: that refused to resolve a perfectly healthy app once, which + // a live test caught. A document with no size bound of its own reads the same + // body as a read we could not finish (`Unreachable`). Offline, so this holds + // whatever any live site happens to serve. + #[tokio::test] + async fn an_over_cap_layer5_body_reads_as_no_declaration() { + let page = format!("{}", " ".repeat(MAX_DERIVATION_ORIGIN_BYTES)); + let client = reqwest::Client::builder().no_proxy().build().expect("client"); + + let (layer5, _) = fetch_well_known( + &client, + &serve_once(page.clone()), + DERIVATION_ORIGIN_PATH, + MAX_DERIVATION_ORIGIN_BYTES, + Overflow::NotTheDocument, + ) + .await; + assert!(matches!(layer5, WellKnown::Absent), "an over-cap Layer 5 body declares nothing"); + + let (unbounded, _) = fetch_well_known( + &client, + &serve_once(page), + LEGACY_MANIFEST_PATH, + MAX_DERIVATION_ORIGIN_BYTES, + Overflow::Unknown, + ) + .await; + assert!( + matches!(&unbounded, WellKnown::Unreachable(e) if e.contains("larger than")), + "an over-cap body of an unbounded document is a read we could not finish" + ); } // Consistency (networked): every origin in a known app's LIVE From 18e07b4e8f2c0f15abb947171d15ba91016a9c78 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 10 Nov 2026 21:10:11 +0000 Subject: [PATCH 3/3] Log a sign-in dropped after II registered it When `record_registration` refuses because the connect's session is gone, or was re-created with another key, while `mcp_register_v2` ran, II has already registered the key and replaced the user's previous MCP session. The user only saw the callback page's message; nothing reached the logs. Warn with the instance, session id, and which of the two happened, after the session lock is released. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VP2vTJm9ipTPD18xW9HyVG --- crates/imcp2-core/src/identities.rs | 41 +++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/crates/imcp2-core/src/identities.rs b/crates/imcp2-core/src/identities.rs index 519a65d..cb71f44 100644 --- a/crates/imcp2-core/src/identities.rs +++ b/crates/imcp2-core/src/identities.rs @@ -987,21 +987,40 @@ impl Identities { ) -> Result<(), String> { let level = access_level(permissions); let now = now_ns(); - let opened = { + let recorded = { let mut sessions = self.sessions.write().await; - let s = sessions - .get_mut(session_id) - .filter(|s| s.pubkey_der == session_der) - .ok_or(SIGN_IN_DROPPED_MSG)?; - if level.is_some() { - s.read_only = level; + match sessions.get_mut(session_id) { + Some(s) if s.pubkey_der == session_der => { + if level.is_some() { + s.read_only = level; + } + Ok(record_grant(s, expiration_ns, now)) + } + Some(_) => Err("re-created with another key"), + None => Err("gone"), } - record_grant(s, expiration_ns, now) }; - if opened { - self.log_session_opened(session_id, expiration_ns); + match recorded { + Ok(opened) => { + if opened { + self.log_session_opened(session_id, expiration_ns); + } + Ok(()) + } + // Worth an operator's attention: II has already registered this key, + // replacing the user's previous MCP session, so the user is left with + // neither until they sign in again. Logged after the lock is released. + Err(session) => { + tracing::warn!( + instance = self.instance.name, + session_id = %session_id, + session, + "sign-in dropped after Internet Identity registered it: its session was \ + reaped or evicted while mcp_register_v2 ran" + ); + Err(SIGN_IN_DROPPED_MSG.to_owned()) + } } - Ok(()) } /// Resolve an optional account `name` at `domain` to its account number