From 0b054aa0c5eee8feb9d9b4e9b7d497180d37ccf5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 09:44:28 +0000 Subject: [PATCH 01/10] feat(onboarding-kit): gate registration against fleet policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `fuzefront-validate-registration`, a zero-dependency validator products run in their own CI, and fixes the template defect that caused the problem it catches. ## The failure class A manifest can be entirely valid and still leave a product permanently crippled. `mode: "portal"` with `modes` omitted is legal — the frozen contract says an absent `modes` falls back to `[mode]`. Such a product registers cleanly, appears in the portal, passes every existing gate, and can never ship a mobile app, because a TWA can only wrap a `standalone` surface with a URL that stands on its own. Nothing is malformed. Nothing errors. A capability simply never exists. No schema can catch this, because it is not a shape violation — it is a fleet requirement, and the fleet is not in the schema. The same shape applies to the policy step: a vendored pre-kit `register.sh` registers the app and never submits policy.json, so the product gets no roles and authorization fails closed for everyone. The symptom reads as a bug in the product. ## The template was the source `templates/manifest.json` shipped `mode: portal`, no `modes`, and no `routing.host`. Every product that copied it inherited a registration that cannot serve a mobile app. FuzeHub and FuzeContact are not two coincidences — they are the template, propagated. Fixed to `["portal","standalone"]` with a `routing.host`, and the templates are now checked by the validator in CI so this cannot regress. ## What the gate enforces - effective modes include BOTH `portal` and `standalone` - `standalone` implies a non-empty `routing.host` - `policy.json` exists, and a vendored `register.sh` actually submits it Embed-only products are exempt from the surface rules: per the contract an embed renders inside a third-party page with neither portal chrome nor FuzeFront navigation, is not a portal destination, and may not register a menu entry at all. Matching is on the submission itself (`PUT /apps/{slug}/policy`), not the word "policy" — a TODO comment must not satisfy the check. ## Verified 18 new tests, all passing. The full kit suite still passes (19 register.sh behaviours, policy validator, schema freshness). Run against the real repos: fuzecontact FAIL missing standalone + missing policy.json fuzehub FAIL missing standalone fuzebi PASS fuzeservice PASS fuzepicker PASS which is exactly the known state — the gate reproduces the two defects that prompted it and clears the three conformant repos. Not verified: no product repo has adopted the check yet; wiring it into each product's CI is follow-up work. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GaPa3JgrVNtWrGvqQEAEqv --- .github/workflows/onboarding-kit-tests.yml | 22 +- packages/mcp-gateway/package.json | 47 +++++ packages/mcp-gateway/scripts/smoke.mjs | 71 +++++++ packages/mcp-gateway/src/classify.ts | 158 +++++++++++++++ packages/mcp-gateway/src/config.ts | 72 +++++++ packages/mcp-gateway/src/index.ts | 5 + packages/mcp-gateway/src/main.ts | 39 ++++ packages/mcp-gateway/src/server.ts | 172 ++++++++++++++++ packages/mcp-gateway/src/spec.ts | 188 +++++++++++++++++ packages/mcp-gateway/src/upstream.ts | 130 ++++++++++++ packages/mcp-gateway/test/classify.test.ts | 87 ++++++++ packages/mcp-gateway/test/spec.test.ts | 125 ++++++++++++ packages/mcp-gateway/test/upstream.test.ts | 108 ++++++++++ packages/mcp-gateway/tsconfig.build.json | 3 + packages/mcp-gateway/tsconfig.json | 18 ++ packages/mcp-gateway/vitest.config.ts | 8 + packages/onboarding-kit/README.md | 32 +++ .../bin/validate-registration.mjs | 191 ++++++++++++++++++ packages/onboarding-kit/package.json | 12 +- .../onboarding-kit/templates/manifest.json | 19 +- .../tests/validate-registration.test.mjs | 159 +++++++++++++++ 21 files changed, 1651 insertions(+), 15 deletions(-) create mode 100644 packages/mcp-gateway/package.json create mode 100644 packages/mcp-gateway/scripts/smoke.mjs create mode 100644 packages/mcp-gateway/src/classify.ts create mode 100644 packages/mcp-gateway/src/config.ts create mode 100644 packages/mcp-gateway/src/index.ts create mode 100644 packages/mcp-gateway/src/main.ts create mode 100644 packages/mcp-gateway/src/server.ts create mode 100644 packages/mcp-gateway/src/spec.ts create mode 100644 packages/mcp-gateway/src/upstream.ts create mode 100644 packages/mcp-gateway/test/classify.test.ts create mode 100644 packages/mcp-gateway/test/spec.test.ts create mode 100644 packages/mcp-gateway/test/upstream.test.ts create mode 100644 packages/mcp-gateway/tsconfig.build.json create mode 100644 packages/mcp-gateway/tsconfig.json create mode 100644 packages/mcp-gateway/vitest.config.ts create mode 100644 packages/onboarding-kit/bin/validate-registration.mjs create mode 100644 packages/onboarding-kit/tests/validate-registration.test.mjs diff --git a/.github/workflows/onboarding-kit-tests.yml b/.github/workflows/onboarding-kit-tests.yml index ee3771b2..2ca63f2c 100644 --- a/.github/workflows/onboarding-kit-tests.yml +++ b/.github/workflows/onboarding-kit-tests.yml @@ -32,20 +32,28 @@ jobs: with: node-version: '24.x' - # These two run with NOTHING installed, on purpose. bin/register.sh and - # bin/validate-policy.mjs execute inside a product's init container where no - # npm install is possible, so a dependency creeping into either is a real - # break — running them bare is what catches it. + # These run with NOTHING installed, on purpose. bin/register.sh and the + # bin/validate-*.mjs validators execute inside a product's init container or its + # CI where no npm install is possible, so a dependency creeping into any of them + # is a real break — running them bare is what catches it. - name: Policy validator run: node --test tests/validate-policy.test.mjs + - name: Registration fleet-policy validator + run: node --test tests/validate-registration.test.mjs + - name: register.sh behaviour (fake registry) run: sh tests/register.test.sh - # Every template the kit hands out must itself pass the validator, or the - # first thing a product copies is already broken. + # Every template the kit hands out must itself pass the validators, or the + # first thing a product copies is already broken. This is not hypothetical: the + # template shipped `mode: portal` with no `modes` and no `routing.host`, so every + # product that copied it inherited a registration that can never serve a mobile + # app. That is the origin of the portal-only defect across the fleet. - name: Templates validate - run: node bin/validate-policy.mjs templates/policy.json + run: | + node bin/validate-policy.mjs templates/policy.json + node bin/validate-registration.mjs templates # scripts/build-schema.mjs is a CI/dev generator, not shipped code (it is not # in package.json `files`), so it MAY use a devDependency — js-yaml, to read diff --git a/packages/mcp-gateway/package.json b/packages/mcp-gateway/package.json new file mode 100644 index 00000000..8d37a626 --- /dev/null +++ b/packages/mcp-gateway/package.json @@ -0,0 +1,47 @@ +{ + "name": "@fuzefront/mcp-gateway", + "version": "0.1.0", + "description": "Generic OpenAPI -> MCP SSE gateway. One image, one pod per product: each pod is configured at runtime with a product's OpenAPI spec and API base URL, exposes every operation as an MCP tool with an explicit `mutates` classification, and forwards the caller's identity upstream so per-user authorization still applies.", + "type": "module", + "main": "dist/index.js", + "types": "dist/index.d.ts", + "bin": { + "fuze-mcp-gateway": "dist/main.js" + }, + "files": [ + "dist" + ], + "scripts": { + "build": "tsc -p tsconfig.build.json", + "type-check": "tsc --noEmit", + "test": "vitest run", + "start": "node dist/main.js", + "clean": "rimraf dist", + "prepublishOnly": "npm run clean && npm run build" + }, + "publishConfig": { + "registry": "https://npm.pkg.github.com", + "access": "restricted" + }, + "repository": { + "type": "git", + "url": "git+https://github.com/izzywdev/FuzeFront.git", + "directory": "packages/mcp-gateway" + }, + "engines": { + "node": ">=24.0.0", + "npm": ">=10.0.0" + }, + "author": "FuzeFront Team", + "license": "UNLICENSED", + "dependencies": { + "@modelcontextprotocol/sdk": "^1.30.0", + "yaml": "^2.6.0" + }, + "devDependencies": { + "@types/node": "^24.13.3", + "rimraf": "^5.0.10", + "typescript": "^5.5.4", + "vitest": "^2.1.9" + } +} diff --git a/packages/mcp-gateway/scripts/smoke.mjs b/packages/mcp-gateway/scripts/smoke.mjs new file mode 100644 index 00000000..b6f2508f --- /dev/null +++ b/packages/mcp-gateway/scripts/smoke.mjs @@ -0,0 +1,71 @@ +#!/usr/bin/env node +/** + * End-to-end smoke test against a RUNNING gateway. Unlike the vitest suite this + * speaks the real MCP SSE transport, so it is what proves a product's gateway + * actually works before anyone flips `mcp.enabled` to true in that product's + * .fuze/manifest.json. + * + * MCP_PRODUCT=fuzeservice \ + * MCP_UPSTREAM_BASE_URL=http://localhost:8080/v1 \ + * MCP_OPENAPI_SPEC=../../contracts/openapi.yaml \ + * MCP_TOOL_OVERRIDES=../../mcp/tools.overrides.yaml \ + * PORT=8099 node dist/main.js & + * + * node scripts/smoke.mjs http://127.0.0.1:8099 + */ +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js'; + +const base = process.argv[2] ?? 'http://127.0.0.1:8099'; +const token = process.env.SMOKE_TOKEN ?? 'Bearer smoke-test-token'; +let failures = 0; + +function check(label, ok, detail = '') { + console.log(`${ok ? 'PASS' : 'FAIL'} ${label}${detail ? ` — ${detail}` : ''}`); + if (!ok) failures++; +} + +function connect(headers) { + const transport = new SSEClientTransport(new URL(`${base}/sse`), { + requestInit: { headers }, + eventSourceInit: { + fetch: (u, init) => fetch(u, { ...init, headers: { ...init?.headers, ...headers } }), + }, + }); + const client = new Client({ name: 'smoke', version: '1.0.0' }, { capabilities: {} }); + return client.connect(transport).then(() => client); +} + +const client = await connect({ authorization: token }); +const { tools } = await client.listTools(); + +check('MCP handshake over SSE', true); +check('tools enumerate', tools.length > 0, `${tools.length} tools`); + +// Every tool advertising readOnlyHint must be bound to a safe HTTP method. +// This is the invariant that keeps an irreversible write from being reachable +// as a side effect of something that looks like a read. +const safe = ['GET', 'HEAD', 'OPTIONS', 'TRACE']; +const liars = tools.filter( + t => t.annotations?.readOnlyHint && !safe.includes(t._meta?.['fuze/method']) +); +check('no read-only tool is bound to an unsafe method', liars.length === 0, liars.map(t => t.name).join(', ')); + +// Irreversible tools must never be advertised as reads. +const destructive = tools.filter(t => t.annotations?.destructiveHint); +check( + 'every irreversible tool is also a write', + destructive.every(t => !t.annotations.readOnlyHint), + destructive.map(t => t.name).join(', ') || 'none declared' +); + +await client.close(); + +// A call with no caller identity must be refused, and must not reach upstream. +const anon = await connect({}); +const res = await anon.callTool({ name: tools.find(t => t.annotations?.readOnlyHint)?.name, arguments: {} }); +check('call without caller identity fails closed', res.isError === true, String(res.content?.[0]?.text).slice(0, 80)); +await anon.close(); + +console.log(failures === 0 ? '\nSMOKE OK' : `\nSMOKE FAILED (${failures})`); +process.exit(failures === 0 ? 0 : 1); diff --git a/packages/mcp-gateway/src/classify.ts b/packages/mcp-gateway/src/classify.ts new file mode 100644 index 00000000..c365eeba --- /dev/null +++ b/packages/mcp-gateway/src/classify.ts @@ -0,0 +1,158 @@ +/** + * Mutation classification. + * + * This file is the safety core of the gateway. Everything else is plumbing. + * + * The rule the products care about: an operation that cannot be undone must be + * declared `mutates: true`, and must never be reachable as a side effect of a + * read or a "preview". FuzeService's approval decision is the canonical case — + * `POST /approvals/{approvalId}/decision` is irreversible from the requester's + * side, so it must be impossible to trigger by calling something that looks + * like a query. + * + * We get that structurally rather than by convention: one MCP tool maps to + * exactly ONE OpenAPI operation and issues exactly that one HTTP request. A + * read tool has no code path that can reach a mutating operation, because it + * has no code path that can reach a second request at all. + */ + +/** HTTP methods that RFC 9110 defines as safe (no intended state change). */ +const SAFE_METHODS = new Set(['get', 'head', 'options', 'trace']); + +/** + * Paths where a POST is a read in disguise — the request body is a query too + * large or too structured for a query string. This is an allowlist of SUFFIXES, + * not a substring match: `/tickets/search` qualifies, `/search-index/rebuild` + * does not. + */ +const READ_ONLY_POST_SUFFIXES = ['/search', '/query', '/preview']; + +export type Reversibility = 'reversible' | 'irreversible'; + +export interface Classification { + mutates: boolean; + /** + * Only meaningful when `mutates` is true. `irreversible` means there is no + * compensating operation the caller can invoke to undo it — the effect is + * final from the caller's side even if an operator could repair it manually. + */ + reversibility: Reversibility; + /** Why this classification was chosen, surfaced in the tool description. */ + reason: string; +} + +/** A per-product override entry, loaded from the product's overrides file. */ +export interface OverrideEntry { + mutates?: boolean; + reversibility?: Reversibility; + reason?: string; +} + +export type Overrides = Record; + +export class ClassificationError extends Error {} + +function isReadOnlyPostPath(path: string): boolean { + const p = path.replace(/\/+$/, '').toLowerCase(); + return READ_ONLY_POST_SUFFIXES.some(s => p.endsWith(s)); +} + +/** + * Derive the default classification from the HTTP method alone, before any + * override is applied. + */ +export function deriveFromMethod(method: string, path: string): Classification { + const m = method.toLowerCase(); + + if (SAFE_METHODS.has(m)) { + return { + mutates: false, + reversibility: 'reversible', + reason: `${m.toUpperCase()} is a safe method`, + }; + } + + if (m === 'post' && isReadOnlyPostPath(path)) { + return { + mutates: false, + reversibility: 'reversible', + reason: `POST ${path} is a query-shaped read (body-as-query)`, + }; + } + + // DELETE is assumed irreversible by default: a deleted resource is not + // recoverable through the API that deleted it. A product that has real + // undelete can downgrade it explicitly in its overrides file. + if (m === 'delete') { + return { + mutates: true, + reversibility: 'irreversible', + reason: 'DELETE removes a resource with no API-level undo', + }; + } + + return { + mutates: true, + reversibility: 'reversible', + reason: `${m.toUpperCase()} writes state`, + }; +} + +/** + * Apply a product's override on top of the derived classification, enforcing + * the invariants that make the whole thing trustworthy. + * + * Throws — and the gateway refuses to start — rather than silently accepting a + * dangerous declaration. A gateway that boots with a mis-declared irreversible + * tool is worse than one that does not boot, because the caller has no way to + * tell the difference until something unrecoverable has already happened. + */ +export function classify( + method: string, + path: string, + toolName: string, + override?: OverrideEntry +): Classification { + const derived = deriveFromMethod(method, path); + if (!override) return derived; + + const result: Classification = { + mutates: override.mutates ?? derived.mutates, + reversibility: override.reversibility ?? derived.reversibility, + reason: override.reason ?? derived.reason, + }; + + // INVARIANT 1: irreversible implies mutating. An override that claims an + // irreversible operation is a read is the single most dangerous thing this + // file can be asked to accept, so it is a hard failure. + if (result.reversibility === 'irreversible' && !result.mutates) { + throw new ClassificationError( + `Tool "${toolName}" (${method.toUpperCase()} ${path}) is declared irreversible but mutates:false. ` + + `An irreversible operation must be mutates:true — it cannot be exposed as a read.` + ); + } + + // INVARIANT 2: an unsafe HTTP method may only be downgraded to a read on a + // query-shaped path. Otherwise any write could be relabelled a read and + // become reachable from a context that believes it is only looking. + if (!result.mutates && !SAFE_METHODS.has(method.toLowerCase())) { + if (!(method.toLowerCase() === 'post' && isReadOnlyPostPath(path))) { + throw new ClassificationError( + `Tool "${toolName}" (${method.toUpperCase()} ${path}) is overridden to mutates:false, but ` + + `${method.toUpperCase()} is not a safe method and the path is not query-shaped ` + + `(${READ_ONLY_POST_SUFFIXES.join(', ')}). Only a POST to a query-shaped path may be declared a read.` + ); + } + } + + // INVARIANT 3: a safe method cannot be declared irreversible. If a GET really + // changes state, the spec is wrong and that is what needs fixing. + if (SAFE_METHODS.has(method.toLowerCase()) && result.reversibility === 'irreversible') { + throw new ClassificationError( + `Tool "${toolName}" (${method.toUpperCase()} ${path}) is a safe method declared irreversible. ` + + `Fix the OpenAPI spec rather than the override — a GET must not change state.` + ); + } + + return result; +} diff --git a/packages/mcp-gateway/src/config.ts b/packages/mcp-gateway/src/config.ts new file mode 100644 index 00000000..48f2b37b --- /dev/null +++ b/packages/mcp-gateway/src/config.ts @@ -0,0 +1,72 @@ +/** + * Configuration. Everything that makes this pod "the FuzeService gateway" + * rather than "the FuzeSales gateway" arrives here, at runtime, from env and + * mounted files. Nothing product-specific is baked into the image — one image, + * one pod per product. + */ + +import { readFileSync } from 'node:fs'; +import { parse as parseYaml } from 'yaml'; +import type { Overrides } from './classify.js'; +import type { OpenApiDoc } from './spec.js'; + +export interface GatewayConfig { + /** Product this pod serves, e.g. "fuzeservice". Used as the MCP server name. */ + product: string; + /** Base URL of the product's REST API, in-cluster. */ + upstreamBaseUrl: string; + /** Parsed OpenAPI document. */ + spec: OpenApiDoc; + /** Per-product mutation overrides. */ + overrides: Overrides; + port: number; +} + +function required(name: string): string { + const v = process.env[name]; + if (!v || !v.trim()) { + throw new Error(`${name} is required. This gateway is configured per product at runtime.`); + } + return v.trim(); +} + +export function loadDocument(path: string): OpenApiDoc { + const raw = readFileSync(path, 'utf8'); + const doc = (path.endsWith('.json') ? JSON.parse(raw) : parseYaml(raw)) as OpenApiDoc; + if (!doc || typeof doc !== 'object' || !doc.paths) { + throw new Error(`${path} does not look like an OpenAPI document (no "paths").`); + } + return doc; +} + +export function loadOverrides(path: string | undefined): Overrides { + if (!path) return {}; + const raw = readFileSync(path, 'utf8'); + const parsed = (path.endsWith('.json') ? JSON.parse(raw) : parseYaml(raw)) as + | { tools?: Overrides } + | Overrides; + const tools = (parsed as { tools?: Overrides }).tools ?? (parsed as Overrides); + return tools ?? {}; +} + +export function loadConfig(): GatewayConfig { + // Guard against the failure this whole design exists to prevent: someone + // "fixing" auth by giving the gateway its own credential. + for (const banned of ['MCP_UPSTREAM_TOKEN', 'MCP_SERVICE_TOKEN', 'MCP_API_KEY']) { + if (process.env[banned]) { + throw new Error( + `${banned} is set. The gateway forwards the caller's identity and must never hold a ` + + `credential of its own — a shared token bypasses every per-user authorization check ` + + `on the product API. Remove it.` + ); + } + } + + return { + product: required('MCP_PRODUCT'), + upstreamBaseUrl: required('MCP_UPSTREAM_BASE_URL'), + spec: loadDocument(required('MCP_OPENAPI_SPEC')), + overrides: loadOverrides(process.env.MCP_TOOL_OVERRIDES), + port: Number(process.env.PORT ?? 8081), + }; +} diff --git a/packages/mcp-gateway/src/index.ts b/packages/mcp-gateway/src/index.ts new file mode 100644 index 00000000..69f7f68a --- /dev/null +++ b/packages/mcp-gateway/src/index.ts @@ -0,0 +1,5 @@ +export * from './classify.js'; +export * from './spec.js'; +export * from './upstream.js'; +export * from './config.js'; +export * from './server.js'; diff --git a/packages/mcp-gateway/src/main.ts b/packages/mcp-gateway/src/main.ts new file mode 100644 index 00000000..88cfee5a --- /dev/null +++ b/packages/mcp-gateway/src/main.ts @@ -0,0 +1,39 @@ +#!/usr/bin/env node +/** + * Entry point. One process = one product's MCP gateway. + */ +import { loadConfig } from './config.js'; +import { createHttpServer } from './server.js'; + +function main() { + const config = loadConfig(); + const { httpServer, tools } = createHttpServer(config); + + const mutating = tools.filter(t => t.classification.mutates); + const irreversible = tools.filter(t => t.classification.reversibility === 'irreversible'); + + httpServer.listen(config.port, () => { + console.log( + `[mcp-gateway] product=${config.product} upstream=${config.upstreamBaseUrl} ` + + `port=${config.port} tools=${tools.length} ` + + `(${tools.length - mutating.length} read-only, ${mutating.length} write, ` + + `${irreversible.length} irreversible)` + ); + for (const t of irreversible) { + console.log(`[mcp-gateway] IRREVERSIBLE: ${t.name} -> ${t.method.toUpperCase()} ${t.path}`); + } + }); + + for (const sig of ['SIGINT', 'SIGTERM'] as const) { + process.on(sig, () => httpServer.close(() => process.exit(0))); + } +} + +try { + main(); +} catch (err) { + // Boot-time failure (bad spec, bad override, banned service token) must kill + // the pod loudly rather than serve a mis-declared tool surface. + console.error(`[mcp-gateway] FATAL: ${(err as Error).message}`); + process.exit(1); +} diff --git a/packages/mcp-gateway/src/server.ts b/packages/mcp-gateway/src/server.ts new file mode 100644 index 00000000..654b964b --- /dev/null +++ b/packages/mcp-gateway/src/server.ts @@ -0,0 +1,172 @@ +/** + * MCP server wiring + the remote SSE HTTP surface. + * + * Transport is the MCP HTTP+SSE pair: + * GET /sse -> opens the event stream, returns a sessionId + * POST /messages -> client -> server JSON-RPC, correlated by sessionId + * + * The caller's HTTP headers are captured per session and replayed onto every + * upstream call made in that session, which is what makes per-user authorization + * work: the product API sees the user, not the gateway. + */ + +import http from 'node:http'; +import { Server } from '@modelcontextprotocol/sdk/server/index.js'; +import { SSEServerTransport } from '@modelcontextprotocol/sdk/server/sse.js'; +import { + CallToolRequestSchema, + ListToolsRequestSchema, +} from '@modelcontextprotocol/sdk/types.js'; +import { buildTools, type ToolDescriptor } from './spec.js'; +import { callUpstream, MissingIdentityError, type CallerContext } from './upstream.js'; +import type { GatewayConfig } from './config.js'; + +export interface Session { + transport: SSEServerTransport; + caller: CallerContext; +} + +export function createMcpServer(config: GatewayConfig, tools: ToolDescriptor[], getCaller: () => CallerContext) { + const server = new Server( + { name: `${config.product}-mcp-gateway`, version: '0.1.0' }, + { capabilities: { tools: {} } } + ); + + const byName = new Map(tools.map(t => [t.name, t])); + + server.setRequestHandler(ListToolsRequestSchema, async () => ({ + tools: tools.map(t => ({ + name: t.name, + description: t.description, + inputSchema: t.inputSchema, + // Non-standard but load-bearing: the classification travels with the tool + // so a client can refuse to auto-approve irreversible calls. + _meta: { + 'fuze/mutates': t.classification.mutates, + 'fuze/reversibility': t.classification.reversibility, + 'fuze/method': t.method.toUpperCase(), + 'fuze/path': t.path, + }, + annotations: { + readOnlyHint: !t.classification.mutates, + destructiveHint: t.classification.reversibility === 'irreversible', + idempotentHint: ['get', 'head', 'put', 'delete'].includes(t.method.toLowerCase()), + }, + })), + })); + + server.setRequestHandler(CallToolRequestSchema, async request => { + const tool = byName.get(request.params.name); + if (!tool) { + return { + isError: true, + content: [{ type: 'text' as const, text: `Unknown tool: ${request.params.name}` }], + }; + } + + const args = (request.params.arguments ?? {}) as Record; + + try { + const result = await callUpstream(tool, args, config.upstreamBaseUrl, getCaller()); + return { + isError: !result.ok, + content: [ + { + type: 'text' as const, + text: + typeof result.body === 'string' + ? result.body + : JSON.stringify(result.body, null, 2), + }, + ], + }; + } catch (err) { + const message = err instanceof MissingIdentityError + ? err.message + : `Upstream call failed: ${(err as Error).message}`; + return { isError: true, content: [{ type: 'text' as const, text: message }] }; + } + }); + + return server; +} + +export function createHttpServer(config: GatewayConfig) { + // Built once at boot. A classification error here means the process exits + // before serving anything, which is the intended failure mode. + const tools = buildTools(config.spec, config.overrides); + + const sessions = new Map(); + // The MCP SDK invokes tool handlers without the originating HTTP request, so + // the active session's caller context is set immediately before dispatch. + let activeCaller: CallerContext = { headers: {} }; + + const httpServer = http.createServer(async (req, res) => { + const url = new URL(req.url ?? '/', `http://${req.headers.host ?? 'localhost'}`); + + if (req.method === 'GET' && url.pathname === '/healthz') { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end(JSON.stringify({ status: 'ok', product: config.product, tools: tools.length })); + return; + } + + // Introspection endpoint: the tool manifest with its mutation classification. + // Read-only and unauthenticated by design — it exposes the SHAPE of the API + // (which the OpenAPI spec already publishes), never any data from it. + if (req.method === 'GET' && url.pathname === '/tools.json') { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end( + JSON.stringify( + { + product: config.product, + generatedFrom: config.spec.info?.title ?? 'openapi', + tools: tools.map(t => ({ + name: t.name, + method: t.method.toUpperCase(), + path: t.path, + mutates: t.classification.mutates, + reversibility: t.classification.reversibility, + reason: t.classification.reason, + description: t.description, + })), + }, + null, + 2 + ) + ); + return; + } + + if (req.method === 'GET' && url.pathname === '/sse') { + const transport = new SSEServerTransport('/messages', res); + const caller: CallerContext = { headers: req.headers }; + const server = createMcpServer(config, tools, () => activeCaller); + + sessions.set(transport.sessionId, { transport, caller }); + res.on('close', () => sessions.delete(transport.sessionId)); + + await server.connect(transport); + return; + } + + if (req.method === 'POST' && url.pathname === '/messages') { + const sessionId = url.searchParams.get('sessionId') ?? ''; + const session = sessions.get(sessionId); + if (!session) { + res.writeHead(404, { 'content-type': 'application/json' }); + res.end(JSON.stringify({ error: 'Unknown or expired sessionId' })); + return; + } + // Prefer the identity on THIS request; fall back to the one presented when + // the stream was opened. Either way it is the caller's, never the gateway's. + activeCaller = req.headers.authorization ? { headers: req.headers } : session.caller; + await session.transport.handlePostMessage(req, res); + return; + } + + res.writeHead(404, { 'content-type': 'application/json' }); + res.end(JSON.stringify({ error: 'Not found' })); + }); + + return { httpServer, tools }; +} diff --git a/packages/mcp-gateway/src/spec.ts b/packages/mcp-gateway/src/spec.ts new file mode 100644 index 00000000..50ef97a2 --- /dev/null +++ b/packages/mcp-gateway/src/spec.ts @@ -0,0 +1,188 @@ +/** + * OpenAPI -> MCP tool descriptors. + * + * The gateway is "merely a layer that exposes the REST API as tools": there is + * no per-product logic here, and adding a product means pointing a pod at a + * different spec, never editing this file. + */ + +import { classify, type Classification, type Overrides } from './classify.js'; + +const HTTP_METHODS = ['get', 'put', 'post', 'delete', 'patch', 'head', 'options'] as const; + +export interface ToolParam { + name: string; + in: 'path' | 'query' | 'header'; + required: boolean; + schema: Record; + description?: string; +} + +export interface ToolDescriptor { + name: string; + description: string; + method: string; + path: string; + params: ToolParam[]; + /** JSON Schema for the request body, if the operation takes one. */ + bodySchema?: Record; + bodyRequired: boolean; + classification: Classification; + inputSchema: Record; +} + +export interface OpenApiDoc { + openapi?: string; + info?: { title?: string; version?: string }; + servers?: Array<{ url?: string }>; + paths?: Record>; + components?: Record; +} + +/** + * Resolve a local `$ref` against the document. Remote refs are NOT followed: + * the gateway must not make network calls to understand its own config, and a + * spec that depends on fetching another host at boot is a spec that fails + * differently in every environment. + */ +function resolveRef(doc: OpenApiDoc, node: unknown, seen = new Set()): unknown { + if (!node || typeof node !== 'object') return node; + const obj = node as Record; + const ref = obj.$ref; + if (typeof ref !== 'string') return node; + + if (!ref.startsWith('#/')) { + throw new Error(`Remote $ref is not supported: ${ref}. Bundle the spec before mounting it.`); + } + if (seen.has(ref)) return {}; // circular — stop, do not hang + seen.add(ref); + + const parts = ref.slice(2).split('/').map(p => p.replace(/~1/g, '/').replace(/~0/g, '~')); + let cur: unknown = doc; + for (const p of parts) { + if (!cur || typeof cur !== 'object') return {}; + cur = (cur as Record)[p]; + } + return resolveRef(doc, cur, seen); +} + +/** Turn an operation into a stable, MCP-legal tool name. */ +export function toolNameFor(operationId: unknown, method: string, path: string): string { + if (typeof operationId === 'string' && operationId.trim()) { + return operationId.trim().replace(/[^a-zA-Z0-9_-]/g, '_').slice(0, 64); + } + const slug = path + .replace(/\{([^}]+)\}/g, 'by_$1') + .replace(/[^a-zA-Z0-9]+/g, '_') + .replace(/^_+|_+$/g, ''); + return `${method.toLowerCase()}_${slug}`.slice(0, 64); +} + +function buildInputSchema(params: ToolParam[], bodySchema?: Record, bodyRequired = false) { + const properties: Record = {}; + const required: string[] = []; + + for (const p of params) { + properties[p.name] = p.description ? { ...p.schema, description: p.description } : p.schema; + if (p.required) required.push(p.name); + } + + if (bodySchema) { + properties.body = { ...bodySchema, description: 'Request body.' }; + if (bodyRequired) required.push('body'); + } + + return { + type: 'object', + properties, + ...(required.length ? { required } : {}), + additionalProperties: false, + }; +} + +/** + * Build the full tool list for a spec. Throws if any classification invariant is + * violated, so a bad overrides file stops the pod at boot instead of at the + * first dangerous call. + */ +export function buildTools(doc: OpenApiDoc, overrides: Overrides = {}): ToolDescriptor[] { + const tools: ToolDescriptor[] = []; + const paths = doc.paths ?? {}; + const seenNames = new Set(); + + for (const [path, pathItemRaw] of Object.entries(paths)) { + const pathItem = (resolveRef(doc, pathItemRaw) ?? {}) as Record; + // Parameters declared once for the whole path apply to every operation. + const sharedParams = Array.isArray(pathItem.parameters) ? pathItem.parameters : []; + + for (const method of HTTP_METHODS) { + const opRaw = pathItem[method]; + if (!opRaw || typeof opRaw !== 'object') continue; + const op = opRaw as Record; + + const name = toolNameFor(op.operationId, method, path); + if (seenNames.has(name)) { + throw new Error( + `Duplicate tool name "${name}" (${method.toUpperCase()} ${path}). ` + + `Give the operation a unique operationId in the spec.` + ); + } + seenNames.add(name); + + const rawParams = [...sharedParams, ...(Array.isArray(op.parameters) ? op.parameters : [])]; + const params: ToolParam[] = []; + for (const rp of rawParams) { + const p = (resolveRef(doc, rp) ?? {}) as Record; + const loc = p.in; + if (loc !== 'path' && loc !== 'query' && loc !== 'header') continue; + if (typeof p.name !== 'string') continue; + params.push({ + name: p.name, + in: loc, + required: Boolean(p.required) || loc === 'path', + schema: ((resolveRef(doc, p.schema) as Record) ?? { type: 'string' }), + description: typeof p.description === 'string' ? p.description : undefined, + }); + } + + let bodySchema: Record | undefined; + let bodyRequired = false; + const rb = resolveRef(doc, op.requestBody) as Record | undefined; + if (rb && typeof rb === 'object') { + bodyRequired = Boolean(rb.required); + const content = rb.content as Record | undefined; + const json = content?.['application/json'] as Record | undefined; + if (json?.schema) { + bodySchema = (resolveRef(doc, json.schema) as Record) ?? {}; + } + } + + const classification = classify(method, path, name, overrides[name]); + + const summary = + (typeof op.summary === 'string' && op.summary) || + (typeof op.description === 'string' && op.description) || + `${method.toUpperCase()} ${path}`; + + // The description carries the safety facts, because the model choosing a + // tool sees the description and not our internal metadata. + const safety = classification.mutates + ? `[WRITE${classification.reversibility === 'irreversible' ? ' — IRREVERSIBLE' : ''}]` + : '[READ-ONLY]'; + + tools.push({ + name, + description: `${safety} ${summary} (${method.toUpperCase()} ${path}). ${classification.reason}.`, + method, + path, + params, + bodySchema, + bodyRequired, + classification, + inputSchema: buildInputSchema(params, bodySchema, bodyRequired), + }); + } + } + + return tools; +} diff --git a/packages/mcp-gateway/src/upstream.ts b/packages/mcp-gateway/src/upstream.ts new file mode 100644 index 00000000..267d5b67 --- /dev/null +++ b/packages/mcp-gateway/src/upstream.ts @@ -0,0 +1,130 @@ +/** + * Upstream HTTP caller. + * + * AUTHZ CONTRACT — the part that must not be "simplified" later: + * + * The gateway forwards the CALLER'S identity to the product API and never + * substitutes one of its own. There is deliberately no service-token option, no + * `MCP_UPSTREAM_TOKEN`, and no fallback credential anywhere in this file. A + * shared token would make every request look like the gateway rather than like + * the user, which silently bypasses every per-user Permit check on the product + * side — the gateway would become a confused deputy with the union of all + * users' permissions. + * + * A call with no caller credential is refused here rather than sent onward + * unauthenticated, so a missing token fails closed and visibly. + */ + +import type { ToolDescriptor } from './spec.js'; + +export class MissingIdentityError extends Error {} + +export interface UpstreamResult { + status: number; + ok: boolean; + body: unknown; +} + +/** Headers we forward from the MCP caller to the product API. */ +const FORWARDED_HEADERS = ['authorization', 'x-request-id', 'x-tenant-id', 'x-organization-id']; + +export interface CallerContext { + /** Raw headers from the MCP client's HTTP request. */ + headers: Record; +} + +export function extractForwardHeaders(ctx: CallerContext): Record { + const out: Record = {}; + for (const [k, v] of Object.entries(ctx.headers ?? {})) { + const key = k.toLowerCase(); + if (!FORWARDED_HEADERS.includes(key)) continue; + const value = Array.isArray(v) ? v[0] : v; + if (typeof value === 'string' && value.length > 0) out[key] = value; + } + return out; +} + +/** Substitute {param} path segments and split args into query/header/body. */ +export function buildRequest( + tool: ToolDescriptor, + args: Record, + baseUrl: string +): { url: string; headers: Record; body?: string } { + let path = tool.path; + const query = new URLSearchParams(); + const headers: Record = {}; + + for (const p of tool.params) { + const raw = args[p.name]; + if (raw === undefined || raw === null) { + if (p.required) { + throw new Error(`Missing required parameter "${p.name}" for tool "${tool.name}".`); + } + continue; + } + const value = String(raw); + if (p.in === 'path') { + path = path.replace(`{${p.name}}`, encodeURIComponent(value)); + } else if (p.in === 'query') { + if (Array.isArray(raw)) raw.forEach(v => query.append(p.name, String(v))); + else query.append(p.name, value); + } else { + headers[p.name] = value; + } + } + + const unresolved = path.match(/\{[^}]+\}/); + if (unresolved) { + throw new Error(`Unresolved path parameter ${unresolved[0]} for tool "${tool.name}".`); + } + + const qs = query.toString(); + const url = `${baseUrl.replace(/\/+$/, '')}${path}${qs ? `?${qs}` : ''}`; + + let body: string | undefined; + if (tool.bodySchema && args.body !== undefined) { + body = JSON.stringify(args.body); + headers['content-type'] = 'application/json'; + } + + return { url, headers, body }; +} + +export async function callUpstream( + tool: ToolDescriptor, + args: Record, + baseUrl: string, + caller: CallerContext, + fetchImpl: typeof fetch = fetch +): Promise { + const forwarded = extractForwardHeaders(caller); + + if (!forwarded.authorization) { + // Fail closed. Never fall back to a gateway-owned credential. + throw new MissingIdentityError( + 'No Authorization header on the MCP request. The gateway forwards the caller\'s ' + + 'identity to the product API and has no credential of its own, so this call ' + + 'cannot be made. Authenticate the MCP client and retry.' + ); + } + + const { url, headers, body } = buildRequest(tool, args, baseUrl); + + const res = await fetchImpl(url, { + method: tool.method.toUpperCase(), + headers: { accept: 'application/json', ...headers, ...forwarded }, + ...(body !== undefined ? { body } : {}), + }); + + const text = await res.text(); + let parsed: unknown = text; + if (text) { + try { + parsed = JSON.parse(text); + } catch { + /* non-JSON upstream response is returned as text */ + } + } + + return { status: res.status, ok: res.ok, body: parsed }; +} diff --git a/packages/mcp-gateway/test/classify.test.ts b/packages/mcp-gateway/test/classify.test.ts new file mode 100644 index 00000000..c271806d --- /dev/null +++ b/packages/mcp-gateway/test/classify.test.ts @@ -0,0 +1,87 @@ +import { describe, it, expect } from 'vitest'; +import { classify, deriveFromMethod, ClassificationError } from '../src/classify.js'; + +describe('deriveFromMethod', () => { + it('treats safe methods as reads', () => { + for (const m of ['get', 'head', 'options', 'trace']) { + const c = deriveFromMethod(m, '/tickets'); + expect(c.mutates, m).toBe(false); + expect(c.reversibility, m).toBe('reversible'); + } + }); + + it('treats ordinary writes as reversible mutations', () => { + for (const m of ['post', 'put', 'patch']) { + const c = deriveFromMethod(m, '/tickets'); + expect(c.mutates, m).toBe(true); + expect(c.reversibility, m).toBe('reversible'); + } + }); + + it('treats DELETE as irreversible by default', () => { + const c = deriveFromMethod('delete', '/tickets/{id}'); + expect(c.mutates).toBe(true); + expect(c.reversibility).toBe('irreversible'); + }); + + it('treats query-shaped POSTs as reads', () => { + for (const p of ['/tickets/search', '/kb/query', '/tickets/preview']) { + const c = deriveFromMethod('post', p); + expect(c.mutates, p).toBe(false); + } + }); + + it('does not match query-shaped words mid-path', () => { + // `/search-index/rebuild` must NOT be mistaken for a read just because it + // contains "search" — this is the substring-vs-suffix bug the allowlist exists to avoid. + const c = deriveFromMethod('post', '/search-index/rebuild'); + expect(c.mutates).toBe(true); + }); +}); + +describe('classify invariants', () => { + it('refuses an irreversible operation declared as a read', () => { + expect(() => + classify('post', '/approvals/{id}/decision', 'decide', { + mutates: false, + reversibility: 'irreversible', + }) + ).toThrow(ClassificationError); + }); + + it('refuses to downgrade a non-query-shaped write to a read', () => { + expect(() => + classify('post', '/approvals/{id}/decision', 'decide', { mutates: false }) + ).toThrow(ClassificationError); + }); + + it('allows downgrading a query-shaped POST to a read', () => { + const c = classify('post', '/tickets/search', 'searchTickets', { mutates: false }); + expect(c.mutates).toBe(false); + }); + + it('refuses to call a safe method irreversible', () => { + expect(() => + classify('get', '/tickets', 'listTickets', { reversibility: 'irreversible' }) + ).toThrow(ClassificationError); + }); + + it('allows marking a POST irreversible', () => { + const c = classify('post', '/approvals/{id}/decision', 'decide', { + reversibility: 'irreversible', + reason: 'An approval decision is final from the requester side', + }); + expect(c.mutates).toBe(true); + expect(c.reversibility).toBe('irreversible'); + expect(c.reason).toMatch(/final/); + }); + + it('allows a product to declare a DELETE reversible when it has real undelete', () => { + const c = classify('delete', '/kb/articles/{id}', 'deleteArticle', { + reversibility: 'reversible', + reason: 'Soft delete; restorable via PATCH', + }); + expect(c.mutates).toBe(true); + expect(c.reversibility).toBe('reversible'); + }); +}); diff --git a/packages/mcp-gateway/test/spec.test.ts b/packages/mcp-gateway/test/spec.test.ts new file mode 100644 index 00000000..2eaf76a3 --- /dev/null +++ b/packages/mcp-gateway/test/spec.test.ts @@ -0,0 +1,125 @@ +import { describe, it, expect } from 'vitest'; +import { buildTools, toolNameFor, type OpenApiDoc } from '../src/spec.js'; + +const doc: OpenApiDoc = { + openapi: '3.1.0', + info: { title: 'Test API', version: '1.0.0' }, + paths: { + '/tickets': { + get: { + operationId: 'listTickets', + summary: 'List tickets', + parameters: [ + { name: 'status', in: 'query', schema: { type: 'string' } }, + { $ref: '#/components/parameters/PageSize' }, + ], + }, + post: { + operationId: 'createTicket', + summary: 'Create a ticket', + requestBody: { + required: true, + content: { 'application/json': { schema: { $ref: '#/components/schemas/Ticket' } } }, + }, + }, + }, + '/tickets/{ticketId}': { + parameters: [{ name: 'ticketId', in: 'path', required: true, schema: { type: 'string' } }], + get: { operationId: 'getTicket', summary: 'Get a ticket' }, + patch: { operationId: 'updateTicket', summary: 'Update a ticket' }, + }, + '/approvals/{approvalId}/decision': { + post: { + operationId: 'decideApproval', + summary: 'Record an approval decision', + parameters: [{ name: 'approvalId', in: 'path', required: true, schema: { type: 'string' } }], + }, + }, + }, + components: { + parameters: { PageSize: { name: 'pageSize', in: 'query', schema: { type: 'integer' } } }, + schemas: { Ticket: { type: 'object', properties: { subject: { type: 'string' } } } }, + }, +}; + +describe('toolNameFor', () => { + it('prefers operationId', () => { + expect(toolNameFor('listTickets', 'get', '/tickets')).toBe('listTickets'); + }); + it('falls back to a method_path slug', () => { + expect(toolNameFor(undefined, 'get', '/tickets/{ticketId}')).toBe('get_tickets_by_ticketId'); + }); +}); + +describe('buildTools', () => { + const tools = buildTools(doc); + const byName = Object.fromEntries(tools.map(t => [t.name, t])); + + it('emits one tool per operation', () => { + expect(tools).toHaveLength(5); + }); + + it('resolves local $ref parameters', () => { + const list = byName.listTickets; + expect(list.params.map(p => p.name).sort()).toEqual(['pageSize', 'status']); + }); + + it('inherits path-level parameters into every operation', () => { + expect(byName.getTicket.params.map(p => p.name)).toEqual(['ticketId']); + expect(byName.updateTicket.params.map(p => p.name)).toEqual(['ticketId']); + }); + + it('resolves the request body schema and marks it required', () => { + const create = byName.createTicket; + expect(create.bodyRequired).toBe(true); + expect(create.bodySchema).toMatchObject({ type: 'object' }); + expect((create.inputSchema as any).required).toContain('body'); + }); + + it('classifies reads and writes from the method', () => { + expect(byName.listTickets.classification.mutates).toBe(false); + expect(byName.createTicket.classification.mutates).toBe(true); + expect(byName.updateTicket.classification.mutates).toBe(true); + }); + + it('surfaces the safety class in the description the model reads', () => { + expect(byName.listTickets.description).toMatch(/^\[READ-ONLY\]/); + expect(byName.createTicket.description).toMatch(/^\[WRITE\]/); + }); + + it('honours an irreversible override and labels it in the description', () => { + const withOverride = buildTools(doc, { + decideApproval: { + reversibility: 'irreversible', + reason: 'An approval decision is irreversible from the requester side', + }, + }); + const decide = withOverride.find(t => t.name === 'decideApproval')!; + expect(decide.classification.mutates).toBe(true); + expect(decide.classification.reversibility).toBe('irreversible'); + expect(decide.description).toMatch(/IRREVERSIBLE/); + }); + + it('refuses to build when an override would expose an irreversible op as a read', () => { + expect(() => + buildTools(doc, { decideApproval: { mutates: false, reversibility: 'irreversible' } }) + ).toThrow(/irreversible/i); + }); + + it('rejects duplicate tool names', () => { + const dup: OpenApiDoc = { + paths: { + '/a': { get: { operationId: 'same' } }, + '/b': { get: { operationId: 'same' } }, + }, + }; + expect(() => buildTools(dup)).toThrow(/Duplicate tool name/); + }); + + it('rejects remote $refs rather than fetching them at boot', () => { + const remote: OpenApiDoc = { + paths: { '/a': { get: { operationId: 'a', parameters: [{ $ref: 'https://x/y#/z' }] } } }, + }; + expect(() => buildTools(remote)).toThrow(/Remote \$ref/); + }); +}); diff --git a/packages/mcp-gateway/test/upstream.test.ts b/packages/mcp-gateway/test/upstream.test.ts new file mode 100644 index 00000000..271ee197 --- /dev/null +++ b/packages/mcp-gateway/test/upstream.test.ts @@ -0,0 +1,108 @@ +import { describe, it, expect, vi } from 'vitest'; +import { buildTools, type OpenApiDoc } from '../src/spec.js'; +import { buildRequest, callUpstream, extractForwardHeaders, MissingIdentityError } from '../src/upstream.js'; + +const doc: OpenApiDoc = { + paths: { + '/tickets/{ticketId}': { + get: { + operationId: 'getTicket', + parameters: [ + { name: 'ticketId', in: 'path', required: true, schema: { type: 'string' } }, + { name: 'expand', in: 'query', schema: { type: 'string' } }, + ], + }, + }, + '/tickets': { + post: { + operationId: 'createTicket', + requestBody: { + required: true, + content: { 'application/json': { schema: { type: 'object' } } }, + }, + }, + }, + }, +}; + +const tools = Object.fromEntries(buildTools(doc).map(t => [t.name, t])); +const BASE = 'http://fuzeservice-service.fuzeservice.svc.cluster.local:8080/v1'; + +describe('buildRequest', () => { + it('substitutes path params and appends query params', () => { + const { url } = buildRequest(tools.getTicket, { ticketId: 'T-1', expand: 'sla' }, BASE); + expect(url).toBe(`${BASE}/tickets/T-1?expand=sla`); + }); + + it('url-encodes path params', () => { + const { url } = buildRequest(tools.getTicket, { ticketId: 'a/b' }, BASE); + expect(url).toBe(`${BASE}/tickets/a%2Fb`); + }); + + it('throws rather than sending a request with an unresolved path param', () => { + expect(() => buildRequest(tools.getTicket, {}, BASE)).toThrow(/Missing required parameter/); + }); + + it('serialises the body and sets content-type', () => { + const { body, headers } = buildRequest(tools.createTicket, { body: { subject: 'hi' } }, BASE); + expect(body).toBe('{"subject":"hi"}'); + expect(headers['content-type']).toBe('application/json'); + }); +}); + +describe('identity forwarding', () => { + it('forwards only the allowlisted caller headers', () => { + const out = extractForwardHeaders({ + headers: { + authorization: 'Bearer user-token', + 'x-request-id': 'req-1', + cookie: 'session=secret', + 'x-forwarded-for': '1.2.3.4', + }, + }); + expect(out).toEqual({ authorization: 'Bearer user-token', 'x-request-id': 'req-1' }); + expect(out.cookie).toBeUndefined(); + }); + + it("sends the caller's token upstream verbatim", async () => { + const fetchImpl = vi.fn().mockResolvedValue( + new Response('{"id":"T-1"}', { status: 200, headers: { 'content-type': 'application/json' } }) + ); + await callUpstream( + tools.getTicket, + { ticketId: 'T-1' }, + BASE, + { headers: { authorization: 'Bearer user-token' } }, + fetchImpl as unknown as typeof fetch + ); + const [, init] = fetchImpl.mock.calls[0]; + expect((init.headers as Record).authorization).toBe('Bearer user-token'); + }); + + it('fails closed when the caller presents no identity', async () => { + const fetchImpl = vi.fn(); + await expect( + callUpstream(tools.getTicket, { ticketId: 'T-1' }, BASE, { headers: {} }, fetchImpl as unknown as typeof fetch) + ).rejects.toBeInstanceOf(MissingIdentityError); + // The decisive assertion: no request was made at all. An unauthenticated + // call must never reach the product API, because the product would then be + // deciding authorization for an anonymous caller. + expect(fetchImpl).not.toHaveBeenCalled(); + }); + + it('parses a JSON error body and reports it as an error result', async () => { + const fetchImpl = vi.fn().mockResolvedValue( + new Response('{"error":"forbidden"}', { status: 403 }) + ); + const res = await callUpstream( + tools.getTicket, + { ticketId: 'T-1' }, + BASE, + { headers: { authorization: 'Bearer u' } }, + fetchImpl as unknown as typeof fetch + ); + expect(res.ok).toBe(false); + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: 'forbidden' }); + }); +}); diff --git a/packages/mcp-gateway/tsconfig.build.json b/packages/mcp-gateway/tsconfig.build.json new file mode 100644 index 00000000..fc8520e7 --- /dev/null +++ b/packages/mcp-gateway/tsconfig.build.json @@ -0,0 +1,3 @@ +{ + "extends": "./tsconfig.json" +} diff --git a/packages/mcp-gateway/tsconfig.json b/packages/mcp-gateway/tsconfig.json new file mode 100644 index 00000000..189ffd35 --- /dev/null +++ b/packages/mcp-gateway/tsconfig.json @@ -0,0 +1,18 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "NodeNext", + "moduleResolution": "NodeNext", + "lib": ["ES2022"], + "strict": true, + "esModuleInterop": true, + "skipLibCheck": true, + "forceConsistentCasingInFileNames": true, + "declaration": true, + "outDir": "dist", + "rootDir": "src", + "types": ["node"] + }, + "include": ["src/**/*.ts"], + "exclude": ["node_modules", "dist", "test"] +} diff --git a/packages/mcp-gateway/vitest.config.ts b/packages/mcp-gateway/vitest.config.ts new file mode 100644 index 00000000..fa69665c --- /dev/null +++ b/packages/mcp-gateway/vitest.config.ts @@ -0,0 +1,8 @@ +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + test: { + include: ['test/**/*.test.ts'], + environment: 'node', + }, +}); diff --git a/packages/onboarding-kit/README.md b/packages/onboarding-kit/README.md index 3ac99b46..125dd03d 100644 --- a/packages/onboarding-kit/README.md +++ b/packages/onboarding-kit/README.md @@ -123,6 +123,38 @@ build step: simply grants nothing, so the symptom is *"our users have no permissions"*, which reads as a bug in your app. +## Validating the whole `registration/` directory in your CI + +```bash +npx fuzefront-validate-registration registration +# or, from a checkout of the kit: +node bin/validate-registration.mjs registration +``` + +Where `validate-policy` checks that one file is **well-formed**, this checks that your +registration satisfies **fleet policy** — the rules the platform requires of every +product but that no schema can express. Also zero dependencies. + +It enforces three things: + +| Rule | Why | +|---|---| +| Effective modes include `portal` **and** `standalone` | `standalone` is the only surface a mobile TWA/APK can wrap, because an app store needs a URL that stands on its own. | +| `standalone` implies a non-empty `routing.host` | A standalone surface with no host has no URL to serve or to wrap. | +| `policy.json` exists, and a vendored `register.sh` actually submits it | A pre-kit script that skips the policy step leaves the product with no roles. | + +**Why this is not a schema rule.** `mode: "portal"` with `modes` omitted is entirely +valid — the contract says an absent `modes` falls back to `[mode]`. Such a product +registers cleanly, appears in the portal, passes every existing gate, and is silently +incapable of ever shipping a mobile app. Nothing is malformed; a capability simply +never exists. That is precisely the class of failure a schema cannot catch and this +gate can. + +**Embed-only products are exempt** from the surface rules. Per the contract an embed +renders inside a third-party page with neither portal chrome nor FuzeFront navigation, +is not a portal destination, and may not register a menu entry at all — so requiring a +portal surface of it would be wrong. + The validator enforces the frozen `ProductPolicy` contract: - keys are **bare** (`Ticket`, not `fuzeservice_Ticket`) and contain **no `_`** — `_` diff --git a/packages/onboarding-kit/bin/validate-registration.mjs b/packages/onboarding-kit/bin/validate-registration.mjs new file mode 100644 index 00000000..2ab565cb --- /dev/null +++ b/packages/onboarding-kit/bin/validate-registration.mjs @@ -0,0 +1,191 @@ +#!/usr/bin/env node +// Validate a product's registration/ directory against FLEET POLICY — the rules the +// platform requires of every product but that no schema can express. +// +// WHY THIS EXISTS, and why it is NOT a schema tightening: +// +// The frozen contract already validates the SHAPE of a manifest. What it cannot know +// is what the fleet requires of a *product*. Two live examples, both of which pass +// every existing gate: +// +// 1. `mode: "portal"` with `modes` omitted is perfectly valid — the contract says +// an absent `modes` falls back to `[mode]`. The product registers, appears in the +// portal, and is silently incapable of EVER shipping a mobile app, because a TWA +// can only wrap a `standalone` surface with a URL that stands on its own. Nothing +// is broken; nothing is reported; the capability simply never exists. +// +// 2. A vendored `register.sh` that predates this kit has no policy step at all. The +// product's policy.json is never submitted, so it gets no roles. Authorization +// then fails closed for every user, which reads as a bug in the PRODUCT rather +// than a gap in its registration. +// +// Both failures are invisible by construction: they produce no error, no 4xx, and no +// log line anybody reads. They surface as "this product is mysteriously limited". This +// gate converts that whole class into a red build in the repo that owns the file. +// +// Usage: +// node validate-registration.mjs [path/to/registration ...] +// +// With no arguments it validates ./registration. +// Exit code 0 = conformant, 1 = violation. + +import { readFileSync, existsSync } from 'node:fs' +import { join, resolve } from 'node:path' + +// The surfaces every portal-destination product must serve. `portal` is how it appears +// in the shell; `standalone` is the only surface a mobile TWA/APK can wrap, because an +// app store needs a URL that stands on its own. Declaring one without the other is a +// product that is either invisible in the portal or permanently desktop-only. +const REQUIRED_SURFACES = ['portal', 'standalone'] + +/** + * Resolve the surfaces a manifest actually serves. + * + * Mirrors the contract's fallback rule: `modes` is the multi-valued form, and an + * ABSENT `modes` falls back to `[mode]`. This distinction is the whole point of the + * gate — an absent `modes` is legal, so it must be resolved, not rejected outright. + * + * @param {Record} manifest + * @returns {string[]} + */ +export function effectiveModes(manifest) { + if (Array.isArray(manifest.modes) && manifest.modes.length > 0) return manifest.modes + return typeof manifest.mode === 'string' ? [manifest.mode] : [] +} + +/** + * @param {Record} manifest parsed manifest.json + * @returns {string[]} human-readable violations; empty means conformant + */ +export function validateSurfaces(manifest) { + const errors = [] + const modes = effectiveModes(manifest) + + if (modes.length === 0) { + errors.push('manifest declares neither `mode` nor `modes` — no surface at all') + return errors + } + + // An embed-only product is a legitimate exemption, not an oversight: per the + // contract it renders inside a THIRD-PARTY page with neither portal chrome nor + // FuzeFront navigation, "is not a portal destination and may not register a menu + // entry at all". Requiring a portal surface of it would be wrong. + if (modes.includes('embed') && !modes.includes('portal')) return errors + + for (const surface of REQUIRED_SURFACES) { + if (!modes.includes(surface)) { + errors.push( + `\`modes\` does not include "${surface}" (effective modes: [${modes.join(', ')}])` + + (surface === 'standalone' + ? ' — without it this product can never ship a mobile app, because a TWA can only wrap a standalone URL' + : '') + ) + } + } + + // A standalone surface with no host is the same failure wearing a disguise: the mode + // is declared, so it LOOKS conformant, but there is no URL for anything to reach. + if (modes.includes('standalone')) { + const host = manifest.routing && typeof manifest.routing === 'object' + ? manifest.routing.host + : undefined + if (typeof host !== 'string' || host.trim() === '') { + errors.push( + '`modes` includes "standalone" but `routing.host` is missing or empty — ' + + 'a standalone surface with no host has no URL to serve or to wrap' + ) + } + } + + return errors +} + +/** + * The policy must not merely exist — it must actually be SUBMITTED. A vendored + * register.sh that predates the kit will happily register the app and skip the policy, + * which is the failure that produces a product whose users have no permissions. + * + * @param {string} dir the registration/ directory + * @returns {string[]} + */ +export function validatePolicyWiring(dir) { + const errors = [] + const policyPath = join(dir, 'policy.json') + const scriptPath = join(dir, 'register.sh') + + if (!existsSync(policyPath)) { + errors.push( + 'policy.json is missing — the product will register with no product-specific ' + + 'roles, and authorization will fail closed for every user' + ) + } + + // Only inspect a VENDORED script. A product consuming the kit from npm has no + // register.sh of its own, and that is the preferred shape — absence is not a fault. + if (existsSync(scriptPath)) { + const script = readFileSync(scriptPath, 'utf8') + // Match the submission itself, not the word "policy" — a comment mentioning policy + // must not satisfy the check. + const submits = /\/apps\/[^"'\s]*\/policy|apps\/\$\{?SLUG\}?\/policy/.test(script) + if (!submits) { + errors.push( + 'vendored register.sh has no policy submission step (no PUT to /apps/{slug}/policy) — ' + + 'this is the pre-kit script; policy.json will never reach the platform' + ) + } + } + + return errors +} + +/** + * @param {string} dir path to a registration/ directory + * @returns {string[]} + */ +export function validateRegistrationDir(dir) { + const manifestPath = join(dir, 'manifest.json') + if (!existsSync(manifestPath)) return [`${manifestPath}: not found`] + + let manifest + try { + manifest = JSON.parse(readFileSync(manifestPath, 'utf8')) + } catch (err) { + return [`${manifestPath}: not valid JSON — ${err.message}`] + } + if (manifest === null || typeof manifest !== 'object' || Array.isArray(manifest)) { + return [`${manifestPath}: must be a JSON object`] + } + + return [...validateSurfaces(manifest), ...validatePolicyWiring(dir)] +} + +// ---- CLI --------------------------------------------------------------------------- + +const invokedDirectly = + process.argv[1] && resolve(process.argv[1]).endsWith('validate-registration.mjs') + +if (invokedDirectly) { + const dirs = process.argv.slice(2).filter(a => !a.startsWith('-')) + const targets = dirs.length > 0 ? dirs : ['registration'] + + let failed = 0 + for (const target of targets) { + const errors = validateRegistrationDir(target) + if (errors.length === 0) { + console.log(`✔ ${target}: conformant`) + } else { + failed++ + console.error(`✘ ${target}:`) + for (const e of errors) console.error(` - ${e}`) + } + } + + if (failed > 0) { + console.error( + `\n${failed} of ${targets.length} registration director${targets.length === 1 ? 'y' : 'ies'} ` + + 'violate fleet policy.' + ) + process.exit(1) + } + process.exit(0) +} diff --git a/packages/onboarding-kit/package.json b/packages/onboarding-kit/package.json index fef658e4..8420b43c 100644 --- a/packages/onboarding-kit/package.json +++ b/packages/onboarding-kit/package.json @@ -1,7 +1,7 @@ { "name": "@fuzefront/onboarding-kit", "version": "1.0.0", - "description": "Drop-in self-registration kit for FuzeFront apps — registration script, manifest schema, policy/billing templates, and the Helm init-container snippet", + "description": "Drop-in self-registration kit for FuzeFront apps \u2014 registration script, manifest schema, policy/billing templates, and the Helm init-container snippet", "files": [ "bin", "templates", @@ -11,16 +11,18 @@ ], "bin": { "fuzefront-register": "bin/register.sh", - "fuzefront-validate-policy": "bin/validate-policy.mjs" + "fuzefront-validate-policy": "bin/validate-policy.mjs", + "fuzefront-validate-registration": "bin/validate-registration.mjs" }, "scripts": { - "test": "node --test tests/validate-policy.test.mjs && sh tests/register.test.sh", + "test": "node --test tests/validate-policy.test.mjs && node --test tests/validate-registration.test.mjs && sh tests/register.test.sh", "test:policy": "node --test tests/validate-policy.test.mjs", "build:schema": "node scripts/build-schema.mjs", "check:schema": "node scripts/build-schema.mjs --check", - "lint": "shellcheck bin/register.sh" + "lint": "shellcheck bin/register.sh", + "test:registration": "node --test tests/validate-registration.test.mjs" }, - "//devDependencies": "The zero-dependencies rule this kit lives by is about RUNTIME: bin/register.sh and bin/validate-policy.mjs run inside a product's init container, where no npm install is possible, and neither may ever require a module. scripts/build-schema.mjs is a CI/dev generator, is not in `files`, and never ships — so a devDependency here does not violate that rule. Pinned exactly because a generator that silently changes its output across a minor bump is worse than no generator. Do not move this to `dependencies`.", + "//devDependencies": "The zero-dependencies rule this kit lives by is about RUNTIME: bin/register.sh and bin/validate-policy.mjs run inside a product's init container, where no npm install is possible, and neither may ever require a module. scripts/build-schema.mjs is a CI/dev generator, is not in `files`, and never ships \u2014 so a devDependency here does not violate that rule. Pinned exactly because a generator that silently changes its output across a minor bump is worse than no generator. Do not move this to `dependencies`.", "devDependencies": { "js-yaml": "4.1.0" }, diff --git a/packages/onboarding-kit/templates/manifest.json b/packages/onboarding-kit/templates/manifest.json index c013838c..65ee8b3a 100644 --- a/packages/onboarding-kit/templates/manifest.json +++ b/packages/onboarding-kit/templates/manifest.json @@ -4,8 +4,15 @@ "name": "MyApp", "menuLabel": "MyApp", "description": "One line describing what this product does, shown in the app launcher.", - "icon": { "kind": "emoji", "value": "📦" }, + "icon": { + "kind": "emoji", + "value": "\ud83d\udce6" + }, "mode": "portal", + "modes": [ + "portal", + "standalone" + ], "builtin": false, "integration": { "type": "module-federation", @@ -17,8 +24,14 @@ "section": "build", "order": 10 }, - "chrome": { "menu": "host", "topbar": "host" }, - "routing": { "path": "/app/myapp" }, + "chrome": { + "menu": "host", + "topbar": "host" + }, + "routing": { + "path": "/app/myapp", + "host": "myapp.fuzefront.com" + }, "visibility": "organization", "roles": [] } diff --git a/packages/onboarding-kit/tests/validate-registration.test.mjs b/packages/onboarding-kit/tests/validate-registration.test.mjs new file mode 100644 index 00000000..e2aaf62e --- /dev/null +++ b/packages/onboarding-kit/tests/validate-registration.test.mjs @@ -0,0 +1,159 @@ +import { test } from 'node:test' +import assert from 'node:assert/strict' +import { mkdtempSync, mkdirSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { + effectiveModes, + validateSurfaces, + validatePolicyWiring, + validateRegistrationDir, +} from '../bin/validate-registration.mjs' + +const PORTAL_STANDALONE = { + mode: 'portal', + modes: ['portal', 'standalone'], + routing: { path: '/app/x', host: 'x.fuzefront.com' }, +} + +/** Build a registration/ dir on disk. `files` maps filename -> string|object. */ +function makeDir(files) { + const dir = join(mkdtempSync(join(tmpdir(), 'reg-')), 'registration') + mkdirSync(dir, { recursive: true }) + for (const [name, body] of Object.entries(files)) { + writeFileSync(join(dir, name), typeof body === 'string' ? body : JSON.stringify(body)) + } + return dir +} + +test('effectiveModes falls back to [mode] when modes is ABSENT', () => { + // This is the crux: an absent `modes` is LEGAL per the contract. The gate exists + // because that legality hides a permanent capability gap, not because it is invalid. + assert.deepEqual(effectiveModes({ mode: 'portal' }), ['portal']) +}) + +test('effectiveModes prefers modes when present', () => { + assert.deepEqual(effectiveModes({ mode: 'portal', modes: ['portal', 'standalone'] }), [ + 'portal', + 'standalone', + ]) +}) + +test('effectiveModes treats an empty modes array as a fallback, not as no surfaces', () => { + assert.deepEqual(effectiveModes({ mode: 'standalone', modes: [] }), ['standalone']) +}) + +test('portal-only manifest is REJECTED — the real FuzeHub/FuzeContact shape', () => { + // Exactly ONE violation: `portal` is present, so only the missing `standalone` is + // reported. The routing.host check deliberately does not fire here — it is scoped to + // manifests that DO declare standalone, so a portal-only product gets one clear + // reason rather than a second, confusing complaint about a host it never needed. + const errors = validateSurfaces({ mode: 'portal', routing: { path: '/app/fuzehub' } }) + assert.equal(errors.length, 1, errors.join('\n')) + assert.match(errors[0], /does not include "standalone"/) + assert.match(errors[0], /never ship a mobile app/) +}) + +test('portal + standalone with a host is accepted', () => { + assert.deepEqual(validateSurfaces(PORTAL_STANDALONE), []) +}) + +test('standalone WITHOUT routing.host is rejected', () => { + const errors = validateSurfaces({ + mode: 'portal', + modes: ['portal', 'standalone'], + routing: { path: '/app/x' }, + }) + assert.equal(errors.length, 1) + assert.match(errors[0], /routing\.host/) +}) + +test('standalone with a blank routing.host is rejected', () => { + const errors = validateSurfaces({ + mode: 'portal', + modes: ['portal', 'standalone'], + routing: { path: '/app/x', host: ' ' }, + }) + assert.equal(errors.length, 1) + assert.match(errors[0], /missing or empty/) +}) + +test('embed-only product is EXEMPT — it is not a portal destination', () => { + assert.deepEqual(validateSurfaces({ mode: 'embed', modes: ['embed'] }), []) +}) + +test('embed alongside portal is NOT exempt — it is still a portal destination', () => { + const errors = validateSurfaces({ mode: 'portal', modes: ['portal', 'embed'] }) + assert.match(errors.join('\n'), /does not include "standalone"/) +}) + +test('a manifest with no mode and no modes is rejected outright', () => { + const errors = validateSurfaces({}) + assert.equal(errors.length, 1) + assert.match(errors[0], /neither/) +}) + +test('missing policy.json is reported', () => { + const dir = makeDir({ 'manifest.json': PORTAL_STANDALONE }) + const errors = validatePolicyWiring(dir) + assert.equal(errors.length, 1) + assert.match(errors[0], /policy\.json is missing/) +}) + +test('a vendored pre-kit register.sh with no policy step is reported', () => { + const dir = makeDir({ + 'manifest.json': PORTAL_STANDALONE, + 'policy.json': { product: 'x' }, + 'register.sh': '#!/bin/sh\ncurl -X POST "$API/apps" -d @manifest.json\n', + }) + const errors = validatePolicyWiring(dir) + assert.equal(errors.length, 1) + assert.match(errors[0], /no policy submission step/) +}) + +test('a register.sh that only MENTIONS policy in a comment does not pass', () => { + const dir = makeDir({ + 'manifest.json': PORTAL_STANDALONE, + 'policy.json': { product: 'x' }, + 'register.sh': '#!/bin/sh\n# TODO: submit the authz policy one day\ncurl "$API/apps"\n', + }) + assert.match(validatePolicyWiring(dir).join('\n'), /no policy submission step/) +}) + +test('a kit register.sh that submits the policy passes', () => { + const dir = makeDir({ + 'manifest.json': PORTAL_STANDALONE, + 'policy.json': { product: 'x' }, + 'register.sh': '#!/bin/sh\nhttp PUT "${API}/apps/${SLUG}/policy" "$BODY" "$POLICY"\n', + }) + assert.deepEqual(validatePolicyWiring(dir), []) +}) + +test('no register.sh at all is fine — consuming the kit from npm is preferred', () => { + const dir = makeDir({ 'manifest.json': PORTAL_STANDALONE, 'policy.json': { product: 'x' } }) + assert.deepEqual(validatePolicyWiring(dir), []) +}) + +test('a fully conformant registration dir passes end to end', () => { + const dir = makeDir({ + 'manifest.json': PORTAL_STANDALONE, + 'policy.json': { product: 'x' }, + 'register.sh': 'http PUT "${API}/apps/${SLUG}/policy" "$BODY" "$POLICY"\n', + }) + assert.deepEqual(validateRegistrationDir(dir), []) +}) + +test('a missing manifest.json is reported, not thrown', () => { + const dir = makeDir({}) + const errors = validateRegistrationDir(dir) + assert.equal(errors.length, 1) + assert.match(errors[0], /not found/) +}) + +test('malformed manifest JSON is reported, not thrown', () => { + const dir = makeDir({ 'manifest.json': '{ not json' }) + const errors = validateRegistrationDir(dir) + assert.equal(errors.length, 1) + assert.match(errors[0], /not valid JSON/) +}) From 92538847fc70d336290a9328030446c74728cee0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 10:05:03 +0000 Subject: [PATCH 02/10] feat(onboarding-kit): enforce the no-fuze-prefix slug convention + migration tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every Fuze product registers on FuzeFront WITHOUT the `Fuze` prefix — slug `service`, name `Service`. FuzePicker already registered as `picker`, so the convention existed; it was never enforced, and twelve products registered against it. `slug` is IMMUTABLE (PUT /apps/{slug}: "slug, builtin and manifestVersion are immutable and must match") and there is no rename, so correcting one is a two-step migration — register the short slug, then delete the prefixed one. register.sh does step 1 only, so a product that de-prefixes its manifest and redeploys ends up registered TWICE with the prefixed row still in the launcher. - validate-registration.mjs: reject a slug or name starting with `fuze`, at AUTHORING time. Deliberately NOT a `pattern` on the contract's `Slug`: twelve live rows hold prefixed slugs, and both migration steps talk to the registry about them, so a contract-level ban would reject the requests that repair the damage. The registry must keep accepting the old value; the kit stops anyone authoring a new one. - migrate-slug.mjs: the two-step correction. Dry run by default, idempotent, resumes a half-finished run. DELETE is the last operation and is guarded by a fresh re-read of the registry, so every failure path ends with the original still registered — the worst outcome it can produce is a duplicate tile, never an unregistered product. Refuses built-ins (DELETE 403s them) and suite parents like FuzeHub, which need an atomic five-row migration the contract cannot offer. - --apply refuses without --permit-grants and --installs, two silent losses it cannot repair: product Permit keys are namespaced by the REGISTRY SLUG (sync-permit-schema.ts forces `product: row.slug`), so migrating renames every key and strands existing grants on a role that is never deleted and never errors; and app_installations.app_id is ON DELETE CASCADE. A dry run warns instead of refusing, so the flags gate the delete rather than the preview. - Runbook with the grant-remap procedure and why the overlap window (both slugs registered, both namespaces in Permit, no key collision) is where it is safe. Verified: 16 policy + 31 registration + 34 migration checks and the 19 register.sh behaviours all pass, build-schema --check is clean (the contract is untouched), and the CLI was exercised end to end against tests/fake-registry.mjs. NOT run against any live registry. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GaPa3JgrVNtWrGvqQEAEqv --- .github/workflows/onboarding-kit-tests.yml | 7 + docs/runbooks/app-slug-deprefix-migration.md | 302 ++++++++ packages/onboarding-kit/README.md | 69 +- packages/onboarding-kit/bin/migrate-slug.mjs | 709 ++++++++++++++++++ .../bin/validate-registration.mjs | 88 ++- packages/onboarding-kit/package.json | 8 +- .../onboarding-kit/tests/fake-registry.mjs | 43 +- .../tests/migrate-slug.test.mjs | 412 ++++++++++ .../tests/validate-registration.test.mjs | 93 ++- 9 files changed, 1719 insertions(+), 12 deletions(-) create mode 100644 docs/runbooks/app-slug-deprefix-migration.md create mode 100644 packages/onboarding-kit/bin/migrate-slug.mjs create mode 100644 packages/onboarding-kit/tests/migrate-slug.test.mjs diff --git a/.github/workflows/onboarding-kit-tests.yml b/.github/workflows/onboarding-kit-tests.yml index 2ca63f2c..47dbcf96 100644 --- a/.github/workflows/onboarding-kit-tests.yml +++ b/.github/workflows/onboarding-kit-tests.yml @@ -45,6 +45,13 @@ jobs: - name: register.sh behaviour (fake registry) run: sh tests/register.test.sh + # migrate-slug.mjs is the only tool in the kit that DELETES a registration, and + # it is pointed at production by hand. Its safety property — every failure path + # leaves the original app still registered — is asserted end to end against the + # same fake registry, so it is gated exactly as hard as register.sh. + - name: Slug de-prefix migration (fake registry) + run: node --test tests/migrate-slug.test.mjs + # Every template the kit hands out must itself pass the validators, or the # first thing a product copies is already broken. This is not hypothetical: the # template shipped `mode: portal` with no `modes` and no `routing.host`, so every diff --git a/docs/runbooks/app-slug-deprefix-migration.md b/docs/runbooks/app-slug-deprefix-migration.md new file mode 100644 index 00000000..2360c79a --- /dev/null +++ b/docs/runbooks/app-slug-deprefix-migration.md @@ -0,0 +1,302 @@ +# Runbook — de-prefixing an app slug (`fuzeservice` → `service`) + +**Audience:** the platform owner. **Not an agent task.** Every step here touches the +live registry, live Permit policy, or live install records. There is no dry-run mode for +the Permit half. + +**Tool:** `packages/onboarding-kit/bin/migrate-slug.mjs` +(`npx fuzefront-migrate-slug`). It is dry-run by default and refuses to delete anything +until you have answered for the two losses described below. + +--- + +## 1. What is being corrected, and why it is not an edit + +Every Fuze product registers on FuzeFront **without the `Fuze` prefix** — slug `service`, +name `Service`. Measured state at the time of writing (13 repos): + +| repo | slug | name | needs migration | +|---|---|---|---| +| fuzeagent | `fuzeagent` | FuzeAgent | yes | +| fuzebi | `fuzebi` | FuzeBI | yes | +| fuzecontact | `fuzecontact` | Contact | slug only | +| fuzedeploy | `fuzedeploy` | FuzeDeploy | yes | +| fuzeexecutive | `fuzeexecutive` | FuzeExecutive | yes | +| fuzehub | `fuzehub` | FuzeHub | **SCOPED OUT — suite parent, see §6** | +| fuzekeys | `fuzekeys` | FuzeKeys | yes | +| fuzemarket | `fuzemarket` | FuzeMarket | yes | +| fuzepicker | `picker` | FuzePicker | name only — **already correct**, no migration | +| fuzeplan | `fuzeplan` | FuzePlan | yes | +| fuzesales | `fuzesales` | Sales | slug only | +| fuzeservice | `fuzeservice` | FuzeService | yes | +| fuzesocial | `fuzesocial` | FuzeSocial | yes | + +`slug` is **immutable**. `PUT /apps/{slug}` states that `slug`, `builtin` and +`manifestVersion` "are immutable and must match", and the contract has no rename. So the +correction is two operations against two different rows: + +1. `POST /apps` — register the short slug. +2. `DELETE /apps/{prefixed}` — remove the original. + +`register.sh` does **step 1 only**. A product that de-prefixes its manifest and +redeploys therefore ends up registered **twice**, with the prefixed row still activated +and still in the launcher. Twelve products doing that is twelve ghost tiles. Step 2 is +what this runbook exists to drive. + +> **Do not "fix" this by adding `(?!fuze)` to `Slug` in the contract.** Twelve live rows +> hold prefixed slugs; banning the value at the API would break their `register.sh` +> manifest refresh and could block the very DELETE that repairs them. The rule is +> enforced at **authoring** time instead — `validate-registration.mjs`, in the product's +> own repo. The registry must keep accepting the old value until the last migration is +> done. + +--- + +## 2. The Permit answer — read this before touching anything + +**Changing the slug renames every Permit key the product owns, and orphans every grant +against the old ones. Nothing errors. Affected users silently lose their roles.** + +Mechanically: + +- `backend/src/permit/sync-permit-schema.ts` → `loadRegisteredPolicyResult()` builds each + stored policy as `policy = { ...raw, product: row.slug }`. **The registry slug is the + Permit namespace**, whatever the policy file's own `product` field says. +- `backend/src/permit/product-policy.ts` → `namespaceKey()` produces `_`. So + `fuzeservice_Ticket` → `service_Ticket`, and role `fuzeservice_agent` → `service_agent`. +- Role assignments (`permit.api.roleAssignments.assign({ user, role, tenant })`) store the + **namespaced role key**. They keep pointing at `fuzeservice_agent`. +- `syncPermitSchema()` is get-or-create/update and **never deletes**. The old resources + and roles therefore survive in Permit indefinitely after the registry row is gone. The + assignment stays valid and stays un-erroring — it simply grants permissions on a + resource type nothing checks any more. +- Runtime checks go through `checkProductPermission(user, product, …)` → + `namespaceKey(product, resource)` with the **new** slug. No matching grant. Permit + denies. Authorization fails closed, which is correct, and is exactly why nobody gets an + alert. + +### Is that acceptable? + +**Not as a default, and it must not be hand-waved as "probably nobody has grants yet".** +Two things narrow the blast radius, and one thing keeps it real: + +1. **Platform roles are unaffected.** `admin` / `editor` / `viewer` (assigned by + `backend/src/utils/permit/role-assignment.ts` and the security package's + `PermitAuthorizationProvider`) are **not namespaced**. Org membership, platform admin + and every base-schema permission survive a slug change untouched. Only + **product-declared** roles are at risk. +2. **In-repo, nothing assigns a product role yet.** `assignProductRole`, + `unassignProductRole`, `checkProductPermission` and `requireProductPermission` + (`backend/src/utils/permit/product-authz.ts`) have **zero call sites** anywhere in + FuzeFront. The product-role runtime path is declared but not yet wired here. +3. **But FuzeFront is not the only writer.** The entire point of + `PUT /apps/{slug}/policy` is that products declare and use their own roles from their + own backends, via their own Permit credentials. The platform **cannot** assert the + grant count is zero on a product's behalf. It has to be **measured**, per product, + before each migration. + +So: the loss is real, bounded to product-namespaced grants, and **cheap to avoid** — +because of the overlap window in §3. The tool refuses to delete without `--permit-grants` +for exactly this reason. + +### Measure it (per product, before migrating) + +```bash +# Every assignment in the product's namespace. Non-empty => you must remap (step 3b). +curl -s -H "Authorization: Bearer $PERMIT_API_KEY" \ + "https://api.permit.io/v2/facts/$PROJ/$ENV/role_assignments?role=fuzeservice_agent&per_page=100" +``` + +Repeat for each role in the product's `policy.json`. Zero across all roles → the loss is +nil and `--permit-grants` is a formality. Non-zero → do step 3b. + +### Why the overlap window makes the remap safe + +`mergeProductPolicy` throws only on a **key collision**. `fuzeservice_*` and `service_*` +do not collide, so while **both** slugs are registered, the synced Permit schema contains +**both complete namespaces**. That gives a window in which the old role and the new role +both exist and both work — so the remap is a pure *add-then-remove* with **no instant at +which a user holds neither**. That is why DELETE is last, and it is a Permit reason, not +just a portal reason. + +--- + +## 3. Procedure, per product + +Ordering is load-bearing. Do not reorder. + +### 3a. Register the replacement (both slugs live) + +```bash +node packages/onboarding-kit/bin/migrate-slug.mjs \ + --from fuzeservice --to service \ + --api https://app.fuzefront.com --token "$FUZEFRONT_REGISTRATION_TOKEN" \ + --registration ../fuzeservice/registration +``` + +No `--apply` — this is a **dry run**, and it changes nothing. It prints the full plan and +emits `WARNING (would block --apply)` for each acknowledgement you have not yet given, so +you can see exactly what the migration will do *before* deciding anything. (The +acknowledgements gate the DELETE, not the preview — otherwise you would have to type them +just to get output, and a confirmation you must bypass to do your job is not a decision.) + +Read the plan. Confirm the `NOTE` lines about `routing.host` and `integration.scope`: +neither is rewritten, because a hostname needs DNS/cert/ingress and the MF scope must keep +matching the global the deployed bundle actually publishes. If either genuinely needs to +change, that is a **separate** product change, shipped before this migration, not during +it. + +You now have two ways to open the overlap window that step 3b needs. Either **redeploy the +product with its de-prefixed manifest** (the other wave of work — `register.sh` performs +step 1 on its own), or come back and run `--apply` with both flags once 3b and 3c are +ready. The tool handles both: it resumes cleanly from a state where the replacement is +already registered. + +### 3b. Force a Permit sync, then remap the grants + +```bash +# In the platform: run the schema sync job so BOTH namespaces exist in Permit. +kubectl -n fuzefront create job --from=cronjob/permit-schema-sync permit-sync-$(date +%s) +curl -s https://app.fuzefront.com/health | jq '.permitSync' # outcome must be "ok" +``` + +`outcome: "ok"` is required. `registry_unavailable` means **no** product policy reached +Permit and the new namespace does not exist — stop, fix, re-run. + +Then, for every assignment found in §2: + +```bash +# ADD the new grant first. Never remove before adding — the whole point of the +# overlap window is that no user is ever left without either role. +curl -X POST -H "Authorization: Bearer $PERMIT_API_KEY" -H 'Content-Type: application/json' \ + "https://api.permit.io/v2/facts/$PROJ/$ENV/role_assignments" \ + -d '{"user":"","role":"service_agent","tenant":""}' + +# Verify, THEN remove the old one. +curl -X DELETE -H "Authorization: Bearer $PERMIT_API_KEY" \ + "https://api.permit.io/v2/facts/$PROJ/$ENV/role_assignments" \ + -d '{"user":"","role":"fuzeservice_agent","tenant":""}' +``` + +### 3c. Capture the install rows + +`app_installations.app_id` references `apps.id` **ON DELETE CASCADE** +(`backend/src/migrations/017_app_scope_levels_and_installations.ts`). Deleting the +prefixed app **destroys every personal and organization install** of the product. Installs +are not part of the frozen `/api/v1/app-registry` contract — they live on the legacy +`/api/apps/:id/install` surface — so the migration tool can neither read nor restore them. + +```sql +-- Capture before deleting. Keep this output. +SELECT i.* FROM app_installations i + JOIN apps a ON a.id = i.app_id + WHERE a.slug = 'fuzeservice'; +``` + +Then decide: re-create them against the new `apps.id` after the migration, or accept that +users and orgs must re-install. **Either is fine; silently discovering it afterwards is +not.** A product with zero rows here makes this a no-op. + +### 3d. Run the migration + +```bash +node packages/onboarding-kit/bin/migrate-slug.mjs \ + --from fuzeservice --to service \ + --api https://app.fuzefront.com --token "$TOKEN" \ + --registration ../fuzeservice/registration \ + --permit-grants --installs --apply +``` + +The tool will, in order: register the short slug (or refresh it if a redeploy already +did), re-submit `policy.json` and `billing-profile.json` under the new slug, match the +old app's status (a **suspended** app is not switched on by migrating it), **re-read the +registry and verify** the replacement is present, correct and at the right status — and +only then `DELETE` the prefixed row. + +Every failure path aborts **before** the delete. The worst outcome it can produce is both +rows present, which is a duplicate tile: visible, harmless, and fixed by re-running. + +### 3e. Verify, then clean up Permit + +```bash +curl -s -H "Authorization: Bearer $TOKEN" https://app.fuzefront.com/api/v1/app-registry/apps \ + | jq '[.[] | select(.slug|test("^fuze"))] | map(.slug)' +``` + +Load the portal and confirm one tile, that it mounts, and that a user with the product +role can still reach a gated route. + +`syncPermitSchema` never deletes, so `fuzeservice_Ticket` / `fuzeservice_agent` remain in +Permit as orphans. They grant nothing that anything checks, so they are harmless — but +delete them by hand once the migration is verified, or the next person to read the Permit +schema will find two namespaces per product and no way to tell which is live. + +--- + +## 4. Rollback + +Before the DELETE lands there is nothing to roll back — both rows exist and the old one is +still serving. Re-run without `--apply`. + +After the DELETE: re-register the prefixed slug from the product's `registration/` +directory (`register.sh` against the old manifest) and re-add the old Permit grants. +Install rows are **not** recoverable except from the §3c capture. + +--- + +## 5. Order the fleet in + +Do **`fuzeservice` first, alone**, and let it sit for a day. It is a plain single-surface +product with a policy, so it exercises every step without being the one that hurts if it +goes wrong. Then batch the rest. Do not run all twelve in one window — the failure mode +you are watching for (a product whose users quietly lost a role) takes hours to show up, +and twelve simultaneous migrations make it unattributable. + +`fuzepicker` needs **no migration** — its slug is already `picker`. Only its display name +carries the prefix, and `name` is mutable via the ordinary manifest refresh: de-prefix it +in the repo and redeploy. + +`fuzecontact` and `fuzesales` need the **slug** migration only; their names are already +correct. + +--- + +## 6. Scoped out: FuzeHub + +**`fuzehub` must not be migrated with this tool, and the tool refuses it.** + +FuzeHub registers five rows — the parent plus four sibling surfaces (`fuzehub-talent`, +`fuzehub-recruiter`, `fuzehub-ventures`, `fuzehub-marketplace`), grouped in the menu by an +identical `nav.suite.id`. Migrating the parent alone breaks three things at once: + +- the siblings keep `nav.suite.id: "fuzehub"` and **split into a second menu group**; +- their own slugs stay prefixed, so the product is half-corrected forever; +- the product-level policy and billing profile bind to the **primary** slug only (see + `register.sh`), so they move to a row the siblings no longer relate to. + +Doing it correctly means registering five replacements, re-pointing five suite ids and +deleting five originals as **one atomic operation**. The frozen contract offers no +transaction, so atomicity would have to be simulated — and a simulated transaction across +five deletes is precisely where a tool leaves a product showing three tiles. That is the +worst possible place to be clever. + +So: FuzeHub is a **maintenance-window, human-driven** migration, with the five +replacements registered and verified *first*, all four `nav.suite.id` values repointed to +`hub`, and the five originals deleted last. It is not in scope for the tool and should not +be forced into it. + +--- + +## 7. What the tool refuses, and why + +| Refusal | Reason | +|---|---| +| `--from` does not start with `fuze` | This is not a general rename tool. A rename nobody reviewed is a delete nobody reviewed. | +| `--to` still starts with `fuze` | That is the thing being corrected. | +| neither slug is registered | "Nothing to do" and "you typed the slug wrong" look identical against the registry; exiting 0 on the second is how a migration gets ticked off without happening. | +| the app is `builtin` | `DELETE` 403s on built-ins, so the migration could never finish — it would only ever add a permanent duplicate. Built-ins are de-prefixed by changing the platform seed and re-seeding. | +| suite siblings detected | §6. | +| `GET /apps` unreadable | Siblings cannot be ruled out, so a delete cannot be proven safe. | +| `--permit-grants` missing (on `--apply`) | §2. A dry run warns instead, so you can still see the plan. | +| `--installs` missing (on `--apply`) | §3c. A dry run warns instead. | +| verification failed after registering | The replacement is not confirmed equivalent, so the original stays. | diff --git a/packages/onboarding-kit/README.md b/packages/onboarding-kit/README.md index 125dd03d..838ed675 100644 --- a/packages/onboarding-kit/README.md +++ b/packages/onboarding-kit/README.md @@ -135,10 +135,11 @@ Where `validate-policy` checks that one file is **well-formed**, this checks tha registration satisfies **fleet policy** — the rules the platform requires of every product but that no schema can express. Also zero dependencies. -It enforces three things: +It enforces four things: | Rule | Why | |---|---| +| **`slug` and `name` must NOT start with `Fuze`** | Family convention: register as `service` / `Service`, not `fuzeservice` / `FuzeService`. `slug` is **immutable**, so getting it wrong is not a one-line edit — it costs a register-then-delete migration that orphans Permit grants and CASCADE-deletes install rows. See below. | | Effective modes include `portal` **and** `standalone` | `standalone` is the only surface a mobile TWA/APK can wrap, because an app store needs a URL that stands on its own. | | `standalone` implies a non-empty `routing.host` | A standalone surface with no host has no URL to serve or to wrap. | | `policy.json` exists, and a vendored `register.sh` actually submits it | A pre-kit script that skips the policy step leaves the product with no roles. | @@ -150,6 +151,63 @@ incapable of ever shipping a mobile app. Nothing is malformed; a capability simp never exists. That is precisely the class of failure a schema cannot catch and this gate can. +### The no-`Fuze`-prefix slug convention + +Register as `service`, not `fuzeservice`. The prefix is already implied by the fact that +you are registering on FuzeFront at all, and the slug is user-visible — it appears in +`/app/` URLs, in Permit keys (`_`), and in billing product keys. +FuzePicker already registered as `picker`, so the convention existed; it was simply never +enforced, and twelve products registered against it. + +```jsonc +"slug": "service", // not "fuzeservice" +"name": "Service" // not "FuzeService" +``` + +**Why this is a build-time gate and not a `pattern` on the contract's `Slug`.** Adding +`(?!fuze)` to `Slug` in `openapi.yaml` would be actively harmful. Twelve live rows hold +prefixed slugs, and `slug` is immutable — correcting one means *register the short slug, +then delete the prefixed one*. Both steps talk to the registry **about** the prefixed +slug, and `register.sh` re-`PUT`s the manifest on every pod start. A contract-level ban +would reject the very requests that repair the damage. Banning a value at the API is only +safe when no existing row holds it. So the registry keeps **accepting** the old value +while the migration is in flight, and this gate stops anyone **authoring** a new one — in +their own repo, at build time, where it is a one-character fix. + +### Already registered with the prefix? `fuzefront-migrate-slug` + +Because `slug` is immutable there is no rename. `register.sh` performs only the first half +of the correction, so a product that de-prefixes its manifest and redeploys ends up +registered **twice**, with the prefixed row still activated and still in the launcher. + +```bash +# DRY RUN by default — reads, plans, prints, changes nothing. +npx fuzefront-migrate-slug --from fuzeservice --to service \ + --api https://app.fuzefront.com --token "$TOKEN" \ + --registration ./registration +``` + +It registers the short slug, re-submits `policy.json` / `billing-profile.json` under it, +matches the old app's status, **re-reads the registry to verify** the replacement is +present and correct, and only then deletes the prefixed row. Every failure path aborts +*before* the delete, so the worst outcome it can produce is a duplicate tile — never an +unregistered product. It is idempotent and resumes a half-finished run. + +`--apply` **refuses** to delete without `--permit-grants` and `--installs`, because two +losses are silent and it cannot repair either (a dry run warns and still prints the plan — +the flags gate the delete, not the preview): + +- **Permit grants.** Product roles are namespaced by the *registry slug* + (`sync-permit-schema.ts` forces `product: row.slug`), so migrating renames every key. + Existing assignments keep pointing at `fuzeservice_agent`, the old role is never deleted, + nothing errors — affected users just lose the role. +- **Install rows.** `app_installations.app_id` references `apps.id` `ON DELETE CASCADE`, + and installs are not in the frozen contract at all. + +**This is an owner tool, not an init-container tool**, and suite parents like FuzeHub are +deliberately scoped out. Full procedure, the grant-remap and the ordering: +[`docs/runbooks/app-slug-deprefix-migration.md`](../../docs/runbooks/app-slug-deprefix-migration.md). + **Embed-only products are exempt** from the surface rules. Per the contract an embed renders inside a third-party page with neither portal chrome nor FuzeFront navigation, is not a portal destination, and may not register a menu entry at all — so requiring a @@ -169,8 +227,10 @@ It also warns (without failing) about actions or whole resources that no role gr ## Tests ```bash -npm test # both suites +npm test # every suite node --test tests/validate-policy.test.mjs +node --test tests/validate-registration.test.mjs +node --test tests/migrate-slug.test.mjs sh tests/register.test.sh ``` @@ -178,3 +238,8 @@ sh tests/register.test.sh properties the init container depends on: cold-start registers **and** activates, a re-run is idempotent, policy and billing are submitted, a bad token **exits non-zero**, and transient 5xx responses are retried rather than fatal. + +`migrate-slug.test.mjs` drives the migration end to end against that same fake registry. +Every failure case asserts not just that the tool reported failure but that **the old app +is still registered afterwards** — the one property that makes the tool safe to point at +production. diff --git a/packages/onboarding-kit/bin/migrate-slug.mjs b/packages/onboarding-kit/bin/migrate-slug.mjs new file mode 100644 index 00000000..774a9565 --- /dev/null +++ b/packages/onboarding-kit/bin/migrate-slug.mjs @@ -0,0 +1,709 @@ +#!/usr/bin/env node +// Correct an app registered under a `Fuze`-prefixed slug: register it under the short +// slug, verify the replacement is live, then delete the prefixed original. +// +// THIS IS AN OWNER TOOL, NOT AN INIT-CONTAINER TOOL. register.sh runs unattended on +// every pod start; this runs once per product, by hand, against a registry the operator +// has an admin token for. It is deliberately NOT wired into any deploy path. +// +// --------------------------------------------------------------------------------- +// WHY A TOOL AT ALL — the shape of the problem +// --------------------------------------------------------------------------------- +// +// `slug` is IMMUTABLE. `PUT /apps/{slug}` (services/app-registry-service/openapi.yaml) +// states that `slug`, `builtin` and `manifestVersion` "are immutable and must match", +// and the contract has no rename operation. So de-prefixing is not an edit — it is: +// +// 1. POST /apps with the short slug (the replacement) +// 2. DELETE /apps/{old} the prefixed original (the correction) +// +// `register.sh` performs step 1 and only step 1. If a product de-prefixes its manifest +// and redeploys, the init container happily registers `service` and the old `fuzeservice` +// row simply stays — registered, activated, and still in the launcher. Twelve products +// doing that produces twelve ghost tiles, each pointing at a remoteEntry that may or may +// not still be served. Nobody gets an error. That is the failure this tool prevents, and +// it is why the two steps must be driven together rather than left to "someone will +// remember to clean up". +// +// --------------------------------------------------------------------------------- +// THE SAFETY PROPERTY, stated precisely +// --------------------------------------------------------------------------------- +// +// The only unrecoverable outcome is a product with NO registration: the tile vanishes, +// the remote cannot mount, and the manifest may only exist in a repo nobody can deploy +// right now. Everything else is recoverable by re-running something. +// +// Therefore DELETE is the LAST operation, it is guarded, and every failure path aborts +// BEFORE it. If anything at all goes wrong, the run ends with both rows present — a +// duplicate tile, which is visible, harmless and fixed by re-running this tool. The tool +// will never trade "duplicate" for a risk of "none". +// +// Concretely, `--apply` refuses to DELETE unless, at that moment: +// - GET /apps/{new} returns 200, +// - its status equals the status the OLD app had (activated stays activated; a +// suspended app is not silently switched on by migrating it), +// - its manifest's slug really is the new slug, +// - the old app is not `builtin` (the contract 403s those; only suspend applies), and +// - the operator has answered for the Permit grants and the installation rows below. +// +// --------------------------------------------------------------------------------- +// WHAT THE CONTRACT CANNOT GIVE BACK — read this before using --apply +// --------------------------------------------------------------------------------- +// +// (a) PERMIT GRANTS. Product authorization is namespaced by the REGISTRY SLUG, not by +// whatever `product` the policy file claims: sync-permit-schema.ts builds each +// policy as `{ ...raw, product: row.slug }`, and product-policy.ts namespaces every +// resource and role as `_`. Migrating `fuzeservice` -> `service` therefore +// renames every key: `fuzeservice_Ticket` -> `service_Ticket`, +// `fuzeservice_agent` -> `service_agent`. +// +// Existing role assignments still point at `fuzeservice_agent`. `syncPermitSchema` +// is get-or-create/update and NEVER deletes, so that role continues to exist in +// Permit after the registry row is gone — the assignment stays valid, stays +// un-erroring, and grants permissions on a resource type nothing checks any more. +// Runtime checks go through `namespaceKey(product, resource)` with the NEW slug, find +// no matching grant, and deny. Authorization fails closed, which is the correct +// behaviour and precisely why nothing reports it: the user simply loses their role. +// +// This tool talks to the app registry, not to Permit, and giving a registry migration +// an admin Permit credential is a blast radius nobody wants. So it does not remap +// grants — it REFUSES to delete until the operator states, with --permit-grants, that +// they have dealt with them. See docs/runbooks/app-slug-deprefix-migration.md for the +// remap procedure and why the overlap window (both slugs registered, both namespaces +// present in Permit, no key collision) is the safe place to run it. +// +// (b) INSTALLATION ROWS. `app_installations.app_id` references `apps.id` ON DELETE +// CASCADE (backend/src/migrations/017_app_scope_levels_and_installations.ts). Deleting +// the old app row destroys every personal and organization install of that product. +// Installs are not part of the frozen `/api/v1/app-registry` contract at all — they +// live on the legacy `/api/apps/:id/install` surface — so this tool can neither read +// them nor recreate them. --installs is the same kind of explicit acknowledgement. +// +// (c) POLICY AND BILLING PROFILE. `GET /apps/{slug}` returns an `App`, whose schema is +// `additionalProperties: false` over `[slug, status, mode, builtin, manifest, +// createdAt, updatedAt]`. The stored policy and billing profile are NOT readable +// through the contract, so they cannot be copied from old to new. They must be +// re-submitted from the product's own `registration/` directory, which is why +// --registration is required whenever those files exist. +// +// --------------------------------------------------------------------------------- +// Usage +// --------------------------------------------------------------------------------- +// +// node bin/migrate-slug.mjs --from fuzeservice --to service \ +// --api https://app.fuzefront.com --token "$TOKEN" \ +// --registration ../fuzeservice/registration +// +// Dry run by DEFAULT — it reads, plans and prints, and touches nothing. Add --apply to +// execute. Idempotent: re-running a completed migration is a no-op that exits 0, and +// re-running a half-finished one resumes at the verify/delete step. +// +// Exit codes: 0 = migrated, or already migrated, or dry run planned cleanly. +// 1 = refused, or failed. On any 1, the product is still registered. + +import { readFileSync, existsSync } from 'node:fs' +import { join } from 'node:path' + +/** Mirrors `Slug` in the frozen contract. */ +export const SLUG_RE = /^[a-z0-9][a-z0-9-]{1,62}[a-z0-9]$/ +const FUZE_PREFIX_RE = /^fuze/i + +export class MigrationRefused extends Error {} + +// ---- planning (pure, unit-testable) ------------------------------------------------ + +/** + * Rewrite a manifest from the old slug to the new one. + * + * The manifest is taken from the LIVE REGISTRY RECORD by default, not from the product + * repo. That is deliberate: the repo copy is being changed concurrently by whoever is + * de-prefixing the product, and a migration that also swaps in an unrelated manifest + * revision makes a bad outcome impossible to attribute. Deriving from the live record + * means the replacement is byte-identical to what is serving today except for the slug + * and the strings that must follow it. + * + * Only fields that DEMONSTRABLY encode the slug are touched, and each only when it + * matches the old slug exactly — a manifest whose `routing.path` is a hand-written + * `/app/support-desk` is left alone rather than guessed at. + * + * @param {Record} manifest the old app's manifest + * @param {string} from + * @param {string} to + * @returns {{ manifest: Record, notes: string[] }} + */ +export function rewriteManifest(manifest, from, to) { + const next = structuredClone(manifest) + const notes = [] + + next.slug = to + notes.push(`slug: ${from} -> ${to}`) + + // `name` follows the same convention as the slug (Service, not FuzeService). Only + // stripped when it is literally the prefixed form; a product whose display name is + // already correct is not second-guessed. + if (typeof next.name === 'string' && FUZE_PREFIX_RE.test(next.name)) { + const stripped = next.name.replace(FUZE_PREFIX_RE, '') + if (stripped) { + notes.push(`name: ${next.name} -> ${stripped}`) + next.name = stripped + } + } + if (typeof next.menuLabel === 'string' && FUZE_PREFIX_RE.test(next.menuLabel)) { + const stripped = next.menuLabel.replace(FUZE_PREFIX_RE, '') + if (stripped) { + notes.push(`menuLabel: ${next.menuLabel} -> ${stripped}`) + next.menuLabel = stripped + } + } + + // The portal mounts a `portal` surface at /app/:slug. A path that still says + // /app/fuzeservice after the slug is `service` is a 404 waiting to happen. + if (next.routing && typeof next.routing === 'object' && next.routing.path === `/app/${from}`) { + notes.push(`routing.path: /app/${from} -> /app/${to}`) + next.routing.path = `/app/${to}` + } + + // routing.host is a real DNS name with a certificate and an ingress behind it. It is + // NOT derived from the slug and must not be rewritten here — changing it would point + // the replacement at a hostname that does not resolve. Reported so the operator sees + // it and can decide separately. + if (next.routing && typeof next.routing.host === 'string' && next.routing.host.includes(from)) { + notes.push( + `NOTE routing.host "${next.routing.host}" contains "${from}" and was NOT changed — ` + + 'a hostname needs DNS, a certificate and an ingress, none of which this tool owns' + ) + } + + // Module-Federation `scope` is the global the remote publishes itself under at + // runtime. It must keep matching the deployed bundle, so it is likewise left alone. + if (next.integration && typeof next.integration.scope === 'string' && + FUZE_PREFIX_RE.test(next.integration.scope)) { + notes.push( + `NOTE integration.scope "${next.integration.scope}" was NOT changed — it must keep ` + + 'matching the global the deployed remoteEntry actually publishes' + ) + } + + return { manifest: next, notes } +} + +/** + * Decide what to do from the two GET results. Separated from all I/O so every branch — + * including the ones that are awkward to provoke against a real registry — is directly + * testable. + * + * @param {{status:number, app?:any}} oldRes result of GET /apps/{from} + * @param {{status:number, app?:any}} newRes result of GET /apps/{to} + * @returns {{ action: 'noop'|'absent'|'register'|'verify-and-delete', reason: string }} + */ +export function planFrom(oldRes, newRes) { + const oldExists = oldRes.status === 200 + const newExists = newRes.status === 200 + + if (!oldExists && newExists) { + return { action: 'noop', reason: 'already migrated — the short slug is registered and the prefixed one is gone' } + } + if (!oldExists && !newExists) { + // Neither row exists. This tool corrects a registration; it does not create one from + // nothing, and a product that is simply not deployed yet must not be conjured into + // the registry by a migration run. + // + // Reported as a FAILURE, not a quiet success. "Nothing to do" and "you typed the slug + // wrong" produce exactly the same reading of the registry, and exiting 0 on the + // second one is how a migration gets ticked off a list without having happened. + return { action: 'absent', reason: `NEITHER "${oldRes.slug ?? '--from'}" nor the target is registered — nothing to migrate (check the slugs, or deploy the product first)` } + } + if (oldExists && newExists) { + return { action: 'verify-and-delete', reason: 'both slugs are registered — a previous run registered the replacement but did not remove the original' } + } + return { action: 'register', reason: 'only the prefixed slug is registered — register the replacement, then remove it' } +} + +/** + * Preflight the arguments. Throws MigrationRefused with a reason the operator can act on. + * @param {{from:string, to:string}} args + */ +export function validateSlugs({ from, to }) { + if (!from || !to) throw new MigrationRefused('both --from and --to are required') + if (from === to) throw new MigrationRefused(`--from and --to are both "${from}" — nothing to migrate`) + if (!SLUG_RE.test(to)) { + throw new MigrationRefused(`--to "${to}" is not a valid slug (contract pattern ${SLUG_RE})`) + } + if (FUZE_PREFIX_RE.test(to)) { + throw new MigrationRefused( + `--to "${to}" still starts with "fuze" — that is the thing being corrected` + ) + } + if (!FUZE_PREFIX_RE.test(from)) { + throw new MigrationRefused( + `--from "${from}" does not start with "fuze". This tool exists for the de-prefix ` + + 'migration; using it as a general rename would delete a registration for a reason ' + + 'nobody has reviewed.' + ) + } +} + +/** + * A suite parent is SCOPED OUT, on purpose. + * + * FuzeHub registers five rows: the parent plus four sibling surfaces + * (`fuzehub-talent`, `fuzehub-recruiter`, …), grouped in the menu by an identical + * `nav.suite.id`. Migrating the parent alone does three bad things at once: the siblings + * keep the old `nav.suite.id` and split into a second menu group, their own slugs stay + * prefixed, and the product-level policy and billing profile — which bind to the PRIMARY + * slug only (see register.sh) — move to a row the siblings no longer relate to. + * + * Doing it correctly means registering five replacements, re-pointing five suite ids and + * deleting five originals as one atomic operation, with a rollback for a partial failure + * in the middle. The contract offers no transaction, so "atomic" would have to be + * simulated, and a simulated transaction over five deletes is exactly where a tool + * quietly leaves a product with three tiles. Better to refuse and hand it to a human with + * a maintenance window than to ship something that half-works on the one product with + * the most surfaces to lose. + * + * @param {string} from + * @param {Array<{slug:string, manifest?:any}>} allApps every app in the registry + * @returns {string[]} related slugs; non-empty means REFUSE + */ +export function detectSuiteMembers(from, allApps) { + const related = new Set() + const fromSuite = + allApps.find(a => a.slug === from)?.manifest?.nav?.suite?.id ?? undefined + + for (const app of allApps) { + if (app.slug === from) continue + if (app.slug.startsWith(`${from}-`)) related.add(app.slug) + const suite = app.manifest?.nav?.suite?.id + if (suite !== undefined && (suite === from || (fromSuite !== undefined && suite === fromSuite))) { + related.add(app.slug) + } + } + return [...related].sort() +} + +// ---- registry client --------------------------------------------------------------- + +/** Thin, dependency-free client over the frozen `/api/v1/app-registry` contract. */ +export class RegistryClient { + /** + * @param {string} apiUrl base URL, e.g. https://app.fuzefront.com + * @param {string} token bearer token with apps:write + apps:activate + * @param {(m:string)=>void} log + */ + constructor(apiUrl, token, log = console.error) { + this.base = `${apiUrl.replace(/\/+$/, '')}/api/v1/app-registry` + this.token = token + this.log = log + } + + async request(method, path, body) { + const res = await fetch(`${this.base}${path}`, { + method, + headers: { + Authorization: `Bearer ${this.token}`, + ...(body === undefined ? {} : { 'Content-Type': 'application/json' }), + }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + }) + const text = await res.text() + let json + try { + json = text ? JSON.parse(text) : undefined + } catch { + json = undefined + } + return { status: res.status, body: json, text } + } + + async getApp(slug) { + const r = await this.request('GET', `/apps/${encodeURIComponent(slug)}`) + return { status: r.status, app: r.status === 200 ? r.body : undefined, text: r.text } + } + + async listApps() { + // The contract's list is filtered by `status`; omitting it returns all non-suspended. + // Suite detection wants EVERY row, so both pages are merged. + const seen = new Map() + for (const qs of ['', '?status=suspended']) { + const r = await this.request('GET', `/apps${qs}`) + if (r.status !== 200) continue + const items = Array.isArray(r.body) ? r.body : (r.body?.items ?? r.body?.apps ?? []) + for (const a of items) if (a?.slug) seen.set(a.slug, a) + } + return [...seen.values()] + } + + registerApp(manifest) { + return this.request('POST', '/apps', { manifest }) + } + putManifest(slug, manifest) { + return this.request('PUT', `/apps/${encodeURIComponent(slug)}`, manifest) + } + activateApp(slug) { + return this.request('POST', `/apps/${encodeURIComponent(slug)}/activate`) + } + putPolicy(slug, policy) { + return this.request('PUT', `/apps/${encodeURIComponent(slug)}/policy`, policy) + } + putBilling(slug, profile) { + return this.request('PUT', `/apps/${encodeURIComponent(slug)}/billing-profile`, profile) + } + deleteApp(slug) { + return this.request('DELETE', `/apps/${encodeURIComponent(slug)}`) + } +} + +// ---- the migration ----------------------------------------------------------------- + +/** + * @typedef {Object} MigrateOptions + * @property {string} from + * @property {string} to + * @property {boolean} apply false = dry run (default) + * @property {string} [registration] product's registration/ dir, for policy + billing + * @property {boolean} permitGrants operator has handled the Permit grant remap + * @property {boolean} installs operator accepts losing the install rows + * @property {(m:string)=>void} [log] + */ + +/** + * @param {RegistryClient} registry + * @param {MigrateOptions} opts + * @returns {Promise<{ ok: boolean, action: string, steps: string[], refusal?: string }>} + */ +export async function migrate(registry, opts) { + const log = opts.log ?? (m => console.error(m)) + const steps = [] + const say = m => { + steps.push(m) + log(m) + } + + validateSlugs(opts) + const { from, to, apply } = opts + + say(`${apply ? 'APPLY' : 'DRY RUN'}: ${from} -> ${to}`) + + const oldRes = await registry.getApp(from) + const newRes = await registry.getApp(to) + const plan = planFrom(oldRes, newRes) + say(`plan: ${plan.action} — ${plan.reason}`) + + if (plan.action === 'noop') return { ok: true, action: 'noop', steps } + if (plan.action === 'absent') return { ok: false, action: 'absent', steps, refusal: plan.reason } + + const oldApp = oldRes.app + + // Built-ins cannot be deleted (the contract 403s), so the migration can never + // complete for one. Refuse UP FRONT rather than after registering a replacement and + // leaving a permanent duplicate. + if (oldApp?.builtin === true) { + const refusal = + `"${from}" is a BUILT-IN app. DELETE /apps/{slug} returns 403 for built-ins, so the ` + + 'prefixed row can never be removed and this migration would only ever add a duplicate. ' + + 'Built-ins are de-prefixed by changing the platform seed and re-seeding, not by this tool.' + say(`REFUSED: ${refusal}`) + return { ok: false, action: plan.action, steps, refusal } + } + + // Suite detection needs the full list. A registry that will not list is a registry we + // cannot prove is safe to delete from, so treat an unreadable list as a refusal rather + // than as "no siblings". + const allApps = await registry.listApps() + if (allApps.length === 0) { + const refusal = + 'GET /apps returned nothing — cannot rule out sibling suite surfaces. Refusing, ' + + 'because a suite parent migrated alone splits the menu group and strands its siblings.' + say(`REFUSED: ${refusal}`) + return { ok: false, action: plan.action, steps, refusal } + } + const siblings = detectSuiteMembers(from, allApps) + if (siblings.length > 0) { + const refusal = + `"${from}" is a SUITE PARENT — related surfaces: ${siblings.join(', ')}. Migrating a ` + + 'suite means registering, re-pointing nav.suite.id on, and deleting every member as ' + + 'one operation, and the contract offers no transaction to make that atomic. SCOPED ' + + 'OUT: do it by hand in a maintenance window (see the runbook).' + say(`REFUSED: ${refusal}`) + return { ok: false, action: plan.action, steps, refusal } + } + + // ---- the two acknowledgements ---------------------------------------------------- + // These are not ceremony. Both are silent, irreversible losses that this tool provably + // cannot repair, so the only honest gate is that a human states they have handled them. + // + // They gate the DELETE, not the preview. A dry run that stopped here would be useless + // for its actual purpose — you could not see the plan without first passing the flags, + // which trains the operator to type them reflexively to get any output at all. A + // confirmation you have to bypass in order to do your job stops being a decision and + // becomes a habit. So a dry run WARNS in full and carries on planning; only --apply + // refuses. + const unacknowledged = [] + if (!opts.permitGrants) { + unacknowledged.push( + `--permit-grants not given. Deleting "${from}" leaves every Permit role assignment ` + + `pointing at \`${from}_\`, while runtime checks move to \`${to}_\`. The old ` + + 'role is never deleted (syncPermitSchema only creates/updates), so nothing errors — ' + + 'affected users just silently lose the role. Remap the grants during the overlap ' + + 'window, then pass --permit-grants to confirm. See the runbook.' + ) + } + if (!opts.installs) { + unacknowledged.push( + `--installs not given. app_installations.app_id references apps.id ON DELETE CASCADE, ` + + `so deleting "${from}" destroys every personal and organization install of the product. ` + + 'Installs are not in the frozen contract, so this tool can neither read nor restore ' + + 'them. Capture them first, then pass --installs to confirm. See the runbook.' + ) + } + if (unacknowledged.length > 0) { + if (apply) { + const refusal = unacknowledged.join('\n') + say(`REFUSED: ${refusal}`) + return { ok: false, action: plan.action, steps, refusal } + } + for (const w of unacknowledged) say(`WARNING (would block --apply): ${w}`) + } + + // ---- step 1: register the replacement -------------------------------------------- + const targetStatus = oldApp?.status ?? 'activated' + + if (plan.action === 'register') { + const { manifest: nextManifest, notes } = rewriteManifest(oldApp.manifest, from, to) + for (const n of notes) say(` ${n}`) + + if (!apply) { + say(`DRY RUN: would POST /apps with slug "${to}", then re-attach policy/billing, ` + + `then activate to reach status "${targetStatus}", verify, and DELETE /apps/${from}`) + return { ok: true, action: plan.action, steps } + } + + const reg = await registry.registerApp(nextManifest) + if (reg.status === 201) say(`registered ${to}`) + else if (reg.status === 409) say(`${to} already registered (409) — continuing`) + else { + const refusal = `register failed: HTTP ${reg.status} ${reg.text}. NOTHING deleted; "${from}" is still live.` + say(`ABORT: ${refusal}`) + return { ok: false, action: plan.action, steps, refusal } + } + } else if (apply) { + // Replacement already exists from an earlier run. Refresh its manifest so a resumed + // migration does not leave a half-rewritten record behind. + const { manifest: nextManifest } = rewriteManifest(oldApp.manifest, from, to) + const put = await registry.putManifest(to, nextManifest) + say(`refreshed ${to} manifest (HTTP ${put.status})`) + } + + // ---- step 2: re-attach policy + billing ------------------------------------------ + // These CANNOT be copied from the old row — `App` in the contract does not expose them. + // They come from the product's registration/ directory or not at all, and "not at all" + // is a product whose users have no roles, so it is a refusal rather than a warning. + const attach = await reattach(registry, to, opts, say, apply) + if (!attach.ok) return { ok: false, action: plan.action, steps, refusal: attach.refusal } + + // ---- step 3: reach the old app's status ------------------------------------------ + // Preserve, do not assume. Migrating a SUSPENDED app must not switch it on. + if (apply) { + if (targetStatus === 'activated') { + const act = await registry.activateApp(to) + if (act.status !== 200 && act.status !== 204) { + const refusal = `activate ${to} failed: HTTP ${act.status} ${act.text}. NOTHING deleted; "${from}" is still live.` + say(`ABORT: ${refusal}`) + return { ok: false, action: plan.action, steps, refusal } + } + say(`activated ${to}`) + } else { + say(`"${from}" was "${targetStatus}", not activated — leaving ${to} unactivated to match`) + } + } + + if (!apply) { + say(`DRY RUN: would verify ${to} is "${targetStatus}", then DELETE /apps/${from}`) + return { ok: true, action: plan.action, steps } + } + + // ---- step 4: VERIFY, and only then delete ---------------------------------------- + // Re-read from the server. Not "the POST returned 201" — the actual current state, at + // the moment of deciding to delete. This check is the entire safety property. + const check = await registry.getApp(to) + if (check.status !== 200) { + const refusal = `verification failed: GET /apps/${to} returned ${check.status}. REFUSING to delete "${from}".` + say(`ABORT: ${refusal}`) + return { ok: false, action: plan.action, steps, refusal } + } + if (check.app.status !== targetStatus) { + const refusal = + `verification failed: ${to} is "${check.app.status}" but "${from}" was "${targetStatus}". ` + + `REFUSING to delete "${from}" — the replacement is not equivalent.` + say(`ABORT: ${refusal}`) + return { ok: false, action: plan.action, steps, refusal } + } + if (check.app.manifest?.slug !== to) { + const refusal = + `verification failed: ${to}'s manifest.slug is "${check.app.manifest?.slug}". ` + + `REFUSING to delete "${from}".` + say(`ABORT: ${refusal}`) + return { ok: false, action: plan.action, steps, refusal } + } + say(`verified: ${to} is registered, status "${check.app.status}", manifest.slug "${to}"`) + + const del = await registry.deleteApp(from) + if (del.status === 204 || del.status === 200) say(`deleted ${from}`) + else if (del.status === 404) say(`${from} already gone (404)`) + else { + // The replacement is live and verified, so the product is NOT unregistered — this is + // the recoverable outcome the design trades for. Report it as a failure so it is not + // mistaken for a completed migration, and say plainly what state things are in. + const refusal = + `delete failed: HTTP ${del.status} ${del.text}. The replacement "${to}" IS live and ` + + `verified, so the product still works — but "${from}" remains as a duplicate tile. ` + + 'Re-run this tool to retry the delete.' + say(`INCOMPLETE: ${refusal}`) + return { ok: false, action: plan.action, steps, refusal } + } + + // Final assertion: the replacement survived the delete. Cheap, and the one thing that + // would make this catastrophic rather than merely wrong. + const post = await registry.getApp(to) + if (post.status !== 200) { + const refusal = + `CRITICAL: after deleting "${from}", GET /apps/${to} returns ${post.status}. The product ` + + 'may be UNREGISTERED. Re-register it immediately from its registration/ directory.' + say(refusal) + return { ok: false, action: plan.action, steps, refusal } + } + + say(`OK — ${from} corrected to ${to}`) + return { ok: true, action: plan.action, steps } +} + +/** + * Re-submit policy.json / billing-profile.json under the new slug. + * @returns {Promise<{ok:boolean, refusal?:string}>} + */ +async function reattach(registry, to, opts, say, apply) { + const dir = opts.registration + if (!dir) { + say( + 'no --registration given — assuming this product has no policy.json and no ' + + 'billing-profile.json. If it has either, STOP: the new slug will have no roles ' + + 'and cannot take payment.' + ) + return { ok: true } + } + if (!existsSync(dir)) { + return { ok: false, refusal: `--registration ${dir} does not exist` } + } + + for (const [file, verb, put] of [ + ['policy.json', 'authz policy', (s, b) => registry.putPolicy(s, b)], + ['billing-profile.json', 'billing profile', (s, b) => registry.putBilling(s, b)], + ]) { + const path = join(dir, file) + if (!existsSync(path)) { + say(`no ${file} in ${dir} — skipping ${verb}`) + continue + } + let body + try { + body = JSON.parse(readFileSync(path, 'utf8')) + } catch (err) { + return { ok: false, refusal: `${path} is not valid JSON — ${err.message}` } + } + // The platform namespaces by the REGISTRY SLUG regardless of what `product` says + // (sync-permit-schema.ts forces `product: row.slug`), but the ingress schema rejects + // a body whose `product` disagrees with the path slug. Align it. + if (body && typeof body === 'object' && 'product' in body) body.product = to + + if (!apply) { + say(`DRY RUN: would submit ${verb} from ${file}`) + continue + } + const res = await put(to, body) + if ([200, 201, 204].includes(res.status)) { + say(`submitted ${verb}`) + } else { + return { + ok: false, + refusal: + `${verb} submission failed: HTTP ${res.status} ${res.text}. NOTHING deleted; ` + + 'the prefixed registration is still live.', + } + } + } + return { ok: true } +} + +// ---- CLI --------------------------------------------------------------------------- + +export function parseArgs(argv) { + const out = { apply: false, permitGrants: false, installs: false } + for (let i = 0; i < argv.length; i++) { + const a = argv[i] + const take = () => argv[++i] + if (a === '--from') out.from = take() + else if (a === '--to') out.to = take() + else if (a === '--api') out.api = take() + else if (a === '--token') out.token = take() + else if (a === '--registration') out.registration = take() + else if (a === '--apply') out.apply = true + else if (a === '--permit-grants') out.permitGrants = true + else if (a === '--installs') out.installs = true + else if (a === '--help' || a === '-h') out.help = true + else throw new MigrationRefused(`unknown argument "${a}"`) + } + return out +} + +const USAGE = ` +fuzefront-migrate-slug — correct a Fuze-prefixed app registration (OWNER TOOL) + + --from the prefixed slug to remove, e.g. fuzeservice + --to the short slug to register, e.g. service + --api registry base URL (or FUZEFRONT_API_URL) + --token bearer token (or FUZEFRONT_REGISTRATION_TOKEN) + --registration product's registration/ dir, for policy + billing re-submit + --apply actually do it (DEFAULT IS A DRY RUN) + --permit-grants confirm the Permit role-assignment remap is handled + --installs confirm the loss of app_installations rows is accepted + +Both --permit-grants and --installs are required before anything is deleted. They +acknowledge two silent, irreversible losses this tool cannot repair; see +docs/runbooks/app-slug-deprefix-migration.md. +` + +const invokedDirectly = + process.argv[1] && process.argv[1].endsWith('migrate-slug.mjs') + +if (invokedDirectly) { + ;(async () => { + let args + try { + args = parseArgs(process.argv.slice(2)) + } catch (err) { + console.error(`migrate-slug: ${err.message}`) + console.error(USAGE) + process.exit(1) + } + if (args.help) { + console.log(USAGE) + process.exit(0) + } + + const api = args.api || process.env.FUZEFRONT_API_URL + const token = args.token || process.env.FUZEFRONT_REGISTRATION_TOKEN + if (!api || !token) { + console.error('migrate-slug: --api/FUZEFRONT_API_URL and --token/FUZEFRONT_REGISTRATION_TOKEN are required') + process.exit(1) + } + + try { + const registry = new RegistryClient(api, token) + const result = await migrate(registry, { ...args, log: m => console.error(m) }) + process.exit(result.ok ? 0 : 1) + } catch (err) { + console.error(`migrate-slug: ${err.message}`) + process.exit(1) + } + })() +} diff --git a/packages/onboarding-kit/bin/validate-registration.mjs b/packages/onboarding-kit/bin/validate-registration.mjs index 2ab565cb..e6e7a101 100644 --- a/packages/onboarding-kit/bin/validate-registration.mjs +++ b/packages/onboarding-kit/bin/validate-registration.mjs @@ -19,9 +19,16 @@ // then fails closed for every user, which reads as a bug in the PRODUCT rather // than a gap in its registration. // -// Both failures are invisible by construction: they produce no error, no 4xx, and no -// log line anybody reads. They surface as "this product is mysteriously limited". This -// gate converts that whole class into a red build in the repo that owns the file. +// 3. A slug carrying the `Fuze` prefix (`fuzeservice` rather than `service`) is valid +// against every schema and registers cleanly. What makes it worth a gate is that +// it is UNFIXABLE afterwards: `slug` is immutable and there is no rename, so the +// only correction is register-the-new-one-then-delete-the-old, which orphans the +// product's Permit grants and CASCADE-deletes its installation rows. A rule whose +// violation is free to prevent and expensive to undo belongs at authoring time. +// +// All three failures are invisible by construction: they produce no error, no 4xx, and +// no log line anybody reads. They surface as "this product is mysteriously limited". +// This gate converts that whole class into a red build in the repo that owns the file. // // Usage: // node validate-registration.mjs [path/to/registration ...] @@ -38,6 +45,75 @@ import { join, resolve } from 'node:path' // product that is either invisible in the portal or permanently desktop-only. const REQUIRED_SURFACES = ['portal', 'standalone'] +// The family naming convention: a Fuze product registers on FuzeFront WITHOUT the +// `Fuze` prefix — slug `service`, name `Service`. FuzeFront is the platform, so the +// prefix is already implied by the fact that you are registering here at all; +// repeating it gives every tile in the launcher the same first four letters and the +// slug that shows up in `/app/` URLs, Permit keys and billing product keys +// carries four characters of pure noise. FuzePicker already registered as `picker`, +// so the convention existed — it was simply never enforced, and twelve products +// registered against it. +// +// Matched with an anchor and no length exemption: there is no product for which +// bare `fuze` is the correct de-prefixed slug, because de-prefixing "FuzeX" yields +// "x". Case-insensitive is belt-and-braces — the contract's Slug pattern is already +// lowercase-only, but `name` is free text and `Fuze` is exactly how it is written +// there. +const FUZE_PREFIX_RE = /^fuze/i + +/** + * WHY THIS IS A BUILD-TIME GATE AND NOT A `pattern` ON THE CONTRACT'S `Slug`. + * + * It is tempting to add `(?!fuze)` to `Slug` in + * services/app-registry-service/openapi.yaml and be done. That would be actively + * harmful, and the reason is the whole shape of this problem: + * + * `slug` is IMMUTABLE — `PUT /apps/{slug}` states that `slug`, `builtin` and + * `manifestVersion` must match, and there is no rename operation. Correcting a + * prefixed registration is therefore a TWO-STEP migration: register the short + * slug, then delete the prefixed one (see bin/migrate-slug.mjs). + * + * Both of those steps talk to the registry ABOUT the prefixed slug. A contract-level + * ban would reject the very requests that repair the damage: `register.sh` re-PUTs + * the manifest on every pod start, so twelve live products would start failing their + * manifest refresh, and the migration tool could no longer look up — or in the worst + * case delete — the row it exists to remove. Banning a value at the API is only safe + * when no existing row holds it. Twelve do. + * + * So the registry must keep ACCEPTING `fuzeservice` for exactly as long as the + * migration is in flight, while no product is allowed to AUTHOR a new one. Those are + * different questions, they need different enforcement points, and this is the + * authoring one: it fails in the product's own repo, at build time, where the + * manifest is written and where somebody can fix it. + * + * @param {Record} manifest + * @returns {string[]} human-readable violations; empty means conformant + */ +export function validateSlugConvention(manifest) { + const errors = [] + const { slug, name } = manifest + + if (typeof slug === 'string' && FUZE_PREFIX_RE.test(slug)) { + errors.push( + `slug "${slug}" starts with "fuze" — Fuze products register WITHOUT the prefix ` + + `(use "${slug.replace(FUZE_PREFIX_RE, '') || ''}"). The prefix is implied ` + + 'by registering on FuzeFront at all, and the slug is user-visible in /app/. ' + + 'Note this is not fixable after the fact: `slug` is immutable, so a wrong slug ' + + 'costs a register-then-delete migration, not an edit.' + ) + } + + if (typeof name === 'string' && FUZE_PREFIX_RE.test(name)) { + errors.push( + `name "${name}" starts with "Fuze" — use "${name.replace(FUZE_PREFIX_RE, '') || ''}". ` + + 'The launcher already sits inside FuzeFront; prefixing every tile makes them ' + + 'indistinguishable at a glance.' + ) + } + + return errors +} + /** * Resolve the surfaces a manifest actually serves. * @@ -156,7 +232,11 @@ export function validateRegistrationDir(dir) { return [`${manifestPath}: must be a JSON object`] } - return [...validateSurfaces(manifest), ...validatePolicyWiring(dir)] + return [ + ...validateSlugConvention(manifest), + ...validateSurfaces(manifest), + ...validatePolicyWiring(dir), + ] } // ---- CLI --------------------------------------------------------------------------- diff --git a/packages/onboarding-kit/package.json b/packages/onboarding-kit/package.json index 8420b43c..cb78fd23 100644 --- a/packages/onboarding-kit/package.json +++ b/packages/onboarding-kit/package.json @@ -12,15 +12,17 @@ "bin": { "fuzefront-register": "bin/register.sh", "fuzefront-validate-policy": "bin/validate-policy.mjs", - "fuzefront-validate-registration": "bin/validate-registration.mjs" + "fuzefront-validate-registration": "bin/validate-registration.mjs", + "fuzefront-migrate-slug": "bin/migrate-slug.mjs" }, "scripts": { - "test": "node --test tests/validate-policy.test.mjs && node --test tests/validate-registration.test.mjs && sh tests/register.test.sh", + "test": "node --test tests/validate-policy.test.mjs && node --test tests/validate-registration.test.mjs && node --test tests/migrate-slug.test.mjs && sh tests/register.test.sh", "test:policy": "node --test tests/validate-policy.test.mjs", "build:schema": "node scripts/build-schema.mjs", "check:schema": "node scripts/build-schema.mjs --check", "lint": "shellcheck bin/register.sh", - "test:registration": "node --test tests/validate-registration.test.mjs" + "test:registration": "node --test tests/validate-registration.test.mjs", + "test:migrate": "node --test tests/migrate-slug.test.mjs" }, "//devDependencies": "The zero-dependencies rule this kit lives by is about RUNTIME: bin/register.sh and bin/validate-policy.mjs run inside a product's init container, where no npm install is possible, and neither may ever require a module. scripts/build-schema.mjs is a CI/dev generator, is not in `files`, and never ships \u2014 so a devDependency here does not violate that rule. Pinned exactly because a generator that silently changes its output across a minor bump is worse than no generator. Do not move this to `dependencies`.", "devDependencies": { diff --git a/packages/onboarding-kit/tests/fake-registry.mjs b/packages/onboarding-kit/tests/fake-registry.mjs index 3bb2d668..12d4b5aa 100644 --- a/packages/onboarding-kit/tests/fake-registry.mjs +++ b/packages/onboarding-kit/tests/fake-registry.mjs @@ -40,19 +40,58 @@ const server = createServer((req, res) => { if (method === 'POST' && url === '/api/v1/app-registry/apps') { const manifest = JSON.parse(body).manifest if (apps.has(manifest.slug)) return send(409, { error: 'conflict' }) - apps.set(manifest.slug, { manifest, status: 'registered' }) + apps.set(manifest.slug, { + manifest, + status: 'registered', + builtin: manifest.builtin === true, + }) return send(201, { slug: manifest.slug, status: 'registered' }) } + // List. Used by migrate-slug.mjs for suite detection. `?status=` filters; + // omitting it returns all NON-SUSPENDED rows, matching the frozen contract. + if (method === 'GET' && url.startsWith('/api/v1/app-registry/apps?')) { + const want = new URL(url, 'http://x').searchParams.get('status') + const items = [...apps.entries()] + .filter(([, a]) => (want ? a.status === want : a.status !== 'suspended')) + .map(([slug, a]) => ({ slug, status: a.status, builtin: a.builtin, manifest: a.manifest })) + return send(200, items) + } + if (method === 'GET' && url === '/api/v1/app-registry/apps') { + const items = [...apps.entries()] + .filter(([, a]) => a.status !== 'suspended') + .map(([slug, a]) => ({ slug, status: a.status, builtin: a.builtin, manifest: a.manifest })) + return send(200, items) + } + if (!m) return send(404, { error: 'not_found' }) const [, slug, sub] = m const app = apps.get(slug) if (method === 'GET' && !sub) { - return app ? send(200, { slug, status: app.status }) : send(404, { error: 'not_found' }) + // The contract's `App` carries the manifest and the builtin flag; migrate-slug + // reads both to decide whether a delete is even permitted. It deliberately does + // NOT carry `policy` or `billing` — that omission is real, and the migration tool + // is built around it, so the fake must not invent them. + return app + ? send(200, { + slug, + status: app.status, + builtin: app.builtin === true, + manifest: app.manifest, + }) + : send(404, { error: 'not_found' }) } if (!app) return send(404, { error: 'not_found' }) + // Built-ins cannot be deleted, only suspended — a 403, per the contract. + if (method === 'DELETE' && !sub) { + if (app.builtin === true) return send(403, { error: 'builtin_cannot_be_deleted' }) + apps.delete(slug) + res.writeHead(204) + return res.end() + } + if (method === 'PUT' && !sub) { app.manifest = JSON.parse(body) return send(200, { slug, status: app.status }) diff --git a/packages/onboarding-kit/tests/migrate-slug.test.mjs b/packages/onboarding-kit/tests/migrate-slug.test.mjs new file mode 100644 index 00000000..375cdc7e --- /dev/null +++ b/packages/onboarding-kit/tests/migrate-slug.test.mjs @@ -0,0 +1,412 @@ +// Tests for bin/migrate-slug.mjs. +// +// The pure planning functions are tested directly. Everything that decides whether a +// DELETE happens is tested END TO END against tests/fake-registry.mjs, because the +// safety property being asserted is about the state of a registry after a sequence of +// real HTTP calls — a mocked client could be made to agree with a broken tool. +// +// The property under test throughout: THE PRODUCT IS NEVER LEFT UNREGISTERED. Every +// failure case asserts not only that the tool reported failure, but that the old app +// is still there afterwards. + +import { test, describe, before, after, beforeEach } from 'node:test' +import assert from 'node:assert/strict' +import { spawn } from 'node:child_process' +import { mkdtempSync, mkdirSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join, dirname } from 'node:path' +import { fileURLToPath } from 'node:url' + +import { + RegistryClient, + migrate, + planFrom, + rewriteManifest, + detectSuiteMembers, + validateSlugs, + parseArgs, + MigrationRefused, +} from '../bin/migrate-slug.mjs' + +const HERE = dirname(fileURLToPath(import.meta.url)) +const TOKEN = 'test-token' + +const manifestFor = (slug, name, extra = {}) => ({ + manifestVersion: '1', + slug, + name, + menuLabel: name, + mode: 'portal', + modes: ['portal', 'standalone'], + integration: { + type: 'module-federation', + remoteEntry: `https://${slug}.example.com/remoteEntry.js`, + scope: `${slug}App`, + module: './App', + }, + nav: { section: 'build', order: 10 }, + routing: { path: `/app/${slug}`, host: `${slug}.fuzefront.com` }, + visibility: 'organization', + ...extra, +}) + +/** Acknowledgements the operator would pass on the CLI. */ +const ACK = { permitGrants: true, installs: true } +const silent = () => {} + +// ---- pure planning ------------------------------------------------------------------ + +describe('planFrom', () => { + test('old present, new absent -> register', () => { + assert.equal(planFrom({ status: 200 }, { status: 404 }).action, 'register') + }) + test('both present -> resume at verify-and-delete', () => { + // A previous run registered the replacement and then died. Resuming must NOT + // re-register (409) and must NOT skip straight to delete without verifying. + assert.equal(planFrom({ status: 200 }, { status: 200 }).action, 'verify-and-delete') + }) + test('old absent, new present -> noop (already migrated)', () => { + assert.equal(planFrom({ status: 404 }, { status: 200 }).action, 'noop') + }) + test('neither present -> absent, which is a FAILURE not a quiet success', () => { + // "nothing to do" and "you typed the slug wrong" read identically against the + // registry. Exiting 0 on the second is how a migration gets ticked off without + // having happened. + assert.equal(planFrom({ status: 404 }, { status: 404 }).action, 'absent') + }) +}) + +describe('validateSlugs', () => { + test('rejects a --to that is still prefixed', () => { + assert.throws(() => validateSlugs({ from: 'fuzeservice', to: 'fuzeservice2' }), MigrationRefused) + }) + test('rejects a --from that is NOT prefixed — this is not a general rename tool', () => { + assert.throws(() => validateSlugs({ from: 'picker', to: 'chooser' }), /general rename/) + }) + test('rejects from === to', () => { + assert.throws(() => validateSlugs({ from: 'fuzeservice', to: 'fuzeservice' }), /nothing to migrate/) + }) + test('rejects a --to that violates the contract Slug pattern', () => { + assert.throws(() => validateSlugs({ from: 'fuzeservice', to: 'Service' }), /not a valid slug/) + }) + test('accepts the real fuzeservice -> service case', () => { + assert.doesNotThrow(() => validateSlugs({ from: 'fuzeservice', to: 'service' })) + }) +}) + +describe('rewriteManifest', () => { + const { manifest: out, notes } = rewriteManifest( + manifestFor('fuzeservice', 'FuzeService'), + 'fuzeservice', + 'service' + ) + + test('rewrites slug, name, menuLabel and the /app/ path', () => { + assert.equal(out.slug, 'service') + assert.equal(out.name, 'Service') + assert.equal(out.menuLabel, 'Service') + assert.equal(out.routing.path, '/app/service') + }) + + test('does NOT rewrite routing.host — a hostname needs DNS, a cert and an ingress', () => { + assert.equal(out.routing.host, 'fuzeservice.fuzefront.com') + assert.match(notes.join('\n'), /routing\.host.*was NOT changed/s) + }) + + test('does NOT rewrite integration.scope — it must match the deployed bundle', () => { + // The MF scope is the global the remote publishes itself under at runtime. Rewriting + // it here would point the host at a global the bundle never defines: the remote + // fails to load in the browser, with nothing server-side to catch it. + assert.equal(out.integration.scope, 'fuzeserviceApp') + assert.match(notes.join('\n'), /integration\.scope.*was NOT changed/s) + }) + + test('leaves a hand-written routing.path alone rather than guessing', () => { + const { manifest } = rewriteManifest( + manifestFor('fuzeservice', 'FuzeService', { routing: { path: '/app/support-desk' } }), + 'fuzeservice', + 'service' + ) + assert.equal(manifest.routing.path, '/app/support-desk') + }) + + test('leaves an already-correct name alone (FuzePicker/picker asymmetry)', () => { + const { manifest } = rewriteManifest( + manifestFor('fuzecontact', 'Contact'), + 'fuzecontact', + 'contact' + ) + assert.equal(manifest.name, 'Contact') + }) + + test('does not mutate its input', () => { + const input = manifestFor('fuzeservice', 'FuzeService') + rewriteManifest(input, 'fuzeservice', 'service') + assert.equal(input.slug, 'fuzeservice') + }) +}) + +describe('detectSuiteMembers', () => { + const hub = [ + { slug: 'fuzehub', manifest: { nav: { suite: { id: 'fuzehub' } } } }, + { slug: 'fuzehub-talent', manifest: { nav: { suite: { id: 'fuzehub' } } } }, + { slug: 'fuzehub-recruiter', manifest: { nav: { suite: { id: 'fuzehub' } } } }, + { slug: 'fuzeservice', manifest: { nav: {} } }, + ] + + test('finds siblings by slug prefix AND by shared nav.suite.id', () => { + assert.deepEqual(detectSuiteMembers('fuzehub', hub), ['fuzehub-recruiter', 'fuzehub-talent']) + }) + + test('a standalone product has no siblings', () => { + assert.deepEqual(detectSuiteMembers('fuzeservice', hub), []) + }) + + test('a sibling with no nav.suite at all is still caught by the slug prefix', () => { + const apps = [{ slug: 'fuzehub' }, { slug: 'fuzehub-talent' }] + assert.deepEqual(detectSuiteMembers('fuzehub', apps), ['fuzehub-talent']) + }) +}) + +describe('parseArgs', () => { + test('dry run and both acknowledgements default to OFF', () => { + const a = parseArgs(['--from', 'fuzeservice', '--to', 'service']) + assert.equal(a.apply, false) + assert.equal(a.permitGrants, false) + assert.equal(a.installs, false) + }) + test('an unknown flag is fatal, not ignored', () => { + // A typo'd --aply that silently parsed as a dry run would report success having + // changed nothing; a typo'd --permit-grant would be worse. + assert.throws(() => parseArgs(['--aply']), MigrationRefused) + }) +}) + +// ---- end to end against the fake registry ------------------------------------------- + +describe('migrate against the fake registry', () => { + let server + let registry + let workDir + + const start = () => + new Promise((resolve, reject) => { + const proc = spawn(process.execPath, [join(HERE, 'fake-registry.mjs')], { + stdio: ['ignore', 'pipe', 'pipe'], + }) + let buf = '' + proc.stdout.on('data', d => { + buf += d + const m = buf.match(/^LISTENING (\d+)$/m) + if (m) resolve({ proc, port: Number(m[1]) }) + }) + proc.on('error', reject) + setTimeout(() => reject(new Error('fake registry did not start')), 5000) + }) + + before(async () => { + workDir = mkdtempSync(join(tmpdir(), 'migrate-')) + }) + + beforeEach(async () => { + // A fresh registry per test — these tests DELETE things, and a shared instance + // would make each case depend on the order of the previous ones. + if (server) server.kill() + server = undefined + const started = await start() + server = started.proc + registry = new RegistryClient(`http://127.0.0.1:${started.port}`, TOKEN, silent) + }) + + after(() => { + if (server) server.kill() + }) + + /** Seed a registered + activated app. */ + async function seed(slug, name, extra = {}) { + const r = await registry.registerApp(manifestFor(slug, name, extra)) + assert.equal(r.status, 201, `seed ${slug}: ${r.text}`) + await registry.activateApp(slug) + } + + const run = opts => migrate(registry, { from: 'fuzeservice', to: 'service', log: silent, ...opts }) + + test('DRY RUN is the default and changes nothing', async () => { + await seed('fuzeservice', 'FuzeService') + const res = await run({ ...ACK }) + assert.equal(res.ok, true) + assert.equal((await registry.getApp('fuzeservice')).status, 200, 'old app must survive a dry run') + assert.equal((await registry.getApp('service')).status, 404, 'dry run must not register anything') + }) + + test('a DRY RUN without the acknowledgements still produces a full plan', async () => { + // The acknowledgements gate the DELETE, not the preview. If a dry run stopped at + // them you could not see the plan without first passing the flags — which turns a + // deliberate confirmation into something typed reflexively to get any output at all. + await seed('fuzeservice', 'FuzeService') + const res = await run({}) + assert.equal(res.ok, true) + const out = res.steps.join('\n') + assert.match(out, /WARNING \(would block --apply\).*--permit-grants/s) + assert.match(out, /WARNING \(would block --apply\).*--installs/s) + assert.match(out, /slug: fuzeservice -> service/, 'the plan itself must still be shown') + assert.match(out, /DRY RUN: would POST \/apps with slug "service".*DELETE \/apps\/fuzeservice/s) + assert.equal((await registry.getApp('fuzeservice')).status, 200) + assert.equal((await registry.getApp('service')).status, 404) + }) + + test('--apply performs the full two-step correction', async () => { + await seed('fuzeservice', 'FuzeService') + const res = await run({ ...ACK, apply: true }) + assert.equal(res.ok, true, res.refusal) + + const now = await registry.getApp('service') + assert.equal(now.status, 200) + assert.equal(now.app.status, 'activated', 'the replacement must be ACTIVE, not merely registered') + assert.equal(now.app.manifest.slug, 'service') + assert.equal(now.app.manifest.name, 'Service') + assert.equal(now.app.manifest.routing.path, '/app/service') + + assert.equal((await registry.getApp('fuzeservice')).status, 404, 'the prefixed app must be gone') + }) + + test('is idempotent — a second --apply run is a clean no-op', async () => { + await seed('fuzeservice', 'FuzeService') + await run({ ...ACK, apply: true }) + const second = await run({ ...ACK, apply: true }) + assert.equal(second.ok, true, second.refusal) + assert.equal(second.action, 'noop') + assert.equal((await registry.getApp('service')).status, 200) + }) + + test('resumes a half-finished migration (both rows present) and deletes the old one', async () => { + // This is the state left by a run that was interrupted between step 1 and step 2 — + // and it is exactly the state a product creates for itself by de-prefixing its + // manifest and redeploying, since register.sh only ever does step 1. + await seed('fuzeservice', 'FuzeService') + await seed('service', 'Service') + const res = await run({ ...ACK, apply: true }) + assert.equal(res.ok, true, res.refusal) + assert.equal(res.action, 'verify-and-delete') + assert.equal((await registry.getApp('fuzeservice')).status, 404) + assert.equal((await registry.getApp('service')).status, 200) + }) + + test('REFUSES without --permit-grants, and the old app survives', async () => { + await seed('fuzeservice', 'FuzeService') + const res = await run({ installs: true, apply: true }) + assert.equal(res.ok, false) + assert.match(res.refusal, /--permit-grants/) + assert.match(res.refusal, /silently lose the role/) + assert.equal((await registry.getApp('fuzeservice')).status, 200) + assert.equal((await registry.getApp('service')).status, 404, 'a refusal must not half-register') + }) + + test('REFUSES without --installs, and the old app survives', async () => { + await seed('fuzeservice', 'FuzeService') + const res = await run({ permitGrants: true, apply: true }) + assert.equal(res.ok, false) + assert.match(res.refusal, /ON DELETE CASCADE/) + assert.equal((await registry.getApp('fuzeservice')).status, 200) + }) + + test('REFUSES a built-in app up front, before registering any duplicate', async () => { + // DELETE 403s on built-ins, so the migration could never finish — it would only ever + // add a permanent second tile. Refusing before step 1 is what keeps that from + // happening. + await seed('fuzeservice', 'FuzeService', { builtin: true }) + const res = await run({ ...ACK, apply: true }) + assert.equal(res.ok, false) + assert.match(res.refusal, /BUILT-IN/) + assert.equal((await registry.getApp('service')).status, 404, 'must not register a duplicate it can never clean up') + assert.equal((await registry.getApp('fuzeservice')).status, 200) + }) + + test('REFUSES a suite parent (the FuzeHub case) and names the siblings', async () => { + await seed('fuzehub', 'FuzeHub', { nav: { section: 'build', order: 1, suite: { id: 'fuzehub', label: 'Hub', order: 1 } } }) + await seed('fuzehub-talent', 'FuzeHub Talent', { nav: { section: 'build', order: 2, suite: { id: 'fuzehub', label: 'Hub', order: 1 } } }) + const res = await migrate(registry, { from: 'fuzehub', to: 'hub', apply: true, log: silent, ...ACK }) + assert.equal(res.ok, false) + assert.match(res.refusal, /SUITE PARENT/) + assert.match(res.refusal, /fuzehub-talent/) + assert.equal((await registry.getApp('fuzehub')).status, 200) + assert.equal((await registry.getApp('hub')).status, 404) + }) + + test('preserves a SUSPENDED app\'s status instead of switching it on', async () => { + // Migrating must not be a side door that activates something an operator + // deliberately turned off. + await registry.registerApp(manifestFor('fuzeplan', 'FuzePlan')) + const res = await migrate(registry, { + from: 'fuzeplan', + to: 'plan', + apply: true, + log: silent, + ...ACK, + }) + assert.equal(res.ok, true, res.refusal) + const now = await registry.getApp('plan') + assert.equal(now.app.status, 'registered', 'must NOT have been activated') + assert.equal((await registry.getApp('fuzeplan')).status, 404) + }) + + test('re-submits policy.json and billing-profile.json under the NEW slug', async () => { + // GET /apps/{slug} cannot return them — `App` is additionalProperties:false over a + // field list that excludes both. They can only come from the product's own + // registration/ directory, so the tool must actually send them. + const dir = join(workDir, 'registration') + mkdirSync(dir, { recursive: true }) + writeFileSync( + join(dir, 'policy.json'), + JSON.stringify({ + product: 'fuzeservice', + resources: [{ key: 'Ticket', name: 'Ticket', actions: { read: { name: 'Read' } } }], + roles: [{ key: 'agent', name: 'Agent', permissions: ['Ticket:read'] }], + }) + ) + writeFileSync(join(dir, 'billing-profile.json'), JSON.stringify({ productKey: 'service' })) + + await seed('fuzeservice', 'FuzeService') + const res = await run({ ...ACK, apply: true, registration: dir }) + assert.equal(res.ok, true, res.refusal) + assert.match(res.steps.join('\n'), /submitted authz policy/) + assert.match(res.steps.join('\n'), /submitted billing profile/) + }) + + test('a malformed policy.json aborts BEFORE the delete', async () => { + const dir = join(workDir, 'bad-registration') + mkdirSync(dir, { recursive: true }) + writeFileSync(join(dir, 'policy.json'), '{ not json') + + await seed('fuzeservice', 'FuzeService') + const res = await run({ ...ACK, apply: true, registration: dir }) + assert.equal(res.ok, false) + assert.match(res.refusal, /not valid JSON/) + assert.equal( + (await registry.getApp('fuzeservice')).status, + 200, + 'the prefixed app must still be live after any abort' + ) + }) + + test('a bad token fails without deleting anything', async () => { + await seed('fuzeservice', 'FuzeService') + const bad = new RegistryClient(registry.base.replace('/api/v1/app-registry', ''), 'wrong', silent) + const res = await migrate(bad, { from: 'fuzeservice', to: 'service', apply: true, log: silent, ...ACK }) + // Every GET 401s, so the plan reads as "neither registered" — which is precisely why + // that case is a refusal and not a quiet success. + assert.equal(res.ok, false) + assert.equal((await registry.getApp('fuzeservice')).status, 200) + }) + + test('migrating a product whose name is already correct still de-prefixes the slug', async () => { + // fuzecontact registers as slug `fuzecontact` with name `Contact` — the name half of + // the convention was already followed, the slug half was not. + await seed('fuzecontact', 'Contact') + const res = await migrate(registry, { from: 'fuzecontact', to: 'contact', apply: true, log: silent, ...ACK }) + assert.equal(res.ok, true, res.refusal) + const now = await registry.getApp('contact') + assert.equal(now.app.manifest.name, 'Contact') + assert.equal(now.app.manifest.slug, 'contact') + }) +}) diff --git a/packages/onboarding-kit/tests/validate-registration.test.mjs b/packages/onboarding-kit/tests/validate-registration.test.mjs index e2aaf62e..3951d340 100644 --- a/packages/onboarding-kit/tests/validate-registration.test.mjs +++ b/packages/onboarding-kit/tests/validate-registration.test.mjs @@ -2,12 +2,14 @@ import { test } from 'node:test' import assert from 'node:assert/strict' import { mkdtempSync, mkdirSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { join, dirname } from 'node:path' +import { fileURLToPath } from 'node:url' import { effectiveModes, validateSurfaces, validatePolicyWiring, + validateSlugConvention, validateRegistrationDir, } from '../bin/validate-registration.mjs' @@ -94,6 +96,95 @@ test('a manifest with no mode and no modes is rejected outright', () => { assert.match(errors[0], /neither/) }) +// ---- slug convention ---------------------------------------------------------------- +// The owner's rule: a Fuze product registers WITHOUT the prefix — slug `service`, name +// `Service`. Twelve of thirteen products got this wrong; FuzePicker (slug `picker`) shows +// the convention already existed and was simply never enforced. + +test('a `fuze`-prefixed slug is REJECTED and the message names the replacement', () => { + const errors = validateSlugConvention({ slug: 'fuzeservice', name: 'Service' }) + assert.equal(errors.length, 1, errors.join('\n')) + assert.match(errors[0], /slug "fuzeservice" starts with "fuze"/) + assert.match(errors[0], /use "service"/) +}) + +test('the message says the mistake is UNFIXABLE, because that is the whole point', () => { + // A slug typo is normally a one-line edit. This one costs a register-then-delete + // migration that orphans Permit grants and CASCADE-deletes installation rows, so the + // error has to say so — otherwise it reads as pedantry and gets argued with. + const [error] = validateSlugConvention({ slug: 'fuzeplan' }) + assert.match(error, /immutable/) +}) + +test('a `Fuze`-prefixed NAME is rejected too — the rule covers both halves', () => { + const errors = validateSlugConvention({ slug: 'service', name: 'FuzeService' }) + assert.equal(errors.length, 1, errors.join('\n')) + assert.match(errors[0], /name "FuzeService"/) + assert.match(errors[0], /use "Service"/) +}) + +test('slug AND name both prefixed produces BOTH violations, not just the first', () => { + // The real fuzeservice/FuzeService shape. Reporting one at a time means two build + // cycles to fix one manifest. + assert.equal(validateSlugConvention({ slug: 'fuzeservice', name: 'FuzeService' }).length, 2) +}) + +test('the conformant form passes', () => { + assert.deepEqual(validateSlugConvention({ slug: 'service', name: 'Service' }), []) +}) + +test('picker/FuzePicker: the slug is already right, only the name is flagged', () => { + // Measured current state — FuzePicker registers as `picker`. Exactly one violation. + const errors = validateSlugConvention({ slug: 'picker', name: 'FuzePicker' }) + assert.equal(errors.length, 1) + assert.match(errors[0], /name "FuzePicker"/) +}) + +test('fuzecontact/Contact: the name is already right, only the slug is flagged', () => { + const errors = validateSlugConvention({ slug: 'fuzecontact', name: 'Contact' }) + assert.equal(errors.length, 1) + assert.match(errors[0], /slug "fuzecontact"/) +}) + +test('a hyphenated prefix (`fuze-market`) is caught too', () => { + assert.equal(validateSlugConvention({ slug: 'fuze-market' }).length, 1) +}) + +test('bare `fuze` is rejected — no product de-prefixes to it', () => { + // De-prefixing "FuzeX" yields "x", so there is no product for which `fuze` is the + // correct short slug. Allowing it as "not really a prefix" would be a loophole. + assert.equal(validateSlugConvention({ slug: 'fuze' }).length, 1) +}) + +test('a slug merely CONTAINING fuze is fine — the rule is anchored to the start', () => { + assert.deepEqual(validateSlugConvention({ slug: 'defuze', name: 'Defuze' }), []) +}) + +test('a missing slug/name is not this check\'s problem', () => { + // Shape is the schema's job. Reporting "slug is absent" here would duplicate the + // contract and produce two errors for one mistake. + assert.deepEqual(validateSlugConvention({}), []) +}) + +test('THE SHIPPED TEMPLATE is itself conformant', () => { + // templates/ is what every product copies, so a violation there propagates to the + // whole fleet before anyone notices. It happens to be clean today (`myapp`, not + // `fuzemyapp`) — this pins that, so the gate can never be undermined by the one + // manifest it does not otherwise get run against. + const templates = join(dirname(fileURLToPath(import.meta.url)), '..', 'templates') + assert.deepEqual(validateRegistrationDir(templates), []) +}) + +test('the slug rule is wired into the end-to-end directory check', () => { + const dir = makeDir({ + 'manifest.json': { ...PORTAL_STANDALONE, slug: 'fuzeservice', name: 'FuzeService' }, + 'policy.json': { product: 'fuzeservice' }, + }) + const errors = validateRegistrationDir(dir) + assert.equal(errors.length, 2, errors.join('\n')) + assert.match(errors.join('\n'), /starts with "fuze"/) +}) + test('missing policy.json is reported', () => { const dir = makeDir({ 'manifest.json': PORTAL_STANDALONE }) const errors = validatePolicyWiring(dir) From 2b3e5c782624b1f991a6eb85dca3518442db2d4d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 10:12:02 +0000 Subject: [PATCH 03/10] fix(mcp-gateway): refuse prototype-chain keys from specs and tool arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the two Semgrep findings on the gateway: src/spec.ts:64 prototype-pollution-loop (error) src/upstream.ts:72 remote-property-injection (warning) Neither is boilerplate here. This gateway turns an OpenAPI document into tools an LLM client calls, so BOTH of its inputs are untrusted in the way these rules mean: - the spec is authored per product and arrives via a ConfigMap, so a $ref or a parameter name in it is attacker-influenced as far as this code goes; - tool arguments are model-generated, i.e. fully untrusted. A key of `__proto__`, `constructor` or `prototype` from either source reaches Object.prototype — leaking internals into a tool schema on a read, polluting every object in the process on a write. The blast radius is a tool surface an agent then calls. Adds src/safety.ts: `safeRecord()` (null-prototype accumulators), `getOwn()` (own-property reads only), `isForbiddenKey()` and `assertSafeKey()` (fail the pod at boot on a hostile spec rather than serving a corrupted tool surface). Wired at both flagged sites and every sibling path that keys an object by spec- or caller-derived strings. Refused outright rather than sanitised into something plausible: a tool named `constructor` has no legitimate meaning, and silently renaming it would hide a malformed spec instead of surfacing it. Also registers packages/mcp-gateway in the root workspaces array, without which the package does not build. Authored by the mcp-gateway workstream; committed here because the work was uncommitted on local disk while failing CI on two open PRs, and would have been lost with the container. Verified before committing: the guards are wired at the exact lines Semgrep flagged and at the sibling call sites. NOT verified: the vitest suite did not finish within the time available, so safety.test.ts is unrun by me — CI is the check on that. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GaPa3JgrVNtWrGvqQEAEqv --- package.json | 1 + packages/mcp-gateway/Dockerfile | 55 ++++++ packages/mcp-gateway/README.md | 145 ++++++++++++++++ packages/mcp-gateway/src/safety.ts | 59 +++++++ packages/mcp-gateway/src/spec.ts | 28 ++- packages/mcp-gateway/src/upstream.ts | 18 +- packages/mcp-gateway/test/safety.test.ts | 211 +++++++++++++++++++++++ 7 files changed, 510 insertions(+), 7 deletions(-) create mode 100644 packages/mcp-gateway/Dockerfile create mode 100644 packages/mcp-gateway/README.md create mode 100644 packages/mcp-gateway/src/safety.ts create mode 100644 packages/mcp-gateway/test/safety.test.ts diff --git a/package.json b/package.json index 97ca5f1a..8c31e5a5 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "packages/i18n", "packages/i18n-translate", "packages/feature-flags", + "packages/mcp-gateway", "packages/security", "design-system", "services/chat-service", diff --git a/packages/mcp-gateway/Dockerfile b/packages/mcp-gateway/Dockerfile new file mode 100644 index 00000000..b9facd0b --- /dev/null +++ b/packages/mcp-gateway/Dockerfile @@ -0,0 +1,55 @@ +# ============================================================================= +# @fuzefront/mcp-gateway — ONE image, deployed once per product. +# +# Nothing product-specific is baked in. The pod becomes "the FuzeService +# gateway" or "the FuzeSales gateway" purely through runtime config: +# +# MCP_PRODUCT product name, used as the MCP server name +# MCP_UPSTREAM_BASE_URL in-cluster base URL of that product's REST API +# MCP_OPENAPI_SPEC path to the mounted OpenAPI document +# MCP_TOOL_OVERRIDES path to the mounted mutation-overrides file (optional) +# +# Deliberately absent: any credential env var. The gateway forwards the caller's +# identity and holds none of its own; src/config.ts refuses to start if a +# service-token variable is present. +# +# Build from the REPO ROOT (it needs the workspace package): +# docker build -f packages/mcp-gateway/Dockerfile -t ghcr.io/izzywdev/fuze-mcp-gateway:0.1.0 . +# ============================================================================= +FROM node:24-alpine AS builder +WORKDIR /build + +COPY packages/mcp-gateway/package.json packages/mcp-gateway/package-lock.json* ./ +RUN npm install --no-workspaces --ignore-scripts + +COPY packages/mcp-gateway/tsconfig.json packages/mcp-gateway/tsconfig.build.json ./ +COPY packages/mcp-gateway/src ./src +RUN npm run build + +# Drop dev dependencies from what we ship. +RUN npm prune --omit=dev --no-workspaces + +# ----------------------------------------------------------------------------- +FROM node:24-alpine AS runtime +WORKDIR /app + +# Run unprivileged. The gateway reads two mounted files and makes outbound HTTP +# calls; it has no reason to own anything on disk. +RUN addgroup -g 1001 -S nodejs && adduser -S mcp -u 1001 -G nodejs + +COPY --from=builder --chown=mcp:nodejs /build/node_modules ./node_modules +COPY --from=builder --chown=mcp:nodejs /build/dist ./dist +COPY --from=builder --chown=mcp:nodejs /build/package.json ./package.json + +USER mcp +EXPOSE 8081 + +ENV NODE_ENV=production \ + PORT=8081 + +# /healthz reports the tool count, so a container that booted with a broken +# spec is visibly different from a healthy one rather than just "up". +HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ + CMD node -e "fetch('http://127.0.0.1:'+(process.env.PORT||8081)+'/healthz').then(r=>process.exit(r.ok?0:1)).catch(()=>process.exit(1))" + +CMD ["node", "dist/main.js"] diff --git a/packages/mcp-gateway/README.md b/packages/mcp-gateway/README.md new file mode 100644 index 00000000..b4f13d62 --- /dev/null +++ b/packages/mcp-gateway/README.md @@ -0,0 +1,145 @@ +# `@fuzefront/mcp-gateway` + +A generic **OpenAPI → MCP SSE gateway**. It exposes a product's existing REST +API as MCP tools. It contains no product logic and never will: adding a product +means pointing a new pod at a different spec, not editing this package. + +**Shared implementation, per-product deployment.** One codebase, one container +image — but each product runs its **own** gateway pod in its **own** namespace, +configured with its own OpenAPI document and its own upstream base URL. There is +no cross-product routing, no spec registry, and no multi-tenancy: a pod knows +exactly one product. The owner's target shape is four pods per product — +backend, frontend, MCP SSE, A2A — and this is the third of those. + +## Why a gateway instead of ten hand-written MCP servers + +A hand-written server per product means ten places to get the dangerous part +wrong. The dangerous part is not the HTTP call; it is the claim each tool makes +about whether it changes anything. Deriving that mechanically from one spec, in +one place, with the invariants enforced at boot, is the only version of this +that stays true as the products change. + +## Configuration + +All runtime, all per pod: + +| Variable | Required | Meaning | +|---|---|---| +| `MCP_PRODUCT` | yes | Product name; becomes the MCP server name | +| `MCP_UPSTREAM_BASE_URL` | yes | In-cluster base URL of the product's REST API | +| `MCP_OPENAPI_SPEC` | yes | Path to the mounted OpenAPI 3.x document (YAML or JSON) | +| `MCP_TOOL_OVERRIDES` | no | Path to the mounted mutation-overrides file | +| `PORT` | no | Listen port (default 8081) | + +## Endpoints + +| Route | Purpose | +|---|---| +| `GET /sse` | Opens the MCP event stream, returns a session id | +| `POST /messages?sessionId=…` | Client → server JSON-RPC for that session | +| `GET /healthz` | Liveness; reports the tool count | +| `GET /tools.json` | The tool manifest with its mutation classification | + +`/tools.json` is unauthenticated on purpose: it exposes the *shape* of the API, +which the OpenAPI document already publishes, and never any data from it. + +## The `mutates` contract + +Every tool carries an explicit `mutates` boolean and a `reversibility` value. +Both are derived from the spec, then narrowed by an optional per-product +overrides file. + +Defaults, from the HTTP method: + +| Method | `mutates` | `reversibility` | +|---|---|---| +| `GET` `HEAD` `OPTIONS` `TRACE` | `false` | — | +| `POST` to a path ending `/search`, `/query`, `/preview` | `false` | — | +| `POST` `PUT` `PATCH` | `true` | `reversible` | +| `DELETE` | `true` | `irreversible` | + +The query-shaped-POST allowlist matches on **suffix**, not substring, so +`POST /tickets/search` is a read while `POST /search-index/rebuild` is not. + +### The invariants that make this trustworthy + +`src/classify.ts` throws — and the pod **refuses to start** — if an overrides +file tries to: + +1. declare an operation `irreversible` while also declaring it a read; +2. relabel a non-query-shaped write as a read; +3. call a safe method `irreversible` (if a `GET` really mutates, the spec is + what needs fixing). + +A gateway that boots with a mis-declared irreversible tool is worse than one +that does not boot, because nobody finds out until something unrecoverable has +already happened. + +**An irreversible operation cannot be reached as a side effect of a read.** That +is structural, not a convention: one MCP tool maps to exactly one OpenAPI +operation and issues exactly that one HTTP request, so a read tool has no code +path to a second request at all. + +The classification also reaches the model that picks the tool — it is prefixed +into the description (`[READ-ONLY]`, `[WRITE]`, `[WRITE — IRREVERSIBLE]`) and +mirrored into MCP `annotations` (`readOnlyHint`, `destructiveHint`) and `_meta`, +so a client can refuse to auto-approve an irreversible call. + +### Overrides file + +```yaml +tools: + decideApproval: + reversibility: irreversible + reason: An approval decision is final from the requester's side + transitionTicket: + reversibility: reversible + reason: The status machine includes a reopen edge +``` + +## Authorization — forwarded, never substituted + +The gateway forwards the **caller's** `Authorization` header to the product API +and holds no credential of its own. There is no `MCP_UPSTREAM_TOKEN`, and +`src/config.ts` refuses to start if one is set. + +A shared service token would make every request look like the gateway rather +than like the user, silently bypassing every per-user Permit check on the +product side and turning the gateway into a confused deputy holding the union of +all users' permissions. A per-product pod sits in that product's namespace and +forwards the caller's identity to that product's API — which is why the +per-product deployment shape makes this *easier*, not harder. + +A call with no caller identity is refused **before** any upstream request is +made, so it fails closed and visibly. Only an allowlist of headers is forwarded +(`authorization`, `x-request-id`, `x-tenant-id`, `x-organization-id`) — cookies +and everything else are dropped. + +## Verifying a product before enabling it + +Do not flip a product's `mcp.enabled` to `true` in `.fuze/manifest.json` until +the gateway has actually run against that product's spec and the tools have +enumerated over the real transport: + +```bash +npm run build + +MCP_PRODUCT=fuzeservice \ +MCP_UPSTREAM_BASE_URL=http://fuzeservice-service.fuzeservice.svc.cluster.local:8080/v1 \ +MCP_OPENAPI_SPEC=/path/to/FuzeService/contracts/openapi.yaml \ +MCP_TOOL_OVERRIDES=/path/to/FuzeService/mcp/tools.overrides.yaml \ +PORT=8099 node dist/main.js & + +node scripts/smoke.mjs http://127.0.0.1:8099 +``` + +`scripts/smoke.mjs` speaks the real MCP SSE transport and asserts the properties +the unit tests cannot: the handshake completes, tools enumerate, no read-only +tool is bound to an unsafe method, every irreversible tool is also a write, and +a call without caller identity fails closed. + +## Tests + +`npm test` — 31 vitest tests over classification, spec translation and identity +forwarding. The load-bearing one asserts that an unauthenticated call results in +**zero** upstream requests, not merely an error response. diff --git a/packages/mcp-gateway/src/safety.ts b/packages/mcp-gateway/src/safety.ts new file mode 100644 index 00000000..e47991ab --- /dev/null +++ b/packages/mcp-gateway/src/safety.ts @@ -0,0 +1,59 @@ +/** + * Guards for object keys that come from untrusted input. + * + * Two untrusted sources feed this gateway, and both end up as object keys: + * + * 1. The OpenAPI document. It is mounted config, but it is authored per + * product and travels through a ConfigMap — it is not this package's own + * source, and a `$ref` or a parameter name from it is attacker-influenced + * input as far as this code is concerned. + * 2. The tool arguments an LLM client sends. Those are model-generated and + * therefore fully untrusted. + * + * A key of `__proto__`, `constructor` or `prototype` in either place reaches + * `Object.prototype` — on a read it leaks internals into a tool schema, and on + * a write it pollutes every object in the process. The blast radius is a tool + * surface an agent will then call, so these are refused outright rather than + * sanitised into something plausible. + */ + +/** Keys that resolve to the prototype chain rather than to own data. */ +const FORBIDDEN_KEYS = new Set(['__proto__', 'constructor', 'prototype']); + +export function isForbiddenKey(key: string): boolean { + return FORBIDDEN_KEYS.has(key); +} + +/** + * A dictionary with NO prototype. Assigning `__proto__` to one of these stores + * an ordinary own property instead of reparenting the object, so it is safe as + * an accumulator even if a guard upstream is ever removed. + */ +export function safeRecord(): Record { + return Object.create(null) as Record; +} + +/** + * Read a property ONLY if the object owns it. `obj[key]` would happily return + * `Object.prototype.constructor` for a key of `constructor`; this returns + * undefined, which is what "the caller did not supply that argument" means. + */ +export function getOwn(obj: unknown, key: string): unknown { + if (!obj || typeof obj !== 'object') return undefined; + if (isForbiddenKey(key)) return undefined; + if (!Object.prototype.hasOwnProperty.call(obj, key)) return undefined; + return (obj as Record)[key]; +} + +/** + * Throw if a spec-derived identifier would touch the prototype chain. Used at + * build time so a hostile spec fails the pod at boot rather than producing a + * tool whose behaviour depends on `Object.prototype`. + */ +export function assertSafeKey(key: string, context: string): void { + if (isForbiddenKey(key)) { + throw new Error( + `Refusing "${key}" as ${context}: it resolves to the object prototype rather than to data.` + ); + } +} diff --git a/packages/mcp-gateway/src/spec.ts b/packages/mcp-gateway/src/spec.ts index 50ef97a2..ab4e88cf 100644 --- a/packages/mcp-gateway/src/spec.ts +++ b/packages/mcp-gateway/src/spec.ts @@ -7,6 +7,7 @@ */ import { classify, type Classification, type Overrides } from './classify.js'; +import { assertSafeKey, getOwn, isForbiddenKey, safeRecord } from './safety.js'; const HTTP_METHODS = ['get', 'put', 'post', 'delete', 'patch', 'head', 'options'] as const; @@ -58,10 +59,27 @@ function resolveRef(doc: OpenApiDoc, node: unknown, seen = new Set()): u seen.add(ref); const parts = ref.slice(2).split('/').map(p => p.replace(/~1/g, '/').replace(/~0/g, '~')); + + // Validate EVERY segment before walking any of them. Checking lazily inside + // the loop would only reject a hostile segment when traversal happened to + // reach it, so `#/absent/__proto__` would pass simply because `absent` is + // missing — the pointer would be judged safe for the wrong reason. + for (const p of parts) { + // A segment of __proto__/constructor/prototype walks onto Object.prototype + // instead of into the document. A spec containing one is hostile or broken, + // and either way the tool built from it would describe the JavaScript + // runtime rather than the product's API. + if (isForbiddenKey(p)) { + throw new Error( + `Refusing $ref "${ref}": segment "${p}" resolves to the object prototype, not to the document.` + ); + } + } + let cur: unknown = doc; for (const p of parts) { if (!cur || typeof cur !== 'object') return {}; - cur = (cur as Record)[p]; + cur = getOwn(cur, p); } return resolveRef(doc, cur, seen); } @@ -79,7 +97,9 @@ export function toolNameFor(operationId: unknown, method: string, path: string): } function buildInputSchema(params: ToolParam[], bodySchema?: Record, bodyRequired = false) { - const properties: Record = {}; + // Null-prototype accumulator: keyed by spec-supplied parameter names, so a + // plain `{}` here would be a prototype-pollution sink. + const properties = safeRecord(); const required: string[] = []; for (const p of params) { @@ -121,6 +141,7 @@ export function buildTools(doc: OpenApiDoc, overrides: Overrides = {}): ToolDesc const op = opRaw as Record; const name = toolNameFor(op.operationId, method, path); + assertSafeKey(name, `a tool name (${method.toUpperCase()} ${path})`); if (seenNames.has(name)) { throw new Error( `Duplicate tool name "${name}" (${method.toUpperCase()} ${path}). ` + @@ -136,6 +157,9 @@ export function buildTools(doc: OpenApiDoc, overrides: Overrides = {}): ToolDesc const loc = p.in; if (loc !== 'path' && loc !== 'query' && loc !== 'header') continue; if (typeof p.name !== 'string') continue; + // The parameter name becomes a key in the tool's input schema and is + // used to index the caller's arguments, so it must not be a prototype key. + assertSafeKey(p.name, `a parameter name on ${method.toUpperCase()} ${path}`); params.push({ name: p.name, in: loc, diff --git a/packages/mcp-gateway/src/upstream.ts b/packages/mcp-gateway/src/upstream.ts index 267d5b67..890015a3 100644 --- a/packages/mcp-gateway/src/upstream.ts +++ b/packages/mcp-gateway/src/upstream.ts @@ -16,6 +16,7 @@ */ import type { ToolDescriptor } from './spec.js'; +import { getOwn, safeRecord } from './safety.js'; export class MissingIdentityError extends Error {} @@ -34,7 +35,7 @@ export interface CallerContext { } export function extractForwardHeaders(ctx: CallerContext): Record { - const out: Record = {}; + const out = safeRecord(); for (const [k, v] of Object.entries(ctx.headers ?? {})) { const key = k.toLowerCase(); if (!FORWARDED_HEADERS.includes(key)) continue; @@ -52,10 +53,16 @@ export function buildRequest( ): { url: string; headers: Record; body?: string } { let path = tool.path; const query = new URLSearchParams(); - const headers: Record = {}; + // Null-prototype: keyed by spec-supplied header parameter names. + const headers = safeRecord(); for (const p of tool.params) { - const raw = args[p.name]; + // OWN-property read only. `args[p.name]` would resolve `constructor` to + // Object.prototype.constructor and stringify a function into the URL; + // `getOwn` returns undefined for anything the caller did not actually send. + // buildTools already rejects prototype-keyed parameter names, so this is the + // second of two independent guards rather than the only one. + const raw = getOwn(args, p.name); if (raw === undefined || raw === null) { if (p.required) { throw new Error(`Missing required parameter "${p.name}" for tool "${tool.name}".`); @@ -82,8 +89,9 @@ export function buildRequest( const url = `${baseUrl.replace(/\/+$/, '')}${path}${qs ? `?${qs}` : ''}`; let body: string | undefined; - if (tool.bodySchema && args.body !== undefined) { - body = JSON.stringify(args.body); + const bodyArg = getOwn(args, 'body'); + if (tool.bodySchema && bodyArg !== undefined) { + body = JSON.stringify(bodyArg); headers['content-type'] = 'application/json'; } diff --git a/packages/mcp-gateway/test/safety.test.ts b/packages/mcp-gateway/test/safety.test.ts new file mode 100644 index 00000000..d73bfdd6 --- /dev/null +++ b/packages/mcp-gateway/test/safety.test.ts @@ -0,0 +1,211 @@ +/** + * Prototype-pollution / property-injection regression tests. + * + * These drive the REAL code paths with hostile keys rather than unit-testing the + * guards in isolation, because the guards are only worth anything if they sit on + * the path a malicious OpenAPI document or a model-generated argument actually + * takes. Each test also asserts Object.prototype is untouched afterwards — a fix + * that stops throwing but starts polluting would otherwise pass. + */ +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { buildTools, type OpenApiDoc } from '../src/spec.js'; +import { buildRequest, extractForwardHeaders, callUpstream } from '../src/upstream.js'; +import { getOwn, safeRecord, isForbiddenKey } from '../src/safety.js'; + +const BASE = 'http://upstream.local/v1'; + +// Anything a polluting payload might have written. +const CANARIES = ['polluted', 'isAdmin', 'x-injected'] as const; + +function expectCleanPrototype() { + for (const key of CANARIES) { + expect(({} as Record)[key], `Object.prototype.${key}`).toBeUndefined(); + } +} + +afterEach(() => { + expectCleanPrototype(); + for (const key of CANARIES) delete (Object.prototype as Record)[key]; +}); + +describe('safety helpers', () => { + it('identifies the prototype-resolving keys', () => { + expect(isForbiddenKey('__proto__')).toBe(true); + expect(isForbiddenKey('constructor')).toBe(true); + expect(isForbiddenKey('prototype')).toBe(true); + expect(isForbiddenKey('ticketId')).toBe(false); + }); + + it('getOwn does not read through the prototype chain', () => { + // A plain object "has" a constructor via its prototype; getOwn must not see it. + expect(getOwn({}, 'constructor')).toBeUndefined(); + expect(getOwn({}, 'toString')).toBeUndefined(); + expect(getOwn({ ticketId: 'T-1' }, 'ticketId')).toBe('T-1'); + }); + + it('safeRecord stores __proto__ as ordinary data instead of reparenting', () => { + const r = safeRecord(); + r['__proto__'] = { polluted: true }; + expect(Object.getPrototypeOf(r)).toBeNull(); + expectCleanPrototype(); + }); +}); + +describe('spec parsing rejects hostile documents', () => { + it('refuses a $ref that points through __proto__', () => { + const doc: OpenApiDoc = { + paths: { + '/a': { + get: { + operationId: 'a', + parameters: [{ $ref: '#/__proto__/polluted' }], + }, + }, + }, + }; + expect(() => buildTools(doc)).toThrow(/prototype/i); + }); + + it('refuses a $ref that points through constructor/prototype', () => { + const doc: OpenApiDoc = { + paths: { + '/a': { get: { operationId: 'a', parameters: [{ $ref: '#/components/constructor/x' }] } }, + }, + // Present so traversal genuinely reaches the `constructor` segment; without + // it the pointer would die on a missing key and the test would pass for + // the wrong reason. + components: { parameters: {} }, + }; + expect(() => buildTools(doc)).toThrow(/prototype/i); + }); + + it('refuses a hostile segment even when an earlier segment does not exist', () => { + // `absent` is missing, so a lazily-checked guard would never reach + // `__proto__` and would wrongly accept the pointer. + const doc: OpenApiDoc = { + paths: { '/a': { get: { operationId: 'a', parameters: [{ $ref: '#/absent/__proto__' }] } } }, + }; + expect(() => buildTools(doc)).toThrow(/prototype/i); + }); + + it('refuses a parameter literally named __proto__', () => { + const doc: OpenApiDoc = { + paths: { + '/a': { + get: { + operationId: 'a', + parameters: [{ name: '__proto__', in: 'query', schema: { polluted: true } }], + }, + }, + }, + }; + expect(() => buildTools(doc)).toThrow(/prototype/i); + }); + + it('refuses an operationId that resolves to a prototype key', () => { + const doc: OpenApiDoc = { paths: { '/a': { get: { operationId: 'constructor' } } } }; + expect(() => buildTools(doc)).toThrow(/prototype/i); + }); + + it('builds a null-prototype input schema so a schema key cannot pollute', () => { + const doc: OpenApiDoc = { + paths: { + '/a': { + get: { + operationId: 'a', + parameters: [{ name: 'ok', in: 'query', schema: { type: 'string' } }], + }, + }, + }, + }; + const [tool] = buildTools(doc); + expect(Object.getPrototypeOf(tool.inputSchema.properties)).toBeNull(); + }); +}); + +describe('argument handling resists property injection', () => { + const doc: OpenApiDoc = { + paths: { + '/tickets/{ticketId}': { + get: { + operationId: 'getTicket', + parameters: [ + { name: 'ticketId', in: 'path', required: true, schema: { type: 'string' } }, + { name: 'expand', in: 'query', schema: { type: 'string' } }, + ], + }, + }, + }, + }; + const tool = buildTools(doc)[0]; + + it('ignores inherited properties on the arguments object', () => { + // A model-supplied argument bag whose PROTOTYPE carries `expand`. Reading it + // with plain bracket notation would smuggle an attacker-chosen query value + // into the upstream URL; getOwn must not see it. + const hostile = Object.create({ expand: 'injected-via-prototype' }) as Record; + hostile.ticketId = 'T-1'; + + const { url } = buildRequest(tool, hostile, BASE); + expect(url).toBe(`${BASE}/tickets/T-1`); + expect(url).not.toContain('injected-via-prototype'); + }); + + it('does not stringify Object.prototype.constructor into the request', () => { + // If a spec ever slipped a `constructor` parameter past the build-time guard, + // the RUNTIME read must still yield nothing rather than a function source. + // Plain `args['constructor']` would return Object.prototype.constructor and + // splice `function Object() { [native code] }` into the query string. + const rogue = { + ...tool, + params: [ + ...tool.params, + { name: 'constructor', in: 'query' as const, required: false, schema: {} }, + ], + }; + const { url } = buildRequest(rogue, { ticketId: 'T-1' }, BASE); + expect(url).toBe(`${BASE}/tickets/T-1`); + expect(url).not.toMatch(/function|native code/i); + }); + + it('a __proto__ key in the request body is serialised, never applied', () => { + const bodyTool = buildTools({ + paths: { + '/t': { + post: { + operationId: 'create', + requestBody: { content: { 'application/json': { schema: { type: 'object' } } } }, + }, + }, + }, + })[0]; + + const { body } = buildRequest(bodyTool, { body: JSON.parse('{"__proto__":{"polluted":true}}') }, BASE); + expect(typeof body).toBe('string'); + expectCleanPrototype(); + }); + + it('forwards headers into a null-prototype object', () => { + const out = extractForwardHeaders({ headers: { authorization: 'Bearer t' } }); + expect(Object.getPrototypeOf(out)).toBeNull(); + expect(out.authorization).toBe('Bearer t'); + }); + + it('a hostile upstream JSON response cannot pollute the prototype', async () => { + // The gateway JSON.parses whatever the product API returns. JSON.parse does + // not apply __proto__, but this pins that we never hand that payload to a + // merge/assign that would. + const fetchImpl = vi.fn().mockResolvedValue( + new Response('{"__proto__":{"polluted":true},"ok":1}', { status: 200 }) + ); + const res = await callUpstream( + tool, + { ticketId: 'T-1' }, + BASE, + { headers: { authorization: 'Bearer t' } }, + fetchImpl as unknown as typeof fetch + ); + expect(res.ok).toBe(true); + expectCleanPrototype(); + }); +}); From bab43764e4fd70929c12e83081dceeab97e15a41 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 10:14:51 +0000 Subject: [PATCH 04/10] fix(ci): drop packages/mcp-gateway from root workspaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My earlier commit added `packages/mcp-gateway` to the root `workspaces` array without regenerating package-lock.json. `npm ci` requires the two to agree, so it failed outright, taking gate-frontend-build, Lint & Test, NPM Security Audit, Identity UI + Security, Client tests, Backend tests and Notify Team with it. Those failures were all mine. The obvious fix — regenerate the lockfile — is wrong here. Doing it in this container produced a 4,895-line diff (120 dependency versions removed, 233 added) because this box runs Node 22 while the repo mandates Node 24, so npm re-resolved the whole tree. Landing that inside a slug-migration PR would smuggle a large unreviewed dependency bump through a change nobody is reviewing for dependencies. Removing the line unbreaks `npm ci` with a one-line diff and touches no dependency. Nothing depends on @fuzefront/mcp-gateway yet and it has no CI job, so it loses nothing today — it still builds via its own package.json/tsconfig. Registering it as a workspace belongs in its own PR, generated on Node 24, where the lockfile diff is the point of review rather than a side effect. Verified: `npm ci --dry-run` passes against the unmodified lockfile. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GaPa3JgrVNtWrGvqQEAEqv --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 8c31e5a5..97ca5f1a 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,6 @@ "packages/i18n", "packages/i18n-translate", "packages/feature-flags", - "packages/mcp-gateway", "packages/security", "design-system", "services/chat-service", From 6c1d46a6940910b68d1a1322611478d93e2f007d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 10:17:57 +0000 Subject: [PATCH 05/10] fix(mcp-gateway): build tool schemas via Map, not dynamic-key assignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clears the last Semgrep prototype-pollution-loop alert on spec.ts. This is a real restructure, not a suppression — no nosemgrep, no scan-config narrowing. The flagged line was: properties[p.name] = ... // inside a for-loop, key from the spec Two guards already stood behind it, and both were invisible to a pattern scanner: 1. every parameter name passes assertSafeKey() when the spec is parsed, so __proto__/constructor/prototype throw long before this point; 2. `properties` was already Object.create(null), where assigning __proto__ stores an ordinary own property rather than reparenting the object. So the code was safe twice over and the alert was, strictly, a false positive. It is still worth removing rather than arguing with: the safety of that line depended on a guard sitting in a different function, which is exactly the kind of coupling that breaks silently when someone later refactors the parse path. Accumulating in a Map removes the dependency. A Map cannot reach Object.prototype at all, so the loop is immune by construction rather than by a precondition. The result is then materialised into a null-prototype object with defineProperty, which writes an own data property under any key without consulting the prototype chain. Output is unchanged: JSON.stringify treats a null-prototype object with enumerable own properties exactly like a plain one, so the emitted tool schema is byte-identical. Verified: tsc --noEmit clean; 45/45 vitest pass across spec, upstream, safety and classify. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GaPa3JgrVNtWrGvqQEAEqv --- packages/mcp-gateway/src/spec.ts | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/packages/mcp-gateway/src/spec.ts b/packages/mcp-gateway/src/spec.ts index ab4e88cf..c7572cb5 100644 --- a/packages/mcp-gateway/src/spec.ts +++ b/packages/mcp-gateway/src/spec.ts @@ -97,21 +97,42 @@ export function toolNameFor(operationId: unknown, method: string, path: string): } function buildInputSchema(params: ToolParam[], bodySchema?: Record, bodyRequired = false) { - // Null-prototype accumulator: keyed by spec-supplied parameter names, so a - // plain `{}` here would be a prototype-pollution sink. - const properties = safeRecord(); + // Accumulate in a Map rather than assigning into an object under a + // spec-supplied key. A Map cannot reach Object.prototype at all, so this is + // immune by construction rather than by a guard someone could later delete — + // and it carries no `obj[dynamicKey] = …` shape for a scanner to flag. + // + // Two guards already stood behind this line: parameter names pass + // assertSafeKey() at parse time, and the accumulator was already + // null-prototype. Both still hold for the materialised result below; this + // removes the last write-by-dynamic-key rather than suppressing the warning. + const collected = new Map(); const required: string[] = []; for (const p of params) { - properties[p.name] = p.description ? { ...p.schema, description: p.description } : p.schema; + collected.set(p.name, p.description ? { ...p.schema, description: p.description } : p.schema); if (p.required) required.push(p.name); } if (bodySchema) { - properties.body = { ...bodySchema, description: 'Request body.' }; + collected.set('body', { ...bodySchema, description: 'Request body.' }); if (bodyRequired) required.push('body'); } + // Materialise into a null-prototype object. defineProperty writes an own data + // property under any key without consulting the prototype chain, and + // JSON.stringify treats the result exactly like a plain object, so the + // emitted tool schema is byte-identical to before. + const properties = safeRecord(); + for (const [key, value] of collected) { + Object.defineProperty(properties, key, { + value, + enumerable: true, + writable: true, + configurable: true, + }); + } + return { type: 'object', properties, From 2c97d87c77227f1b2dfe4ff676d430b1837a814e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 10:58:30 +0000 Subject: [PATCH 06/10] fix(mcp-gateway): smoke check must allow query-shaped POST reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scripts/smoke.mjs judged "is this tool bound to a read?" on the HTTP verb alone: const safe = ['GET', 'HEAD', 'OPTIONS', 'TRACE']; That is strictly stricter than the gateway it is smoke-testing. src/classify.ts defines READ_ONLY_POST_SUFFIXES = ['/search', '/query', '/preview'] and treats a POST to such a path as a read — the body is a query too large or too structured for a query string. So for any product whose contract contains `POST /tickets/search`, the gateway would classify it correctly as a read and this smoke check would then report that same tool as a liar and fail. A verification step that rejects behaviour the implementation is specified to have is worse than no check: it fails on correct input, and the obvious way to "fix" it is to mislabel the tool. It never fired because FuzeService's contract — the only spec smoke has run against — contains no query-shaped POST. Found by an agent converging a different product, not by the suite. Now mirrors classify.ts exactly, including the suffix-not-substring rule: `/tickets/search` qualifies, `/search-index/rebuild` does not. Uses the `fuze/path` metadata the server already emits alongside `fuze/method`. The stronger invariant is untouched: an irreversible tool may never be advertised as a read, and classify.ts still refuses a read override on anything that is not a safe method or a query-shaped POST. Verified: node --check clean; 45/45 vitest pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GaPa3JgrVNtWrGvqQEAEqv --- packages/mcp-gateway/scripts/smoke.mjs | 32 ++++++++++++++++++++------ 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/packages/mcp-gateway/scripts/smoke.mjs b/packages/mcp-gateway/scripts/smoke.mjs index b6f2508f..f9684381 100644 --- a/packages/mcp-gateway/scripts/smoke.mjs +++ b/packages/mcp-gateway/scripts/smoke.mjs @@ -42,13 +42,31 @@ const { tools } = await client.listTools(); check('MCP handshake over SSE', true); check('tools enumerate', tools.length > 0, `${tools.length} tools`); -// Every tool advertising readOnlyHint must be bound to a safe HTTP method. -// This is the invariant that keeps an irreversible write from being reachable -// as a side effect of something that looks like a read. -const safe = ['GET', 'HEAD', 'OPTIONS', 'TRACE']; -const liars = tools.filter( - t => t.annotations?.readOnlyHint && !safe.includes(t._meta?.['fuze/method']) -); +// Every tool advertising readOnlyHint must be bound to a safe HTTP method — OR +// to a query-shaped POST, where the body is a query too large or too structured +// for a query string. +// +// This MUST mirror READ_ONLY_POST_SUFFIXES in src/classify.ts. It previously +// judged on the verb alone, which is strictly stricter than the gateway itself +// and therefore wrong in a way that only shows up later: the gateway would +// correctly classify `POST /tickets/search` as a read, and then this smoke check +// would report that same tool as a liar and fail a spec the gateway handles +// fine. It never fired because FuzeService's contract has no query-shaped POST. +// +// A suffix match, not a substring one: `/tickets/search` qualifies, +// `/search-index/rebuild` does not. +const SAFE_METHODS = ['GET', 'HEAD', 'OPTIONS', 'TRACE']; +const READ_ONLY_POST_SUFFIXES = ['/search', '/query', '/preview']; + +const boundToRead = t => { + const method = t._meta?.['fuze/method']; + if (SAFE_METHODS.includes(method)) return true; + if (method !== 'POST') return false; + const path = t._meta?.['fuze/path'] ?? ''; + return READ_ONLY_POST_SUFFIXES.some(s => path.endsWith(s)); +}; + +const liars = tools.filter(t => t.annotations?.readOnlyHint && !boundToRead(t)); check('no read-only tool is bound to an unsafe method', liars.length === 0, liars.map(t => t.name).join(', ')); // Irreversible tools must never be advertised as reads. From 31bee237307358a0c2295e6a1f53e9558e127bdf Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 11:00:15 +0000 Subject: [PATCH 07/10] fix(mcp-gateway): remove the last dynamic-key writes in upstream.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clears the prototype-pollution-loop alert that survived the spec.ts fix. I restructured buildInputSchema and assumed that was the only site; it was not. Two loops in upstream.ts still wrote `obj[key] = value` with a runtime key: extractForwardHeaders out[key] = value key from caller headers buildRequest headers[p.name] = … key from the spec Both are, today, genuinely safe — and neither guard is visible at the write: - `out[key]` is reached only after `FORWARDED_HEADERS.includes(key)`, an allowlist that no prototype key is on; - `headers[p.name]` uses a name that already passed assertSafeKey() when the spec was parsed, in a different module. That is the same fragility the spec.ts commit removed: correctness resting on a precondition enforced somewhere else. Widen the allowlist, or refactor the parse path, and the write silently becomes reachable. Both now use Object.defineProperty, which writes an own data property without consulting the prototype chain. Behaviour is unchanged — the accumulators were already null-prototype, and enumerable own properties serialise identically. Verified: tsc --noEmit clean; 45/45 vitest pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GaPa3JgrVNtWrGvqQEAEqv --- packages/mcp-gateway/src/upstream.ts | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/packages/mcp-gateway/src/upstream.ts b/packages/mcp-gateway/src/upstream.ts index 890015a3..8f79c6fe 100644 --- a/packages/mcp-gateway/src/upstream.ts +++ b/packages/mcp-gateway/src/upstream.ts @@ -40,7 +40,19 @@ export function extractForwardHeaders(ctx: CallerContext): Record 0) out[key] = value; + // defineProperty rather than `out[key] = value`: writes an own data property + // without consulting the prototype chain. `key` is already constrained by the + // FORWARDED_HEADERS allowlist above, so this cannot currently be reached with + // a prototype key — but the allowlist is one edit away from being widened, + // and this way the safety does not depend on remembering that. + if (typeof value === 'string' && value.length > 0) { + Object.defineProperty(out, key, { + value, + enumerable: true, + writable: true, + configurable: true, + }); + } } return out; } @@ -76,7 +88,17 @@ export function buildRequest( if (Array.isArray(raw)) raw.forEach(v => query.append(p.name, String(v))); else query.append(p.name, value); } else { - headers[p.name] = value; + // Same reasoning as extractForwardHeaders: an own data property written + // without touching the prototype chain. `p.name` has already passed + // assertSafeKey() when the spec was parsed, so this is belt-and-braces — + // but that guard lives in another module, and a header name is + // spec-supplied input reaching an object key. + Object.defineProperty(headers, p.name, { + value, + enumerable: true, + writable: true, + configurable: true, + }); } } From 44fa960eb6c8db894d89956125d0ebf8ad301797 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 11:02:22 +0000 Subject: [PATCH 08/10] ci(mcp-gateway): build and publish the gateway image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing published ghcr.io/izzywdev/fuze-mcp-gateway, and that is currently the single blocker to MCP working anywhere in the family. Every product deploys its OWN gateway pod running this one image, configured with that product's OpenAPI document. FuzeService, FuzePlan, FuzeAgent, FuzeContact, FuzeKeys, FuzeMarket, FuzeSocial and FuzePicker have all merged or opened the Helm templates for that pod — and every one of them is pinned OFF, because flipping mcp.enabled without a published image is an ImagePullBackOff. So the wiring is done fleet-wide and none of it can run. Publishes :sha and :latest on a master push touching packages/mcp-gateway/**, with an optional extra tag via workflow_dispatch for cutting 0.1.0 (the tag the product charts currently reference). Three deliberate choices: - A PR BUILDS but does NOT push. An unreviewed image tag that products could pull is worse than no image: the charts reference a floating tag, so a bad push would propagate without anyone merging anything. - Typecheck and the unit suite run BEFORE the build, not after. This gateway decides whether a tool is advertised as a safe read or an irreversible write. Getting that wrong is not a broken build — it is an agent taking an unrecoverable action believing it is reversible. That must not reach a registry, and a test that runs after the push does not prevent it. - Actions are pinned to the SHAs already used elsewhere in this repo, matching the convention introduced on onboarding-kit-tests.yml. Product charts should pin :sha rather than :latest — a floating tag on the component that classifies destructiveness is not something to roll forward silently. Not verified: the image has not been built here (no Docker in this environment). The Dockerfile is pre-existing and its EXPOSE/CMD are consistent with the chart templates that reference it; the first master run is the real test. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GaPa3JgrVNtWrGvqQEAEqv --- .github/workflows/mcp-gateway-image.yml | 96 +++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 .github/workflows/mcp-gateway-image.yml diff --git a/.github/workflows/mcp-gateway-image.yml b/.github/workflows/mcp-gateway-image.yml new file mode 100644 index 00000000..94040265 --- /dev/null +++ b/.github/workflows/mcp-gateway-image.yml @@ -0,0 +1,96 @@ +name: MCP gateway image + +# Builds and publishes ghcr.io/izzywdev/fuze-mcp-gateway. +# +# WHY THIS EXISTS: every product in the family deploys its OWN MCP pod running +# this one image, configured with that product's OpenAPI document. Several +# products have already merged the Helm templates for that pod — and every one of +# them is pinned OFF, because nothing published the image. Flipping `mcp.enabled` +# without it is an ImagePullBackOff. +# +# So this is the single gate between "MCP pods are wired" and "MCP pods can run", +# for the whole fleet rather than for this repo. + +on: + push: + branches: [master] + paths: + - 'packages/mcp-gateway/**' + - '.github/workflows/mcp-gateway-image.yml' + # A PR builds but does NOT push: an unreviewed image tag that products could + # pull is worse than no image at all. + pull_request: + paths: + - 'packages/mcp-gateway/**' + - '.github/workflows/mcp-gateway-image.yml' + workflow_dispatch: + inputs: + tag: + description: 'Extra tag to publish (e.g. 0.1.0). :sha and :latest are always published on master.' + required: false + type: string + +jobs: + image: + name: build & publish + runs-on: ubuntu-latest + permissions: + contents: read + packages: write + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 + + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version: '24.x' + + # Test before building. A gateway that mis-classifies a tool as read-only + # when it is irreversible is not a broken build — it is an agent taking an + # unrecoverable action believing it is safe. That must not reach a registry. + - name: Install + working-directory: packages/mcp-gateway + run: npm install --no-audit --no-fund + + - name: Typecheck + working-directory: packages/mcp-gateway + run: npx tsc --noEmit -p tsconfig.json + + - name: Unit tests + working-directory: packages/mcp-gateway + run: npx vitest run + + - uses: docker/setup-buildx-action@v4 + + - name: Log in to GHCR + if: github.event_name != 'pull_request' + uses: docker/login-action@v3 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Compute image tag + id: tag + run: echo "sha=${GITHUB_SHA::12}" >> "$GITHUB_OUTPUT" + + # `latest` is published for convenience, but product charts should pin the + # :sha tag — a floating tag on a component that decides whether a tool is + # destructive is not something to roll forward silently. + - name: Build & push + uses: docker/build-push-action@v7 + with: + context: packages/mcp-gateway + file: packages/mcp-gateway/Dockerfile + push: ${{ github.event_name != 'pull_request' }} + tags: | + ghcr.io/izzywdev/fuze-mcp-gateway:${{ steps.tag.outputs.sha }} + ghcr.io/izzywdev/fuze-mcp-gateway:latest + ${{ inputs.tag && format('ghcr.io/izzywdev/fuze-mcp-gateway:{0}', inputs.tag) || '' }} + cache-from: type=gha,scope=mcp-gateway + cache-to: type=gha,mode=max,scope=mcp-gateway + + - name: Report + if: github.event_name != 'pull_request' + run: | + echo "::notice::Published ghcr.io/izzywdev/fuze-mcp-gateway:${{ steps.tag.outputs.sha }}" + echo "Products can now set mcp.enabled=true with image.tag=${{ steps.tag.outputs.sha }}" >> "$GITHUB_STEP_SUMMARY" From be2d40b95055406ac01a0efd057fa6db096074a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 11:03:54 +0000 Subject: [PATCH 09/10] fix(ci): build the gateway image from the repo root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My own bug, one commit old. I set `context: packages/mcp-gateway`, but the Dockerfile's COPY paths are repo-root-relative: COPY packages/mcp-gateway/package.json packages/mcp-gateway/package-lock.json* ./ With the narrower context there is no `packages/` directory inside it, so every COPY misses and the build fails. release.yml already establishes the right convention — `context: .` with `file: backend/Dockerfile` — and this now matches it. Worth noting the failure mode rather than just the fix: the build step ran far enough to upload a buildx artifact before failing, so the job looked like it had built something. The error was ~700 lines above the log tail, which is why a tail-only read of a red job is misleading here. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GaPa3JgrVNtWrGvqQEAEqv --- .github/workflows/mcp-gateway-image.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/mcp-gateway-image.yml b/.github/workflows/mcp-gateway-image.yml index 94040265..60f226f6 100644 --- a/.github/workflows/mcp-gateway-image.yml +++ b/.github/workflows/mcp-gateway-image.yml @@ -79,7 +79,11 @@ jobs: - name: Build & push uses: docker/build-push-action@v7 with: - context: packages/mcp-gateway + # Repo ROOT, not packages/mcp-gateway. The Dockerfile's COPY paths are + # root-relative (`COPY packages/mcp-gateway/package.json …`), so a + # narrower context makes every COPY miss. Same convention as + # release.yml, which builds `backend/Dockerfile` from `.`. + context: . file: packages/mcp-gateway/Dockerfile push: ${{ github.event_name != 'pull_request' }} tags: | From f7e72f1942f8c7cad810bceb85eff6ac42e297d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 11:04:44 +0000 Subject: [PATCH 10/10] ci(mcp-gateway): pin the docker actions to SHAs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Semgrep flagged three mutable action tags in the workflow I just added, and it is right — more so here than in a typical workflow. This one PUSHES an image that every product in the family pulls, so an action owner silently repointing a tag is a direct path to publishing a malicious image fleet-wide. That is the exact scenario the rule cites (trivy-action, kics-github-action). Pinned to SHAs already established in this repo, rather than invented: docker/setup-buildx-action bb05f3f5519dd87d3ba754cc423b652a5edd6d2c docker/build-push-action 53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7.3.0 The build-push SHA is annotated v7.3.0 in security.yml, so swapping it for the @v7 I wrote is a pin, not a version change — worth checking, since the repo also carries @v5 usages and pinning to one of those would have silently downgraded the action under a step. docker/login-action is deliberately LEFT on @v3. No verified SHA for it exists anywhere in this repo, and this session's GitHub scope cannot read the upstream action's tags to resolve one. A plausible-looking 40-hex string would either break the workflow or pin to something nobody actually checked — which is worse than the mutable tag, because it looks verified. The reasoning is recorded in a comment at the call site so the next person can finish the job rather than rediscover the gap. Two of three findings genuinely closed; the third is reported, not suppressed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GaPa3JgrVNtWrGvqQEAEqv --- .github/workflows/mcp-gateway-image.yml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/mcp-gateway-image.yml b/.github/workflows/mcp-gateway-image.yml index 60f226f6..28f42465 100644 --- a/.github/workflows/mcp-gateway-image.yml +++ b/.github/workflows/mcp-gateway-image.yml @@ -59,8 +59,15 @@ jobs: working-directory: packages/mcp-gateway run: npx vitest run - - uses: docker/setup-buildx-action@v4 + - uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c # v3 + # NOT pinned to a SHA, deliberately. Semgrep flags this and it is right in + # principle — but no verified SHA for docker/login-action exists anywhere in + # this repo, and this repo's GitHub scope cannot read the upstream action's + # tags to resolve one. Inventing a plausible-looking 40-hex string would + # either break the workflow or, worse, pin to something nobody checked. + # Pinning the two actions whose SHAs ARE established here is a real + # reduction; leaving this one honest and visible is better than a guess. - name: Log in to GHCR if: github.event_name != 'pull_request' uses: docker/login-action@v3 @@ -77,7 +84,7 @@ jobs: # :sha tag — a floating tag on a component that decides whether a tool is # destructive is not something to roll forward silently. - name: Build & push - uses: docker/build-push-action@v7 + uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7.3.0 with: # Repo ROOT, not packages/mcp-gateway. The Dockerfile's COPY paths are # root-relative (`COPY packages/mcp-gateway/package.json …`), so a