From f8ee6a9e8a4050a6e9d8c2e3d5c8d40d0ed90915 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 13 Sep 2026 09:58:58 +0000 Subject: [PATCH 1/5] Reconnect live MCP sharing after device login or registration failure --- README.md | 22 +++++++--- src/cli-schema.mjs | 2 +- src/integrations.mjs | 18 +++++++-- src/mcp-share.mjs | 8 ++-- src/tui.mjs | 22 ++++++++-- test/mcp-share.test.mjs | 89 +++++++++++++++++++++++++++++++++++++++-- 6 files changed, 140 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 8d4dcc74..a03a5379 100644 --- a/README.md +++ b/README.md @@ -1591,9 +1591,9 @@ session: ```sh moshcode -/mcp answer # default: all session scopes for 8 hours +/mcp answer # default: read-only for 8 hours /mcp answer --ttl 30m # shorter share -/mcp answer --scope sessions:read,sessions:control +/mcp answer --scope sessions:read,sessions:write,sessions:approve,sessions:cancel ``` Paste the printed `https://moshcode.sh/api/v1/mcp/mcs_…` endpoint into a remote @@ -1602,10 +1602,10 @@ specific session and scopes in the browser, and its access token is bound to that one opaque share URL. OAuth authorization code + PKCE, rotating refresh tokens, and device authorization are supported. -The shared server exposes `moshcode_session_read`, `moshcode_session_answer`, -`moshcode_session_approve`, `moshcode_session_send`, and -`moshcode_session_cancel`. Read and control access are separate scopes; the -authorization page shows exactly which ones the client requested. Share +The shared server exposes `session_read`, `session_answer`, `session_approve`, +`session_send`, and `session_cancel`. Reading, writing, approval and interruption +have separate scopes; the authorization page shows which ones the client +requested. Each tool is bound to the shared session. Share management stays with the logged-in Moshcode operator: ```sh @@ -1619,6 +1619,16 @@ moshcode mcp revoke mcs_… writes fail, and revoking or expiring the share invalidates its access and refresh tokens. +If you started the pit before signing in, `/mcp answer` signs you in and starts +its live connection. `/mcp connect` also reconnects the current pit after login. +An unreachable service can be retried without restarting the terminal. An +explicit `MOSHCODE_NO_MIRROR` setting remains respected; unset it and restart +before sharing. Remote approval sends a bounded yes/no to the terminal; it does +not identify a particular engine prompt. Read the current output before acting. +The write scope permits arbitrary terminal input, including commands and a +typed "yes". The approval scope restricts the approval tool; it does not block +someone who already has write permission from answering a prompt themselves. + ### Known MCP servers Some MCP servers are worth remembering by name rather than by npx invocation: diff --git a/src/cli-schema.mjs b/src/cli-schema.mjs index ed48b1f7..8253f9d0 100644 --- a/src/cli-schema.mjs +++ b/src/cli-schema.mjs @@ -989,7 +989,7 @@ export const MCP_VERBS = [ flags: [ ["--ttl ", "60s to 7d", "8h"], ["--name ", "label shown during authorization", "session name"], - ["--scope ", "comma or space-separated session scopes", "all"], + ["--scope ", "sessions:read, sessions:write, sessions:approve, sessions:cancel", "sessions:read"], ["--json", "machine-readable", ""], ], }, diff --git a/src/integrations.mjs b/src/integrations.mjs index 2e292d16..5a955e8d 100644 --- a/src/integrations.mjs +++ b/src/integrations.mjs @@ -14,7 +14,7 @@ import { } from "./plugins.mjs"; import { catalogList, resolveCatalog } from "./mcp-catalog.mjs"; import { MCP_VERBS, PLUGIN_VERBS, SKILL_VERBS } from "./cli-schema.mjs"; -import { connectMcp, createMcpShare, listMcpShares, revokeMcpShare } from "./mcp-share.mjs"; +import { connectMcp, createMcpShare, ensureMcpCredentials, listMcpShares, revokeMcpShare } from "./mcp-share.mjs"; import { acid, ash, bone, ok, err, info } from "./ui.mjs"; function splitKV(pair) { @@ -251,7 +251,7 @@ const anyFailed = (results) => results.some((r) => r.status === "failed"); /** Run `/mcp …`. `tokens` are the words after `mcp`. `run`/`installedSet` are injectable for tests. */ export async function mcpCommand(tokens, { - run, installedSet, sessionId, fetchImpl, credentials, login, + run, installedSet, sessionId, ensureSession, sharingDisabled = false, fetchImpl, credentials, login, } = {}) { const parsed = parseMcp(tokens); if (parsed.list) { printMcpTargets(parsed.json); return 0; } @@ -262,11 +262,23 @@ export async function mcpCommand(tokens, { try { if (parsed.remote.action === "connect") { const creds = await connectMcp(options); + if (ensureSession && !sharingDisabled) await ensureSession(creds, { restart: true }); console.log(ok(`connected${creds?.email ? ` as ${creds.email}` : ""}`)); } else if (parsed.remote.action === "share") { + if (sharingDisabled && !parsed.remote.sessionId) { + throw new Error("remote session sharing is disabled by MOSHCODE_NO_MIRROR; unset it and restart moshcode to share"); + } + let liveSessionId = parsed.remote.sessionId || sessionId; + if (!liveSessionId && ensureSession) { + // A terminal may have started before login, or while the service was + // unavailable. Authenticate first, then register this exact live pit + // with the same credentials used to create the share. + options.credentials = await ensureMcpCredentials(options); + liveSessionId = await ensureSession(options.credentials); + } const share = await createMcpShare({ ...options, - sessionId: parsed.remote.sessionId || sessionId, + sessionId: liveSessionId, name: parsed.remote.name, ttl: parsed.remote.ttl, scope: parsed.remote.scope?.replace(/,/g, " "), diff --git a/src/mcp-share.mjs b/src/mcp-share.mjs index 19c9061e..fd46c77f 100644 --- a/src/mcp-share.mjs +++ b/src/mcp-share.mjs @@ -15,7 +15,7 @@ export function parseMcpTtl(value) { return seconds; } -async function credentialsFor({ credentials, login = loginDevice } = {}) { +export async function ensureMcpCredentials({ credentials, login = loginDevice } = {}) { let creds = credentials === undefined ? loadCreds() : credentials; if (!creds?.token) { const result = await login(); @@ -26,7 +26,7 @@ async function credentialsFor({ credentials, login = loginDevice } = {}) { } async function request(path, { method = "GET", body, fetchImpl = fetch, ...options } = {}) { - const creds = await credentialsFor(options); + const creds = await ensureMcpCredentials(options); const api = String(creds.api || process.env.MOSHCODE_API || "https://app.moshcode.sh").replace(/\/+$/, ""); const response = await fetchImpl(`${api}${path}`, { method, @@ -43,7 +43,9 @@ async function request(path, { method = "GET", body, fetchImpl = fetch, ...optio export async function connectMcp({ login = loginDevice, credentials } = {}) { const result = await login(); - return result?.token ? result : (credentials === undefined ? loadCreds() : credentials); + const creds = result?.token ? result : (credentials === undefined ? loadCreds() : credentials); + if (!creds?.token) throw new Error("device authorization completed without storing Moshcode credentials"); + return creds; } export function createMcpShare({ sessionId, name, scope, ttl, ...options } = {}) { diff --git a/src/tui.mjs b/src/tui.mjs index 6ee26292..9a69c00a 100644 --- a/src/tui.mjs +++ b/src/tui.mjs @@ -919,7 +919,7 @@ export async function tui() { const ad = await motd; if (ad) console.log(dim(ad) + "\n"); - const { restoreTee, drainRemote, atPrompt } = await startMirror(); + let { restoreTee, drainRemote, atPrompt } = await startMirror(); // Settings sync, unattended. Started per `tui()` call and stopped in the // teardown below, because the pit is re-entered after an engine session @@ -1192,7 +1192,21 @@ export async function tui() { } if (cmd === "mcp") { rl.close(); - await mcpCommand(rest, { sessionId: activeMirror?.id }); + await mcpCommand(rest, { + sessionId: activeMirror?.id, + sharingDisabled: Boolean(process.env.MOSHCODE_NO_MIRROR), + ensureSession: async (credentials, { restart = false } = {}) => { + if (process.env.MOSHCODE_NO_MIRROR) { + throw new Error("remote session sharing is disabled by MOSHCODE_NO_MIRROR; unset it and restart moshcode to share"); + } + if (restart && activeMirror) await stopMirror(restoreTee); + if (!activeMirror) ({ restoreTee, drainRemote, atPrompt } = await startMirror({ credentials })); + if (!activeMirror?.id) { + throw new Error("could not register this live session; check the connection and retry /mcp answer"); + } + return activeMirror.id; + }, + }); rl = mkrl(); continue; } @@ -1428,7 +1442,7 @@ export async function tui() { * Entirely optional — not logged in, or the app unreachable, and the pit runs * exactly as before. */ -async function startMirror() { +async function startMirror({ credentials } = {}) { const noop = { restoreTee: null, drainRemote: () => {}, atPrompt: () => {} }; // Only mirror a real interactive pit. A piped or scripted run (tests, CI, // `echo /quit | moshcode`) has no human to watch from a browser, and the @@ -1436,7 +1450,7 @@ async function startMirror() { if (!process.stdin.isTTY || process.env.MOSHCODE_NO_MIRROR) return noop; let mirror; - try { mirror = createMirror({ version: moshcodeVersion() || "", cwd: process.cwd() }); } + try { mirror = createMirror({ version: moshcodeVersion() || "", cwd: process.cwd(), credentials }); } catch { return noop; } let started = false; diff --git a/test/mcp-share.test.mjs b/test/mcp-share.test.mjs index 36177d69..fdd02b3d 100644 --- a/test/mcp-share.test.mjs +++ b/test/mcp-share.test.mjs @@ -18,11 +18,11 @@ async function quietly(fn) { } test("remote MCP verbs parse separately from server registration", () => { - assert.deepEqual(parseMcp(["answer", "--ttl", "8h", "--scope", "sessions:read,sessions:control", "--json"]), { + assert.deepEqual(parseMcp(["answer", "--ttl", "8h", "--scope", "sessions:read,sessions:write", "--json"]), { remote: { action: "share", ttl: "8h", - scope: "sessions:read,sessions:control", + scope: "sessions:read,sessions:write", json: true, }, }); @@ -44,7 +44,7 @@ test("mcp answer shares the active TUI session through the authenticated API", a request = { url, options, body: JSON.parse(options.body) }; return json({ id: "mcs_share", endpoint: "https://moshcode.sh/api/v1/mcp/mcs_share", expires_at: 1 }); }; - const result = await quietly(() => mcpCommand(["answer", "--ttl", "8h", "--scope", "sessions:read,sessions:control"], { + const result = await quietly(() => mcpCommand(["answer", "--ttl", "8h", "--scope", "sessions:read,sessions:write"], { sessionId: "session-live", credentials: { api: "https://app.example.test", token: "mck_test" }, fetchImpl, @@ -56,7 +56,7 @@ test("mcp answer shares the active TUI session through the authenticated API", a assert.deepEqual(request.body, { session_id: "session-live", ttl_seconds: 28800, - scope: "sessions:read sessions:control", + scope: "sessions:read sessions:write", }); assert.match(result.lines.join("\n"), /https:\/\/moshcode\.sh\/api\/v1\/mcp\/mcs_share/); }); @@ -69,3 +69,84 @@ test("mcp answer refuses to invent a session outside the live TUI", async () => assert.equal(result.code, 1); assert.match(result.lines.join("\n"), /no live session/); }); + +test("a pit started before login authenticates before registering and sharing the same session", async () => { + const steps = []; + const creds = { api: "https://app.example.test", token: "mck_new" }; + const result = await quietly(() => mcpCommand(["answer"], { + credentials: null, + login: async () => { steps.push("login"); return creds; }, + ensureSession: async (authenticated) => { + assert.equal(authenticated, creds); steps.push("register"); return "new-live-session"; + }, + fetchImpl: async (url, options) => { + assert.equal(options.headers.authorization, "Bearer mck_new"); + assert.equal(JSON.parse(options.body).session_id, "new-live-session"); + steps.push("share"); + return json({ id: "mcs_new", endpoint: "https://moshcode.sh/api/v1/mcp/mcs_new", expires_at: 1 }); + }, + })); + assert.equal(result.code, 0); + assert.deepEqual(steps, ["login", "register", "share"]); +}); + +test("retrying a failed live registration does not create a stale share", async () => { + let attempts = 0, shares = 0; + const options = { + credentials: { api: "https://app.example.test", token: "mck_test" }, + ensureSession: async () => { if (++attempts === 1) throw new Error("offline; retry /mcp answer"); return "live"; }, + fetchImpl: async () => { shares++; return json({ id: "mcs_new", endpoint: "https://moshcode.sh/api/v1/mcp/mcs_new", expires_at: 1 }); }, + }; + assert.equal((await quietly(() => mcpCommand(["answer"], options))).code, 1); + assert.equal(shares, 0); + assert.equal((await quietly(() => mcpCommand(["answer"], options))).code, 0); + assert.equal(shares, 1); +}); + +test("connect re-registers the pit with the newly authenticated operator", async () => { + const creds = { token: "new-account" }; + let called = false; + const result = await quietly(() => mcpCommand(["connect"], { + credentials: null, + login: async () => creds, + ensureSession: async (actual, options) => { + assert.equal(actual, creds); assert.deepEqual(options, { restart: true }); called = true; + }, + })); + assert.equal(result.code, 0); assert.equal(called, true); +}); + +test("connect cannot claim success without credentials", async () => { + const result = await quietly(() => mcpCommand(["connect"], { credentials: null, login: async () => ({}) })); + assert.equal(result.code, 1); + assert.match(result.lines.join("\n"), /without storing/); +}); + +test("an explicitly selected session never starts an unrelated mirror", async () => { + const result = await quietly(() => mcpCommand(["answer", "--session", "chosen-session"], { + credentials: { token: "mck_test" }, + ensureSession: async () => { throw new Error("must not create another session"); }, + fetchImpl: async (_url, options) => { + assert.equal(JSON.parse(options.body).session_id, "chosen-session"); + return json({ id: "share", endpoint: "https://example.test/share", expires_at: 1 }); + }, + })); + assert.equal(result.code, 0); +}); + +test("disabled mirroring refuses local shares before authentication but still permits login", async () => { + let logins = 0; + const options = { + sharingDisabled: true, + credentials: null, + login: async () => { logins++; return { token: "mck_test" }; }, + ensureSession: async () => { throw new Error("must not mirror"); }, + fetchImpl: async () => { throw new Error("must not share"); }, + }; + const refused = await quietly(() => mcpCommand(["answer"], options)); + assert.equal(refused.code, 1); + assert.match(refused.lines.join("\n"), /MOSHCODE_NO_MIRROR/); + assert.equal(logins, 0); + assert.equal((await quietly(() => mcpCommand(["connect"], options))).code, 0); + assert.equal(logins, 1); +}); From d8ef069cff1c917ee74ad62aff85861e9b977996 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 13 Sep 2026 09:57:27 +0000 Subject: [PATCH 2/5] Enforce scoped session grants and revoke undelivered MCP controls --- apps/pwa/MCP.md | 76 ++++++ apps/pwa/src/lib/mcp-audit.mjs | 16 ++ apps/pwa/src/lib/mcp-auth.mjs | 165 +++++++++---- .../src/migrations/022_mcp_grants_audit.sql | 35 +++ apps/pwa/src/routes/cli.mjs | 7 +- apps/pwa/src/routes/mcp-oauth.mjs | 28 ++- apps/pwa/src/routes/mcp.mjs | 113 ++++++--- apps/pwa/src/routes/sessions.mjs | 12 +- apps/pwa/test/mcp-migration.test.mjs | 51 ++++ apps/pwa/test/mcp-security.test.mjs | 222 ++++++++++++++++++ apps/pwa/test/mcp-session-share.test.mjs | 13 +- 11 files changed, 640 insertions(+), 98 deletions(-) create mode 100644 apps/pwa/MCP.md create mode 100644 apps/pwa/src/lib/mcp-audit.mjs create mode 100644 apps/pwa/src/migrations/022_mcp_grants_audit.sql create mode 100644 apps/pwa/test/mcp-migration.test.mjs create mode 100644 apps/pwa/test/mcp-security.test.mjs diff --git a/apps/pwa/MCP.md b/apps/pwa/MCP.md new file mode 100644 index 00000000..0f92b720 --- /dev/null +++ b/apps/pwa/MCP.md @@ -0,0 +1,76 @@ +# Remote session MCP + +The canonical resource is `https://moshcode.sh/api/v1/mcp/`. +OAuth remains on `https://app.moshcode.sh`. The front door must proxy the share +endpoint, owner share-management endpoints, and per-share protected-resource +metadata without changing resource identifiers. `MCP_PUBLIC_ORIGIN` controls the +share host; `PUBLIC_ORIGIN` remains the existing account and OAuth authority. + +New shares default to `sessions:read`. Request additional permissions explicitly: + +| Scope | Bound-share tools | +| --- | --- | +| `sessions:read` | `session_read` | +| `sessions:write` | `session_send`, `session_answer` | +| `sessions:approve` | `session_approve` (`approve` or `deny` only) | +| `sessions:cancel` | `session_cancel` | + +Write, approval and cancellation also include read access. A share automatically +binds tools to its session; a conflicting `session_id` is rejected. Legacy +`moshcode_session_*` aliases remain accepted, while `/mcp` retains its old tool +names and `sessions:control` compatibility. Broad control is rejected for new +share grants. Migration 022 converts existing share control consent to the +same explicit permissions. Existing tokens for the same owner, client, resource +and session are grouped conservatively for replay revocation because previous +rotations did not preserve token lineage. + +These are terminal operations. Write access can type any bounded line, including +an answer to a confirmation. Separate tool scopes do not turn raw terminal bytes +into prompt-specific authorization. Approval queues `yes` or `no`; it does not +claim to identify or correlate a particular engine confirmation. Input is +limited to 50 lines of 500 characters each, with terminal control bytes rejected. + +OAuth code grants require S256 PKCE, the registered redirect, the registered +client and the exact resource. Authorization codes are single-use. Device and +refresh token requests must repeat the exact share `resource`; device polls honor +the advertised interval and increase it by five seconds after `slow_down`. +Only the session owner can consent, through the existing authenticated, +CSRF-protected browser flow. Device secrets are stored hashed. Device flows +started before the storage upgrade should be restarted after deployment. + +Access tokens last at most one hour and never outlive the share. Refresh tokens +rotate; replay revokes the entire authorization grant, including replacement +access/refresh tokens. `POST /oauth/revoke` accepts `token` and `client_id` and +revokes that grant without revealing whether an unknown token existed. Share +revocation and expiry invalidate all access and further token issuance. + +Queued actions carry share provenance. The CLI's atomic queue claim rechecks the +share's owner, session, expiry and revocation. Revocation cancels still-queued +actions; an action already claimed by the CLI may finish. Revocation cannot +recall input already delivered to a terminal. The queue's `{id, body}` wire +format is unchanged. + +`mcp_audit_events` records identities, fixed action names, outcomes and time. +Command/answer text, terminal output and credentials are absent from audit rows. +The operational command queue and output mirror still contain the content they +must deliver; they are not audit storage. + +The HTTP transport uses JSON responses to authenticated POST requests. GET +performs OAuth discovery/challenge and returns 405 for an authenticated client +because the server does not offer a standalone SSE stream. Metadata and machine +preflight responses support CORS; an authenticated browser MCP request must have +a configured origin or one of that OAuth client's registered redirect origins. +Unknown protocol versions and tool calls disguised as notifications are rejected. + +Validation uses isolated local databases and HTTP servers, including ownership, +CSRF, scope separation, exact binding, replay, concurrency, queue revocation, +audit exclusions and an upgrade from the previous database schema: + +```sh +cd apps/pwa +node --test test/mcp-*.test.mjs +``` + +References: [MCP authorization and resource binding](https://modelcontextprotocol.io/specification/2025-06-18/basic/authorization), +[RFC 8628 device polling](https://www.rfc-editor.org/rfc/rfc8628.html#section-3.5), +[RFC 9700 refresh-token protection](https://www.rfc-editor.org/rfc/rfc9700.html#section-4.14). diff --git a/apps/pwa/src/lib/mcp-audit.mjs b/apps/pwa/src/lib/mcp-audit.mjs new file mode 100644 index 00000000..b8fb3576 --- /dev/null +++ b/apps/pwa/src/lib/mcp-audit.mjs @@ -0,0 +1,16 @@ +import { run } from "../db.mjs"; +import { id } from "./crypto.mjs"; + +/** Callers pass identity and a fixed action/outcome only, never request bodies. */ +export async function auditMcp({ userId, clientId = null, shareId = null, sessionId = null, action, outcome }) { + const eventId = id(); + await run( + `INSERT INTO mcp_audit_events (id,user_id,client_id,share_id,session_id,action,outcome,created_at) + VALUES (?,?,?,?,?,?,?,?)`, + [eventId, userId, clientId, shareId, sessionId, action, outcome, Date.now()] + ); + return eventId; +} + +export const finishMcpAudit = (eventId, outcome) => + run(`UPDATE mcp_audit_events SET outcome=? WHERE id=?`, [outcome, eventId]); diff --git a/apps/pwa/src/lib/mcp-auth.mjs b/apps/pwa/src/lib/mcp-auth.mjs index 304604a2..128e1d81 100644 --- a/apps/pwa/src/lib/mcp-auth.mjs +++ b/apps/pwa/src/lib/mcp-auth.mjs @@ -9,8 +9,9 @@ import { config } from "../config.mjs"; import { id, sha256, token } from "./crypto.mjs"; export const MCP_RESOURCE = `${config.origin}/mcp`; -export const MCP_SCOPES = ["sessions:read", "sessions:control"]; -export const ACCESS_TTL_MS = 4 * 60 * 60 * 1000; +export const MCP_SHARE_SCOPES = ["sessions:read", "sessions:write", "sessions:approve", "sessions:cancel"]; +export const MCP_SCOPES = [...MCP_SHARE_SCOPES, "sessions:control"]; +export const ACCESS_TTL_MS = config.mcp.accessTtlMs; export const REFRESH_TTL_MS = 30 * 24 * 60 * 60 * 1000; export const CODE_TTL_MS = 5 * 60 * 1000; @@ -37,7 +38,7 @@ export async function activeMcpShare(shareId, userId = null) { let sql = `SELECT sh.*, s.name AS session_name, s.status AS session_status, s.last_seen_at, s.cwd, s.engine, s.features FROM mcp_shares sh JOIN cli_sessions s ON s.id=sh.session_id - WHERE sh.id=? AND sh.status='active' AND sh.expires_at>?`; + WHERE sh.id=? AND sh.user_id=s.user_id AND sh.status='active' AND sh.expires_at>?`; if (userId) { sql += " AND sh.user_id=?"; args.push(userId); @@ -47,7 +48,7 @@ export async function activeMcpShare(shareId, userId = null) { export async function shareForResource(resource, userId = null) { const shareId = mcpShareIdFromResource(resource); - return shareId ? activeMcpShare(shareId, userId) : null; + return shareId && resource === mcpShareResource(shareId) ? activeMcpShare(shareId, userId) : null; } function shareScopes(share) { @@ -61,12 +62,14 @@ export function normalizeScopes(value, { fallback = ["sessions:read"] } = {}) { const set = new Set(requested.filter((scope) => MCP_SCOPES.includes(scope))); // Controlling a session without being able to identify/read it is not useful, // so make the implication explicit rather than returning a half-working token. - if (set.has("sessions:control")) set.add("sessions:read"); + if (["sessions:control", "sessions:write", "sessions:approve", "sessions:cancel"].some((scope) => set.has(scope))) set.add("sessions:read"); return MCP_SCOPES.filter((scope) => set.has(scope)); } -export const hasScope = (auth, scope) => - new Set(String(auth?.scope || "").split(/\s+/).filter(Boolean)).has(scope); +export const hasScope = (auth, scope) => { + const scopes = new Set(String(auth?.scope || "").split(/\s+/).filter(Boolean)); + return scopes.has(scope) || (auth?.resource === MCP_RESOURCE && scopes.has("sessions:control")); +}; export function validRedirectUri(raw, applicationType = "web") { try { @@ -81,6 +84,7 @@ export function validRedirectUri(raw, applicationType = "web") { } export async function registerOAuthClient(metadata = {}) { + if (metadata.token_endpoint_auth_method && metadata.token_endpoint_auth_method !== "none") throw new Error("only public clients with token_endpoint_auth_method=none are supported"); const applicationType = metadata.application_type === "native" ? "native" : "web"; const redirectUris = Array.isArray(metadata.redirect_uris) ? [...new Set(metadata.redirect_uris.map(String))].slice(0, 20) @@ -126,13 +130,14 @@ export async function validateAuthorizationRequest(params = {}) { if (!client.redirect_uris.includes(redirectUri)) throw new Error("redirect_uri mismatch"); if (params.code_challenge_method !== "S256") throw new Error("PKCE S256 is required"); const challenge = String(params.code_challenge || ""); - if (!/^[A-Za-z0-9_-]{43,128}$/.test(challenge)) throw new Error("invalid code_challenge"); + if (!/^[A-Za-z0-9_-]{43}$/.test(challenge)) throw new Error("invalid code_challenge"); const resource = String(params.resource || MCP_RESOURCE); const share = resource === MCP_RESOURCE ? null : await shareForResource(resource); if (resource !== MCP_RESOURCE && !share) throw new Error("invalid or expired resource"); const rawScopes = String(params.scope || "").trim().split(/\s+/).filter(Boolean); const unknownScopes = rawScopes.filter((scope) => !MCP_SCOPES.includes(scope)); if (unknownScopes.length) throw new Error(`unsupported scope: ${unknownScopes.join(" ")}`); + if (share && rawScopes.includes("sessions:control")) throw new Error("Use separate sessions:write, sessions:approve and sessions:cancel scopes for session shares."); const scopes = normalizeScopes(params.scope); if (!scopes.length) throw new Error("no supported scopes requested"); if (share && scopes.some((scope) => !shareScopes(share).has(scope))) { @@ -203,49 +208,79 @@ export async function exchangeAuthorizationCode({ return issueTokenPair(row); } +async function activeGrantSource(source) { + if (source.resource === MCP_RESOURCE) { + if (!(await userOwnsSession(source.user_id, source.session_id))) throw new Error("session no longer belongs to this account"); + return null; + } + const share = await shareForResource(source.resource, source.user_id); + if (!share || share.session_id !== source.session_id || + String(source.scope).split(/\s+/).some((scope) => !shareScopes(share).has(scope))) { + throw new Error("session share is expired, revoked, or no longer permits this grant"); + } + return share; +} + export async function issueTokenPair(source) { + const share = await activeGrantSource(source); const now = Date.now(); + const family = source.family_id || id(); + await run(`INSERT OR IGNORE INTO mcp_oauth_grants (id,created_at) VALUES (?,?)`, [family, now]); + const grant = await get(`SELECT revoked_at FROM mcp_oauth_grants WHERE id=?`, [family]); + if (!grant || grant.revoked_at != null) throw new Error("authorization grant was revoked"); const access = `mca_${token(32)}`; const refresh = `mcr_${token(40)}`; - await run( - `INSERT INTO mcp_oauth_tokens - (token_hash,token_type,user_id,client_id,scope,resource,session_id,created_at,expires_at,revoked_at) - VALUES (?,?,?,?,?,?,?,?,?,NULL)`, - [sha256(access), "access", source.user_id, source.client_id, source.scope, source.resource, - source.session_id || null, now, now + ACCESS_TTL_MS] - ); - await run( - `INSERT INTO mcp_oauth_tokens - (token_hash,token_type,user_id,client_id,scope,resource,session_id,created_at,expires_at,revoked_at) - VALUES (?,?,?,?,?,?,?,?,?,NULL)`, - [sha256(refresh), "refresh", source.user_id, source.client_id, source.scope, source.resource, - source.session_id || null, now, now + REFRESH_TTL_MS] - ); - return { - access_token: access, - token_type: "Bearer", - expires_in: Math.floor(ACCESS_TTL_MS / 1000), - refresh_token: refresh, - scope: source.scope, - }; + const accessExpiry = Math.min(now + ACCESS_TTL_MS, share ? Number(share.expires_at) : Infinity); + const refreshExpiry = Math.min(now + REFRESH_TTL_MS, share ? Number(share.expires_at) : Infinity); + for (const [raw, kind, expires] of [[access, "access", accessExpiry], [refresh, "refresh", refreshExpiry]]) { + await run( + `INSERT INTO mcp_oauth_tokens + (token_hash,token_type,user_id,client_id,scope,resource,session_id,created_at,expires_at,revoked_at,family_id) + VALUES (?,?,?,?,?,?,?,?,?,NULL,?)`, + [sha256(raw), kind, source.user_id, source.client_id, source.scope, source.resource, + source.session_id || null, now, expires, family] + ); + } + return { access_token: access, token_type: "Bearer", expires_in: Math.max(0, Math.floor((accessExpiry - now) / 1000)), + refresh_token: refresh, scope: source.scope }; +} + +async function revokeTokenFamily(family) { + await run(`UPDATE mcp_oauth_grants SET revoked_at=COALESCE(revoked_at,?) WHERE id=?`, [Date.now(), family]); + await run(`UPDATE mcp_oauth_tokens SET revoked_at=COALESCE(revoked_at,?) WHERE family_id=?`, [Date.now(), family]); } -export async function rotateRefreshToken({ refreshToken, clientId }) { +export async function rotateRefreshToken({ refreshToken, clientId, resource = MCP_RESOURCE, scope }) { const hash = sha256(String(refreshToken || "")); - const row = await get( - `SELECT * FROM mcp_oauth_tokens - WHERE token_hash=? AND token_type='refresh' AND revoked_at IS NULL AND expires_at > ?`, - [hash, Date.now()] - ); - if (!row || row.client_id !== String(clientId || "")) throw new Error("invalid refresh_token"); + const row = await get(`SELECT * FROM mcp_oauth_tokens WHERE token_hash=? AND token_type='refresh'`, [hash]); + if (!row || row.client_id !== String(clientId || "") || row.resource !== String(resource || "")) { + throw new Error("invalid refresh_token or resource mismatch"); + } + if (row.revoked_at != null) { + // A consumed refresh token is evidence of replay. Kill its replacements, + // including a replacement being inserted concurrently with this request. + await revokeTokenFamily(row.family_id); + throw new Error("refresh_token already used; authorization grant revoked"); + } + if (Number(row.expires_at) <= Date.now()) throw new Error("invalid or expired refresh_token"); + await activeGrantSource(row); + if (scope !== undefined && String(scope) !== row.scope) throw new Error("refresh scope must match the original grant"); const revoked = await run( - `UPDATE mcp_oauth_tokens SET revoked_at=? WHERE token_hash=? AND revoked_at IS NULL`, - [Date.now(), hash] + `UPDATE mcp_oauth_tokens SET revoked_at=? WHERE token_hash=? AND revoked_at IS NULL`, [Date.now(), hash] ); - if (!revoked.rowsAffected) throw new Error("refresh_token already used"); + if (!revoked.rowsAffected) { + await revokeTokenFamily(row.family_id); + throw new Error("refresh_token already used; authorization grant revoked"); + } return issueTokenPair(row); } +export async function revokeOAuthToken({ rawToken, clientId }) { + const row = await get(`SELECT family_id FROM mcp_oauth_tokens WHERE token_hash=? AND client_id=?`, + [sha256(String(rawToken || "")), String(clientId || "")]); + if (row) await revokeTokenFamily(row.family_id); +} + export function bearerToken(req) { const header = String(req.get?.("authorization") || req.headers?.authorization || ""); const match = /^Bearer\s+(.+)$/i.exec(header); @@ -255,13 +290,33 @@ export function bearerToken(req) { export async function accessForToken(raw, resource = MCP_RESOURCE) { if (!raw) return null; return get( - `SELECT * FROM mcp_oauth_tokens - WHERE token_hash=? AND token_type='access' AND revoked_at IS NULL - AND expires_at > ? AND resource = ?`, + `SELECT t.* FROM mcp_oauth_tokens t JOIN mcp_oauth_grants g ON g.id=t.family_id + WHERE t.token_hash=? AND t.token_type='access' AND t.revoked_at IS NULL + AND g.revoked_at IS NULL AND t.expires_at > ? AND t.resource = ?`, [sha256(raw), Date.now(), resource] ); } +async function validMcpOrigin(req, auth) { + const origin = req.get?.("origin"); + if (!origin) return true; // Non-browser MCP clients do not send Origin. + const client = await oauthClient(auth.client_id); + const allowed = new Set([config.origin, config.mcp.origin, + ...(client?.redirect_uris || []).map((uri) => new URL(uri).origin)]); + return allowed.has(origin); +} + +/** Public machine endpoints never use browser cookies as authorization. */ +export function mcpCors(req, res, next) { + if (!/^\/(?:\.well-known\/oauth-|oauth\/(?:token|register|device_authorization|revoke)$|mcp$|api\/v1\/mcp(?:\/|$))/.test(req.path)) return next(); + res.set("Access-Control-Allow-Origin", "*"); + res.set("Access-Control-Expose-Headers", "WWW-Authenticate, MCP-Protocol-Version"); + res.set("Access-Control-Allow-Headers", "Authorization, Content-Type, Accept, MCP-Protocol-Version, Mcp-Method, Mcp-Name"); + res.set("Access-Control-Allow-Methods", "GET, POST, DELETE, OPTIONS"); + if (req.method === "OPTIONS") return res.status(204).end(); + next(); +} + export async function requireMcpAccess(req, res, next) { const auth = await accessForToken(bearerToken(req)); if (!auth) { @@ -269,6 +324,7 @@ export async function requireMcpAccess(req, res, next) { res.set("WWW-Authenticate", `Bearer resource_metadata="${metadata}"`); return res.status(401).json({ error: "invalid_token" }); } + if (!(await validMcpOrigin(req, auth))) return res.status(403).json({ error: "invalid_origin" }); req.mcpAuth = auth; next(); } @@ -277,10 +333,12 @@ export async function requireMcpShareAccess(req, res, next) { const share = await activeMcpShare(req.params.shareId); const resource = mcpShareResource(req.params.shareId); const auth = share ? await accessForToken(bearerToken(req), resource) : null; - if (!share || !auth || auth.user_id !== share.user_id || auth.session_id !== share.session_id) { + if (!share || !auth || auth.user_id !== share.user_id || auth.session_id !== share.session_id || + String(auth.scope).split(/\s+/).some((scope) => !shareScopes(share).has(scope))) { res.set("WWW-Authenticate", `Bearer resource_metadata="${mcpShareMetadata(req.params.shareId)}"`); return res.status(401).json({ error: "invalid_token" }); } + if (!(await validMcpOrigin(req, auth))) return res.status(403).json({ error: "invalid_origin" }); await run(`UPDATE mcp_shares SET last_used_at=? WHERE id=?`, [Date.now(), share.id]); req.mcpAuth = auth; req.mcpShare = share; @@ -307,7 +365,7 @@ export async function createMcpDeviceAuthorization({ clientId, resource, scope } const share = await shareForResource(resource); if (!client || !share) throw new Error("invalid client_id or session resource"); const rawScopes = String(scope || "").trim().split(/\s+/).filter(Boolean); - if (rawScopes.some((value) => !MCP_SCOPES.includes(value))) throw new Error("unsupported scope"); + if (rawScopes.some((value) => !MCP_SHARE_SCOPES.includes(value))) throw new Error("unsupported scope; use sessions:read/write/approve/cancel for a session share"); const scopes = normalizeScopes(scope); if (!scopes.length || scopes.some((value) => !shareScopes(share).has(value))) { throw new Error("requested scope is not allowed by this session share"); @@ -319,12 +377,12 @@ export async function createMcpDeviceAuthorization({ clientId, resource, scope } shortCode = userCode(); } const now = Date.now(); - const ttlMs = 10 * 60 * 1000; + const ttlMs = Math.min(10 * 60 * 1000, Number(share.expires_at) - now); await run( `INSERT INTO device_codes (device_code,user_code,status,name,interval_s,created_at,expires_at,kind,client_id,share_id,resource,scope) VALUES (?,?,?,?,?,?,?,'mcp',?,?,?,?)`, - [deviceCode, shortCode, "pending", client.client_name, 5, now, now + ttlMs, + [sha256(deviceCode), shortCode, "pending", client.client_name, 5, now, Math.min(now + ttlMs, Number(share.expires_at)), client.client_id, share.id, resource, scopes.join(" ")] ); return { deviceCode, userCode: shortCode, interval: 5, expiresIn: Math.floor(ttlMs / 1000) }; @@ -334,11 +392,22 @@ function grantError(code, message) { return Object.assign(new Error(message), { oauthError: code }); } -export async function exchangeMcpDeviceCode({ deviceCode, clientId }) { - const row = await get(`SELECT * FROM device_codes WHERE device_code=? AND kind='mcp'`, [String(deviceCode || "")]); +export async function exchangeMcpDeviceCode({ deviceCode, clientId, resource }) { + const row = await get(`SELECT * FROM device_codes WHERE device_code=? AND kind='mcp'`, [sha256(String(deviceCode || ""))]); if (!row || row.client_id !== String(clientId || "") || Number(row.expires_at) <= Date.now()) { throw grantError("expired_token", "device code is invalid or expired"); } + if (row.resource !== String(resource || "")) throw grantError("invalid_grant", "resource mismatch"); + const currentShare = await activeMcpShare(row.share_id); + if (!currentShare) throw grantError("invalid_grant", "session share is expired or revoked"); + if (["pending", "approved"].includes(row.status)) { + const now = Date.now(); + const last = Number(row.last_polled_at || 0); + const fast = last > 0 && now - last < Number(row.interval_s) * 1000; + const poll = await run(`UPDATE device_codes SET last_polled_at=?, interval_s=interval_s+? + WHERE device_code=? AND COALESCE(last_polled_at,0)=?`, [now, fast ? 5 : 0, row.device_code, last]); + if (fast || !poll.rowsAffected) throw grantError("slow_down", "poll less frequently; increase the interval by five seconds"); + } if (row.status === "pending") throw grantError("authorization_pending", "authorization is still pending"); if (row.status === "denied") throw grantError("access_denied", "authorization was denied"); if (row.status !== "approved" || !row.user_id) throw grantError("expired_token", "device code is no longer valid"); diff --git a/apps/pwa/src/migrations/022_mcp_grants_audit.sql b/apps/pwa/src/migrations/022_mcp_grants_audit.sql new file mode 100644 index 00000000..0aa2ff91 --- /dev/null +++ b/apps/pwa/src/migrations/022_mcp_grants_audit.sql @@ -0,0 +1,35 @@ +ALTER TABLE mcp_oauth_tokens ADD COLUMN family_id TEXT; +CREATE TABLE IF NOT EXISTS mcp_oauth_grants ( + id TEXT PRIMARY KEY, + created_at INTEGER NOT NULL, + revoked_at INTEGER +); +-- Previous rotations did not retain lineage. Conservatively group existing +-- tokens for the same owner/client/resource/session so replay of an older +-- consumed refresh also revokes the currently active replacement. +UPDATE mcp_oauth_tokens SET family_id = user_id || ':' || client_id || ':' || resource || ':' || COALESCE(session_id,''); +INSERT OR IGNORE INTO mcp_oauth_grants (id,created_at) SELECT family_id,MIN(created_at) FROM mcp_oauth_tokens GROUP BY family_id; +CREATE INDEX IF NOT EXISTS idx_mcp_token_family ON mcp_oauth_tokens(family_id); + +ALTER TABLE device_codes ADD COLUMN last_polled_at INTEGER; +ALTER TABLE session_commands ADD COLUMN mcp_share_id TEXT REFERENCES mcp_shares(id); +CREATE INDEX IF NOT EXISTS idx_session_commands_mcp_share ON session_commands(mcp_share_id,status); + +-- Preserve existing broad consent; new shares require explicit granular scopes. +UPDATE mcp_shares SET scopes=REPLACE(scopes,'sessions:control','sessions:write sessions:approve sessions:cancel'); +UPDATE mcp_oauth_tokens SET scope=REPLACE(scope,'sessions:control','sessions:write sessions:approve sessions:cancel') WHERE resource LIKE '%/api/v1/mcp/%'; +UPDATE mcp_oauth_codes SET scope=REPLACE(scope,'sessions:control','sessions:write sessions:approve sessions:cancel') WHERE resource LIKE '%/api/v1/mcp/%'; +UPDATE device_codes SET scope=REPLACE(scope,'sessions:control','sessions:write sessions:approve sessions:cancel') WHERE kind='mcp'; + +-- Deliberately no arguments, command text, answer bodies, output or token data. +CREATE TABLE IF NOT EXISTS mcp_audit_events ( + id TEXT PRIMARY KEY, + user_id TEXT NOT NULL, + client_id TEXT, + share_id TEXT, + session_id TEXT, + action TEXT NOT NULL, + outcome TEXT NOT NULL, + created_at INTEGER NOT NULL +); +CREATE INDEX IF NOT EXISTS idx_mcp_audit_share ON mcp_audit_events(share_id,created_at); diff --git a/apps/pwa/src/routes/cli.mjs b/apps/pwa/src/routes/cli.mjs index 5ff33252..c0c58889 100644 --- a/apps/pwa/src/routes/cli.mjs +++ b/apps/pwa/src/routes/cli.mjs @@ -12,6 +12,7 @@ import { requireAuth, csrfInput } from "../lib/session.mjs"; import { createApiKey, bearer, userForApiKey } from "../lib/apikey.mjs"; import { balance } from "../lib/credits.mjs"; import { config } from "../config.mjs"; +import { auditMcp } from "../lib/mcp-audit.mjs"; export const cliRouter = Router(); @@ -147,7 +148,7 @@ cliRouter.get("/device", requireAuth, async (req, res) => { : denied ? `
request denied — you can close this tab.
` : mcpRequest - ? `

