From e62a69dbeeacc0295557abc8096670f3b6703f74 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 17:20:10 +0000 Subject: [PATCH 01/13] Push the dashboard verdict to a Statuspage component status.internetcomputer.org is a hosted Atlassian Statuspage: its components live in the Statuspage service, not in a repository, so the way to surface MCP health there is to push it. Add an optional pusher to the status dashboard that maps each probe run's overall verdict onto one Statuspage component and PATCHes it through the Statuspage REST API. - monitoring/mcp-status/statuspage.js: the pusher. Mapping: pass -> operational, warn -> degraded_performance, fail -> partial_outage, or major_outage when every MCP endpoint check failed (server down or unreachable). Pushes are change-driven (plus once at startup, since a restarted process cannot know the remote state), the interval is floored at 15 s, overlapping ticks join the in-flight run, push failures are logged (API key redacted) and retried, and the timer is unref'd so the pusher never keeps the process alive. - monitoring/mcp-status/server.js: start the pusher when the three STATUSPAGE_* variables are set; off by default, so existing deployments are unaffected. It reuses the /api/status probe cache, so it adds at most one probe run per interval. - monitoring/mcp-status/statuspage.test.js: unit tests (stubbed fetch, no network) for the mapping, config resolution, the PATCH request shape, change-only pushes, and retry-after-failure. - deploy/native/imcp-status.service: load an optional /etc/imcp-status/statuspage.env (created by the operator, mode 600) so the API key stays out of the repository and the rendered unit. - monitoring/mcp-status/README.md: setup (create the component in the Statuspage admin, mint an API key), variables, and the mapping table. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NWw9GUtUFEcTi75SnfnPi3 --- deploy/native/imcp-status.service | 7 + monitoring/mcp-status/README.md | 39 ++++ monitoring/mcp-status/server.js | 19 ++ monitoring/mcp-status/statuspage.js | 254 +++++++++++++++++++++++ monitoring/mcp-status/statuspage.test.js | 215 +++++++++++++++++++ 5 files changed, 534 insertions(+) create mode 100644 monitoring/mcp-status/statuspage.js create mode 100644 monitoring/mcp-status/statuspage.test.js diff --git a/deploy/native/imcp-status.service b/deploy/native/imcp-status.service index 3ece4b4..3c7438a 100644 --- a/deploy/native/imcp-status.service +++ b/deploy/native/imcp-status.service @@ -14,6 +14,13 @@ WorkingDirectory=/opt/imcp2/monitoring/mcp-status # can never be steered elsewhere -- including by the server's own /version. # Binds to 127.0.0.1; Caddy publishes it under https:///status/. Environment=MCP_STATUS_ALLOWED_HOSTS=__ALLOWED_HOSTS__ +# Optional: mirror the verdict to an Atlassian Statuspage component (e.g. on +# status.internetcomputer.org). The file holds STATUSPAGE_PAGE_ID, +# STATUSPAGE_API_KEY and STATUSPAGE_COMPONENT_ID (see the mcp-status README). +# It is created by the operator (mode 600, owner root), never by deploy.sh, so +# the API key stays out of this repository and of the rendered unit file. The +# leading "-" makes it optional: without the file the pusher never starts. +EnvironmentFile=-/etc/imcp-status/statuspage.env ExecStart=/usr/bin/node /opt/imcp2/monitoring/mcp-status/server.js --port 8137 --host 127.0.0.1 --mcp https://__DOMAIN__ Restart=always RestartSec=5 diff --git a/monitoring/mcp-status/README.md b/monitoring/mcp-status/README.md index c5a535c..3c47532 100644 --- a/monitoring/mcp-status/README.md +++ b/monitoring/mcp-status/README.md @@ -107,6 +107,45 @@ the host and runs it as the `imcp-status.service` systemd unit (bound to publishes it at `https:///status/`, and the CI workflow runs the unit tests below before rolling out. See the deploy README for details. +### Publishing to an Atlassian Statuspage (status.internetcomputer.org) + +`server.js` can mirror the dashboard's overall verdict to one component on an +Atlassian Statuspage — the mechanism that puts MCP health on +[status.internetcomputer.org](https://status.internetcomputer.org/), which is +hosted SaaS whose components cannot be served from a repository, only driven +through its [REST API](https://developer.statuspage.io/). The pusher +(`statuspage.js`) is **off by default** and starts only when all three of these +are set: + +| Variable | Value | +| -------- | ----- | +| `STATUSPAGE_PAGE_ID` | The page id (visible in the Statuspage admin, or as `page.id` in `https:///api/v2/status.json`). | +| `STATUSPAGE_API_KEY` | An API key minted in the Statuspage admin (read from the environment, never argv; never logged). | +| `STATUSPAGE_COMPONENT_ID` | The id of the component to drive. | + +One-time setup in the Statuspage admin: create the component (e.g. **"ICP MCP +server"**, optionally inside an existing group), mint an API key, and note the +page and component ids. On a deployed host, put the three variables in +`/etc/imcp-status/statuspage.env` (mode `600`, owner root) — the systemd unit +loads that file if present — and `systemctl restart imcp-status`. + +The mapping is intentionally coarse (a public status page is not the detailed +dashboard): + +| Dashboard verdict | Statuspage component status | +| ----------------- | --------------------------- | +| `pass` | `operational` | +| `warn` | `degraded_performance` | +| `fail`, but some MCP endpoint checks still pass | `partial_outage` | +| `fail` with every MCP endpoint check failing | `major_outage` | + +The pusher re-evaluates every `STATUSPAGE_PUSH_INTERVAL_MS` (default 60 s, +floor 15 s), reusing the same cached probe runs as dashboard visitors, and +PATCHes the component **only when the mapped status changes** (plus once at +startup, since a restarted process cannot know the remote state) — so a healthy +steady state costs one API call per process lifetime. A failed push is logged +and retried on the next interval; it never affects the dashboard itself. + ## Why a standalone tool (and not a page in the II frontend)? The II frontend is a **static, prerendered, `ssr: false` SvelteKit app** served diff --git a/monitoring/mcp-status/server.js b/monitoring/mcp-status/server.js index 986542c..6ec9339 100644 --- a/monitoring/mcp-status/server.js +++ b/monitoring/mcp-status/server.js @@ -22,6 +22,10 @@ import { readFile } from "node:fs/promises"; import { fileURLToPath } from "node:url"; import { dirname, join } from "node:path"; import { runDashboard } from "./checks.js"; +import { + resolveStatuspageConfig, + startStatuspagePusher, +} from "./statuspage.js"; const here = dirname(fileURLToPath(import.meta.url)); @@ -163,3 +167,18 @@ server.listen(port, host, () => { ` monitoring: ${defaults.mcpOrigin ?? "https://mcp.beta.id.ai (default)"}\n`, ); }); + +// Optionally mirror the verdict to an Atlassian Statuspage component (e.g. on +// status.internetcomputer.org). Off unless the STATUSPAGE_* variables are set — +// see statuspage.js and the README. The pusher shares getReport's cache with +// dashboard visitors, so it adds at most one probe run per interval. +const { config: statuspageConfig, warning: statuspageWarning } = + resolveStatuspageConfig(); +if (statuspageWarning) console.error(statuspageWarning); +if (statuspageConfig) { + startStatuspagePusher({ getReport, config: statuspageConfig }); + process.stdout.write( + ` statuspage: pushing to component ${statuspageConfig.componentId} ` + + `(page ${statuspageConfig.pageId}) every ${statuspageConfig.intervalMs} ms\n`, + ); +} diff --git a/monitoring/mcp-status/statuspage.js b/monitoring/mcp-status/statuspage.js new file mode 100644 index 0000000..6a54751 --- /dev/null +++ b/monitoring/mcp-status/statuspage.js @@ -0,0 +1,254 @@ +// Publish the dashboard's verdict to an Atlassian Statuspage component. +// +// status.internetcomputer.org is a hosted Atlassian Statuspage: its components +// live in the Statuspage service, not in any repository, so the way to surface +// MCP health there is to push it. This module maps a DashboardReport onto one +// Statuspage component status and PATCHes that component through the Statuspage +// REST API (https://developer.statuspage.io/) on a fixed interval, from the +// same process that already runs the probes (`server.js`). +// +// The pusher is entirely optional and off by default: it starts only when all +// three STATUSPAGE_* variables below are set, so existing deployments are +// unaffected. Pushes are change-driven — the component is PATCHed when the +// mapped status differs from the last successfully pushed one (plus once at +// startup, since a restarted process cannot know the remote state) — so a +// healthy steady state costs one API call per process lifetime, well inside +// Statuspage's rate limit of about one request per second. +// +// Configuration (environment; see also the deploy unit's EnvironmentFile): +// STATUSPAGE_PAGE_ID Statuspage page id (e.g. "kc2llmsd16bk"). +// STATUSPAGE_API_KEY API key minted in the Statuspage admin. It is +// read from the environment (never argv, so it +// doesn't show in `ps`) and never logged. +// STATUSPAGE_COMPONENT_ID Id of the component to drive. The component +// itself is created once, by hand, in the +// Statuspage admin (e.g. "ICP MCP server"). +// STATUSPAGE_PUSH_INTERVAL_MS Interval between checks (default 60000, +// floored at 15000 so a typo cannot hammer the +// monitored server or the Statuspage API). +// STATUSPAGE_API_BASE API base URL override — for tests and mocks +// only (default https://api.statuspage.io/v1). + +/** Default Statuspage REST API base URL. */ +export const DEFAULT_STATUSPAGE_API_BASE = "https://api.statuspage.io/v1"; + +/** Default and minimum push-loop intervals, in milliseconds. */ +export const DEFAULT_PUSH_INTERVAL_MS = 60_000; +export const MIN_PUSH_INTERVAL_MS = 15_000; + +/** + * @typedef {"operational" | "degraded_performance" | "partial_outage" | "major_outage"} ComponentStatus + * + * @typedef {Object} StatuspageConfig + * @property {string} pageId + * @property {string} apiKey + * @property {string} componentId + * @property {string} apiBase + * @property {number} intervalMs + */ + +/** + * Map a dashboard report onto a Statuspage component status. + * + * "pass" and "warn" map directly. For "fail" the severity depends on where the + * failure is: when every check in the "endpoints" section failed, the MCP + * server itself is down or unreachable (major outage); any other failure — + * some endpoints broken, or a linked Internet Identity instance unhealthy — + * degrades the service without taking it out entirely (partial outage). + * Anything unrecognised also lands in the "fail" branch: a monitor should fail + * loud, not quietly report green on input it doesn't understand. + * + * @param {import("./checks.js").DashboardReport | undefined} report + * @returns {ComponentStatus} + */ +export const componentStatusFor = (report) => { + if (report?.overall === "pass") return "operational"; + if (report?.overall === "warn") return "degraded_performance"; + const endpoints = Array.isArray(report?.sections) + ? report.sections.find((s) => s?.id === "endpoints") + : undefined; + const allEndpointsDown = + endpoints !== undefined && + Array.isArray(endpoints.checks) && + endpoints.checks.length > 0 && + endpoints.checks.every((c) => c?.status === "fail"); + return allEndpointsDown ? "major_outage" : "partial_outage"; +}; + +// Page and component ids are URL path segments; Statuspage ids are plain +// alphanumerics. Rejecting anything else keeps a mis-pasted value (a URL, a +// quoted string) from producing a confusing request instead of a clear error. +const ID_SHAPE = /^[a-z0-9]+$/i; + +/** + * Resolve the pusher configuration from the environment. + * + * Returns no config (pusher off) when none of the variables are set; returns a + * warning instead of a config when the configuration is present but unusable + * (some variables missing, or an id that is not a plain Statuspage id), so a + * typo shows up in the logs rather than as a silently missing status feed. + * + * @param {Record} [env] + * @returns {{ config?: StatuspageConfig, warning?: string }} + */ +export const resolveStatuspageConfig = (env = process.env) => { + const pageId = env.STATUSPAGE_PAGE_ID?.trim() || undefined; + const apiKey = env.STATUSPAGE_API_KEY?.trim() || undefined; + const componentId = env.STATUSPAGE_COMPONENT_ID?.trim() || undefined; + + const missing = [ + ...(pageId ? [] : ["STATUSPAGE_PAGE_ID"]), + ...(apiKey ? [] : ["STATUSPAGE_API_KEY"]), + ...(componentId ? [] : ["STATUSPAGE_COMPONENT_ID"]), + ]; + if (missing.length === 3) return {}; + if (missing.length > 0) { + return { + warning: `statuspage: pusher disabled — partial configuration (missing ${missing.join(", ")})`, + }; + } + for (const [name, value] of [ + ["STATUSPAGE_PAGE_ID", pageId], + ["STATUSPAGE_COMPONENT_ID", componentId], + ]) { + if (!ID_SHAPE.test(/** @type {string} */ (value))) { + return { + warning: `statuspage: pusher disabled — ${name} does not look like a Statuspage id (expected letters/digits only)`, + }; + } + } + + // Coerce the interval like resolveConfig does for the probe timeout: garbage + // falls back to the default, and a too-small value is floored rather than + // honoured (each cycle can trigger a full probe run against the monitored + // server, and Statuspage rate-limits its API). + const rawInterval = env.STATUSPAGE_PUSH_INTERVAL_MS + ? Number(env.STATUSPAGE_PUSH_INTERVAL_MS) + : DEFAULT_PUSH_INTERVAL_MS; + const intervalMs = + Number.isFinite(rawInterval) && rawInterval > 0 + ? Math.max(rawInterval, MIN_PUSH_INTERVAL_MS) + : DEFAULT_PUSH_INTERVAL_MS; + + const apiBase = ( + env.STATUSPAGE_API_BASE || DEFAULT_STATUSPAGE_API_BASE + ).replace(/\/+$/, ""); + + return { + config: { + pageId: /** @type {string} */ (pageId), + apiKey: /** @type {string} */ (apiKey), + componentId: /** @type {string} */ (componentId), + apiBase, + intervalMs, + }, + }; +}; + +/** + * PATCH the component's status on Statuspage. + * + * Errors are thrown with the HTTP status only — never the response body, which + * is under the remote service's control, and never anything derived from the + * API key. + * + * @param {StatuspageConfig} config + * @param {ComponentStatus} status + * @param {typeof fetch} [fetchImpl] injectable for tests + */ +export const pushComponentStatus = async (config, status, fetchImpl = fetch) => { + const url = `${config.apiBase}/pages/${encodeURIComponent(config.pageId)}/components/${encodeURIComponent(config.componentId)}`; + const res = await fetchImpl(url, { + method: "PATCH", + headers: { + authorization: `OAuth ${config.apiKey}`, + "content-type": "application/json", + }, + body: JSON.stringify({ component: { status } }), + }); + if (res.status !== 200) { + throw new Error(`Statuspage API responded ${res.status}`); + } +}; + +/** + * Reduce an error to a single safe log line: message only (no stack), control + * characters stripped (so a network error cannot forge log entries — same + * rationale as server.js's sanitiser), capped in length, and with the API key + * redacted should it ever leak into a message. + * + * @param {unknown} e + * @param {string} apiKey + * @returns {string} + */ +const describeError = (e, apiKey) => { + const raw = String((e && /** @type {any} */ (e).message) || e).slice(0, 300); + let out = ""; + for (const ch of raw) { + const code = /** @type {number} */ (ch.codePointAt(0)); + const dangerous = + code < 0x20 || + code === 0x7f || + (code >= 0x80 && code <= 0x9f) || + code === 0x2028 || + code === 0x2029; + out += dangerous ? " " : ch; + } + return apiKey ? out.split(apiKey).join("[redacted]") : out; +}; + +/** + * Start the periodic pusher. Each tick obtains a report (through the caller's + * cached `getReport`, so ticks and dashboard visitors share probe runs), maps + * it, and PATCHes the component only when the mapped status changed since the + * last successful push. A failed tick — probe or API — is logged and retried + * on the next interval; it never throws and never takes the server down. The + * interval timer is unref'd so the pusher alone never keeps the process alive. + * + * @param {Object} args + * @param {() => Promise} args.getReport + * @param {StatuspageConfig} args.config + * @param {typeof fetch} [args.fetchImpl] injectable for tests + * @param {(line: string) => void} [args.log] + * @returns {{ stop: () => void, tick: () => Promise }} + */ +export const startStatuspagePusher = ({ + getReport, + config, + fetchImpl = fetch, + log = (line) => console.error(line), +}) => { + /** @type {ComponentStatus | undefined} */ + let lastPushed; + /** @type {Promise | null} */ + let inFlight = null; + + // A slow probe or API call must not stack a second run on top of itself: + // while one is in flight, further ticks join it instead of starting anew + // (which also makes manual ticks in tests deterministic). + const tick = () => { + if (inFlight) return inFlight; + inFlight = (async () => { + try { + const status = componentStatusFor(await getReport()); + if (status !== lastPushed) { + await pushComponentStatus(config, status, fetchImpl); + lastPushed = status; + log(`statuspage: component ${config.componentId} -> ${status}`); + } + } catch (e) { + log( + `statuspage: push failed (will retry): ${describeError(e, config.apiKey)}`, + ); + } + })().finally(() => { + inFlight = null; + }); + return inFlight; + }; + + void tick(); + const timer = setInterval(tick, config.intervalMs); + timer.unref?.(); + return { stop: () => clearInterval(timer), tick }; +}; diff --git a/monitoring/mcp-status/statuspage.test.js b/monitoring/mcp-status/statuspage.test.js new file mode 100644 index 0000000..2a3990f --- /dev/null +++ b/monitoring/mcp-status/statuspage.test.js @@ -0,0 +1,215 @@ +// Unit tests for the Statuspage pusher. +// Run with: node --test monitoring/mcp-status/statuspage.test.js +// or: cd monitoring/mcp-status && npm test +// +// These tests inject a stubbed `fetch`, so they make no real network calls. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + DEFAULT_PUSH_INTERVAL_MS, + DEFAULT_STATUSPAGE_API_BASE, + MIN_PUSH_INTERVAL_MS, + componentStatusFor, + pushComponentStatus, + resolveStatuspageConfig, + startStatuspagePusher, +} from "./statuspage.js"; + +/** Minimal report with the fields the mapping reads. */ +const report = (overall, endpointStatuses = []) => ({ + overall, + sections: [ + { + id: "endpoints", + title: "MCP server endpoints", + status: endpointStatuses.includes("fail") ? "fail" : "pass", + checks: endpointStatuses.map((status, i) => ({ id: `c${i}`, status })), + }, + ], +}); + +const FULL_ENV = { + STATUSPAGE_PAGE_ID: "kc2llmsd16bk", + STATUSPAGE_API_KEY: "sk-test-key", + STATUSPAGE_COMPONENT_ID: "abcdef123456", +}; + +test("componentStatusFor maps the overall verdict", () => { + assert.equal(componentStatusFor(report("pass")), "operational"); + assert.equal(componentStatusFor(report("warn")), "degraded_performance"); +}); + +test("componentStatusFor: a partial endpoint failure is a partial outage", () => { + assert.equal( + componentStatusFor(report("fail", ["pass", "fail", "pass"])), + "partial_outage", + ); +}); + +test("componentStatusFor: every endpoint check down is a major outage", () => { + assert.equal( + componentStatusFor(report("fail", ["fail", "fail", "fail"])), + "major_outage", + ); +}); + +test("componentStatusFor: a non-endpoint failure is a partial outage", () => { + // Overall "fail" caused by another section (e.g. linked II unhealthy): + // the endpoints section itself is green. + assert.equal( + componentStatusFor(report("fail", ["pass", "pass"])), + "partial_outage", + ); +}); + +test("componentStatusFor fails loud on unrecognised input", () => { + assert.equal(componentStatusFor(undefined), "partial_outage"); + assert.equal( + componentStatusFor(/** @type {any} */ ({ overall: "bogus" })), + "partial_outage", + ); +}); + +test("resolveStatuspageConfig: unset env means pusher off, silently", () => { + assert.deepEqual(resolveStatuspageConfig({}), {}); +}); + +test("resolveStatuspageConfig: partial config warns and stays off", () => { + const { config, warning } = resolveStatuspageConfig({ + STATUSPAGE_PAGE_ID: "kc2llmsd16bk", + }); + assert.equal(config, undefined); + assert.match(warning, /STATUSPAGE_API_KEY/); + assert.match(warning, /STATUSPAGE_COMPONENT_ID/); + assert.doesNotMatch(warning, /STATUSPAGE_PAGE_ID/); +}); + +test("resolveStatuspageConfig: full config resolves with defaults", () => { + const { config, warning } = resolveStatuspageConfig(FULL_ENV); + assert.equal(warning, undefined); + assert.deepEqual(config, { + pageId: "kc2llmsd16bk", + apiKey: "sk-test-key", + componentId: "abcdef123456", + apiBase: DEFAULT_STATUSPAGE_API_BASE, + intervalMs: DEFAULT_PUSH_INTERVAL_MS, + }); +}); + +test("resolveStatuspageConfig: interval is floored and garbage falls back", () => { + const floored = resolveStatuspageConfig({ + ...FULL_ENV, + STATUSPAGE_PUSH_INTERVAL_MS: "1000", + }); + assert.equal(floored.config?.intervalMs, MIN_PUSH_INTERVAL_MS); + const garbage = resolveStatuspageConfig({ + ...FULL_ENV, + STATUSPAGE_PUSH_INTERVAL_MS: "abc", + }); + assert.equal(garbage.config?.intervalMs, DEFAULT_PUSH_INTERVAL_MS); +}); + +test("resolveStatuspageConfig: a non-id value warns and stays off", () => { + const { config, warning } = resolveStatuspageConfig({ + ...FULL_ENV, + STATUSPAGE_COMPONENT_ID: "https://example.com/x", + }); + assert.equal(config, undefined); + assert.match(warning, /STATUSPAGE_COMPONENT_ID/); +}); + +test("pushComponentStatus PATCHes the component with the OAuth header", async () => { + const { config } = resolveStatuspageConfig(FULL_ENV); + /** @type {{ url: string, init: RequestInit }[]} */ + const calls = []; + await pushComponentStatus(config, "operational", async (url, init) => { + calls.push({ url, init }); + return { status: 200 }; + }); + assert.equal(calls.length, 1); + assert.equal( + calls[0].url, + `${DEFAULT_STATUSPAGE_API_BASE}/pages/kc2llmsd16bk/components/abcdef123456`, + ); + assert.equal(calls[0].init.method, "PATCH"); + assert.equal(calls[0].init.headers.authorization, "OAuth sk-test-key"); + assert.deepEqual(JSON.parse(calls[0].init.body), { + component: { status: "operational" }, + }); +}); + +test("pushComponentStatus throws on a non-200 without leaking details", async () => { + const { config } = resolveStatuspageConfig(FULL_ENV); + await assert.rejects( + pushComponentStatus(config, "operational", async () => ({ status: 401 })), + (e) => { + assert.match(e.message, /401/); + assert.doesNotMatch(e.message, /sk-test-key/); + return true; + }, + ); +}); + +test("pusher pushes on change only, and once at startup", async () => { + const { config } = resolveStatuspageConfig(FULL_ENV); + const reports = [report("pass"), report("pass"), report("warn")]; + let pushes = []; + const pusher = startStatuspagePusher({ + getReport: async () => reports.shift(), + config, + fetchImpl: async (_url, init) => { + pushes.push(JSON.parse(init.body).component.status); + return { status: 200 }; + }, + log: () => {}, + }); + pusher.stop(); + // The constructor fires the first tick; awaiting tick() joins it while it is + // in flight, so drive the sequence with three awaited ticks. + await pusher.tick(); // joins the startup tick ("pass") — first push + await pusher.tick(); // "pass" again — no push + await pusher.tick(); // "warn" — pushes the change + assert.deepEqual(pushes, ["operational", "degraded_performance"]); +}); + +test("pusher retries after a failed push instead of marking it done", async () => { + const { config } = resolveStatuspageConfig(FULL_ENV); + let apiUp = false; + const pushes = []; + const logs = []; + const pusher = startStatuspagePusher({ + getReport: async () => report("pass"), + config, + fetchImpl: async (_url, init) => { + if (!apiUp) return { status: 500 }; + pushes.push(JSON.parse(init.body).component.status); + return { status: 200 }; + }, + log: (line) => logs.push(line), + }); + pusher.stop(); + await pusher.tick(); // API still down: logged, not thrown, not recorded + assert.equal(pushes.length, 0); + assert.ok(logs.some((l) => l.includes("push failed"))); + assert.ok(!logs.some((l) => l.includes("sk-test-key"))); + apiUp = true; + await pusher.tick(); // same status as the failed attempt: must push now + assert.deepEqual(pushes, ["operational"]); +}); + +test("pusher survives a probe failure and logs it", async () => { + const { config } = resolveStatuspageConfig(FULL_ENV); + const logs = []; + const pusher = startStatuspagePusher({ + getReport: async () => { + throw new Error("probe exploded"); + }, + config, + fetchImpl: async () => ({ status: 200 }), + log: (line) => logs.push(line), + }); + pusher.stop(); + await pusher.tick(); + assert.ok(logs.some((l) => l.includes("probe exploded"))); +}); From 21ab489c22549be55ecff2839013c98680bfc77e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 11:03:10 +0000 Subject: [PATCH 02/13] Harden the Statuspage pusher: timer cap, request timeout, redact-first Three review findings, each with a regression test: - Cap the push interval below Node's signed 32-bit timer limit. setInterval coerces larger delays to 1 ms, so an uncapped oversized STATUSPAGE_PUSH_INTERVAL_MS would fire near-continuously - the exact opposite of the minimum-interval floor's intent. - Bound the Statuspage PATCH with AbortSignal.timeout (10 s). A stalled request kept the tick's in-flight promise pending forever, and since later ticks join the in-flight run, the pusher would wedge and the public component would stay stale indefinitely. - Redact the API key from error messages before truncating them. Capping first could cut the message mid-key, leaving a prefix that no longer matched the full-key replacement and so leaked into the log. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NWw9GUtUFEcTi75SnfnPi3 --- monitoring/mcp-status/statuspage.js | 40 +++++++++++++++++++++--- monitoring/mcp-status/statuspage.test.js | 34 ++++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/monitoring/mcp-status/statuspage.js b/monitoring/mcp-status/statuspage.js index 6a54751..5a48383 100644 --- a/monitoring/mcp-status/statuspage.js +++ b/monitoring/mcp-status/statuspage.js @@ -36,6 +36,22 @@ export const DEFAULT_STATUSPAGE_API_BASE = "https://api.statuspage.io/v1"; export const DEFAULT_PUSH_INTERVAL_MS = 60_000; export const MIN_PUSH_INTERVAL_MS = 15_000; +/** + * Maximum push-loop interval: Node's timers take a signed 32-bit delay, and + * setInterval coerces anything larger to 1 ms (with a TimeoutOverflowWarning) — + * so an uncapped oversized value would fire near-continuously, the exact + * opposite of what the operator asked for and of the minimum-interval floor. + */ +export const MAX_PUSH_INTERVAL_MS = 2 ** 31 - 1; + +/** + * Timeout for each Statuspage API request. Without one, a stalled request + * would keep the tick's in-flight promise pending forever — every later tick + * joins the in-flight run rather than starting a new one, so the pusher would + * wedge and the public component would stay stale indefinitely. + */ +export const PUSH_TIMEOUT_MS = 10_000; + /** * @typedef {"operational" | "degraded_performance" | "partial_outage" | "major_outage"} ComponentStatus * @@ -119,15 +135,19 @@ export const resolveStatuspageConfig = (env = process.env) => { } // Coerce the interval like resolveConfig does for the probe timeout: garbage - // falls back to the default, and a too-small value is floored rather than + // falls back to the default, a too-small value is floored rather than // honoured (each cycle can trigger a full probe run against the monitored - // server, and Statuspage rate-limits its API). + // server, and Statuspage rate-limits its API), and a too-large value is + // capped below Node's 32-bit timer limit (see MAX_PUSH_INTERVAL_MS). const rawInterval = env.STATUSPAGE_PUSH_INTERVAL_MS ? Number(env.STATUSPAGE_PUSH_INTERVAL_MS) : DEFAULT_PUSH_INTERVAL_MS; const intervalMs = Number.isFinite(rawInterval) && rawInterval > 0 - ? Math.max(rawInterval, MIN_PUSH_INTERVAL_MS) + ? Math.min( + Math.max(rawInterval, MIN_PUSH_INTERVAL_MS), + MAX_PUSH_INTERVAL_MS, + ) : DEFAULT_PUSH_INTERVAL_MS; const apiBase = ( @@ -165,6 +185,9 @@ export const pushComponentStatus = async (config, status, fetchImpl = fetch) => "content-type": "application/json", }, body: JSON.stringify({ component: { status } }), + // Bound the request like the dashboard probes bound theirs: a stalled + // PATCH must become a retryable error, not an eternally in-flight tick. + signal: AbortSignal.timeout(PUSH_TIMEOUT_MS), }); if (res.status !== 200) { throw new Error(`Statuspage API responded ${res.status}`); @@ -182,7 +205,14 @@ export const pushComponentStatus = async (config, status, fetchImpl = fetch) => * @returns {string} */ const describeError = (e, apiKey) => { - const raw = String((e && /** @type {any} */ (e).message) || e).slice(0, 300); + // Redact BEFORE truncating: capping first could cut the message in the + // middle of an embedded key, and the surviving prefix would no longer match + // the full-key replacement below. + const full = String((e && /** @type {any} */ (e).message) || e); + const raw = (apiKey ? full.split(apiKey).join("[redacted]") : full).slice( + 0, + 300, + ); let out = ""; for (const ch of raw) { const code = /** @type {number} */ (ch.codePointAt(0)); @@ -194,7 +224,7 @@ const describeError = (e, apiKey) => { code === 0x2029; out += dangerous ? " " : ch; } - return apiKey ? out.split(apiKey).join("[redacted]") : out; + return out; }; /** diff --git a/monitoring/mcp-status/statuspage.test.js b/monitoring/mcp-status/statuspage.test.js index 2a3990f..32e7b1a 100644 --- a/monitoring/mcp-status/statuspage.test.js +++ b/monitoring/mcp-status/statuspage.test.js @@ -9,6 +9,7 @@ import assert from "node:assert/strict"; import { DEFAULT_PUSH_INTERVAL_MS, DEFAULT_STATUSPAGE_API_BASE, + MAX_PUSH_INTERVAL_MS, MIN_PUSH_INTERVAL_MS, componentStatusFor, pushComponentStatus, @@ -110,6 +111,16 @@ test("resolveStatuspageConfig: interval is floored and garbage falls back", () = assert.equal(garbage.config?.intervalMs, DEFAULT_PUSH_INTERVAL_MS); }); +test("resolveStatuspageConfig: an oversized interval is capped, not overflowed", () => { + // Node's setInterval coerces delays above 2^31-1 ms to 1 ms, which would turn + // a mistyped huge interval into near-continuous probing. + const { config } = resolveStatuspageConfig({ + ...FULL_ENV, + STATUSPAGE_PUSH_INTERVAL_MS: "9999999999999", + }); + assert.equal(config?.intervalMs, MAX_PUSH_INTERVAL_MS); +}); + test("resolveStatuspageConfig: a non-id value warns and stays off", () => { const { config, warning } = resolveStatuspageConfig({ ...FULL_ENV, @@ -137,6 +148,8 @@ test("pushComponentStatus PATCHes the component with the OAuth header", async () assert.deepEqual(JSON.parse(calls[0].init.body), { component: { status: "operational" }, }); + // A stalled request must abort rather than pin the tick in flight forever. + assert.ok(calls[0].init.signal instanceof AbortSignal); }); test("pushComponentStatus throws on a non-200 without leaking details", async () => { @@ -198,6 +211,27 @@ test("pusher retries after a failed push instead of marking it done", async () = assert.deepEqual(pushes, ["operational"]); }); +test("pusher redacts the API key even when it straddles the log cap", async () => { + const { config } = resolveStatuspageConfig(FULL_ENV); + const logs = []; + const pusher = startStatuspagePusher({ + getReport: async () => { + // Place the key across the 300-character truncation boundary (chars + // 289-300): redaction must run on the full message first, or truncating + // first would leave the surviving prefix "sk-test-ke" unredacted. + throw new Error("x".repeat(289) + "sk-test-key trailing"); + }, + config, + fetchImpl: async () => ({ status: 200 }), + log: (line) => logs.push(line), + }); + pusher.stop(); + await pusher.tick(); + assert.equal(logs.length, 1); + assert.ok(!logs[0].includes("sk-t")); + assert.ok(logs[0].includes("[redacted]")); +}); + test("pusher survives a probe failure and logs it", async () => { const { config } = resolveStatuspageConfig(FULL_ENV); const logs = []; From f6247a9d4f7a97732c64efe934c12fbb231a0914 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 11:11:07 +0000 Subject: [PATCH 03/13] Key the major-outage verdict on availability checks only major_outage was unreachable in a real total outage: the every-check- failed test ran over the whole endpoints section, but that section's auxiliary checks deliberately do not fail when the server is unreachable - version and metadata-consistency degrade to "warn", and mcp-tls can stay "pass" behind a healthy reverse proxy fronting a dead application - so the verdict was always partial_outage. Compute the verdict from an explicit set of service-availability check ids instead (the endpoint probes that report "fail" on a failed request: landing page, discovery documents, the /mcp challenge, and the OAuth endpoints). Add a realistic total-outage report test in which the auxiliary checks are warn/pass and every availability probe fails, and document the distinction in the README's mapping table. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NWw9GUtUFEcTi75SnfnPi3 --- monitoring/mcp-status/README.md | 12 +++++- monitoring/mcp-status/statuspage.js | 48 ++++++++++++++++------ monitoring/mcp-status/statuspage.test.js | 52 ++++++++++++++++++++++-- 3 files changed, 95 insertions(+), 17 deletions(-) diff --git a/monitoring/mcp-status/README.md b/monitoring/mcp-status/README.md index 3c47532..31b70ab 100644 --- a/monitoring/mcp-status/README.md +++ b/monitoring/mcp-status/README.md @@ -136,8 +136,16 @@ dashboard): | ----------------- | --------------------------- | | `pass` | `operational` | | `warn` | `degraded_performance` | -| `fail`, but some MCP endpoint checks still pass | `partial_outage` | -| `fail` with every MCP endpoint check failing | `major_outage` | +| `fail`, but some service-availability checks still pass | `partial_outage` | +| `fail` with every service-availability check failing | `major_outage` | + +"Service-availability checks" are the endpoint probes that report `fail` on a +failed request (landing page, discovery documents, the `/mcp` challenge, and +the OAuth endpoints). Auxiliary checks are excluded from the major-outage +test on purpose: `version` and `metadata-consistency` only degrade to `warn` +when the server is unreachable, and the TLS check can stay green while the +application behind the proxy is down — counting them would make a total +outage report as merely partial. The pusher re-evaluates every `STATUSPAGE_PUSH_INTERVAL_MS` (default 60 s, floor 15 s), reusing the same cached probe runs as dashboard visitors, and diff --git a/monitoring/mcp-status/statuspage.js b/monitoring/mcp-status/statuspage.js index 5a48383..29d289f 100644 --- a/monitoring/mcp-status/statuspage.js +++ b/monitoring/mcp-status/statuspage.js @@ -63,16 +63,39 @@ export const PUSH_TIMEOUT_MS = 10_000; * @property {number} intervalMs */ +/** + * Check ids in the "endpoints" section that directly measure whether the MCP + * service is reachable and serving: each of these probes an endpoint and + * reports "fail" on a failed request. The section's other checks are + * deliberately excluded because they do NOT fail in a total outage — `version` + * and `metadata-consistency` degrade to "warn" when the server is unreachable, + * and `mcp-tls` inspects the TLS layer, which can stay "pass" (a healthy + * reverse proxy in front of a dead application) or degrade to "warn". Testing + * "every check failed" over the whole section would therefore be unsatisfiable + * in a real total outage, and major_outage would never be published. + */ +const AVAILABILITY_CHECK_IDS = new Set([ + "root", + "protected-resource", + "as-metadata", + "mcp-challenge", + "oauth-register", + "oauth-register-allowlist", + "oauth-authorize", + "oauth-token", +]); + /** * Map a dashboard report onto a Statuspage component status. * * "pass" and "warn" map directly. For "fail" the severity depends on where the - * failure is: when every check in the "endpoints" section failed, the MCP - * server itself is down or unreachable (major outage); any other failure — - * some endpoints broken, or a linked Internet Identity instance unhealthy — - * degrades the service without taking it out entirely (partial outage). - * Anything unrecognised also lands in the "fail" branch: a monitor should fail - * loud, not quietly report green on input it doesn't understand. + * failure is: when every service-availability check in the "endpoints" section + * failed (see AVAILABILITY_CHECK_IDS), the MCP server itself is down or + * unreachable (major outage); any other failure — some endpoints broken, or a + * linked Internet Identity instance unhealthy — degrades the service without + * taking it out entirely (partial outage). Anything unrecognised also lands in + * the "fail" branch: a monitor should fail loud, not quietly report green on + * input it doesn't understand. * * @param {import("./checks.js").DashboardReport | undefined} report * @returns {ComponentStatus} @@ -83,12 +106,13 @@ export const componentStatusFor = (report) => { const endpoints = Array.isArray(report?.sections) ? report.sections.find((s) => s?.id === "endpoints") : undefined; - const allEndpointsDown = - endpoints !== undefined && - Array.isArray(endpoints.checks) && - endpoints.checks.length > 0 && - endpoints.checks.every((c) => c?.status === "fail"); - return allEndpointsDown ? "major_outage" : "partial_outage"; + const availability = Array.isArray(endpoints?.checks) + ? endpoints.checks.filter((c) => AVAILABILITY_CHECK_IDS.has(c?.id)) + : []; + const serviceDown = + availability.length > 0 && + availability.every((c) => c.status === "fail"); + return serviceDown ? "major_outage" : "partial_outage"; }; // Page and component ids are URL path segments; Statuspage ids are plain diff --git a/monitoring/mcp-status/statuspage.test.js b/monitoring/mcp-status/statuspage.test.js index 32e7b1a..18747d4 100644 --- a/monitoring/mcp-status/statuspage.test.js +++ b/monitoring/mcp-status/statuspage.test.js @@ -17,7 +17,21 @@ import { startStatuspagePusher, } from "./statuspage.js"; -/** Minimal report with the fields the mapping reads. */ +/** + * Minimal report with the fields the mapping reads. Statuses are assigned to + * real service-availability check ids from checks.js, since the mapping keys + * the major-outage decision on that explicit set. + */ +const AVAILABILITY_IDS = [ + "root", + "protected-resource", + "as-metadata", + "mcp-challenge", + "oauth-register", + "oauth-register-allowlist", + "oauth-authorize", + "oauth-token", +]; const report = (overall, endpointStatuses = []) => ({ overall, sections: [ @@ -25,7 +39,33 @@ const report = (overall, endpointStatuses = []) => ({ id: "endpoints", title: "MCP server endpoints", status: endpointStatuses.includes("fail") ? "fail" : "pass", - checks: endpointStatuses.map((status, i) => ({ id: `c${i}`, status })), + checks: endpointStatuses.map((status, i) => ({ + id: AVAILABILITY_IDS[i] ?? `c${i}`, + status, + })), + }, + ], +}); + +/** + * What the endpoints section actually looks like when the server is entirely + * unreachable: every availability probe fails, but the auxiliary checks do NOT + * — `version` and `metadata-consistency` degrade to "warn", and `mcp-tls` can + * stay "pass" (a healthy reverse proxy fronting a dead application). + */ +const totalOutageReport = () => ({ + overall: "fail", + sections: [ + { + id: "endpoints", + title: "MCP server endpoints", + status: "fail", + checks: [ + ...AVAILABILITY_IDS.map((id) => ({ id, status: "fail" })), + { id: "version", status: "warn" }, + { id: "metadata-consistency", status: "warn" }, + { id: "mcp-tls", status: "pass" }, + ], }, ], }); @@ -48,13 +88,19 @@ test("componentStatusFor: a partial endpoint failure is a partial outage", () => ); }); -test("componentStatusFor: every endpoint check down is a major outage", () => { +test("componentStatusFor: every availability check down is a major outage", () => { assert.equal( componentStatusFor(report("fail", ["fail", "fail", "fail"])), "major_outage", ); }); +test("componentStatusFor: a realistic total outage is a major outage", () => { + // Auxiliary checks (version, metadata-consistency, mcp-tls) do not fail when + // the server is unreachable; they must not veto the major-outage verdict. + assert.equal(componentStatusFor(totalOutageReport()), "major_outage"); +}); + test("componentStatusFor: a non-endpoint failure is a partial outage", () => { // Overall "fail" caused by another section (e.g. linked II unhealthy): // the endpoints section itself is green. From 9b64c90eb7d26b796e39de505f3418dfe92e08d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 11:15:25 +0000 Subject: [PATCH 04/13] Cancel the Statuspage response body after each push Node's fetch (Undici) keeps the underlying connection associated with an unread response body until garbage collection. The pusher never reads the body, and a persistent error response - exactly the case that is retried every interval - would therefore accumulate connections over time. Cancel the body before checking the status, on success and error alike; its content stays out of thrown errors as before, since it is under the remote service's control. Regression test asserts the body is cancelled on both a 200 and an error response. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NWw9GUtUFEcTi75SnfnPi3 --- monitoring/mcp-status/statuspage.js | 10 +++++++++ monitoring/mcp-status/statuspage.test.js | 27 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/monitoring/mcp-status/statuspage.js b/monitoring/mcp-status/statuspage.js index 29d289f..3681ce5 100644 --- a/monitoring/mcp-status/statuspage.js +++ b/monitoring/mcp-status/statuspage.js @@ -213,6 +213,16 @@ export const pushComponentStatus = async (config, status, fetchImpl = fetch) => // PATCH must become a retryable error, not an eternally in-flight tick. signal: AbortSignal.timeout(PUSH_TIMEOUT_MS), }); + // Discard the response body without reading it: Node's fetch (Undici) keeps + // the underlying connection associated with an unread body until garbage + // collection, so a persistent error response retried every interval would + // otherwise accumulate connections. The body is never used — and stays out + // of thrown errors, since its content is under the remote service's control. + try { + await res.body?.cancel(); + } catch { + // An already-consumed or errored body changes nothing about the outcome. + } if (res.status !== 200) { throw new Error(`Statuspage API responded ${res.status}`); } diff --git a/monitoring/mcp-status/statuspage.test.js b/monitoring/mcp-status/statuspage.test.js index 18747d4..938bbea 100644 --- a/monitoring/mcp-status/statuspage.test.js +++ b/monitoring/mcp-status/statuspage.test.js @@ -198,6 +198,33 @@ test("pushComponentStatus PATCHes the component with the OAuth header", async () assert.ok(calls[0].init.signal instanceof AbortSignal); }); +test("pushComponentStatus discards the response body on success and error", async () => { + // Node's fetch keeps the connection tied to an unread body until GC, so the + // pusher must cancel it — including on error responses, which are the ones + // that would be retried (and so accumulate) every interval. + const { config } = resolveStatuspageConfig(FULL_ENV); + const bodyRes = (status) => { + const res = { + status, + cancelled: 0, + body: { + cancel: async () => { + res.cancelled += 1; + }, + }, + }; + return res; + }; + const ok = bodyRes(200); + await pushComponentStatus(config, "operational", async () => ok); + assert.equal(ok.cancelled, 1); + const err = bodyRes(500); + await assert.rejects( + pushComponentStatus(config, "operational", async () => err), + ); + assert.equal(err.cancelled, 1); +}); + test("pushComponentStatus throws on a non-200 without leaking details", async () => { const { config } = resolveStatuspageConfig(FULL_ENV); await assert.rejects( From 3f25245320e9dcaf604e99169cb97a85ef2360e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 11:18:03 +0000 Subject: [PATCH 05/13] Accept any 2xx from the Statuspage API Review feedback: checking res.status !== 200 was stricter than needed. Use res.ok instead - the API returns 200 today, but a successful push should not depend on that staying exact. Test stubs now carry ok like a real Response, and a new test asserts a 204 is accepted. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NWw9GUtUFEcTi75SnfnPi3 --- monitoring/mcp-status/statuspage.js | 4 ++- monitoring/mcp-status/statuspage.test.js | 39 +++++++++++++++--------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/monitoring/mcp-status/statuspage.js b/monitoring/mcp-status/statuspage.js index 3681ce5..92b7cdc 100644 --- a/monitoring/mcp-status/statuspage.js +++ b/monitoring/mcp-status/statuspage.js @@ -223,7 +223,9 @@ export const pushComponentStatus = async (config, status, fetchImpl = fetch) => } catch { // An already-consumed or errored body changes nothing about the outcome. } - if (res.status !== 200) { + // Any 2xx counts as success — the API returns 200 today, but a successful + // push must not depend on that staying exact. + if (!res.ok) { throw new Error(`Statuspage API responded ${res.status}`); } }; diff --git a/monitoring/mcp-status/statuspage.test.js b/monitoring/mcp-status/statuspage.test.js index 938bbea..d574649 100644 --- a/monitoring/mcp-status/statuspage.test.js +++ b/monitoring/mcp-status/statuspage.test.js @@ -76,6 +76,13 @@ const FULL_ENV = { STATUSPAGE_COMPONENT_ID: "abcdef123456", }; +/** Response-like stub carrying the fields pushComponentStatus reads. */ +const httpRes = (status, extra = {}) => ({ + status, + ok: status >= 200 && status < 300, + ...extra, +}); + test("componentStatusFor maps the overall verdict", () => { assert.equal(componentStatusFor(report("pass")), "operational"); assert.equal(componentStatusFor(report("warn")), "degraded_performance"); @@ -182,7 +189,7 @@ test("pushComponentStatus PATCHes the component with the OAuth header", async () const calls = []; await pushComponentStatus(config, "operational", async (url, init) => { calls.push({ url, init }); - return { status: 200 }; + return httpRes(200); }); assert.equal(calls.length, 1); assert.equal( @@ -204,13 +211,10 @@ test("pushComponentStatus discards the response body on success and error", asyn // that would be retried (and so accumulate) every interval. const { config } = resolveStatuspageConfig(FULL_ENV); const bodyRes = (status) => { - const res = { - status, - cancelled: 0, - body: { - cancel: async () => { - res.cancelled += 1; - }, + const res = httpRes(status, { cancelled: 0 }); + res.body = { + cancel: async () => { + res.cancelled += 1; }, }; return res; @@ -225,10 +229,15 @@ test("pushComponentStatus discards the response body on success and error", asyn assert.equal(err.cancelled, 1); }); -test("pushComponentStatus throws on a non-200 without leaking details", async () => { +test("pushComponentStatus accepts any 2xx, not just 200", async () => { + const { config } = resolveStatuspageConfig(FULL_ENV); + await pushComponentStatus(config, "operational", async () => httpRes(204)); +}); + +test("pushComponentStatus throws on a non-2xx without leaking details", async () => { const { config } = resolveStatuspageConfig(FULL_ENV); await assert.rejects( - pushComponentStatus(config, "operational", async () => ({ status: 401 })), + pushComponentStatus(config, "operational", async () => httpRes(401)), (e) => { assert.match(e.message, /401/); assert.doesNotMatch(e.message, /sk-test-key/); @@ -246,7 +255,7 @@ test("pusher pushes on change only, and once at startup", async () => { config, fetchImpl: async (_url, init) => { pushes.push(JSON.parse(init.body).component.status); - return { status: 200 }; + return httpRes(200); }, log: () => {}, }); @@ -268,9 +277,9 @@ test("pusher retries after a failed push instead of marking it done", async () = getReport: async () => report("pass"), config, fetchImpl: async (_url, init) => { - if (!apiUp) return { status: 500 }; + if (!apiUp) return httpRes(500); pushes.push(JSON.parse(init.body).component.status); - return { status: 200 }; + return httpRes(200); }, log: (line) => logs.push(line), }); @@ -295,7 +304,7 @@ test("pusher redacts the API key even when it straddles the log cap", async () = throw new Error("x".repeat(289) + "sk-test-key trailing"); }, config, - fetchImpl: async () => ({ status: 200 }), + fetchImpl: async () => httpRes(200), log: (line) => logs.push(line), }); pusher.stop(); @@ -313,7 +322,7 @@ test("pusher survives a probe failure and logs it", async () => { throw new Error("probe exploded"); }, config, - fetchImpl: async () => ({ status: 200 }), + fetchImpl: async () => httpRes(200), log: (line) => logs.push(line), }); pusher.stop(); From 4dcb51bd066abd71c69a2ad2a86f17fd2fd86c74 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 11:25:48 +0000 Subject: [PATCH 06/13] Keep the periodic pusher from minting OAuth clients The pusher ran the full probe suite every interval, and the suite's Dynamic Client Registration check mints and persists a client_id on the monitored server per run - about 1,440 junk registrations a day at the default 60 s interval, cycling the server's 10,000-entry LRU client store in under a week and churning its persistence. - checks.js: checkMcpEndpoints (and runDashboard) accept mutating:false, which skips only the loopback DCR probe - the one check that changes server state. The allow-list probe stays in both modes: its non-allow-listed redirect is rejected before anything is stored. - server.js: the pusher reuses a recent visitor-triggered full report when one exists (already paid for) and otherwise probes with mutating:false. The non-mutating report is not stored in the visitor cache, which promises the full suite. - checks.test.js: non-mutating mode sends no loopback registration, omits the oauth-register check, and still runs the allow-list probe. - README: document that the pusher's own runs are non-mutating and that DCR health is still verified by interactive visits and CLI runs. The verdict mapping is unaffected: it already computes major_outage from the availability checks present in the report. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NWw9GUtUFEcTi75SnfnPi3 --- monitoring/mcp-status/README.md | 20 ++++++--- monitoring/mcp-status/checks.js | 27 ++++++++++-- monitoring/mcp-status/checks.test.js | 65 ++++++++++++++++++++++++++++ monitoring/mcp-status/server.js | 36 +++++++++++++-- 4 files changed, 136 insertions(+), 12 deletions(-) diff --git a/monitoring/mcp-status/README.md b/monitoring/mcp-status/README.md index 31b70ab..bd931fa 100644 --- a/monitoring/mcp-status/README.md +++ b/monitoring/mcp-status/README.md @@ -148,11 +148,21 @@ application behind the proxy is down — counting them would make a total outage report as merely partial. The pusher re-evaluates every `STATUSPAGE_PUSH_INTERVAL_MS` (default 60 s, -floor 15 s), reusing the same cached probe runs as dashboard visitors, and -PATCHes the component **only when the mapped status changes** (plus once at -startup, since a restarted process cannot know the remote state) — so a healthy -steady state costs one API call per process lifetime. A failed push is logged -and retried on the next interval; it never affects the dashboard itself. +floor 15 s) and PATCHes the component **only when the mapped status changes** +(plus once at startup, since a restarted process cannot know the remote state) +— so a healthy steady state costs one API call per process lifetime. A failed +push is logged and retried on the next interval; it never affects the dashboard +itself. + +Unlike visitor-triggered probe runs, the pusher's own runs are +**non-mutating**: it reuses a recent visitor-triggered report when one exists, +and otherwise probes with the loopback Dynamic Client Registration check +skipped — that is the one probe that changes server state (each run mints and +persists a client_id in the server's LRU-bounded registration store), and an +unattended periodic loop must not grind through that store. The allow-list +check still runs (its non-allow-listed redirect is rejected before anything is +stored), and DCR health is still verified by every interactive dashboard visit +and CLI run. ## Why a standalone tool (and not a page in the II frontend)? diff --git a/monitoring/mcp-status/checks.js b/monitoring/mcp-status/checks.js index e1c8241..d79f987 100644 --- a/monitoring/mcp-status/checks.js +++ b/monitoring/mcp-status/checks.js @@ -176,9 +176,21 @@ export const parseCspDirective = (csp, directive) => { /** * @param {string} mcpOrigin * @param {number} timeoutMs + * @param {{ mutating?: boolean }} [opts] With `mutating: false`, skip the one + * check that changes server state — the loopback Dynamic Client Registration + * probe, which mints and persists a fresh client_id in the server's + * LRU-bounded registration store on every run. Interactive uses (a person + * loading the dashboard, a one-off CLI run) keep the default; unattended + * periodic callers (the Statuspage pusher) must not grind through that store. + * The allow-list check stays in both modes: its non-allow-listed redirect is + * rejected before anything is stored, so it never mutates. * @returns {Promise<{ section: Section, facts: Record }>} */ -export const checkMcpEndpoints = async (mcpOrigin, timeoutMs) => { +export const checkMcpEndpoints = async ( + mcpOrigin, + timeoutMs, + { mutating = true } = {}, +) => { /** @type {CheckResult[]} */ const checks = []; /** @type {Record} */ @@ -397,7 +409,10 @@ export const checkMcpEndpoints = async (mcpOrigin, timeoutMs) => { // with a LOOPBACK redirect: loopback is always permitted (the hosted-redirect // allow-list exempts it), so this is the dependency-free way to confirm a // client can self-register without manual approval, which is what DCR is for. - { + // Skipped in non-mutating mode: this is the one probe that changes server + // state (the minted client_id is persisted in the LRU-bounded registration + // store), so an unattended periodic caller must not run it. + if (mutating) { const url = `${mcpOrigin}/mcp/oauth/register`; const r = await probe(url, { timeoutMs, @@ -1003,13 +1018,17 @@ export const buildSuggestions = (sections, facts) => { /** * Run the full dashboard against the resolved configuration. - * @param {{ mcpOrigin?: string, iiOrigin?: string, timeoutMs?: number }} [overrides] + * @param {{ mcpOrigin?: string, iiOrigin?: string, timeoutMs?: number, mutating?: boolean }} [overrides] + * `mutating: false` skips the one probe that changes server state (the + * loopback Dynamic Client Registration check) — see checkMcpEndpoints. * @returns {Promise} */ export const runDashboard = async (overrides = {}) => { const cfg = resolveConfig(overrides); - const endpoints = await checkMcpEndpoints(cfg.mcpOrigin, cfg.timeoutMs); + const endpoints = await checkMcpEndpoints(cfg.mcpOrigin, cfg.timeoutMs, { + mutating: overrides.mutating !== false, + }); // Which IIs to probe. Normally the ones the server advertises at /version — // it is the authority on its own pairing, and it lists every mount, so both a diff --git a/monitoring/mcp-status/checks.test.js b/monitoring/mcp-status/checks.test.js index 5e8abff..209a4a1 100644 --- a/monitoring/mcp-status/checks.test.js +++ b/monitoring/mcp-status/checks.test.js @@ -303,6 +303,71 @@ test("checkMcpEndpoints passes for a well-behaved server", async () => { } }); +test("checkMcpEndpoints in non-mutating mode never registers a client", async () => { + const origin = "https://mcp.beta.test"; + /** @type {string[]} */ + const registerRedirects = []; + const restore = stubFetch({ + [`GET ${origin}/`]: resp(200, { headers: { "content-type": "text/html" } }), + [`GET ${origin}/version`]: resp(200, { + headers: { "content-type": "application/json" }, + body: JSON.stringify({ version: "0.1.0", commit: "abc123def4567890" }), + }), + [`GET ${origin}/.well-known/oauth-protected-resource`]: resp(200, { + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + authorization_servers: [`${origin}/mcp`], + resource: `${origin}/mcp`, + }), + }), + [`GET ${origin}/.well-known/oauth-authorization-server`]: resp(200, { + body: JSON.stringify({ + issuer: `${origin}/mcp`, + authorization_endpoint: `${origin}/mcp/oauth/authorize`, + token_endpoint: `${origin}/mcp/oauth/token`, + registration_endpoint: `${origin}/mcp/oauth/register`, + code_challenge_methods_supported: ["S256"], + }), + }), + [`POST ${origin}/mcp`]: resp(401, { + headers: { + "www-authenticate": `Bearer resource_metadata="${origin}/.well-known/oauth-protected-resource/mcp"`, + }, + body: JSON.stringify({ error: "invalid_token" }), + }), + [`POST ${origin}/mcp/oauth/register`]: (init) => { + registerRedirects.push( + (JSON.parse(init.body ?? "{}").redirect_uris ?? [])[0] ?? "", + ); + return registerRoute(init); + }, + [`GET ${origin}/mcp/oauth/authorize`]: resp(400, { + body: "missing client_id", + }), + [`POST ${origin}/mcp/oauth/token`]: resp(400, { + body: JSON.stringify({ error: "invalid_grant" }), + }), + }); + try { + const { section } = await checkMcpEndpoints(origin, 2000, { + mutating: false, + }); + // The state-mutating loopback DCR probe is skipped entirely... + assert.equal(byId(section, "oauth-register"), undefined); + // ...while the allow-list probe (rejected before anything is stored) and + // the rest of the suite still run. + assert.equal(byId(section, "oauth-register-allowlist").status, "pass"); + assert.equal(byId(section, "root").status, "pass"); + assert.equal(registerRedirects.length, 1); + assert.ok( + !registerRedirects[0].includes("127.0.0.1"), + "no loopback (minting) registration may be sent in non-mutating mode", + ); + } finally { + restore(); + } +}); + test("checkMcpEndpoints flags a missing OAuth challenge", async () => { const origin = "https://mcp.beta.test"; const restore = stubFetch({ diff --git a/monitoring/mcp-status/server.js b/monitoring/mcp-status/server.js index 6ec9339..be8954a 100644 --- a/monitoring/mcp-status/server.js +++ b/monitoring/mcp-status/server.js @@ -88,6 +88,32 @@ const getReport = (force = false) => { return inFlight; }; +// Report source for the periodic Statuspage pusher. Unlike visitor-triggered +// probes, this runs unattended around the clock, and the full suite's Dynamic +// Client Registration check mints and persists a client_id on the monitored +// server per run — a 60 s loop would grind ~1,440 junk registrations a day +// through the server's LRU-bounded client store. So: reuse a recent +// visitor-triggered full report when one exists (it was already paid for), and +// otherwise probe with `mutating: false`, which skips only the DCR check. The +// non-mutating report is deliberately NOT stored in the visitor cache — it is +// missing that check, and /api/status promises the full suite. +/** @type {Promise | null} */ +let pusherInFlight = null; +/** @param {number} maxAgeMs how recent a full report must be to be reused. */ +const getPusherReport = (maxAgeMs) => { + if (cache.report && Date.now() - cache.at < maxAgeMs) { + return Promise.resolve(cache.report); + } + if (!pusherInFlight) { + pusherInFlight = runDashboard({ ...defaults, mutating: false }).finally( + () => { + pusherInFlight = null; + }, + ); + } + return pusherInFlight; +}; + const sendJson = (res, code, body) => { const payload = JSON.stringify(body); res.writeHead(code, { @@ -170,13 +196,17 @@ server.listen(port, host, () => { // Optionally mirror the verdict to an Atlassian Statuspage component (e.g. on // status.internetcomputer.org). Off unless the STATUSPAGE_* variables are set — -// see statuspage.js and the README. The pusher shares getReport's cache with -// dashboard visitors, so it adds at most one probe run per interval. +// see statuspage.js and the README. The pusher reuses a recent visitor-triggered +// report when one exists and otherwise probes without the state-mutating DCR +// check (see getPusherReport), so it never registers OAuth clients on its own. const { config: statuspageConfig, warning: statuspageWarning } = resolveStatuspageConfig(); if (statuspageWarning) console.error(statuspageWarning); if (statuspageConfig) { - startStatuspagePusher({ getReport, config: statuspageConfig }); + startStatuspagePusher({ + getReport: () => getPusherReport(statuspageConfig.intervalMs), + config: statuspageConfig, + }); process.stdout.write( ` statuspage: pushing to component ${statuspageConfig.componentId} ` + `(page ${statuspageConfig.pageId}) every ${statuspageConfig.intervalMs} ms\n`, From e3105b8be941b330d672da8337d09b0017a67e41 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 11:28:18 +0000 Subject: [PATCH 07/13] Isolate the dashboard's UID and fix the redaction test boundary Two review findings: - Run imcp-status under a dedicated system user (created by deploy.sh) instead of the shared ec2-user. The dashboard process can carry a Statuspage API key in its environment, and same-UID processes can read each other's /proc//environ, so sharing a user with the internet-facing imcp2 service would let a compromise of the main service recover the secret despite the root-only env file. The shipped files under /opt/imcp2 are world-readable, so no ownership changes are needed. - Fix an off-by-one in the redaction regression test: with 289 prefix characters the 11-character key occupied indices 289-299 and survived slice(0, 300) whole, so the vulnerable truncate-before-redact ordering passed the test too. With 290 the key straddles the cap and only an unmatchable prefix would survive, which the test now catches. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NWw9GUtUFEcTi75SnfnPi3 --- deploy/native/deploy.sh | 4 ++++ deploy/native/imcp-status.service | 8 +++++++- monitoring/mcp-status/statuspage.test.js | 10 ++++++---- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/deploy/native/deploy.sh b/deploy/native/deploy.sh index 630abc5..35152ca 100755 --- a/deploy/native/deploy.sh +++ b/deploy/native/deploy.sh @@ -106,6 +106,10 @@ node_major="\$(node -v 2>/dev/null | cut -c2- | cut -d. -f1)" if [ -z "\$node_major" ] || [ "\$node_major" -lt 20 ] 2>/dev/null; then dnf install -y -q nodejs20 >/dev/null 2>&1 || dnf install -y -q nodejs >/dev/null 2>&1 || true fi +# Dedicated UID for the dashboard: it can carry a Statuspage API key in its +# environment, so it must not share a user with the internet-facing imcp2 +# service (same-UID processes can read each other's /proc//environ). +id imcp-status >/dev/null 2>&1 || useradd --system --no-create-home --shell /sbin/nologin imcp-status cat > /etc/systemd/system/imcp-status.service <<'UNIT' $unit_status UNIT diff --git a/deploy/native/imcp-status.service b/deploy/native/imcp-status.service index 3c7438a..bd9002f 100644 --- a/deploy/native/imcp-status.service +++ b/deploy/native/imcp-status.service @@ -5,7 +5,13 @@ Wants=network-online.target [Service] Type=simple -User=ec2-user +# Dedicated system user (created by deploy.sh), NOT the ec2-user the +# internet-facing imcp2.service runs as: this process can carry a Statuspage +# API key in its environment, and same-UID processes can read each other's +# /proc//environ — a compromise of the main service must not be able to +# recover that secret. The shipped files under /opt/imcp2 are world-readable, +# so no ownership changes are needed. +User=imcp-status WorkingDirectory=/opt/imcp2/monitoring/mcp-status # Probe the public deployment end-to-end (through Caddy/TLS). Which Internet # Identity instances to probe is read from this server's own /version, so no II diff --git a/monitoring/mcp-status/statuspage.test.js b/monitoring/mcp-status/statuspage.test.js index d574649..e8ea7df 100644 --- a/monitoring/mcp-status/statuspage.test.js +++ b/monitoring/mcp-status/statuspage.test.js @@ -298,10 +298,12 @@ test("pusher redacts the API key even when it straddles the log cap", async () = const logs = []; const pusher = startStatuspagePusher({ getReport: async () => { - // Place the key across the 300-character truncation boundary (chars - // 289-300): redaction must run on the full message first, or truncating - // first would leave the surviving prefix "sk-test-ke" unredacted. - throw new Error("x".repeat(289) + "sk-test-key trailing"); + // Place the key across the 300-character truncation boundary: with 290 + // prefix characters the 11-character key occupies indices 290-300, so + // truncating BEFORE redacting would leave the unmatchable 10-character + // prefix "sk-test-ke" in the log. (289 would let the whole key fit + // inside slice(0, 300), and the vulnerable ordering would pass too.) + throw new Error("x".repeat(290) + "sk-test-key trailing"); }, config, fetchImpl: async () => httpRes(200), From f4a45045321bb654bc7ff5512e534f332b63a10f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 11:33:37 +0000 Subject: [PATCH 08/13] Share the log sanitiser and tighten the pusher's report reuse Three review findings: - Factor sanitizeForLog out of server.js into log.js and reuse it from statuspage.js's describeError, which carried a near-identical copy. describeError still redacts the API key on the full message BEFORE the shared sanitiser truncates it. Small log.test.js covers the sanitiser directly. - Bound the pusher's report reuse by the dashboard cache TTL as well as the push interval. Reusing anything /api/status already considers expired could let a tick publish stale state, delaying an outage by up to two intervals. - Join a visitor-triggered full probe run that is already in flight instead of starting a second concurrent suite; the full report satisfies the pusher, and probe load stays coalesced. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NWw9GUtUFEcTi75SnfnPi3 --- monitoring/mcp-status/log.js | 34 ++++++++++++++++++++++++ monitoring/mcp-status/log.test.js | 41 +++++++++++++++++++++++++++++ monitoring/mcp-status/server.js | 32 +++++++--------------- monitoring/mcp-status/statuspage.js | 30 +++++---------------- 4 files changed, 92 insertions(+), 45 deletions(-) create mode 100644 monitoring/mcp-status/log.js create mode 100644 monitoring/mcp-status/log.test.js diff --git a/monitoring/mcp-status/log.js b/monitoring/mcp-status/log.js new file mode 100644 index 0000000..86ac82b --- /dev/null +++ b/monitoring/mcp-status/log.js @@ -0,0 +1,34 @@ +// Shared log sanitisation for the IMCP status dashboard. +// +// Extracted from server.js so statuspage.js can reuse it instead of carrying +// a near-identical copy. + +/** + * Reduce a value (an Error or anything else) to a single safe log line: + * message only (no stack), capped in length, with control characters replaced + * by spaces so that a logged error message can never forge or inject + * additional log entries. Covers C0 controls + DEL, the C1 controls + * (U+0080–U+009F, incl. U+009B the 8-bit CSI), and the Unicode line/paragraph + * separators (U+2028/U+2029). Implemented with a codepoint filter to avoid + * embedding control-char literals. + * + * @param {unknown} value + * @returns {string} + */ +export const sanitizeForLog = (value) => { + const input = String( + (value && /** @type {any} */ (value).message) || value, + ).slice(0, 300); + let out = ""; + for (const ch of input) { + const code = /** @type {number} */ (ch.codePointAt(0)); + const dangerous = + code < 0x20 || + code === 0x7f || + (code >= 0x80 && code <= 0x9f) || + code === 0x2028 || + code === 0x2029; + out += dangerous ? " " : ch; + } + return out; +}; diff --git a/monitoring/mcp-status/log.test.js b/monitoring/mcp-status/log.test.js new file mode 100644 index 0000000..0c4795c --- /dev/null +++ b/monitoring/mcp-status/log.test.js @@ -0,0 +1,41 @@ +// Unit tests for the shared log sanitiser. +// Run with: node --test monitoring/mcp-status/log.test.js +// or: cd monitoring/mcp-status && npm test + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { sanitizeForLog } from "./log.js"; + +test("sanitizeForLog strips control characters and separators", () => { + // C0 (LF, CR, ESC), DEL, a C1 control (the 8-bit CSI), and the Unicode line + // and paragraph separators - each must become a plain space. Built from + // codepoints so the test source embeds no control-char literals (same + // rationale as the implementation). + const ctl = (cp) => String.fromCodePoint(cp); + const input = [ + "a", + ctl(0x0a), + "b", + ctl(0x0d), + "c", + ctl(0x1b), + "d", + ctl(0x7f), + "e", + ctl(0x9b), + "f", + ctl(0x2028), + "g", + ctl(0x2029), + "h", + ].join(""); + assert.equal(sanitizeForLog(input), "a b c d e f g h"); +}); + +test("sanitizeForLog caps the length at 300 characters", () => { + assert.equal(sanitizeForLog("x".repeat(500)).length, 300); +}); + +test("sanitizeForLog prefers an Error's message and never its stack", () => { + assert.equal(sanitizeForLog(new Error("boom")), "boom"); +}); diff --git a/monitoring/mcp-status/server.js b/monitoring/mcp-status/server.js index be8954a..8bf7f70 100644 --- a/monitoring/mcp-status/server.js +++ b/monitoring/mcp-status/server.js @@ -22,6 +22,7 @@ import { readFile } from "node:fs/promises"; import { fileURLToPath } from "node:url"; import { dirname, join } from "node:path"; import { runDashboard } from "./checks.js"; +import { sanitizeForLog } from "./log.js"; import { resolveStatuspageConfig, startStatuspagePusher, @@ -101,9 +102,17 @@ const getReport = (force = false) => { let pusherInFlight = null; /** @param {number} maxAgeMs how recent a full report must be to be reused. */ const getPusherReport = (maxAgeMs) => { - if (cache.report && Date.now() - cache.at < maxAgeMs) { + // Bound reuse by the dashboard cache TTL as well as the push interval: + // reusing a report /api/status already considers expired would let a tick + // publish stale state, delaying an outage by up to two intervals. + const maxAge = Math.min(maxAgeMs, CACHE_TTL_MS); + if (cache.report && Date.now() - cache.at < maxAge) { return Promise.resolve(cache.report); } + // A visitor-triggered full run that is already in flight satisfies the + // pusher too — join it rather than doubling the probe load with a second + // concurrent suite. + if (inFlight) return inFlight; if (!pusherInFlight) { pusherInFlight = runDashboard({ ...defaults, mutating: false }).finally( () => { @@ -124,27 +133,6 @@ const sendJson = (res, code, body) => { res.end(payload); }; -// Replace control characters with spaces and cap the length, so that a logged -// error message can never forge or inject additional log entries. Covers C0 -// controls + DEL, the C1 controls (U+0080–U+009F, incl. U+009B the 8-bit CSI), -// and the Unicode line/paragraph separators (U+2028/U+2029). Implemented with a -// codepoint filter to avoid embedding control-char literals. -const sanitizeForLog = (value) => { - const input = String((value && value.message) || value).slice(0, 300); - let out = ""; - for (const ch of input) { - const code = ch.codePointAt(0); - const dangerous = - code < 0x20 || - code === 0x7f || - (code >= 0x80 && code <= 0x9f) || - code === 0x2028 || - code === 0x2029; - out += dangerous ? " " : ch; - } - return out; -}; - const server = http.createServer(async (req, res) => { try { const url = new URL(req.url ?? "/", "http://localhost"); diff --git a/monitoring/mcp-status/statuspage.js b/monitoring/mcp-status/statuspage.js index 92b7cdc..29bf9b3 100644 --- a/monitoring/mcp-status/statuspage.js +++ b/monitoring/mcp-status/statuspage.js @@ -29,6 +29,8 @@ // STATUSPAGE_API_BASE API base URL override — for tests and mocks // only (default https://api.statuspage.io/v1). +import { sanitizeForLog } from "./log.js"; + /** Default Statuspage REST API base URL. */ export const DEFAULT_STATUSPAGE_API_BASE = "https://api.statuspage.io/v1"; @@ -231,36 +233,18 @@ export const pushComponentStatus = async (config, status, fetchImpl = fetch) => }; /** - * Reduce an error to a single safe log line: message only (no stack), control - * characters stripped (so a network error cannot forge log entries — same - * rationale as server.js's sanitiser), capped in length, and with the API key - * redacted should it ever leak into a message. + * Reduce an error to a single safe log line: the API key is redacted first — + * BEFORE the shared sanitiser truncates, since capping first could cut the + * message in the middle of an embedded key and the surviving prefix would no + * longer match the full-key replacement. * * @param {unknown} e * @param {string} apiKey * @returns {string} */ const describeError = (e, apiKey) => { - // Redact BEFORE truncating: capping first could cut the message in the - // middle of an embedded key, and the surviving prefix would no longer match - // the full-key replacement below. const full = String((e && /** @type {any} */ (e).message) || e); - const raw = (apiKey ? full.split(apiKey).join("[redacted]") : full).slice( - 0, - 300, - ); - let out = ""; - for (const ch of raw) { - const code = /** @type {number} */ (ch.codePointAt(0)); - const dangerous = - code < 0x20 || - code === 0x7f || - (code >= 0x80 && code <= 0x9f) || - code === 0x2028 || - code === 0x2029; - out += dangerous ? " " : ch; - } - return out; + return sanitizeForLog(apiKey ? full.split(apiKey).join("[redacted]") : full); }; /** From 7bdd6eff796ed07592800c996f42793e413f8082 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 11:36:17 +0000 Subject: [PATCH 09/13] Run the dashboard from a root-owned copy of its source The dedicated imcp-status UID alone did not isolate the Statuspage API key: the dashboard source was staged under /opt/imcp2 owned by the deploy/SSH user, the same UID the internet-facing imcp2.service runs as, so a compromised main service could edit server.js and have the next dashboard restart execute its code in the secret-bearing service. deploy.sh now installs a root-owned, non-group/other-writable copy of monitoring/mcp-status at /opt/imcp-status on every deploy, and the unit executes that copy. The staged tree under /opt/imcp2/monitoring remains the shipping vehicle only. Deploy README's layout diagram updated. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NWw9GUtUFEcTi75SnfnPi3 --- deploy/native/README.md | 5 ++++- deploy/native/deploy.sh | 11 +++++++++++ deploy/native/imcp-status.service | 10 ++++++---- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/deploy/native/README.md b/deploy/native/README.md index 984a268..191fc56 100644 --- a/deploy/native/README.md +++ b/deploy/native/README.md @@ -8,7 +8,10 @@ managed VPC) and you just want to put the app on it. The repo's ``` build.sh ─────► build-out/imcp2 (cross-built linux/arm64 or amd64 binary) deploy.sh ─────► /opt/imcp2/{imcp2,static} + systemd: imcp2.service - /opt/imcp2/monitoring + systemd: imcp-status.service (dashboard) + /opt/imcp2/monitoring (staging copy of the dashboard source) + /opt/imcp-status + systemd: imcp-status.service (dashboard; + root-owned executable copy, run as the + dedicated imcp-status user) /usr/local/bin/caddy + systemd: caddy.service (TLS) ``` diff --git a/deploy/native/deploy.sh b/deploy/native/deploy.sh index 35152ca..ae66dac 100755 --- a/deploy/native/deploy.sh +++ b/deploy/native/deploy.sh @@ -110,6 +110,17 @@ fi # environment, so it must not share a user with the internet-facing imcp2 # service (same-UID processes can read each other's /proc//environ). id imcp-status >/dev/null 2>&1 || useradd --system --no-create-home --shell /sbin/nologin imcp-status +# Execute a ROOT-OWNED copy of the dashboard source, not the staged copy under +# $REMOTE_DIR: the staging tree is owned (and writable) by the deploy/SSH user, +# which is also the UID the internet-facing imcp2.service runs as — a +# compromised main service could edit server.js there and have the next +# dashboard restart run its code in the secret-bearing service. Refresh the +# copy on every deploy; strip group/other write (files stay world-readable, so +# the unprivileged imcp-status user can run them). +rm -rf /opt/imcp-status +cp -a $REMOTE_DIR/monitoring/mcp-status /opt/imcp-status +chown -R root:root /opt/imcp-status +chmod -R go-w /opt/imcp-status cat > /etc/systemd/system/imcp-status.service <<'UNIT' $unit_status UNIT diff --git a/deploy/native/imcp-status.service b/deploy/native/imcp-status.service index bd9002f..c92bc25 100644 --- a/deploy/native/imcp-status.service +++ b/deploy/native/imcp-status.service @@ -9,10 +9,12 @@ Type=simple # internet-facing imcp2.service runs as: this process can carry a Statuspage # API key in its environment, and same-UID processes can read each other's # /proc//environ — a compromise of the main service must not be able to -# recover that secret. The shipped files under /opt/imcp2 are world-readable, -# so no ownership changes are needed. +# recover that secret. For the same reason the unit executes the ROOT-OWNED +# copy deploy.sh installs at /opt/imcp-status, not the ec2-user-writable +# staging tree under /opt/imcp2/monitoring: code writable by the main +# service's UID would hand the secret to whoever can modify it. User=imcp-status -WorkingDirectory=/opt/imcp2/monitoring/mcp-status +WorkingDirectory=/opt/imcp-status # Probe the public deployment end-to-end (through Caddy/TLS). Which Internet # Identity instances to probe is read from this server's own /version, so no II # origin is configured here. The allowlist is pinned to the deployment's own host @@ -27,7 +29,7 @@ Environment=MCP_STATUS_ALLOWED_HOSTS=__ALLOWED_HOSTS__ # the API key stays out of this repository and of the rendered unit file. The # leading "-" makes it optional: without the file the pusher never starts. EnvironmentFile=-/etc/imcp-status/statuspage.env -ExecStart=/usr/bin/node /opt/imcp2/monitoring/mcp-status/server.js --port 8137 --host 127.0.0.1 --mcp https://__DOMAIN__ +ExecStart=/usr/bin/node /opt/imcp-status/server.js --port 8137 --host 127.0.0.1 --mcp https://__DOMAIN__ Restart=always RestartSec=5 LimitNOFILE=65536 From 378fe89f5cd4ac19364e898f6a0b1a19241e7850 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 11:39:16 +0000 Subject: [PATCH 10/13] Diagnose a Statuspage config that sets only optional variables resolveStatuspageConfig stayed silent whenever the three required variables were absent, even when an optional one ( STATUSPAGE_PUSH_INTERVAL_MS or STATUSPAGE_API_BASE) was set - so a mistyped deployment intending to run the pusher got a silent no-op instead of a diagnosis, contradicting the documented contract that only a completely unset configuration is silent. Warn in that case, naming both the optional variables that were set and the required ones that are missing. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NWw9GUtUFEcTi75SnfnPi3 --- monitoring/mcp-status/statuspage.js | 15 ++++++++++++++- monitoring/mcp-status/statuspage.test.js | 13 +++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/monitoring/mcp-status/statuspage.js b/monitoring/mcp-status/statuspage.js index 29bf9b3..8be3cce 100644 --- a/monitoring/mcp-status/statuspage.js +++ b/monitoring/mcp-status/statuspage.js @@ -143,7 +143,20 @@ export const resolveStatuspageConfig = (env = process.env) => { ...(apiKey ? [] : ["STATUSPAGE_API_KEY"]), ...(componentId ? [] : ["STATUSPAGE_COMPONENT_ID"]), ]; - if (missing.length === 3) return {}; + if (missing.length === 3) { + // Stay silent only when NO Statuspage configuration was supplied at all. + // The optional variables count as "supplied": a deployment that sets only + // STATUSPAGE_PUSH_INTERVAL_MS or STATUSPAGE_API_BASE clearly intended the + // pusher to run, and must get a diagnosis rather than a silent no-op. + const optionalSet = [ + "STATUSPAGE_PUSH_INTERVAL_MS", + "STATUSPAGE_API_BASE", + ].filter((name) => (env[name] ?? "").trim() !== ""); + if (optionalSet.length === 0) return {}; + return { + warning: `statuspage: pusher disabled — ${optionalSet.join(", ")} set but the required variables are missing (${missing.join(", ")})`, + }; + } if (missing.length > 0) { return { warning: `statuspage: pusher disabled — partial configuration (missing ${missing.join(", ")})`, diff --git a/monitoring/mcp-status/statuspage.test.js b/monitoring/mcp-status/statuspage.test.js index e8ea7df..8f0a3be 100644 --- a/monitoring/mcp-status/statuspage.test.js +++ b/monitoring/mcp-status/statuspage.test.js @@ -129,6 +129,19 @@ test("resolveStatuspageConfig: unset env means pusher off, silently", () => { assert.deepEqual(resolveStatuspageConfig({}), {}); }); +test("resolveStatuspageConfig: only optional variables set warns and stays off", () => { + // Silence is reserved for a completely unset configuration: setting only an + // optional variable signals intent to run the pusher and must be diagnosable. + const { config, warning } = resolveStatuspageConfig({ + STATUSPAGE_PUSH_INTERVAL_MS: "30000", + }); + assert.equal(config, undefined); + assert.match(warning, /STATUSPAGE_PUSH_INTERVAL_MS/); + assert.match(warning, /STATUSPAGE_PAGE_ID/); + assert.match(warning, /STATUSPAGE_API_KEY/); + assert.match(warning, /STATUSPAGE_COMPONENT_ID/); +}); + test("resolveStatuspageConfig: partial config warns and stays off", () => { const { config, warning } = resolveStatuspageConfig({ STATUSPAGE_PAGE_ID: "kc2llmsd16bk", From da197b65da6dc470eea6172fc4b4f5f4600c0547 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 11:40:54 +0000 Subject: [PATCH 11/13] Provision /etc/imcp-status during deployment The documented setup step - placing statuspage.env in /etc/imcp-status (mode 600) - could not be completed on a fresh host without an undocumented mkdir, and since the EnvironmentFile is optional, a restart after the attempt would silently leave the pusher disabled. Create the directory root-only (mode 700) in deploy.sh; systemd reads the EnvironmentFile as root, so no other user needs access. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NWw9GUtUFEcTi75SnfnPi3 --- deploy/native/deploy.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/deploy/native/deploy.sh b/deploy/native/deploy.sh index ae66dac..23eecb6 100755 --- a/deploy/native/deploy.sh +++ b/deploy/native/deploy.sh @@ -110,6 +110,11 @@ fi # environment, so it must not share a user with the internet-facing imcp2 # service (same-UID processes can read each other's /proc//environ). id imcp-status >/dev/null 2>&1 || useradd --system --no-create-home --shell /sbin/nologin imcp-status +# Root-only directory for the optional Statuspage credentials +# (/etc/imcp-status/statuspage.env, mode 600 — see the mcp-status README). +# Provisioned here so the documented setup step works on a fresh host; systemd +# reads the EnvironmentFile as root, so no other user needs access. +install -d -m 700 -o root -g root /etc/imcp-status # Execute a ROOT-OWNED copy of the dashboard source, not the staged copy under # $REMOTE_DIR: the staging tree is owned (and writable) by the deploy/SSH user, # which is also the UID the internet-facing imcp2.service runs as — a From ad3dc718ca142155144c38d36d1b629346be3410 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 11:49:36 +0000 Subject: [PATCH 12/13] Skip both registration probes in non-mutating runs Non-mutating mode still sent the allow-list registration request. While the server's hosted-redirect guard holds that stores nothing, but if the guard regressed - the exact failure the probe detects - every run would reach the normal registration path and mint a client, turning the unattended pusher into a store-grinding loop precisely when the server is broken. Non-mutating runs now skip both probes that POST to /oauth/register; interactive dashboard visits and CLI runs still exercise both. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NWw9GUtUFEcTi75SnfnPi3 --- monitoring/mcp-status/README.md | 14 +++++++------- monitoring/mcp-status/checks.js | 23 ++++++++++++++--------- monitoring/mcp-status/checks.test.js | 17 +++++++++-------- 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/monitoring/mcp-status/README.md b/monitoring/mcp-status/README.md index bd931fa..b6f144d 100644 --- a/monitoring/mcp-status/README.md +++ b/monitoring/mcp-status/README.md @@ -156,13 +156,13 @@ itself. Unlike visitor-triggered probe runs, the pusher's own runs are **non-mutating**: it reuses a recent visitor-triggered report when one exists, -and otherwise probes with the loopback Dynamic Client Registration check -skipped — that is the one probe that changes server state (each run mints and -persists a client_id in the server's LRU-bounded registration store), and an -unattended periodic loop must not grind through that store. The allow-list -check still runs (its non-allow-listed redirect is rejected before anything is -stored), and DCR health is still verified by every interactive dashboard visit -and CLI run. +and otherwise probes with both registration checks skipped. The loopback +Dynamic Client Registration probe changes server state on every run (it mints +and persists a client_id in the server's LRU-bounded registration store), and +the allow-list probe would do the same whenever the server's guard regressed — +the very failure it detects — so an unattended periodic loop runs neither. +Both probes are still exercised by every interactive dashboard visit and CLI +run. ## Why a standalone tool (and not a page in the II frontend)? diff --git a/monitoring/mcp-status/checks.js b/monitoring/mcp-status/checks.js index d79f987..5c625fb 100644 --- a/monitoring/mcp-status/checks.js +++ b/monitoring/mcp-status/checks.js @@ -176,14 +176,15 @@ export const parseCspDirective = (csp, directive) => { /** * @param {string} mcpOrigin * @param {number} timeoutMs - * @param {{ mutating?: boolean }} [opts] With `mutating: false`, skip the one - * check that changes server state — the loopback Dynamic Client Registration - * probe, which mints and persists a fresh client_id in the server's - * LRU-bounded registration store on every run. Interactive uses (a person - * loading the dashboard, a one-off CLI run) keep the default; unattended - * periodic callers (the Statuspage pusher) must not grind through that store. - * The allow-list check stays in both modes: its non-allow-listed redirect is - * rejected before anything is stored, so it never mutates. + * @param {{ mutating?: boolean }} [opts] With `mutating: false`, skip both + * registration probes. The loopback Dynamic Client Registration probe mints + * and persists a fresh client_id in the server's LRU-bounded registration + * store on every run. The allow-list probe stores nothing while the server's + * guard holds — but if that guard ever regresses (the exact failure it + * detects), each run would mint a client too, turning an unattended periodic + * caller into a store-grinding loop precisely when the server is broken. + * Interactive uses (a person loading the dashboard, a one-off CLI run) keep + * the default and still exercise both probes. * @returns {Promise<{ section: Section, facts: Record }>} */ export const checkMcpEndpoints = async ( @@ -456,7 +457,11 @@ export const checkMcpEndpoints = async ( // uses a reserved `.invalid` host (RFC 2606) that can never be legitimately // allow-listed by a deployment, so this can't false-alert if the allow-list // is widened via OAUTH_ALLOWED_REDIRECT_PREFIXES. - { + // Also skipped in non-mutating mode: while the guard holds this stores + // nothing, but if it regressed (exactly what this probe detects) each run + // would mint a client — an unattended periodic caller must not become a + // store-grinding loop at the very moment the server is broken. + if (mutating) { const url = `${mcpOrigin}/mcp/oauth/register`; const r = await probe(url, { timeoutMs, diff --git a/monitoring/mcp-status/checks.test.js b/monitoring/mcp-status/checks.test.js index 209a4a1..cea8a11 100644 --- a/monitoring/mcp-status/checks.test.js +++ b/monitoring/mcp-status/checks.test.js @@ -352,16 +352,17 @@ test("checkMcpEndpoints in non-mutating mode never registers a client", async () const { section } = await checkMcpEndpoints(origin, 2000, { mutating: false, }); - // The state-mutating loopback DCR probe is skipped entirely... + // Both registration probes are skipped: the loopback DCR probe always + // mints, and the allow-list probe would mint whenever the server's guard + // regressed — the rest of the suite still runs. assert.equal(byId(section, "oauth-register"), undefined); - // ...while the allow-list probe (rejected before anything is stored) and - // the rest of the suite still run. - assert.equal(byId(section, "oauth-register-allowlist").status, "pass"); + assert.equal(byId(section, "oauth-register-allowlist"), undefined); assert.equal(byId(section, "root").status, "pass"); - assert.equal(registerRedirects.length, 1); - assert.ok( - !registerRedirects[0].includes("127.0.0.1"), - "no loopback (minting) registration may be sent in non-mutating mode", + assert.equal(byId(section, "oauth-token").status, "pass"); + assert.equal( + registerRedirects.length, + 0, + "no registration request of any kind may be sent in non-mutating mode", ); } finally { restore(); From d600d576f55fcf8628e96ce96b8e4d615ef4c7c3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 11:54:34 +0000 Subject: [PATCH 13/13] Update two comments stale since the allow-list skip runDashboard's JSDoc and the getPusherReport comment still said non-mutating mode skips only the DCR probe; it skips both registration probes, and the resulting report is missing two checks - which is also why it must not enter the visitor cache. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NWw9GUtUFEcTi75SnfnPi3 --- monitoring/mcp-status/checks.js | 6 ++++-- monitoring/mcp-status/server.js | 8 +++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/monitoring/mcp-status/checks.js b/monitoring/mcp-status/checks.js index 5c625fb..1c2c8e8 100644 --- a/monitoring/mcp-status/checks.js +++ b/monitoring/mcp-status/checks.js @@ -1024,8 +1024,10 @@ export const buildSuggestions = (sections, facts) => { /** * Run the full dashboard against the resolved configuration. * @param {{ mcpOrigin?: string, iiOrigin?: string, timeoutMs?: number, mutating?: boolean }} [overrides] - * `mutating: false` skips the one probe that changes server state (the - * loopback Dynamic Client Registration check) — see checkMcpEndpoints. + * `mutating: false` skips both registration probes — the loopback Dynamic + * Client Registration check (always mutates) and the allow-list check + * (mutates whenever the guard it tests has regressed) — see + * checkMcpEndpoints. The report then carries neither of those checks. * @returns {Promise} */ export const runDashboard = async (overrides = {}) => { diff --git a/monitoring/mcp-status/server.js b/monitoring/mcp-status/server.js index 8bf7f70..bfbe8f8 100644 --- a/monitoring/mcp-status/server.js +++ b/monitoring/mcp-status/server.js @@ -95,9 +95,11 @@ const getReport = (force = false) => { // server per run — a 60 s loop would grind ~1,440 junk registrations a day // through the server's LRU-bounded client store. So: reuse a recent // visitor-triggered full report when one exists (it was already paid for), and -// otherwise probe with `mutating: false`, which skips only the DCR check. The -// non-mutating report is deliberately NOT stored in the visitor cache — it is -// missing that check, and /api/status promises the full suite. +// otherwise probe with `mutating: false`, which skips both registration probes +// (the DCR check, and the allow-list check that would mutate if the guard it +// tests regressed). The non-mutating report is deliberately NOT stored in the +// visitor cache — it is missing those two checks, and /api/status promises the +// full suite. /** @type {Promise | null} */ let pusherInFlight = null; /** @param {number} maxAgeMs how recent a full report must be to be reused. */