From 831c290384b6aad683b3a850278845b516c530e0 Mon Sep 17 00:00:00 2001 From: Mike Date: Fri, 7 Aug 2026 14:48:16 +0300 Subject: [PATCH 1/3] fix(SHARK-3619/3620/3622): stop the key-lifecycle writes describing a world the data plane does not produce Three defects found by walking create -> call -> restrict -> freeze against prod on 2026-08-07, all the same shape: a reply or a description asserts something the proxy contradicts. SHARK-3619. The create reply said the ready URL "works immediately". A newly minted key answers -32050 "API key not found" under HTTP 401 for roughly 60 to 90 seconds first, and every plausible reading of that error is wrong: create it again, escalate, or report MCP key creation as broken. The claim about TIME is gone; the claim about SETUP (no session, no header) survives, because that is what makes the URL the shortest path to a first call. The create path adds a measured propagation note naming the code, the words the proxy uses, and the verdict. Reveal does not carry it: its key already exists. SHARK-3620. The tool description said "The secret key material is never returned in the tool output" while the reply carries the endpoint token in full plus a URL with it embedded. The approval page was already accurate, so the description now IS that sentence rather than a second wording of it, shared as one constant and pinned by identity rather than by two regexes that can drift apart. Found while wiring that pairing: the approval page CLIPS every effect at 200 characters, and three lines were over it -- create's credential disclosure (cut at "which are live credentials", losing where they land), the platform-key mint's "without asking a human to approve anything and without a second factor", and the bulk logout's blast radius, whose truncation depended on how many sessions the account had. All three now sit under the bound, and no gated page may ship a truncated consequence again. SHARK-3622. freeze neither read the resulting state back nor mentioned the lag, while the allowlist write on the same key one minute earlier did both. It now reads the status back from the control plane -- authoritative immediately, unlike the data-plane propagation the allowlist writes rightly refuse to race -- and states the measured lag per direction: 10 to 21 seconds for freeze, unmeasured for unfreeze rather than borrowed from it. A read-back that disagrees with the request is reported without isError, because a separate route can trail a write and "retry me" here costs a second human approval. Gates: format, lint, typecheck, build clean; 1639/1639 tests; coverage on the changed files 98.3% lines / 90.5% branches / 100% functions. Co-Authored-By: Claude Opus 5 (1M context) --- src/mgmt/tools/createApiKey.ts | 33 +- src/mgmt/tools/endpointToken.ts | 100 +++- src/mgmt/tools/freezeApiKey.ts | 176 +++++- src/mgmt/tools/platformApiKeys.ts | 10 +- src/mgmt/tools/sessions.ts | 22 +- test/mgmt-gated-display.test.ts | 15 + test/mgmt-key-lifecycle-truthfulness.test.ts | 546 +++++++++++++++++++ test/mgmt-sessions.test.ts | 33 +- test/mgmt-tools.test.ts | 25 +- 9 files changed, 914 insertions(+), 46 deletions(-) create mode 100644 test/mgmt-key-lifecycle-truthfulness.test.ts diff --git a/src/mgmt/tools/createApiKey.ts b/src/mgmt/tools/createApiKey.ts index 74bc3c9..f59d47c 100644 --- a/src/mgmt/tools/createApiKey.ts +++ b/src/mgmt/tools/createApiKey.ts @@ -36,7 +36,11 @@ import { MGMT_ADDITIVE } from "./annotations.js"; // point when the account has one) is rendered in ONE place, shared with // mgmt_reveal_api_key, so a field cannot be surfaced on one path and dropped on // the other. That is precisely how enterpriseApiKeys went missing. -import { describeEndpointToken } from "./endpointToken.js"; +import { + describeEndpointToken, + ENDPOINT_TOKEN_DISCLOSURE, + ENDPOINT_TOKEN_DISCLOSURE_LINES, +} from "./endpointToken.js"; /** * SHARK-3513 — the human-facing description of a key creation. @@ -82,8 +86,14 @@ export function registerCreateApiKey({ "Create or get a dedicated per-project API key (JWT) for this " + "account, optionally restricted to a set of blockchains. " + "STATE-CHANGING. Idempotent by index: an existing index returns the " + - "existing key. The secret key material is never returned in the tool " + - "output." + + "existing key. " + + // SHARK-3620: this slot used to read "The secret key material is never + // returned in the tool output", which the reply falsified on every + // successful call — it carries the endpoint token, and a ready URL with + // the token in it. The approval page has always been accurate, so the + // description now IS the approval page's sentence rather than a second + // wording of it. + ENDPOINT_TOKEN_DISCLOSURE + TOTP_DESCRIPTION_SUFFIX + HITL_DESCRIPTION_SUFFIX, inputSchema: z @@ -167,10 +177,14 @@ export function registerCreateApiKey({ // credential-bearing reply has to be told that is what they are // approving, so the two are named separately: the signed material // stays hidden, the usable credential does not. - "The key's signed material (jwt_data) is never shown to the " + - "assistant. The reply DOES carry the key's endpoint token, and " + - "the account's enterprise API keys where it has any, which are " + - "live credentials that land in the conversation transcript.", + // + // SHARK-3620: and the tool DESCRIPTION now renders this same + // constant, because it used to assert the opposite. The wording is + // unchanged here; what changed is that there is only one of it, and + // that it arrives as TWO effect lines — as one 201-character line it + // was clipped by the page's 200-char per-effect bound, exactly at + // the clause naming the transcript. + ...ENDPOINT_TOKEN_DISCLOSURE_LINES, ], account: await accountAddressForDisplay(gateway), }), @@ -262,6 +276,11 @@ export function registerCreateApiKey({ const resolved = await describeEndpointToken({ key: created, worker: deps.worker, + // SHARK-3619: this is the create path, so the key may be seconds old + // and invisible to the RPC proxy for about a minute. Reveal renders + // the same surface without this caveat, because its key already + // exists. + justCreated: true, }); return { content: [ diff --git a/src/mgmt/tools/endpointToken.ts b/src/mgmt/tools/endpointToken.ts index 08ab363..090fb05 100644 --- a/src/mgmt/tools/endpointToken.ts +++ b/src/mgmt/tools/endpointToken.ts @@ -119,8 +119,16 @@ function enterpriseSurface(resolved: WorkerTokenResult): string { * create and reveal must not be able to disagree about it. */ const DATA_CALL_HANDOFF = - "\n\nTO MAKE DATA CALLS WITH IT. The URL above works immediately from any " + - "HTTP client, and that is the shortest path to a first call. The Ankr data " + + "\n\nTO MAKE DATA CALLS WITH IT. The URL above needs no session setup at " + + // SHARK-3619: this used to read "works immediately", which was a claim about + // TIME and was false for a key that had just been minted — the proxy answers + // -32050 for about a minute afterwards. What the sentence is actually for is + // the claim about SETUP: no session, no header, no client. That half is true + // on both paths and is what makes this the shortest route to a first call, so + // it is what survives. The timing caveat belongs to the create path alone and + // lives in NEW_KEY_PROPAGATION_NOTE. + "all: any HTTP client can call it, and that is the shortest path to a " + + "first call. The Ankr data " + "MCP server is different: it binds ONE API key per session, at connect time, " + "so a session that is already open keeps the key it was opened with and " + "cannot be repointed at this one. To reach this key from the data tools, set " + @@ -128,6 +136,85 @@ const DATA_CALL_HANDOFF = "NEW session, which in most clients means reconnecting that server. This key " + "stays valid meanwhile, so nothing has to be created again."; +/** + * SHARK-3620 — ONE sentence about what a key-bearing reply discloses, shared by + * the tool DESCRIPTION and the human approval PAGE. + * + * THE DEFECT IT CLOSES. mgmt_create_api_key's description said "The secret key + * material is never returned in the tool output" while the reply carried the + * endpoint token in full, plus a ready-to-call URL with the token embedded. The + * approval page for the same call was already accurate, and that is the point: + * the two disagreed about whether a live credential lands in the model + * transcript, which is precisely the property a reader checks before deciding + * whether a tool is safe to call in a shared or logged session. + * + * WHY A CONSTANT RATHER THAN TWO CAREFUL WORDINGS (see the LINES form below for + * why there are two of them). Two wordings that agree today + * are two wordings that can drift, and the drift is invisible because each side + * reads fine on its own. Sharing the sentence makes agreement structural: there + * is nothing to keep in sync. The pairing is asserted by identity in + * test/mgmt-key-lifecycle-truthfulness.ts, not by a pair of regexes. + * + * The DISTINCTION is the load-bearing part. There are two secrets here and only + * one of them is withheld, so a summary like "this returns credentials" would be + * true and useless. jwt_data is the input to the exchange and never leaves the + * server; the endpoint token is the result, and it is live. + */ +/** + * TWO LINES, AND THE SPLIT IS LOad-BEARING — found while wiring the pairing + * test. The approval page stores each effect through `clip(e, 200)`, and this + * sentence was 201 characters, so the page a human reads to decide whether a + * credential lands in their transcript was cut at "…which are live credentials…" + * and never reached the words that say where they land. It is the only truncated + * effect on the whole gated surface (test/mgmt-gated-display.test.ts now pins + * that for every call site), and it was the one that mattered most. + * + * Splitting rather than shortening keeps the distinction intact: line one is the + * secret that stays hidden, line two is the credential that does not. + */ +export const ENDPOINT_TOKEN_DISCLOSURE_LINES = [ + "The key's signed material (jwt_data) is never shown to the assistant.", + "The reply DOES carry the key's endpoint token, and the account's " + + "enterprise API keys where it has any, which are live credentials that " + + "land in the conversation transcript.", +]; + +/** The same disclosure as one sentence, for the tool description. */ +export const ENDPOINT_TOKEN_DISCLOSURE = + ENDPOINT_TOKEN_DISCLOSURE_LINES.join(" "); + +/** + * SHARK-3619 — the create path's timing caveat, in the reply that creates the + * expectation. + * + * MEASURED, NOT ESTIMATED. On 2026-08-07 a key created through this server + * answered -32050 on rpc.ankr.com and became callable between 60 and 90 seconds + * later; the transition itself fell inside a 10-second poll window. The number + * carries its date so that a future edit has to change both, the same discipline + * the allowlist writes' 45-100 second window follows. + * + * WHY THE VERDICT IS SPELLED OUT rather than left to the reader. The failure + * this ticket recorded was not that the delay was unmentioned, it was that every + * plausible reading of "API key not found" is wrong: create the key again (which + * costs another human approval and mints nothing), escalate, or report MCP key + * creation as broken. So the note names the code, the exact words the proxy + * uses, and what to do about them. + * + * WHY IT IS NOT IN THE SHARED HANDOFF STRING. mgmt_reveal_api_key renders the + * same endpoint surface for a key that already exists and is therefore already + * known to the proxy. Warning about a wait there would be false in the other + * direction. + */ +export const NEW_KEY_PROPAGATION_NOTE = + "\n\nIF THIS KEY WAS JUST MINTED, THE PROXY NEEDS A MOMENT. The control " + + "plane creates a key at once; the RPC proxy learns it afterwards, measured " + + "at roughly 60 to 90 seconds on 2026-08-07. Until then rpc.ankr.com answers " + + "HTTP 401 with `API key not found` (json-rpc code -32050) for this token. " + + "That is the key not being visible YET, not a failed creation: do not create " + + "it again, and do not report it as broken. This call is idempotent by slot, " + + "so a key that already existed is already known to the proxy and is callable " + + "now."; + /** * Turn a key into something the caller can call, or say plainly why not. * @@ -146,9 +233,17 @@ const DATA_CALL_HANDOFF = export async function describeEndpointToken({ key, worker, + justCreated = false, }: { key: { jwt_data?: string; is_encrypted: boolean; config?: string }; worker?: WorkerClient; + /** + * SHARK-3619 — true only on the create path, where the key may be seconds old + * and the proxy may not know it yet. The caveat rides on the ONE branch that + * hands over a usable URL: the encrypted / no-material / exchange-failed + * branches promise no immediacy to correct. + */ + justCreated?: boolean; }): Promise<{ ok: boolean; text: string }> { if (key.is_encrypted) { return { @@ -181,6 +276,7 @@ export async function describeEndpointToken({ "on the chains the key is scoped to. The same value is what the " + "allowlist, freeze and status tools take as `token`." + DATA_CALL_HANDOFF + + (justCreated ? NEW_KEY_PROPAGATION_NOTE : "") + enterpriseSurface(resolved), }; } catch (e) { diff --git a/src/mgmt/tools/freezeApiKey.ts b/src/mgmt/tools/freezeApiKey.ts index 7c8f97f..1fd525a 100644 --- a/src/mgmt/tools/freezeApiKey.ts +++ b/src/mgmt/tools/freezeApiKey.ts @@ -23,7 +23,11 @@ // x-ankr-totp-token). The totp is never logged or echoed. import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { z } from "zod"; -import { type GatewayClient, GatewayError } from "../gateway/client.js"; +import { + type CounterStatus, + type GatewayClient, + GatewayError, +} from "../gateway/client.js"; import { totpSchema, TOTP_DESCRIPTION_SUFFIX, @@ -40,9 +44,88 @@ import { resolveKeyTarget, } from "./keyAddressing.js"; import { accountAddressForDisplay } from "./whoami.js"; -import { unobservedMeta } from "./writeOutcome.js"; +import { observedMeta, unobservedMeta } from "./writeOutcome.js"; import { MGMT_DESTRUCTIVE } from "./annotations.js"; +// --------------------------------------------------------------------------- +// SHARK-3622 — the two things the allowlist write on the same key already did, +// and this one did not: read the result back, and say the data plane lags. +// --------------------------------------------------------------------------- + +/** + * The measured lag, per DIRECTION, and never averaged into one number. + * + * On 2026-08-07 a freeze took 10 to 21 seconds to stop traffic on rpc.ankr.com + * after the control plane accepted it — faster than the blockchain allowlist's + * 45-100 second window, which is worth knowing before anyone writes a wait into + * a runbook. The UNFREEZE direction was not measured on that run, and printing + * the freeze number for it would be exactly the invented precision this ticket + * is about, so it says what it knows and no more. + * + * WHY THIS MATTERS MORE ON FREEZE THAN ANYWHERE ELSE. Freeze is the tool someone + * reaches for during an incident, when "is it actually stopped yet" is the only + * question. A reply that reads as a completed stop, while the proxy serves for + * another 20 seconds, is the difference between waiting and escalating. + */ +const FREEZE_PROPAGATION_NOTE = + " Enforcement at the RPC proxy follows the control plane rather than " + + "coinciding with it: a freeze was measured at 10 to 21 seconds to take " + + "effect on rpc.ankr.com (2026-08-07), so requests can still be served " + + "briefly after this reply."; + +const UNFREEZE_PROPAGATION_NOTE = + " Enforcement at the RPC proxy follows the control plane rather than " + + "coinciding with it, so traffic takes a moment to resume. Only the freeze " + + "direction has been measured (10 to 21 seconds, 2026-08-07); the unfreeze " + + "direction has not been measured."; + +const propagationNote = (freeze: boolean): string => + freeze ? FREEZE_PROPAGATION_NOTE : UNFREEZE_PROPAGATION_NOTE; + +/** What a status read settled, or why it settled nothing. */ +type StatusReadBack = + { ok: true; status: CounterStatus } | { ok: false; why: string }; + +/** + * Read the key's status straight back from the gateway. + * + * WHY THIS IS NOT THE READ-BACK THE ALLOWLIST WRITES REFUSE TO DO. Their comment + * warns against a check that would "race the 45-100s proxy propagation and + * manufacture false failures", and it is right — about the DATA plane. This read + * asks the CONTROL plane what it now holds, which is the same store the write + * just went to and is authoritative immediately. The two are different + * questions, and conflating them is how the caller ends up doing this read by + * hand anyway. + * + * A THROWN read is not a failed write. The write was already accepted; only the + * observation failed, so the reason travels back as text and the caller keeps a + * non-error result. + */ +async function readBackStatus( + gateway: GatewayClient, + token: string +): Promise { + try { + const status = await gateway.getJwtStatus(token); + // Same defensive shape as createApiKey's `index` guard: request() hands back + // `undefined as unknown as T` for an empty body, and CounterStatus types + // `frozen` as a required boolean, so a `typeof` test is the one TypeScript + // accepts as meaningful against a reply that may not honour the type. + if (!status || typeof status.frozen !== "boolean") { + return { + ok: false, + why: "the status route returned no state in its body", + }; + } + return { ok: true, status }; + } catch (e) { + return { ok: false, why: e instanceof Error ? e.message : String(e) }; + } +} + +const statusLine = (s: CounterStatus): string => + `frozen: ${s.frozen} (suspended: ${s.suspended}, freemium: ${s.freemium})`; + export function registerFreezeApiKey({ server, gateway, @@ -152,29 +235,104 @@ export function registerFreezeApiKey({ // SHARK-3522: say what a bodiless 200 actually proves — that the request // was ACCEPTED — not that the key IS frozen. // - // Deliberately NOT a request-vs-reply comparison like the allowlist and - // notif-config writes: there is nothing to compare. In the gateway source + // There is still nothing to compare IN THE REPLY. In the gateway source // (src/controllers/jwtcontroller.go) UpdateProjectFreezeState is // documented `@Success 200 {string} string ""` and the only Respond* // calls in the handler are error responders, so a success carries an empty // body. freezeJwt is typed Promise for that reason. // + // SHARK-3622: which is why the comparison now comes from a SECOND call. + // The old reply ended by telling the caller to run mgmt_get_api_key_status + // themselves — one gateway read, named in the sentence, that every caller + // had to write and that an agent which did not know to write reported as + // "frozen" while traffic was still being served. Doing it here removes a + // whole class of false "done" for the cost of the read the reply was + // already prescribing. + // // Freezing takes a customer's production traffic down, so overstating it // is operationally expensive in both directions: a human who believes an // unfreeze already took effect stops looking at an outage. const verb = freeze ? "FREEZE" : "UNFREEZE"; + const accepted = + `The gateway ACCEPTED the request to ${verb} API key ${target.label} ` + + `(HTTP 2xx)`; + const note = propagationNote(freeze); + + // SHARK-3622: the freeze route itself still says nothing, so the status + // route is asked. Three outcomes, and each states exactly what it knows. + const readBack = await readBackStatus(gateway, target.token); + if (!readBack.ok) { + return { + content: [ + { + type: "text", + text: + `${accepted}. This route returns no state in its body, so the ` + + `key's resulting status was NOT observed and is not confirmed ` + + `here. A follow-up status read did not settle it either: ` + + `${readBack.why}. Verify with mgmt_get_api_key_status before ` + + `relying on it.${note}`, + }, + ], + _meta: unobservedMeta("mgmt_get_api_key_status"), + }; + } + + const status = readBack.status; + if (status.frozen !== freeze) { + // The gateway's OWN read disagrees with the write it just accepted. + // + // WHY THIS IS NOT isError, when the allowlist writes DO raise it on a + // mismatch. Theirs compares the request against the state carried in + // the SAME reply, where a disagreement can only mean the write did not + // apply. This one compares against a SEPARATE read on a different + // route, which can legitimately trail the write by a moment — so the + // shim cannot tell "did not apply" from "read too early", and + // writeOutcome.ts's rule applies: asserting a failure the code has no + // evidence for is the same defect as asserting a success, pointed the + // other way. `isError` also means "retry me" to most agents, and a + // retry here spends a second human approval on a change that may + // already be in place — which is what the text tells them not to do. + // The uncertainty travels in `_meta.matchesRequest` instead, where a + // client can branch on it without parsing English. + return { + content: [ + { + type: "text", + text: + `${accepted}, but the status read back immediately ` + + `afterwards reports ${statusLine(status)}, which is NOT what ` + + `was requested. Two things produce this: the read raced the ` + + `write, or the write did not apply. Re-read with ` + + `mgmt_get_api_key_status before acting on it; do NOT ` + + `re-issue the ${verb}, which would spend another human ` + + `approval on a change that may already be in place.`, + }, + ], + _meta: { + ...observedMeta(), + frozen: status.frozen, + requested: freeze, + matchesRequest: false, + }, + }; + } + return { content: [ { type: "text", text: - `The gateway ACCEPTED the request to ${verb} API key ${target.label} ` + - `(HTTP 2xx). This route returns no state in its body, so the ` + - `key's resulting status was NOT observed and is not confirmed ` + - `here. Verify with mgmt_get_api_key_status before relying on it.`, + `${accepted}, and the status read back CONFIRMS it: ` + + `${statusLine(status)}.${note}`, }, ], - _meta: unobservedMeta("mgmt_get_api_key_status"), + _meta: { + ...observedMeta(), + frozen: status.frozen, + suspended: status.suspended, + freemium: status.freemium, + }, }; } catch (e) { const authHint = diff --git a/src/mgmt/tools/platformApiKeys.ts b/src/mgmt/tools/platformApiKeys.ts index 631b074..38ffbaf 100644 --- a/src/mgmt/tools/platformApiKeys.ts +++ b/src/mgmt/tools/platformApiKeys.ts @@ -225,9 +225,17 @@ function mintEffects(ttlLabel: string): string[] { "The new key is a BEARER token for the whole Ankr management API, not an " + "RPC endpoint token: it does not fetch chain data, it administers this " + "account.", + // SHARK-3620: TWO lines, because the page clips each effect at 200 + // characters and this one was 202 — cut at "without asking a human to + // approve anyth…", losing the second factor, and losing the sentence that + // tells the human what they are approving. Of every line on the gated + // surface this is the one that must arrive whole: it is the difference + // between minting an admin credential and minting one that also removes the + // gate the human is currently standing at. "Anyone who holds it can do everything this assistant can do here — list, " + "create, edit, freeze and delete API keys, read usage and billing, and " + - "start a payment — without asking a human to approve anything and " + + "start a payment.", + "It does all of that without asking a human to approve anything and " + "without a second factor. Approving this is approving that.", `It works for ${ttlLabel} from now, or until it is deleted, whichever ` + `comes first. It cannot be limited to one chain, one project or one ` + diff --git a/src/mgmt/tools/sessions.ts b/src/mgmt/tools/sessions.ts index e98d47a..3ae9430 100644 --- a/src/mgmt/tools/sessions.ts +++ b/src/mgmt/tools/sessions.ts @@ -747,17 +747,23 @@ export function logoutOthersEffects(input: { }): string[] { const { others, unreadable } = input; const effects = [ - `${others.length} session(s) end immediately. Everything signed in on ` + - `this Ankr login stops working at once: other browsers, other ` + - `machines, other agents, CI jobs and scripts included, whether or not ` + - `anyone remembers they exist.`, + // SHARK-3620: split at the sentence boundary because the page clips every + // effect at 200 characters, and this one crossed the line as soon as the + // count reached one digit — losing "whether or not anyone remembers they + // exist", which is the clause that explains why the blast radius is larger + // than the list a human is looking at. A bound that bites depending on how + // many sessions the account happens to have is the worst kind. + `${others.length} session(s) end immediately.`, + `Everything signed in on this Ankr login stops working at once: other ` + + `browsers, other machines, other agents, CI jobs and scripts included, ` + + `whether or not anyone remembers they exist.`, "Each of them: " + others.map((s) => describeDevice(s.creation_details)).join("; "), "THIS session is NOT ended. This assistant keeps working.", - "No API key, allowlist, payment or account setting is touched. A Platform " + - "API key is a different credential and is NOT a session: it keeps " + - "working, so revoke one with mgmt_delete_platform_api_key if it may " + - "also have leaked.", + "No API key, allowlist, payment or account setting is touched.", + "A Platform API key is a different credential and is NOT a session: it " + + "keeps working, so revoke one with mgmt_delete_platform_api_key if it " + + "may also have leaked.", ]; if (unreadable > 0) { effects.push( diff --git a/test/mgmt-gated-display.test.ts b/test/mgmt-gated-display.test.ts index e0dfea5..e957a38 100644 --- a/test/mgmt-gated-display.test.ts +++ b/test/mgmt-gated-display.test.ts @@ -290,6 +290,21 @@ test("SHARK-3513: EVERY gated call site mints a self-describing display payload" (d.effects ?? []).length > 0, `${entry.tool}: the consequences must be listed` ); + // SHARK-3620: and none of them may be CUT OFF. The store clips every effect + // at 200 characters, silently, and exactly one line on the whole gated + // surface was over it — create's credential disclosure, truncated at + // "…which are live credentials…", losing the clause that says they land in + // the conversation transcript. A consequence a human cannot finish reading + // is not a consequence they were told, and the failure mode is invisible + // from the call site, so it is pinned here for every page rather than for + // the one that happened to be caught. + for (const effect of d.effects ?? []) { + assert.doesNotMatch( + effect, + /…$/, + `${entry.tool}: an effect was truncated by the display bound: ${effect}` + ); + } // SHARK-3577: the two SESSION writes are the only gated pages that carry NO // account, and it is not an omission. `account` is filled from // `GET /auth/users/profile`, which IS account-scoped, so under a selected diff --git a/test/mgmt-key-lifecycle-truthfulness.test.ts b/test/mgmt-key-lifecycle-truthfulness.test.ts new file mode 100644 index 0000000..12db94d --- /dev/null +++ b/test/mgmt-key-lifecycle-truthfulness.test.ts @@ -0,0 +1,546 @@ +// SHARK-3619 / SHARK-3620 / SHARK-3622 — a key-lifecycle write must not +// describe a world different from the one it produces. +// +// All three were found by walking create -> call -> restrict -> freeze against +// prod on 2026-08-07, and they are the same defect pointed in three directions: +// the reply and the tool description each assert something the data plane does +// not do. +// +// - SHARK-3619. The create reply says the ready URL "works immediately". It +// does not: the proxy learns a NEW key roughly 60 to 90 seconds later, and +// until then rpc.ankr.com answers -32050 "API key not found" under HTTP 401. +// Every plausible reaction to that error is wrong (create it again, escalate, +// report MCP key creation as broken), and the reply is what caused it. +// - SHARK-3620. The tool description says "The secret key material is never +// returned in the tool output" while the reply carries the endpoint token in +// full. The approval page for the SAME call is accurate and separates the two +// secrets. A description that denies the disclosure is what makes the +// disclosure invisible. +// - SHARK-3622. freeze neither reads the resulting state back nor mentions the +// data-plane lag, while the blockchain-allowlist write on the same key, one +// minute earlier, does both. +// +// WHY THE PAIRING TESTS ARE STRUCTURAL RATHER THAN TEXTUAL. Asserting that two +// wordings "agree" by matching two regexes is the failure mode that produced +// SHARK-3620 in the first place: both sides pass their own assertion and drift +// apart anyway. So the disclosure is ONE exported constant and the tests assert +// IDENTITY - the description contains it, and the approval page's effects list +// contains it as an element. Changing one and not the other cannot be done. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { createMgmtServer } from "../src/mgmt/server.js"; +import type { + CounterStatus, + GatewayClient, +} from "../src/mgmt/gateway/client.js"; +import type { + WorkerClient, + WorkerTokenResult, +} from "../src/mgmt/gateway/worker.js"; +import { + type MgmtDeps, + createConfirmationStore, +} from "../src/mgmt/tools/confirmation.js"; +import { + ENDPOINT_TOKEN_DISCLOSURE, + ENDPOINT_TOKEN_DISCLOSURE_LINES, + NEW_KEY_PROPAGATION_NOTE, +} from "../src/mgmt/tools/endpointToken.js"; + +const JWT_DATA = "HEADER.PAYLOAD.SIGNATURE"; +const ENDPOINT_TOKEN = "b3d9f1a6c07e4b1e9f2a5c8d7e6b4a3f"; +const ADDRESS = "0xabc0000000000000000000000000000000000001"; +const TEST_SUB = "test-subject"; +const SLOT = 2; + +const keyAt = (index: number) => ({ + index, + jwt_data: JWT_DATA, + is_encrypted: false, + name: "acceptance-2026-08-07", + description: "acceptance key", + config: '{"blockchains":["eth"]}', +}); + +/** + * A gateway whose freeze route answers as the real one does (bodiless 2xx), and + * whose STATUS route is supplied per test - that read is the subject of + * SHARK-3622, so every test states what it returns rather than inheriting it. + */ +function gatewayWith(status: (() => Promise) | undefined): { + gateway: GatewayClient; + calls: string[]; +} { + const calls: string[] = []; + return { + calls, + gateway: { + listJwtTokens: () => { + calls.push("listJwtTokens"); + return Promise.resolve([keyAt(SLOT)]); + }, + getUserProfile: () => Promise.resolve({ address: ADDRESS }), + createAdditionalJwt: () => { + calls.push("createAdditionalJwt"); + return Promise.resolve(keyAt(SLOT)); + }, + freezeJwt: () => { + calls.push("freezeJwt"); + return Promise.resolve(undefined); + }, + getJwtStatus: () => { + calls.push("getJwtStatus"); + // The harness default mirrors the real bodiless-200 shape: request() + // hands back undefined for an empty body. + return ( + status?.() ?? Promise.resolve(undefined as unknown as CounterStatus) + ); + }, + } as unknown as GatewayClient, + }; +} + +const statusOf = (frozen: boolean) => (): Promise => + Promise.resolve({ frozen, suspended: false, freemium: false }); + +/** A worker that resolves the endpoint token, as the live one does. */ +const workerOk = (): WorkerClient => ({ + importJwtToken: () => + Promise.resolve({ + token: ENDPOINT_TOKEN, + tier: "premium", + } as WorkerTokenResult), +}); + +function depsWith(worker?: WorkerClient): { + deps: MgmtDeps; + store: ReturnType; +} { + const confirmations = createConfirmationStore("http://localhost:3100"); + return { + deps: { + confirmations, + sub: TEST_SUB, + issuerUrl: "http://localhost:3100", + mfaEnforced: true, + worker, + }, + store: confirmations, + }; +} + +async function connect( + gateway: GatewayClient, + deps?: MgmtDeps +): Promise { + const server = createMgmtServer(gateway, deps); + const [clientT, serverT] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test", version: "0" }); + await server.connect(serverT); + await client.connect(clientT); + return client; +} + +const textOf = (r: unknown): string => + ((r as { content?: { text?: string }[] }).content ?? []) + .map((c) => c.text ?? "") + .join("\n"); + +const metaOf = (r: unknown): Record => + ((r as { _meta?: Record })._meta ?? {}) as Record< + string, + unknown + >; + +const isError = (r: unknown): boolean => + (r as { isError?: boolean }).isError === true; + +const mintedToken = (text: string): string | undefined => + /confirmToken: ([0-9a-f-]{36})/.exec(text)?.[1]; + +/** Drive a gated tool the way a human does: request, approve, repeat. */ +async function runApproved( + client: Client, + store: ReturnType, + name: string, + args: Record +): Promise { + const first = await client.callTool({ name, arguments: args }); + const token = mintedToken(textOf(first)); + assert.ok(token, `${name} must mint a confirmToken: ${textOf(first)}`); + assert.ok(store.approve(token, TEST_SUB), "approval must succeed"); + return client.callTool({ name, arguments: { ...args, confirmToken: token } }); +} + +/** The display payload a gated call parked for the approval page. */ +async function displayFor( + client: Client, + store: ReturnType, + name: string, + args: Record +): Promise<{ summary: string; effects?: string[] }> { + const first = await client.callTool({ name, arguments: args }); + const token = mintedToken(textOf(first)); + assert.ok(token, `${name} must mint a confirmToken`); + const pending = store.peek(token); + assert.ok(pending?.display, `${name} must park a display payload`); + return pending.display as { summary: string; effects?: string[] }; +} + +const descriptionOf = async (client: Client, name: string): Promise => { + const { tools } = await client.listTools(); + const tool = tools.find((t) => t.name === name); + assert.ok(tool, `${name} must be registered`); + return tool.description ?? ""; +}; + +// --------------------------------------------------------------------------- +// SHARK-3620 — the description and the approval page must say the same thing +// --------------------------------------------------------------------------- + +test("SHARK-3620: create's description does not claim the output withholds the secret", async () => { + const { gateway } = gatewayWith(undefined); + const client = await connect(gateway, depsWith(workerOk()).deps); + try { + const description = await descriptionOf(client, "mgmt_create_api_key"); + + // The exact claim the acceptance run falsified. It is not enough to add a + // truthful sentence beside it; the false one has to be gone. + assert.doesNotMatch( + description, + /secret key material is never returned/i, + "the description must stop denying a disclosure the reply performs" + ); + assert.match( + description, + /endpoint token/i, + "and must name the credential the reply actually carries" + ); + } finally { + await client.close(); + } +}); + +test("SHARK-3620: the description and the approval page carry the SAME disclosure sentence", async () => { + const { gateway } = gatewayWith(undefined); + const { deps, store } = depsWith(workerOk()); + const client = await connect(gateway, deps); + try { + const description = await descriptionOf(client, "mgmt_create_api_key"); + const display = await displayFor(client, store, "mgmt_create_api_key", { + index: SLOT, + name: "acceptance-2026-08-07", + }); + + assert.ok( + description.includes(ENDPOINT_TOKEN_DISCLOSURE), + "the tool description must carry the shared disclosure verbatim" + ); + for (const line of ENDPOINT_TOKEN_DISCLOSURE_LINES) { + assert.ok( + (display.effects ?? []).includes(line), + `the approval page must carry this line verbatim: ${line}` + ); + } + // The page STORES each effect through clip(e, 200). Asserting the lines are + // present in the payload we handed over would pass even if the page then cut + // them, which is what was happening: the 201-character single sentence was + // truncated at "…which are live credentials…" and the human never saw where + // those credentials land. So the assertion is on what the STORE holds. + for (const effect of display.effects ?? []) { + assert.doesNotMatch( + effect, + /…$/, + `an effect a human is meant to read was truncated: ${effect}` + ); + } + } finally { + await client.close(); + } +}); + +test("SHARK-3620: the disclosure names BOTH secrets, and which one is withheld", async () => { + // The point of the sentence is the distinction. A version that said only + // "credentials are returned" would pass an includes() check while losing the + // thing that makes it actionable: jwt_data stays hidden, the endpoint token + // does not. + assert.match(ENDPOINT_TOKEN_DISCLOSURE, /jwt_data/); + assert.match(ENDPOINT_TOKEN_DISCLOSURE, /never shown/i); + assert.match(ENDPOINT_TOKEN_DISCLOSURE, /DOES carry/); + assert.match(ENDPOINT_TOKEN_DISCLOSURE, /endpoint token/i); +}); + +// --------------------------------------------------------------------------- +// SHARK-3619 — the create reply must not promise immediate usability +// --------------------------------------------------------------------------- + +test("SHARK-3619: the create reply does not claim the URL works immediately", async () => { + const { gateway } = gatewayWith(undefined); + const { deps, store } = depsWith(workerOk()); + const client = await connect(gateway, deps); + try { + const res = await runApproved(client, store, "mgmt_create_api_key", { + index: SLOT, + name: "acceptance-2026-08-07", + }); + const text = textOf(res); + + assert.equal(isError(res), false); + assert.doesNotMatch( + text, + /works immediately/i, + "the reply must not assert immediacy the proxy contradicts for a minute" + ); + // The useful half of that sentence must survive: the URL still needs no + // session setup, which is the whole reason it is the shortest path. + assert.match(text, /HTTP client/i); + } finally { + await client.close(); + } +}); + +test("SHARK-3619: the create reply names the wait, the error, and that it is not a failure", async () => { + const { gateway } = gatewayWith(undefined); + const { deps, store } = depsWith(workerOk()); + const client = await connect(gateway, deps); + try { + const text = textOf( + await runApproved(client, store, "mgmt_create_api_key", { + index: SLOT, + name: "acceptance-2026-08-07", + }) + ); + + assert.ok( + text.includes(NEW_KEY_PROPAGATION_NOTE), + "the create reply must carry the propagation note" + ); + // A caller who hits the window must be able to recognise what they are + // looking at from the reply alone: the code, the words the proxy uses, and + // the verdict that it is not a failed creation. + assert.match(text, /-32050/); + assert.match(text, /API key not found/i); + assert.match( + text, + /not a failed creation|not.{0,40}creation.{0,20}fail/i, + "the reply must say the error means not-ready-yet" + ); + assert.match( + text, + /do not create it again|do NOT create it again/i, + "and must head off the reaction that actually happened" + ); + // The number is a MEASUREMENT, so it is stated with its date rather than as + // folklore. Whoever changes it should have to change the date too. + assert.match(text, /60 to 90 seconds/); + assert.match(text, /2026-08-07/); + } finally { + await client.close(); + } +}); + +test("SHARK-3619: reveal does NOT carry the new-key note, because its key already exists", async () => { + // The renderer is shared with create on purpose (SHARK-3543), so the note has + // to be parameterised rather than bolted onto the shared string. Pinning the + // reveal path is what stops the next edit from putting a create-only warning + // on every key the account already has. + const { gateway } = gatewayWith(undefined); + const { deps, store } = depsWith(workerOk()); + const client = await connect(gateway, deps); + try { + const text = textOf( + await runApproved(client, store, "mgmt_reveal_api_key", { index: SLOT }) + ); + + assert.match( + text, + new RegExp(ENDPOINT_TOKEN), + "reveal still hands over the key" + ); + assert.doesNotMatch( + text, + /-32050/, + "an existing key is already known to the proxy; do not warn about a wait" + ); + } finally { + await client.close(); + } +}); + +// --------------------------------------------------------------------------- +// SHARK-3622 — freeze must read the state back, and state the lag +// --------------------------------------------------------------------------- + +test("SHARK-3622: an accepted freeze reports the status it read back", async () => { + const { gateway, calls } = gatewayWith(statusOf(true)); + const { deps, store } = depsWith(workerOk()); + const client = await connect(gateway, deps); + try { + const res = await runApproved(client, store, "mgmt_freeze_api_key", { + index: SLOT, + freeze: true, + }); + const text = textOf(res); + + assert.equal(isError(res), false); + assert.ok( + calls.includes("getJwtStatus"), + "the tool must perform the read it used to delegate to the caller" + ); + assert.match(text, /frozen: true/); + assert.doesNotMatch( + text, + /NOT observed/, + "a state that WAS read back must not be reported as unobserved" + ); + assert.equal(metaOf(res).observed, true); + assert.equal(metaOf(res).frozen, true); + } finally { + await client.close(); + } +}); + +test("SHARK-3622: the freeze reply states the measured data-plane lag", async () => { + const { gateway } = gatewayWith(statusOf(true)); + const { deps, store } = depsWith(workerOk()); + const client = await connect(gateway, deps); + try { + const text = textOf( + await runApproved(client, store, "mgmt_freeze_api_key", { + index: SLOT, + freeze: true, + }) + ); + + assert.match(text, /10 to 21 seconds/, "the measured window, not a guess"); + assert.match(text, /2026-08-07/, "with the date the measurement was taken"); + assert.match( + text, + /still be served|briefly|resume/i, + "and what that means for traffic in the meantime" + ); + } finally { + await client.close(); + } +}); + +test("SHARK-3622: unfreeze does not borrow the freeze direction's measurement", async () => { + // Only the freeze direction was measured. Printing the same number for the + // reverse would be exactly the kind of invented precision these three tickets + // are about. + const { gateway } = gatewayWith(statusOf(false)); + const { deps, store } = depsWith(workerOk()); + const client = await connect(gateway, deps); + try { + const text = textOf( + await runApproved(client, store, "mgmt_freeze_api_key", { + index: SLOT, + freeze: false, + }) + ); + + assert.match(text, /frozen: false/); + assert.match( + text, + /has not been measured|not measured/i, + "the unmeasured direction must say so" + ); + } finally { + await client.close(); + } +}); + +test("SHARK-3622: a status read that carries no state keeps today's unobserved wording", async () => { + // The freeze route answers bodiless, and so may the status route. When the + // read-back settles nothing, the tool must fall back to the claim it can + // support rather than inventing a confirmation. + const { gateway } = gatewayWith(undefined); + const { deps, store } = depsWith(workerOk()); + const client = await connect(gateway, deps); + try { + const res = await runApproved(client, store, "mgmt_freeze_api_key", { + index: SLOT, + freeze: true, + }); + const text = textOf(res); + + assert.equal(isError(res), false, "an accepted write is not a failure"); + assert.match(text, /ACCEPTED the request to FREEZE/); + assert.match(text, /NOT observed/); + assert.match(text, /mgmt_get_api_key_status/); + assert.equal(metaOf(res).observed, false); + assert.equal(metaOf(res).verifyWith, "mgmt_get_api_key_status"); + } finally { + await client.close(); + } +}); + +test("SHARK-3622: a read-back that contradicts the request is reported, not glossed", async () => { + const { gateway } = gatewayWith(statusOf(false)); + const { deps, store } = depsWith(workerOk()); + const client = await connect(gateway, deps); + try { + const res = await runApproved(client, store, "mgmt_freeze_api_key", { + index: SLOT, + freeze: true, + }); + const text = textOf(res); + + // NOT isError, and the distinction is the point. The write was accepted; a + // status read on a different route may simply have trailed it, so the shim + // cannot tell "did not apply" from "read too early". isError additionally + // reads as "retry me", and a retried freeze costs a second human approval + // for a change that may already be in place. The disagreement is carried in + // the text and in _meta.matchesRequest, which is checkable without parsing + // English. + assert.equal(isError(res), false, "an accepted write is not a failed one"); + assert.equal(metaOf(res).observed, true); + assert.equal(metaOf(res).matchesRequest, false); + assert.equal(metaOf(res).requested, true); + assert.match(text, /frozen: false/, "it must print what was actually read"); + assert.match(text, /NOT what was requested/, "and flag the disagreement"); + assert.match(text, /mgmt_get_api_key_status/, "and name the settling read"); + // Re-issuing the write costs another human approval for a change that may + // already be in place, so the reply must not send the caller there. + assert.match( + text, + /do NOT re-issue|do not re-issue/i, + "it must steer to a re-read rather than a second approval" + ); + } finally { + await client.close(); + } +}); + +test("SHARK-3622: a status read that throws leaves the write accepted and unobserved", async () => { + const { gateway } = gatewayWith(() => + Promise.reject(new Error("status route unavailable")) + ); + const { deps, store } = depsWith(workerOk()); + const client = await connect(gateway, deps); + try { + const res = await runApproved(client, store, "mgmt_freeze_api_key", { + index: SLOT, + freeze: true, + }); + const text = textOf(res); + + assert.equal( + isError(res), + false, + "a failed VERIFICATION must not be reported as a failed write" + ); + assert.match(text, /ACCEPTED the request to FREEZE/); + assert.match(text, /NOT observed/); + assert.match( + text, + /status route unavailable/, + "the reason the read-back settled nothing belongs in the reply" + ); + assert.equal(metaOf(res).observed, false); + } finally { + await client.close(); + } +}); diff --git a/test/mgmt-sessions.test.ts b/test/mgmt-sessions.test.ts index 33710e9..f2319c7 100644 --- a/test/mgmt-sessions.test.ts +++ b/test/mgmt-sessions.test.ts @@ -1785,23 +1785,28 @@ test("SHARK-3577: the bulk-logout consent page reads exactly this", async () => }); test("SHARK-3577: the bulk-logout effects are exactly these, with and without unaddressable entries", () => { - // Pinned from the source rather than the stored page, because the store - // CLIPS a long effect and a clipped assertion would only cover its prefix. + // Pinned from the source. It used to say the store "CLIPS a long effect and + // a clipped assertion would only cover its prefix" — which was true, and was + // the workaround rather than the fix: two of these lines were being cut off on + // the page a human approves, one of them depending on how many sessions the + // account happened to have. SHARK-3620 split them under the bound, and + // test/mgmt-gated-display.test.ts now fails any effect that arrives truncated, + // so source and page carry the same words again. assert.deepEqual( logoutOthersEffects({ others: [LAPTOP_SESSION, CI_SESSION], unreadable: 0, }), [ - "2 session(s) end immediately. Everything signed in on this " + - "Ankr login stops working at once: other browsers, other " + - "machines, other agents, CI jobs and scripts included, whether " + - "or not anyone remembers they exist.", + "2 session(s) end immediately.", + "Everything signed in on this Ankr login stops working at once: " + + "other browsers, other machines, other agents, CI jobs and " + + "scripts included, whether or not anyone remembers they exist.", "Each of them: Firefox 122 on Ubuntu 22.04 (desktop); linux " + "(server)", "THIS session is NOT ended. This assistant keeps working.", - "No API key, allowlist, payment or account setting is touched. " + - "A Platform API key is a different credential and is NOT a " + + "No API key, allowlist, payment or account setting is touched.", + "A Platform API key is a different credential and is NOT a " + "session: it keeps working, so revoke one with " + "mgmt_delete_platform_api_key if it may also have leaked.", ] @@ -1809,14 +1814,14 @@ test("SHARK-3577: the bulk-logout effects are exactly these, with and without un assert.deepEqual( logoutOthersEffects({ others: [LAPTOP_SESSION], unreadable: 1 }), [ - "1 session(s) end immediately. Everything signed in on this " + - "Ankr login stops working at once: other browsers, other " + - "machines, other agents, CI jobs and scripts included, whether " + - "or not anyone remembers they exist.", + "1 session(s) end immediately.", + "Everything signed in on this Ankr login stops working at once: " + + "other browsers, other machines, other agents, CI jobs and " + + "scripts included, whether or not anyone remembers they exist.", "Each of them: Firefox 122 on Ubuntu 22.04 (desktop)", "THIS session is NOT ended. This assistant keeps working.", - "No API key, allowlist, payment or account setting is touched. " + - "A Platform API key is a different credential and is NOT a " + + "No API key, allowlist, payment or account setting is touched.", + "A Platform API key is a different credential and is NOT a " + "session: it keeps working, so revoke one with " + "mgmt_delete_platform_api_key if it may also have leaked.", "NOT EVERYTHING: the gateway also returned 1 session entry " + diff --git a/test/mgmt-tools.test.ts b/test/mgmt-tools.test.ts index 7c0c059..bf85a16 100644 --- a/test/mgmt-tools.test.ts +++ b/test/mgmt-tools.test.ts @@ -400,8 +400,15 @@ test("SHARK-3381: gated write reaches the gateway only with totp + approved conf // is asserted by the call below. assert.match(text, /ACCEPTED/); assert.match(text, /FREEZE/); - assert.equal(calls.length, 1); - assert.equal(calls[0].method, "freezeJwt"); + // SHARK-3622: TWO calls now, and which two is the assertion. The write is + // still the only thing this test is about (a gated write reached the gateway), + // but the tool no longer stops there: it reads the resulting status back + // instead of telling the caller to. Pinning the pair by name is what keeps + // that from drifting into an extra write. + assert.deepEqual( + calls.map((c) => c.method), + ["freezeJwt", "getJwtStatus"] + ); // The totp is a shim-side gate for freeze (non-MFA route) — never forwarded // and never echoed. assert.doesNotMatch(text, /123456/); @@ -1416,9 +1423,17 @@ test("SHARK-3522 pass3: freeze reports the request as ACCEPTED, not as an observ "must not assert the key IS frozen from a bodiless 200" ); assert.match(t, /ACCEPTED/); - // It must say WHY it cannot confirm, and name the read-back. - assert.match(t, /no .*body|returns no state|empty body/i); - assert.match(t, /mgmt_get_api_key_status|mgmt_list_api_keys/); + // SHARK-3622 reopened this one. The claim being pinned is unchanged — the + // reply may not assert a state it did not see — but the tool now GOES AND + // SEES: it reads the status back rather than handing the caller the read. So + // a confirmation is allowed here, on the condition that it is sourced from + // that read and says so. What must never come back is a confirmation + // attributed to the bodiless 200, which is what the old wording guarded. + assert.match(t, /read back CONFIRMS it: frozen: true/); + // The unobserved wording still exists and is still correct; it now belongs to + // the branch where the status read settles nothing, and it is asserted there + // (test/mgmt-key-lifecycle-truthfulness.test.ts). + assert.match(t, /mgmt_get_api_key_status|read back/); await client.close(); }); From 33901fba35a457225bb704609eadc246ead35088 Mon Sep 17 00:00:00 2001 From: Mike Date: Fri, 7 Aug 2026 14:52:52 +0300 Subject: [PATCH 2/3] fix(SHARK-3622): state matchesRequest on both observed freeze paths, not only the disagreeing one A client branching on the field would otherwise have to read `undefined` as agreement, which is the trap writeOutcome.ts closed for `observed`. Co-Authored-By: Claude Opus 5 (1M context) --- src/mgmt/tools/freezeApiKey.ts | 6 ++++++ test/mgmt-key-lifecycle-truthfulness.test.ts | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/mgmt/tools/freezeApiKey.ts b/src/mgmt/tools/freezeApiKey.ts index 1fd525a..51233cc 100644 --- a/src/mgmt/tools/freezeApiKey.ts +++ b/src/mgmt/tools/freezeApiKey.ts @@ -332,6 +332,12 @@ export function registerFreezeApiKey({ frozen: status.frozen, suspended: status.suspended, freemium: status.freemium, + // Stated on BOTH observed paths, not just the disagreeing one: a + // client that branches on `matchesRequest` must not have to read + // `undefined` as agreement. That is the same trap writeOutcome.ts + // closed for `observed`. + requested: freeze, + matchesRequest: true, }, }; } catch (e) { diff --git a/test/mgmt-key-lifecycle-truthfulness.test.ts b/test/mgmt-key-lifecycle-truthfulness.test.ts index 12db94d..72afde1 100644 --- a/test/mgmt-key-lifecycle-truthfulness.test.ts +++ b/test/mgmt-key-lifecycle-truthfulness.test.ts @@ -397,6 +397,10 @@ test("SHARK-3622: an accepted freeze reports the status it read back", async () ); assert.equal(metaOf(res).observed, true); assert.equal(metaOf(res).frozen, true); + // Both observed paths state it, so `undefined` never has to be read as + // agreement by a client that branches on this field. + assert.equal(metaOf(res).matchesRequest, true); + assert.equal(metaOf(res).requested, true); } finally { await client.close(); } From b82437a943d9a980ba52cc20bbb12e659d7dc965 Mon Sep 17 00:00:00 2001 From: Mike Date: Fri, 7 Aug 2026 15:36:16 +0300 Subject: [PATCH 3/3] test(SHARK-3622): kill two mutants the first pass left alive, one of them my own vacuous assertion Mutation run over freezeApiKey.ts (Stryker, repo config, 132 mutants) scored 67.42 and surfaced two survivors worth acting on. 1. The date assertion on the freeze propagation note was VACUOUS. The fixture key was named "acceptance-2026-08-07", so /2026-08-07/ matched the key's LABEL in the reply and would have passed with the date deleted from the note entirely -- which is exactly what the surviving mutant did. The fixture key is now "acceptance-run" and the assertion tests the note it was written for. 2. Flipping `||` to `&&` in the read-back guard survived: with an absent body the mutated guard throws reading `.frozen`, the throw is caught one frame later, and the result is still "accepted, not observed" -- passing every assertion. The unobserved test now pins the REASON, and a new case covers the other half of the guard: a body that exists but whose `frozen` is not a boolean, which must not render as `frozen: undefined`. Both re-verified by hand mutation: each mutant now fails, and the source was restored byte-for-byte (md5sum checked, not `git diff`). 1640/1640 tests; format, lint, typecheck, build clean. Co-Authored-By: Claude Opus 5 (1M context) --- test/mgmt-key-lifecycle-truthfulness.test.ts | 44 ++++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/test/mgmt-key-lifecycle-truthfulness.test.ts b/test/mgmt-key-lifecycle-truthfulness.test.ts index 72afde1..a8f2aaf 100644 --- a/test/mgmt-key-lifecycle-truthfulness.test.ts +++ b/test/mgmt-key-lifecycle-truthfulness.test.ts @@ -59,7 +59,7 @@ const keyAt = (index: number) => ({ index, jwt_data: JWT_DATA, is_encrypted: false, - name: "acceptance-2026-08-07", + name: "acceptance-run", description: "acceptance key", config: '{"blockchains":["eth"]}', }); @@ -231,7 +231,7 @@ test("SHARK-3620: the description and the approval page carry the SAME disclosur const description = await descriptionOf(client, "mgmt_create_api_key"); const display = await displayFor(client, store, "mgmt_create_api_key", { index: SLOT, - name: "acceptance-2026-08-07", + name: "acceptance-run", }); assert.ok( @@ -283,7 +283,7 @@ test("SHARK-3619: the create reply does not claim the URL works immediately", as try { const res = await runApproved(client, store, "mgmt_create_api_key", { index: SLOT, - name: "acceptance-2026-08-07", + name: "acceptance-run", }); const text = textOf(res); @@ -309,7 +309,7 @@ test("SHARK-3619: the create reply names the wait, the error, and that it is not const text = textOf( await runApproved(client, store, "mgmt_create_api_key", { index: SLOT, - name: "acceptance-2026-08-07", + name: "acceptance-run", }) ); @@ -476,6 +476,42 @@ test("SHARK-3622: a status read that carries no state keeps today's unobserved w assert.match(text, /mgmt_get_api_key_status/); assert.equal(metaOf(res).observed, false); assert.equal(metaOf(res).verifyWith, "mgmt_get_api_key_status"); + // The REASON, not just the shape. Without this the guard could degrade into + // a TypeError caught one frame later and still produce an unobserved result + // that passes every assertion above (mutation testing found exactly that: + // flipping `||` to `&&` in the guard survived). + assert.match(text, /the status route returned no state in its body/); + } finally { + await client.close(); + } +}); + +test("SHARK-3622: a status reply whose `frozen` is not a boolean settles nothing", async () => { + // The other half of the guard. A body that exists but does not honour the + // type is not "no body", and it is the shape that would otherwise print + // `frozen: undefined` as though it had been observed — the same defect + // createApiKey's `index` guard exists for, one layer in. + const { gateway } = gatewayWith(() => + Promise.resolve({ suspended: false, freemium: false } as CounterStatus) + ); + const { deps, store } = depsWith(workerOk()); + const client = await connect(gateway, deps); + try { + const res = await runApproved(client, store, "mgmt_freeze_api_key", { + index: SLOT, + freeze: true, + }); + const text = textOf(res); + + assert.equal(isError(res), false); + assert.match(text, /NOT observed/); + assert.match(text, /the status route returned no state in its body/); + assert.doesNotMatch( + text, + /frozen: undefined/, + "a missing flag must never be rendered as an observed value" + ); + assert.equal(metaOf(res).observed, false); } finally { await client.close(); }