${esc(mcpRequest.name || "MCP client")} wants to control only ${esc(mcpRequest.share_name || "this Moshcode session")} as ${esc(req.user.email || req.user.display_name)}.

+ ? `

${esc(mcpRequest.name || "MCP client")} wants to connect only to ${esc(mcpRequest.share_name || "this Moshcode session")} as ${esc(req.user.email || req.user.display_name)}.

Scopes: ${esc(mcpRequest.scope || "")}

${csrfInput(req)} @@ -188,6 +189,8 @@ cliRouter.post("/device", requireAuth, async (req, res) => { `UPDATE device_codes SET status = 'denied', user_id = ? WHERE device_code = ? AND status = 'pending'`, [req.user.id, row.device_code] ); + await auditMcp({ userId: req.user.id, clientId: row.client_id, shareId: row.share_id, + action: "oauth.device", outcome: "denied" }); return res.redirect("/device?denied=1"); } if (req.body.decision !== "approve") return res.redirect(`/device?code=${encodeURIComponent(userCode)}`); @@ -199,6 +202,8 @@ cliRouter.post("/device", requireAuth, async (req, res) => { if (!approved.rowsAffected) { return res.redirect(`/device?bad=1${req.body.user_code ? "&code=" + encodeURIComponent(req.body.user_code) : ""}`); } + if (row.kind === "mcp") await auditMcp({ userId: req.user.id, clientId: row.client_id, shareId: row.share_id, + action: "oauth.device", outcome: "allowed" }); res.redirect("/device?done=1"); }); diff --git a/apps/pwa/src/routes/mcp-oauth.mjs b/apps/pwa/src/routes/mcp-oauth.mjs index d1663453..cafc3c72 100644 --- a/apps/pwa/src/routes/mcp-oauth.mjs +++ b/apps/pwa/src/routes/mcp-oauth.mjs @@ -7,6 +7,7 @@ import { Router } from "express"; import { config } from "../config.mjs"; import { page, esc } from "../lib/html.mjs"; import { csrfInput, requireAuth } from "../lib/session.mjs"; +import { auditMcp } from "../lib/mcp-audit.mjs"; import { MCP_RESOURCE, MCP_SCOPES, @@ -19,6 +20,8 @@ import { normalizeScopes, registerOAuthClient, rotateRefreshToken, + revokeOAuthToken, + mcpCors, sessionsForAuthorization, userOwnsSession, validateAuthorizationRequest, @@ -26,6 +29,7 @@ import { export const mcpOAuthMachineRouter = Router(); export const mcpOAuthBrowserRouter = Router(); +mcpOAuthMachineRouter.use(mcpCors); const DEVICE_GRANT = "urn:ietf:params:oauth:grant-type:device_code"; const AS_METADATA = () => ({ @@ -34,6 +38,8 @@ const AS_METADATA = () => ({ token_endpoint: `${config.origin}/oauth/token`, device_authorization_endpoint: `${config.origin}/oauth/device_authorization`, registration_endpoint: `${config.origin}/oauth/register`, + revocation_endpoint: `${config.origin}/oauth/revoke`, + revocation_endpoint_auth_methods_supported: ["none"], response_types_supported: ["code"], response_modes_supported: ["query"], grant_types_supported: ["authorization_code", "refresh_token", DEVICE_GRANT], @@ -121,6 +127,7 @@ mcpOAuthMachineRouter.post("/oauth/token", async (req, res) => { const tokens = await exchangeMcpDeviceCode({ deviceCode: req.body?.device_code, clientId: req.body?.client_id, + resource: req.body?.resource, }); return res.json(tokens); } @@ -138,6 +145,8 @@ mcpOAuthMachineRouter.post("/oauth/token", async (req, res) => { const tokens = await rotateRefreshToken({ refreshToken: req.body?.refresh_token, clientId: req.body?.client_id, + resource: req.body?.resource || MCP_RESOURCE, + scope: req.body?.scope, }); return res.json(tokens); } @@ -153,6 +162,12 @@ mcpOAuthMachineRouter.post("/oauth/token", async (req, res) => { } }); +mcpOAuthMachineRouter.post("/oauth/revoke", async (req, res) => { + await revokeOAuthToken({ rawToken: req.body?.token, clientId: req.body?.client_id }); + // Revocation does not reveal whether a supplied credential ever existed. + res.set("Cache-Control", "no-store").status(200).end(); +}); + function authRedirect(redirectUri, values) { const url = new URL(redirectUri); for (const [key, value] of Object.entries(values)) { @@ -163,6 +178,9 @@ function authRedirect(redirectUri, values) { function describeScope(scope) { if (scope === "sessions:read") return "See your Moshcode sessions and read mirrored terminal output."; + if (scope === "sessions:write") return "Send bounded text input or answer a prompt in this session."; + if (scope === "sessions:approve") return "Send an explicit approve or deny response to this session."; + if (scope === "sessions:cancel") return "Interrupt work running in this session."; if (scope === "sessions:control") return "Queue commands or supported key presses into the selected live session."; return scope; } @@ -250,7 +268,12 @@ mcpOAuthBrowserRouter.post("/oauth/authorize", requireAuth, async (req, res) => return res.status(400).type("text").send(`invalid authorization request: ${error.message}\n`); } + if (auth.share && auth.share.user_id !== req.user.id) { + return res.status(400).type("text").send("That session share does not belong to this account.\n"); + } if (req.body?.decision !== "allow") { + await auditMcp({ userId: req.user.id, clientId: auth.clientId, shareId: auth.share?.id, sessionId: auth.share?.session_id, + action: "oauth.authorize", outcome: "denied" }); return res.redirect(authRedirect(auth.redirectUri, { error: "access_denied", state: auth.state, @@ -258,9 +281,6 @@ mcpOAuthBrowserRouter.post("/oauth/authorize", requireAuth, async (req, res) => })); } - if (auth.share && auth.share.user_id !== req.user.id) { - return res.status(400).type("text").send("That session share does not belong to this account.\n"); - } const sessionId = auth.share?.session_id || String(req.body?.session_id || "").trim() || null; if (sessionId && !(await userOwnsSession(req.user.id, sessionId))) { return res.status(400).type("text").send("That session does not belong to this account.\n"); @@ -276,6 +296,8 @@ mcpOAuthBrowserRouter.post("/oauth/authorize", requireAuth, async (req, res) => codeChallenge: auth.codeChallenge, }); + await auditMcp({ userId: req.user.id, clientId: auth.clientId, shareId: auth.share?.id, sessionId, + action: "oauth.authorize", outcome: "allowed" }); return res.redirect(authRedirect(auth.redirectUri, { code, state: auth.state, diff --git a/apps/pwa/src/routes/mcp.mjs b/apps/pwa/src/routes/mcp.mjs index f95a029a..2c33fa76 100644 --- a/apps/pwa/src/routes/mcp.mjs +++ b/apps/pwa/src/routes/mcp.mjs @@ -9,8 +9,11 @@ import { all, get, run } from "../db.mjs"; import { id, token } from "../lib/crypto.mjs"; import { bearer, userForApiKey } from "../lib/apikey.mjs"; import { config } from "../config.mjs"; +import { auditMcp, finishMcpAudit } from "../lib/mcp-audit.mjs"; import { - MCP_SCOPES, + MCP_SHARE_SCOPES, + mcpShareIdFromResource, + mcpCors, hasScope, mcpShareResource, normalizeScopes, @@ -20,6 +23,7 @@ import { } from "../lib/mcp-auth.mjs"; export const mcpRouter = Router(); +mcpRouter.use(mcpCors); export const MODERN_VERSION = "2026-07-28"; export const LEGACY_VERSIONS = ["2025-11-25", "2025-06-18", "2025-03-26"]; @@ -113,13 +117,14 @@ async function readSession(auth, args = {}) { } function splitCommands(text) { - return String(text ?? "") - .split(/\r\n|\r|\n/) - .map((line) => line.trim()) - .filter(Boolean) - .filter((line) => !line.startsWith(KEY_PREFIX) && !line.startsWith(SIGNAL_PREFIX)) - .slice(0, 50) - .map((line) => line.slice(0, 500)); + if (typeof text !== "string" || text.length > 25000 || /[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]/.test(text)) { + throw new Error("text must be at most 25000 characters without terminal control bytes"); + } + const lines = text.split(/\r\n|\r|\n/).map((line) => line.trim()).filter(Boolean); + if (lines.length > 50 || lines.some((line) => line.length > 500)) { + throw new Error("text must contain at most 50 lines of 500 characters each"); + } + return lines; } async function sendSession(auth, args = {}) { @@ -132,9 +137,9 @@ async function sendSession(auth, args = {}) { for (const [index, body] of lines.entries()) { const commandId = id(); await run( - `INSERT INTO session_commands (id,session_id,body,status,created_at) - VALUES (?,?,?,'queued',?)`, - [commandId, row.id, body, now + index] + `INSERT INTO session_commands (id,session_id,body,status,created_at,mcp_share_id) + VALUES (?,?,?,'queued',?,?)`, + [commandId, row.id, body, now + index, mcpShareIdFromResource(auth.resource)] ); commands.push({ id: commandId, body }); } @@ -154,15 +159,15 @@ async function pressSessionKey(auth, args = {}) { if (!features(row).includes("keys")) throw new Error("this Moshcode session does not advertise remote key support"); const commandId = id(); await run( - `INSERT INTO session_commands (id,session_id,body,status,created_at) - VALUES (?,?,?,'queued',?)`, - [commandId, row.id, KEY_PREFIX + key, Date.now()] + `INSERT INTO session_commands (id,session_id,body,status,created_at,mcp_share_id) + VALUES (?,?,?,'queued',?,?)`, + [commandId, row.id, KEY_PREFIX + key, Date.now(), mcpShareIdFromResource(auth.resource)] ); return { ok: true, session_id: row.id, command_id: commandId, key }; } async function answerSession(auth, args = {}) { - const text = String(args.text || "").trim(); + const text = typeof args.text === "string" ? args.text.trim() : ""; if (!text || text.includes("\n") || text.includes("\r") || text.length > 500) { throw new Error("text must be one line between 1 and 500 characters"); } @@ -180,9 +185,9 @@ async function cancelSession(auth, args = {}) { if (!features(row).includes("signals")) throw new Error("this Moshcode session does not advertise remote interrupt support"); const commandId = id(); await run( - `INSERT INTO session_commands (id,session_id,body,status,created_at) - VALUES (?,?,?,'queued',?)`, - [commandId, row.id, SIGNAL_PREFIX + "interrupt", Date.now()] + `INSERT INTO session_commands (id,session_id,body,status,created_at,mcp_share_id) + VALUES (?,?,?,'queued',?,?)`, + [commandId, row.id, SIGNAL_PREFIX + "interrupt", Date.now(), mcpShareIdFromResource(auth.resource)] ); return { ok: true, session_id: row.id, command_id: commandId, signal: "interrupt" }; } @@ -234,7 +239,7 @@ const TOOL_DEFS = [ additionalProperties: false, }, annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: true }, - requiredScope: "sessions:control", + requiredScope: "sessions:write", }, { name: "moshcode_session_key", @@ -250,7 +255,7 @@ const TOOL_DEFS = [ additionalProperties: false, }, annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: true }, - requiredScope: "sessions:control", + requiredScope: "sessions:write", }, { name: "moshcode_session_answer", @@ -266,7 +271,7 @@ const TOOL_DEFS = [ additionalProperties: false, }, annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false, openWorldHint: false }, - requiredScope: "sessions:control", + requiredScope: "sessions:write", }, { name: "moshcode_session_approve", @@ -282,7 +287,7 @@ const TOOL_DEFS = [ additionalProperties: false, }, annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: true }, - requiredScope: "sessions:control", + requiredScope: "sessions:approve", }, { name: "moshcode_session_cancel", @@ -295,18 +300,27 @@ const TOOL_DEFS = [ additionalProperties: false, }, annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: false }, - requiredScope: "sessions:control", + requiredScope: "sessions:cancel", }, ]; +const shareToolName = (name) => name.replace(/^moshcode_/, ""); +const SHARE_TOOLS = new Set(["session_read", "session_send", "session_answer", "session_approve", "session_cancel"]); + export function toolsFor(auth) { - return TOOL_DEFS - .filter((tool) => hasScope(auth, tool.requiredScope)) - .map(({ requiredScope, ...tool }) => tool); + const shared = Boolean(mcpShareIdFromResource(auth.resource)); + return TOOL_DEFS.filter((tool) => hasScope(auth, tool.requiredScope) && (!shared || SHARE_TOOLS.has(shareToolName(tool.name)))) + .map(({ requiredScope, ...tool }) => { + if (!shared) return tool; + const { session_id, ...properties } = tool.inputSchema.properties; + return { ...tool, name: shareToolName(tool.name), inputSchema: { ...tool.inputSchema, properties, + required: (tool.inputSchema.required || []).filter((name) => name !== "session_id") } }; + }); } async function invokeTool(auth, name, args) { - const def = TOOL_DEFS.find((tool) => tool.name === name); + const shared = Boolean(mcpShareIdFromResource(auth.resource)); + const def = TOOL_DEFS.find((tool) => tool.name === name || (shared && shareToolName(tool.name) === name)); if (!def) throw Object.assign(new Error(`unknown tool: ${name}`), { code: -32602 }); if (!hasScope(auth, def.requiredScope)) { throw Object.assign(new Error(`scope ${def.requiredScope} is required`), { @@ -314,6 +328,13 @@ async function invokeTool(auth, name, args) { requiredScope: def.requiredScope, }); } + if (!args || typeof args !== "object" || Array.isArray(args)) throw new Error("arguments must be an object"); + if (Object.keys(args).some((key) => !Object.hasOwn(def.inputSchema.properties, key))) throw new Error("unknown tool argument"); + if (shared) { + if (args.session_id !== undefined && args.session_id !== auth.session_id) throw new Error("this token is bound to a different session"); + args = { ...args, session_id: auth.session_id }; + } + name = def.name; if (name === "moshcode_sessions_list") return listSessions(auth, args); if (name === "moshcode_session_read") return readSession(auth, args); if (name === "moshcode_session_send") return sendSession(auth, args); @@ -381,6 +402,7 @@ async function dispatch(req, auth, body) { throw Object.assign(new Error("invalid JSON-RPC request"), { code: -32600 }); } const version = versionFor(req, body); + if (version && version !== MODERN_VERSION && !LEGACY_VERSIONS.includes(version)) throw headerMismatch("Unsupported MCP protocol version"); const modern = version === MODERN_VERSION; if (modern) validateModernHeaders(req, body); @@ -414,9 +436,16 @@ async function dispatch(req, auth, body) { if (body.method === "tools/call") { const name = String(body.params?.name || ""); + const known = TOOL_DEFS.find((tool) => tool.name === name || shareToolName(tool.name) === name); + const eventId = await auditMcp({ userId: auth.user_id, clientId: auth.client_id, + shareId: mcpShareIdFromResource(auth.resource), sessionId: auth.session_id, + action: known ? shareToolName(known.name) : "tool.unknown", outcome: "started" }); try { - return toolResult(await invokeTool(auth, name, body.params?.arguments || {}), modern); + const data = await invokeTool(auth, name, body.params?.arguments ?? {}); + await finishMcpAudit(eventId, "allowed"); + return toolResult(data, modern); } catch (error) { + await finishMcpAudit(eventId, error.requiredScope ? "denied" : "failed"); if (error.requiredScope || Number.isInteger(error.code)) throw error; return { ...(modern ? modernResult({}) : {}), @@ -429,7 +458,7 @@ async function dispatch(req, auth, body) { throw Object.assign(new Error(`method not found: ${body.method}`), { code: -32601 }); } -mcpRouter.get("/mcp", (_req, res) => { +mcpRouter.get("/mcp", requireMcpAccess, (_req, res) => { res.set("Allow", "POST"); res.status(405).json({ error: "Moshcode MCP uses stateless HTTP POST." }); }); @@ -437,11 +466,14 @@ mcpRouter.get("/mcp", (_req, res) => { async function handleMcp(req, res) { const body = req.body; // initialized is a JSON-RPC notification in legacy clients and has no response. - if (body?.method === "notifications/initialized" && body?.id === undefined) { - return res.status(202).end(); - } - try { + const version = versionFor(req, body); + if (version && version !== MODERN_VERSION && !LEGACY_VERSIONS.includes(version)) throw headerMismatch("Unsupported MCP protocol version"); + if (version === MODERN_VERSION) validateModernHeaders(req, body); + if (body?.jsonrpc === "2.0" && typeof body.method === "string" && body.method.startsWith("notifications/") && body.id === undefined) return res.status(202).end(); + if (body?.id === undefined || (typeof body.id !== "string" && typeof body.id !== "number")) { + throw Object.assign(new Error("invalid JSON-RPC request id"), { code: -32600, httpStatus: 400 }); + } const result = await dispatch(req, req.mcpAuth, body); // A notification has no JSON-RPC response. if (body?.id === undefined) return res.status(202).end(); @@ -472,9 +504,9 @@ async function apiUser(req, res, next) { function requestedShareScopes(value) { const raw = String(value || "").replace(/,/g, " ").trim(); - const requested = raw ? raw.split(/\s+/) : [...MCP_SCOPES]; - if (requested.some((scope) => !MCP_SCOPES.includes(scope))) return null; - return normalizeScopes(requested.join(" "), { fallback: MCP_SCOPES }); + const requested = raw ? raw.split(/\s+/) : ["sessions:read"]; + if (requested.some((scope) => !MCP_SHARE_SCOPES.includes(scope))) return null; + return normalizeScopes(requested.join(" ")); } mcpRouter.post("/api/v1/mcp/shares", apiUser, async (req, res) => { @@ -484,7 +516,7 @@ mcpRouter.post("/api/v1/mcp/shares", apiUser, async (req, res) => { ); if (!session) return res.status(404).json({ error: "no such session" }); const scopes = requestedShareScopes(req.body?.scope); - if (!scopes?.length) return res.status(400).json({ error: "invalid scope" }); + if (!scopes?.length) return res.status(400).json({ error: "invalid scope; use sessions:read, sessions:write, sessions:approve or sessions:cancel" }); const requestedSeconds = Number(req.body?.ttl_seconds); const ttlMs = Number.isFinite(requestedSeconds) ? Math.min(config.mcp.maxShareTtlMs, Math.max(60_000, Math.floor(requestedSeconds * 1000))) @@ -497,6 +529,7 @@ mcpRouter.post("/api/v1/mcp/shares", apiUser, async (req, res) => { [shareId, session.id, req.apiUser.id, String(req.body?.name || session.name || "Moshcode session").slice(0, 100), scopes.join(" "), now, now + ttlMs] ); + await auditMcp({ userId: req.apiUser.id, shareId, sessionId: session.id, action: "share.create", outcome: "allowed" }); res.status(201).json({ id: shareId, session_id: session.id, @@ -520,7 +553,7 @@ mcpRouter.get("/api/v1/mcp/shares", apiUser, async (req, res) => { name: row.name, endpoint: mcpShareResource(row.id), scopes: String(row.scopes).split(/\s+/).filter(Boolean), - status: row.status, + status: row.status === "active" && Number(row.expires_at) <= Date.now() ? "expired" : row.status, expires_at: Number(row.expires_at), session_live: row.session_status === "live" && Date.now() - Number(row.last_seen_at) < STALE_MS, })), @@ -539,10 +572,12 @@ mcpRouter.delete("/api/v1/mcp/shares/:shareId", apiUser, async (req, res) => { `UPDATE mcp_oauth_tokens SET revoked_at=COALESCE(revoked_at,?) WHERE resource=?`, [now, mcpShareResource(req.params.shareId)] ); + await run(`UPDATE session_commands SET status='cancelled' WHERE mcp_share_id=? AND status='queued'`, [req.params.shareId]); + await auditMcp({ userId: req.apiUser.id, shareId: req.params.shareId, action: "share.revoke", outcome: "allowed" }); res.json({ ok: true }); }); -mcpRouter.get("/api/v1/mcp/:shareId", (_req, res) => { +mcpRouter.get("/api/v1/mcp/:shareId", requireMcpShareAccess, (_req, res) => { res.set("Allow", "POST"); res.status(405).json({ error: "Moshcode MCP uses stateless HTTP POST." }); }); diff --git a/apps/pwa/src/routes/sessions.mjs b/apps/pwa/src/routes/sessions.mjs index 4d69c712..c9309d85 100644 --- a/apps/pwa/src/routes/sessions.mjs +++ b/apps/pwa/src/routes/sessions.mjs @@ -245,6 +245,11 @@ sessionsRouter.get("/api/sessions/:id/commands", cliAuth, async (req, res) => { await run(`UPDATE cli_sessions SET last_seen_at = ? WHERE id = ?`, [Date.now(), session.id]); const claim = async () => { + await run(`UPDATE session_commands SET status='cancelled' + WHERE session_id=? AND status='queued' AND mcp_share_id IS NOT NULL + AND NOT EXISTS (SELECT 1 FROM mcp_shares sh WHERE sh.id=session_commands.mcp_share_id + AND sh.session_id=session_commands.session_id AND sh.user_id=? AND sh.status='active' AND sh.expires_at>?)`, + [session.id, req.apiUser.id, Date.now()]); const queued = await all( `SELECT * FROM session_commands WHERE session_id = ? AND status = 'queued' ORDER BY created_at ASC LIMIT 10`, [session.id] @@ -253,8 +258,11 @@ sessionsRouter.get("/api/sessions/:id/commands", cliAuth, async (req, res) => { for (const c of queued) { // The UPDATE is the lock — only the poll that flips 'queued' runs it. const claimed = await run( - `UPDATE session_commands SET status='claimed', claimed_at=? WHERE id=? AND status='queued'`, - [Date.now(), c.id] + `UPDATE session_commands SET status='claimed', claimed_at=? WHERE id=? AND status='queued' + AND (mcp_share_id IS NULL OR EXISTS (SELECT 1 FROM mcp_shares sh + WHERE sh.id=session_commands.mcp_share_id AND sh.session_id=session_commands.session_id + AND sh.user_id=? AND sh.status='active' AND sh.expires_at>?))`, + [Date.now(), c.id, req.apiUser.id, Date.now()] ); if (claimed.rowsAffected) mine.push({ id: c.id, body: c.body }); } diff --git a/apps/pwa/test/mcp-migration.test.mjs b/apps/pwa/test/mcp-migration.test.mjs new file mode 100644 index 00000000..5c93043c --- /dev/null +++ b/apps/pwa/test/mcp-migration.test.mjs @@ -0,0 +1,51 @@ +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; + +const dir = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-upgrade-")); +process.env.DATABASE_URL = `file:${path.join(dir, "test.db")}`; +process.env.PUBLIC_ORIGIN = "https://app.example.test"; +process.env.MCP_PUBLIC_ORIGIN = "https://gateway.example.test"; +const { run, get, db } = await import("../src/db.mjs"); +const { sha256 } = await import("../src/lib/crypto.mjs"); +const migrations = new URL("../src/migrations/", import.meta.url); +test.after(() => { db.close?.(); fs.rmSync(dir, { recursive: true, force: true }); }); + +test("upgrade preserves existing consent, pairs access/refresh grants and leaves ordinary queued input alone", async () => { + const files = fs.readdirSync(migrations).filter((name) => name.endsWith(".sql") && name < "022").sort(); + for (const name of files) { + const sql = fs.readFileSync(new URL(name, migrations), "utf8"); + for (const statement of sql.split(/;\s*(?:\n|$)/).map((one) => one.trim()).filter(Boolean)) await run(statement); + await run(`INSERT INTO _migrations (name,applied_at) VALUES (?,?)`, [name, Date.now()]); + } + const now = Date.now(); const resource = "https://gateway.example.test/api/v1/mcp/mcs_old"; + await run(`INSERT INTO users (id,email,created_at) VALUES ('u','upgrade@example.test',?)`, [now]); + await run(`INSERT INTO cli_sessions (id,user_id,status,created_at,last_seen_at) VALUES ('s','u','live',?,?)`, [now, now]); + await run(`INSERT INTO mcp_shares (id,session_id,user_id,scopes,status,created_at,expires_at) VALUES ('mcs_old','s','u','sessions:read sessions:control','active',?,?)`, [now, now + 600000]); + await run(`INSERT INTO mcp_oauth_clients (client_id,client_name,redirect_uris,created_at) VALUES ('client','Fixture','["https://client.example.test/cb"]',?)`, [now]); + for (const [raw, type] of [["old-access", "access"], ["old-refresh", "refresh"]]) { + await run(`INSERT INTO mcp_oauth_tokens (token_hash,token_type,user_id,client_id,scope,resource,session_id,created_at,expires_at) VALUES (?,?,'u','client','sessions:read sessions:control',?,'s',?,?)`, [sha256(raw), type, resource, now, now + 600000]); + } + await run(`INSERT INTO mcp_oauth_tokens (token_hash,token_type,user_id,client_id,scope,resource,session_id,created_at,expires_at,revoked_at) VALUES (?,'refresh','u','client','sessions:read sessions:control',?,'s',?,?,?)`, + [sha256("older-consumed-refresh"), resource, now - 1000, now + 600000, now - 500]); + await run(`INSERT INTO session_commands (id,session_id,body,status,created_at) VALUES ('plain','s','fixture','queued',?)`, [now]); + const { migrate } = await import("../src/migrate.mjs"); + await migrate(); + await migrate(); // A restart applies no ALTER twice. + const { accessForToken, requireMcpShareAccess, rotateRefreshToken, MCP_SHARE_SCOPES } = await import("../src/lib/mcp-auth.mjs"); + const access = await accessForToken("old-access", resource); + assert.deepEqual(access.scope.split(" "), MCP_SHARE_SCOPES); + assert.equal((await get(`SELECT family_id FROM mcp_oauth_tokens WHERE token_hash=?`, [sha256("old-refresh")])).family_id, access.family_id); + assert.equal((await get(`SELECT mcp_share_id FROM session_commands WHERE id='plain'`)).mcp_share_id, null); + let accepted = false; + await requireMcpShareAccess({ params: { shareId: "mcs_old" }, get: (name) => name === "authorization" ? "Bearer old-access" : undefined }, + { set() { return this; }, status() { return this; }, json() { throw new Error("existing consent was lost"); } }, () => { accepted = true; }); + assert.equal(accepted, true); + const rotated = await rotateRefreshToken({ refreshToken: "old-refresh", clientId: "client", resource }); + assert.ok(await accessForToken(rotated.access_token, resource)); + await assert.rejects(rotateRefreshToken({ refreshToken: "older-consumed-refresh", clientId: "client", resource }), /already used/); + assert.equal(await accessForToken(rotated.access_token, resource), null); + assert.equal(await accessForToken("old-access", resource), null); +}); diff --git a/apps/pwa/test/mcp-security.test.mjs b/apps/pwa/test/mcp-security.test.mjs new file mode 100644 index 00000000..51ef4cc1 --- /dev/null +++ b/apps/pwa/test/mcp-security.test.mjs @@ -0,0 +1,222 @@ +import assert from "node:assert/strict"; +import crypto from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import express from "express"; +import cookieParser from "cookie-parser"; + +const dir = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-security-")); +process.env.DATABASE_URL = `file:${path.join(dir, "test.db")}`; +process.env.PUBLIC_ORIGIN = "https://app.example.test"; +process.env.MCP_PUBLIC_ORIGIN = "https://gateway.example.test"; +process.env.SESSION_POLL_MS = "50"; +const { migrate } = await import("../src/migrate.mjs"); +await migrate(); +const { run, get, all, db } = await import("../src/db.mjs"); +const auth = await import("../src/lib/mcp-auth.mjs"); +const { createApiKey } = await import("../src/lib/apikey.mjs"); +const { sessionMiddleware, csrfGuard } = await import("../src/lib/session.mjs"); +const { mcpRouter } = await import("../src/routes/mcp.mjs"); +const { mcpOAuthMachineRouter, mcpOAuthBrowserRouter } = await import("../src/routes/mcp-oauth.mjs"); +const { cliRouter } = await import("../src/routes/cli.mjs"); +const { sessionsRouter } = await import("../src/routes/sessions.mjs"); +const app = express(); +app.use(express.json(), express.urlencoded({ extended: false }), cookieParser(), sessionMiddleware); +app.use(mcpOAuthMachineRouter, mcpRouter, csrfGuard, mcpOAuthBrowserRouter, cliRouter, sessionsRouter); +const server = await new Promise((resolve) => { const one = app.listen(0, "127.0.0.1", () => resolve(one)); }); +const base = `http://127.0.0.1:${server.address().port}`; +const keys = {}; +for (const user of ["owner", "other"]) { + await run(`INSERT INTO users (id,email,created_at) VALUES (?,?,?)`, [user, `${user}@example.test`, Date.now()]); + keys[user] = (await createApiKey(user, "fixture")).plaintext; + await run(`INSERT INTO sessions (token,user_id,created_at,expires_at) VALUES (?,?,?,?)`, [`cookie-${user}`, user, Date.now(), Date.now() + 3600000]); +} +const client = await auth.registerOAuthClient({ client_name: "Fixture", redirect_uris: ["https://client.example.test/callback"] }); +const otherClient = await auth.registerOAuthClient({ redirect_uris: ["https://other-client.example.test/callback"] }); +const request = async (url, { method = "GET", bearer, user, csrf = true, body, headers = {} } = {}) => { + const response = await fetch(base + url, { method, redirect: "manual", headers: { + ...(bearer ? { authorization: `Bearer ${bearer}` } : {}), + ...(user ? { cookie: `mc_sess=cookie-${user}; mc_csrf=fixture-csrf`, ...(csrf ? { "x-csrf-token": "fixture-csrf" } : {}) } : {}), + ...(body !== undefined ? { "content-type": "application/json" } : {}), ...headers, + }, ...(body !== undefined ? { body: JSON.stringify(body) } : {}) }); + const text = await response.text(); let data; try { data = JSON.parse(text); } catch { data = text; } + return { status: response.status, headers: response.headers, data }; +}; +async function fixture(scope = "sessions:read sessions:write sessions:approve sessions:cancel") { + const session = crypto.randomUUID(); const now = Date.now(); + await run(`INSERT INTO cli_sessions (id,user_id,name,features,status,created_at,last_seen_at) VALUES (?,'owner','Fixture','["keys","signals"]','live',?,?)`, [session, now, now]); + const result = await request("/api/v1/mcp/shares", { method: "POST", bearer: keys.owner, body: { session_id: session, scope, ttl_seconds: 300 } }); + assert.equal(result.status, 201); + return { session, share: result.data, route: `/api/v1/mcp/${result.data.id}` }; +} +async function codeFor(f, scope = "sessions:read") { + const verifier = crypto.randomBytes(48).toString("base64url"); + const code = await auth.createAuthorizationCode({ userId: "owner", clientId: client.client_id, redirectUri: client.redirect_uris[0], + scope, resource: f.share.endpoint, sessionId: f.session, codeChallenge: auth.pkceChallenge(verifier) }); + return { grant_type: "authorization_code", code, client_id: client.client_id, redirect_uri: client.redirect_uris[0], code_verifier: verifier, resource: f.share.endpoint }; +} +async function grant(f, scope = "sessions:read") { + const response = await request("/oauth/token", { method: "POST", body: await codeFor(f, scope) }); + assert.equal(response.status, 200, JSON.stringify(response.data)); return response.data; +} +const call = (f, token, name, args = {}, extra = {}) => request(f.route, { method: "POST", bearer: token, + body: { jsonrpc: "2.0", id: 1, method: "tools/call", params: { name, arguments: args } }, ...extra }); +const deviceRequest = (device, f) => ({ grant_type: "urn:ietf:params:oauth:grant-type:device_code", device_code: device.device_code, client_id: client.client_id, resource: f.share.endpoint }); +test.after(async () => { await new Promise((resolve) => server.close(resolve)); db.close?.(); fs.rmSync(dir, { recursive: true, force: true }); }); + +test("share creation is owner-only, defaults read-only, and rejects broad control", async () => { + const f = await fixture(); + const other = await request("/api/v1/mcp/shares", { method: "POST", bearer: keys.other, body: { session_id: f.session } }); + assert.equal(other.status, 404); + const broad = await request("/api/v1/mcp/shares", { method: "POST", bearer: keys.owner, body: { session_id: f.session, scope: "sessions:control" } }); + assert.equal(broad.status, 400); assert.match(broad.data.error, /sessions:write/); + const read = await request("/api/v1/mcp/shares", { method: "POST", bearer: keys.owner, body: { session_id: f.session } }); + assert.deepEqual(read.data.scopes, ["sessions:read"]); + assert.equal((await request(`/api/v1/mcp/shares/${f.share.id}`, { method: "DELETE", bearer: keys.other })).status, 404); +}); + +test("metadata and GET challenge identify the exact canonical resource; tokens cannot cross shares", async () => { + const f = await fixture(); const other = await fixture(); const token = await grant(f); + const metadata = await request(`/.well-known/oauth-protected-resource/api/v1/mcp/${f.share.id}`); + assert.equal(metadata.data.resource, f.share.endpoint); assert.deepEqual(metadata.data.authorization_servers, ["https://app.example.test"]); + const unauth = await request(f.route); assert.equal(unauth.status, 401); assert.ok(unauth.headers.get("www-authenticate").includes(f.share.id)); + assert.equal((await request(f.route, { bearer: token.access_token })).status, 405); + assert.equal((await call(other, token.access_token, "session_read")).status, 401); + assert.equal((await request("/mcp", { method: "POST", bearer: token.access_token, body: { jsonrpc: "2.0", id: 1, method: "tools/list" } })).status, 401); + const mismatch = await call(f, token.access_token, "session_read", { session_id: other.session }); + assert.equal(mismatch.data.result.isError, true); assert.match(mismatch.data.result.content[0].text, /different session/); +}); + +test("granular scopes expose bound tools and enforce read/write/approve/cancel independently", async () => { + const f = await fixture(); + const allowed = { "sessions:read": ["session_read"], "sessions:write": ["session_read", "session_send", "session_answer"], + "sessions:approve": ["session_read", "session_approve"], "sessions:cancel": ["session_read", "session_cancel"] }; + for (const [scope, names] of Object.entries(allowed)) { + const token = await grant(f, auth.normalizeScopes(scope).join(" ")); + const listed = await request(f.route, { method: "POST", bearer: token.access_token, body: { jsonrpc: "2.0", id: 1, method: "tools/list" } }); + assert.deepEqual(listed.data.result.tools.map((t) => t.name), names); + for (const tool of listed.data.result.tools) assert.equal(tool.inputSchema.required.includes("session_id"), false); + assert.equal((await call(f, token.access_token, "session_read")).data.result.structuredContent.session.id, f.session); + for (const [name, args] of [["session_send", { text: "fixture text" }], ["session_answer", { text: "fixture answer" }], ["session_approve", { decision: "deny" }], ["session_cancel", {}]]) { + const result = await call(f, token.access_token, name, args); + assert.equal(result.status, names.includes(name) ? 200 : 403, `${scope}: ${name}`); + } + } + const token = await grant(f, "sessions:read sessions:approve"); + const bad = await call(f, token.access_token, "session_approve", { decision: "rm -rf anything" }); + assert.equal(bad.data.result.isError, true); +}); + +test("commands reject truncation and control-byte injection; audits contain metadata only", async () => { + const f = await fixture(); const token = await grant(f, "sessions:read sessions:write"); + for (const text of ["x".repeat(501), "a\n".repeat(51), "\u001bmoshsignal:interrupt", "\u0003"]) { + assert.equal((await call(f, token.access_token, "session_send", { text })).data.result.isError, true); + } + const secret = "fixture body must never appear in audit"; + assert.equal((await call(f, token.access_token, "session_answer", { text: secret })).data.result.isError, undefined); + const events = await all(`SELECT * FROM mcp_audit_events WHERE share_id=?`, [f.share.id]); + assert.ok(events.some((event) => event.action === "session_answer" && event.outcome === "allowed")); + assert.equal(JSON.stringify(events).includes(secret), false); assert.equal(JSON.stringify(events).includes(token.access_token), false); + assert.deepEqual((await all(`SELECT body,mcp_share_id FROM session_commands WHERE session_id=?`, [f.session])).map((row) => [row.body, row.mcp_share_id]), [[secret, f.share.id]]); +}); + +test("PKCE, client, redirect and resource checks precede atomic single-use code consumption", async () => { + const f = await fixture(); const code = await codeFor(f); + for (const changed of [{ code_verifier: "a".repeat(43) }, { client_id: otherClient.client_id }, { redirect_uri: "https://client.example.test/elsewhere" }, { resource: auth.MCP_RESOURCE }, { resource: undefined }]) { + assert.equal((await request("/oauth/token", { method: "POST", body: { ...code, ...changed } })).status, 400); + } + const attempts = await Promise.all([1, 2].map(() => request("/oauth/token", { method: "POST", body: code }))); + assert.deepEqual(attempts.map((one) => one.status).sort(), [200, 400]); + const expired = await codeFor(f); await run(`UPDATE mcp_oauth_codes SET expires_at=0 WHERE resource=?`, [f.share.endpoint]); + assert.equal((await request("/oauth/token", { method: "POST", body: expired })).status, 400); +}); + +test("refresh is resource-bound; replay revokes replacement tokens and explicit revocation is client-bound", async () => { + const f = await fixture(); const first = await grant(f); + const body = { grant_type: "refresh_token", refresh_token: first.refresh_token, client_id: client.client_id, resource: f.share.endpoint }; + for (const changed of [{ resource: auth.MCP_RESOURCE }, { client_id: otherClient.client_id }, { scope: "sessions:write" }, { resource: undefined }]) { + assert.equal((await request("/oauth/token", { method: "POST", body: { ...body, ...changed } })).status, 400); + } + const second = await request("/oauth/token", { method: "POST", body }); assert.equal(second.status, 200); + assert.equal((await call(f, second.data.access_token, "session_read")).status, 200); + assert.equal((await request("/oauth/token", { method: "POST", body })).status, 400); + assert.equal((await call(f, second.data.access_token, "session_read")).status, 401); + assert.equal((await request("/oauth/token", { method: "POST", body: { ...body, refresh_token: second.data.refresh_token } })).status, 400); + const fresh = await grant(f); + await request("/oauth/revoke", { method: "POST", body: { token: fresh.refresh_token, client_id: otherClient.client_id } }); + assert.equal((await call(f, fresh.access_token, "session_read")).status, 200); + await request("/oauth/revoke", { method: "POST", body: { token: fresh.refresh_token, client_id: client.client_id } }); + assert.equal((await call(f, fresh.access_token, "session_read")).status, 401); +}); + +test("expired/revoked shares cannot mint or use tokens, and token lifetimes do not outlive the share", async () => { + const f = await fixture(); const code = await codeFor(f); const token = await grant(f); + assert.ok(token.expires_in <= 300); + await run(`UPDATE mcp_shares SET expires_at=0 WHERE id=?`, [f.share.id]); + assert.equal((await call(f, token.access_token, "session_read")).status, 401); + assert.equal((await request("/oauth/token", { method: "POST", body: code })).status, 400); + assert.equal((await request("/oauth/token", { method: "POST", body: { grant_type: "refresh_token", refresh_token: token.refresh_token, client_id: client.client_id, resource: f.share.endpoint } })).status, 400); + const list = await request("/api/v1/mcp/shares", { bearer: keys.owner }); assert.equal(list.data.shares.find((sh) => sh.id === f.share.id).status, "expired"); +}); + +test("device flow enforces resource, polling interval, owner consent, CSRF and single use", async () => { + const f = await fixture(); + const device = (await request("/oauth/device_authorization", { method: "POST", body: { client_id: client.client_id, resource: f.share.endpoint, scope: "sessions:read" } })).data; + assert.equal(await get(`SELECT 1 FROM device_codes WHERE device_code=?`, [device.device_code]), null); + const body = deviceRequest(device, f); + assert.equal((await request("/oauth/token", { method: "POST", body: { ...body, resource: auth.MCP_RESOURCE } })).data.error, "invalid_grant"); + assert.equal((await request("/oauth/token", { method: "POST", body })).data.error, "authorization_pending"); + assert.equal((await request("/oauth/token", { method: "POST", body })).data.error, "slow_down"); + assert.equal(Number((await get(`SELECT interval_s FROM device_codes WHERE user_code=?`, [device.user_code])).interval_s), 10); + await request("/device", { method: "POST", user: "other", body: { user_code: device.user_code, decision: "approve" } }); + assert.equal((await get(`SELECT status FROM device_codes WHERE user_code=?`, [device.user_code])).status, "pending"); + assert.equal((await request("/device", { method: "POST", user: "owner", csrf: false, body: { user_code: device.user_code, decision: "approve" } })).status, 403); + await request("/device", { method: "POST", user: "owner", body: { user_code: device.user_code, decision: "approve" } }); + await run(`UPDATE device_codes SET last_polled_at=0 WHERE user_code=?`, [device.user_code]); + const issued = await request("/oauth/token", { method: "POST", body }); assert.equal(issued.status, 200); + assert.equal((await call(f, issued.data.access_token, "session_read")).status, 200); + assert.equal((await request("/oauth/token", { method: "POST", body })).data.error, "expired_token"); +}); + +test("browser consent checks owner and CSRF before granting the exact session", async () => { + const f = await fixture(); const verifier = crypto.randomBytes(48).toString("base64url"); + const body = { client_id: client.client_id, response_type: "code", redirect_uri: client.redirect_uris[0], resource: f.share.endpoint, + scope: "sessions:read", code_challenge_method: "S256", code_challenge: auth.pkceChallenge(verifier), decision: "allow", session_id: "not-the-shared-session" }; + assert.equal((await request("/oauth/authorize", { method: "POST", user: "other", body })).status, 400); + assert.equal((await request("/oauth/authorize", { method: "POST", user: "owner", csrf: false, body })).status, 403); + const consent = await request("/oauth/authorize", { method: "POST", user: "owner", body }); assert.equal(consent.status, 302); + const code = new URL(consent.headers.get("location")).searchParams.get("code"); + const issued = await request("/oauth/token", { method: "POST", body: { grant_type: "authorization_code", code, client_id: client.client_id, + redirect_uri: client.redirect_uris[0], code_verifier: verifier, resource: f.share.endpoint } }); + assert.equal(issued.status, 200); assert.equal((await call(f, issued.data.access_token, "session_read")).data.result.structuredContent.session.id, f.session); +}); + +test("share revocation and expiry prevent queued commands from being claimed; claims remain single-use", async () => { + for (const revoke of [true, false]) { + const f = await fixture(); const token = await grant(f, "sessions:read sessions:write"); + await call(f, token.access_token, "session_send", { text: "never deliver this fixture" }); + if (revoke) await request(`/api/v1/mcp/shares/${f.share.id}`, { method: "DELETE", bearer: keys.owner }); + else await run(`UPDATE mcp_shares SET expires_at=0 WHERE id=?`, [f.share.id]); + const polled = await request(`/api/sessions/${f.session}/commands`, { bearer: keys.owner }); + assert.deepEqual(polled.data.commands, []); + assert.equal((await get(`SELECT status FROM session_commands WHERE session_id=?`, [f.session])).status, "cancelled"); + } + const f = await fixture(); const token = await grant(f, "sessions:read sessions:approve"); + await call(f, token.access_token, "session_approve", { decision: "deny" }); + const polls = await Promise.all([1, 2].map(() => request(`/api/sessions/${f.session}/commands`, { bearer: keys.owner }))); + assert.deepEqual(polls.flatMap((one) => one.data.commands.map((command) => command.body)), ["no"]); +}); + +test("browser origins, preflight, protocol versions and notifications are validated", async () => { + const f = await fixture(); const token = await grant(f, "sessions:read sessions:write"); + const preflight = await request(f.route, { method: "OPTIONS", headers: { origin: "https://client.example.test" } }); + assert.equal(preflight.status, 204); assert.equal(preflight.headers.get("access-control-allow-origin"), "*"); + assert.equal((await call(f, token.access_token, "session_read", {}, { headers: { origin: "https://evil.example.test" } })).status, 403); + assert.equal((await call(f, token.access_token, "session_read", {}, { headers: { origin: "https://client.example.test" } })).status, 200); + assert.equal((await call(f, token.access_token, "session_read", {}, { headers: { "mcp-protocol-version": "unknown" } })).status, 400); + assert.equal((await request(f.route, { method: "POST", bearer: token.access_token, body: { jsonrpc: "2.0", method: "notifications/initialized" } })).status, 202); + assert.equal((await request(f.route, { method: "POST", bearer: token.access_token, body: { jsonrpc: "2.0", method: "tools/call", params: { name: "session_send", arguments: { text: "must not queue" } } } })).status, 400); + assert.equal((await all(`SELECT * FROM session_commands WHERE session_id=?`, [f.session])).length, 0); +}); diff --git a/apps/pwa/test/mcp-session-share.test.mjs b/apps/pwa/test/mcp-session-share.test.mjs index 54f79689..f072775e 100644 --- a/apps/pwa/test/mcp-session-share.test.mjs +++ b/apps/pwa/test/mcp-session-share.test.mjs @@ -77,7 +77,7 @@ test("session share: device OAuth binds one opaque endpoint and revocation kills const share = await request("/api/v1/mcp/shares", { method: "POST", bearer: apiKey, - body: { session_id: session.body.id, scope: "sessions:read sessions:control", ttl_seconds: 3600 }, + body: { session_id: session.body.id, scope: "sessions:read sessions:write sessions:approve sessions:cancel", ttl_seconds: 3600 }, }); assert.equal(share.response.status, 201); assert.match(share.body.endpoint, /^https:\/\/moshcode\.example\.test\/api\/v1\/mcp\/mcs_/); @@ -97,7 +97,7 @@ test("session share: device OAuth binds one opaque endpoint and revocation kills code_challenge_method: "S256", code_challenge: "a".repeat(43), resource: share.body.endpoint, - scope: "sessions:read sessions:control", + scope: "sessions:read sessions:write sessions:approve sessions:cancel", }); assert.equal(browserGrant.share.session_id, session.body.id); const device = await request("/oauth/device_authorization", { @@ -105,7 +105,7 @@ test("session share: device OAuth binds one opaque endpoint and revocation kills body: { client_id: client.body.client_id, resource: share.body.endpoint, - scope: "sessions:read sessions:control", + scope: "sessions:read sessions:write sessions:approve sessions:cancel", }, }); assert.equal(device.response.status, 200); @@ -117,17 +117,19 @@ test("session share: device OAuth binds one opaque endpoint and revocation kills grant_type: "urn:ietf:params:oauth:grant-type:device_code", client_id: client.body.client_id, device_code: device.body.device_code, + resource: share.body.endpoint, }, }); assert.equal(pending.body.error, "authorization_pending"); - await run(`UPDATE device_codes SET status='approved', user_id='u1' WHERE device_code=?`, [device.body.device_code]); + await run(`UPDATE device_codes SET status='approved', user_id='u1', last_polled_at=NULL WHERE user_code=?`, [device.body.user_code]); const issued = await request("/oauth/token", { method: "POST", body: { grant_type: "urn:ietf:params:oauth:grant-type:device_code", client_id: client.body.client_id, device_code: device.body.device_code, + resource: share.body.endpoint, }, }); assert.equal(issued.response.status, 200); @@ -138,6 +140,7 @@ test("session share: device OAuth binds one opaque endpoint and revocation kills grant_type: "urn:ietf:params:oauth:grant-type:device_code", client_id: client.body.client_id, device_code: device.body.device_code, + resource: share.body.endpoint, }, }); assert.equal(replay.body.error, "expired_token"); @@ -148,7 +151,7 @@ test("session share: device OAuth binds one opaque endpoint and revocation kills body: { jsonrpc: "2.0", id: 1, method: "tools/list", params: {} }, }); assert.equal(listed.response.status, 200); - assert.equal(listed.body.result.tools.some((tool) => tool.name === "moshcode_session_cancel"), true); + assert.equal(listed.body.result.tools.some((tool) => tool.name === "session_cancel"), true); const cancelled = await request(`/api/v1/mcp/${share.body.id}`, { method: "POST", From 0dd42a12833f2daef08d4b91a9931f677ed11e11 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 13 Sep 2026 10:04:14 +0000 Subject: [PATCH 3/5] Document MCP clients and require PWA security tests for release --- .github/workflows/ci.yml | 1 + .github/workflows/publish.yml | 1 + .github/workflows/test.yml | 1 + README.md | 18 ++++++++++++++++++ apps/pwa/MCP.md | 24 ++++++++++++++++++++++++ package.json | 2 +- 6 files changed, 46 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4aea4b90..c6c4e42d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,6 +34,7 @@ jobs: cache: pnpm - run: pnpm install --frozen-lockfile + - run: pnpm --dir apps/pwa install --frozen-lockfile # typecheck / test default to `pnpm run --if-present