From 2d4e37fc75d9a83b2b68c36adb348d623ce35158 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 30 Oct 2026 23:43:50 -0700 Subject: [PATCH 01/11] feat(connectors): operator-key mode and per-agent binding for hosted MCP (TASK-330) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice 1 of TASK-330, per Vera's read on main 57057cad9 (§2, §3): - HostedMcpEntry gains credentialMode ('oauth' default | 'operator_key'). - credentialFor branches on the entry mode before any OAuth-shaped check: an operator key is served as stored, with no fence, lease or refresh, and the freshness rule does not decide it. - Grant mint on an operator-key connection must name exactly the agent the row is bound to. An omitted audience is refused, not defaulted to every pod member. The check runs before the member default. Not in this slice: the paste-key intake route, the discriminated entry union and its reader census, and the latchkey.ts pins (the last commit, once the walk key exists). Co-Authored-By: Claude Haiku 5.5 --- .../hostedMcpCredential.operatorKey.test.js | 64 +++++++++++++++++++ .../services/hostedMcpOperatorKey.test.js | 38 +++++++++++ backend/routes/grants.ts | 12 ++++ .../services/hostedMcpCredentialService.ts | 11 ++++ backend/services/hostedMcpEntryService.ts | 7 ++ backend/services/hostedMcpOperatorKey.ts | 29 +++++++++ 6 files changed, 161 insertions(+) create mode 100644 backend/__tests__/unit/services/hostedMcpCredential.operatorKey.test.js create mode 100644 backend/__tests__/unit/services/hostedMcpOperatorKey.test.js create mode 100644 backend/services/hostedMcpOperatorKey.ts diff --git a/backend/__tests__/unit/services/hostedMcpCredential.operatorKey.test.js b/backend/__tests__/unit/services/hostedMcpCredential.operatorKey.test.js new file mode 100644 index 000000000..4b93e9dc1 --- /dev/null +++ b/backend/__tests__/unit/services/hostedMcpCredential.operatorKey.test.js @@ -0,0 +1,64 @@ +/** + * `credentialFor` on an operator-key entry (TASK-330, Vera's read §3): the + * stored key is served as it stands, with no fence, no lease and no vendor + * refresh, and the freshness rule does not decide it. + */ +const { credentialFor, HostedMcpCredentialError } = require('../../../services/hostedMcpCredentialService'); + +const KEY_ENTRY = { id: 'latchkey', credentialMode: 'operator_key' }; +const ROW = { + _id: '68e1f2a4b5c6d7e8f9a0b1c2', + status: 'connected', + revokedAt: null, + config: { entryId: 'latchkey', agentId: 'a1', credentialRef: 'ref-key-1' }, +}; + +const deps = (overrides = {}) => { + const bumpGeneration = jest.fn(); + const secretsGet = jest.fn(async (ref) => `token-for-${ref}`); + return { + bumpGeneration, + secretsGet, + overrides: { + entryFor: () => KEY_ENTRY, + secrets: { get: secretsGet, put: jest.fn(), revoke: jest.fn() }, + row: { bumpGeneration, markClientMismatch: jest.fn() }, + now: () => new Date('2026-10-31T06:00:00.000Z'), + ...overrides, + }, + }; +}; + +describe('credentialFor on an operator-key entry', () => { + it('serves the stored key without touching the fence or the vendor', async () => { + const d = deps(); + const out = await credentialFor(ROW, d.overrides); + expect(out).toEqual({ token: 'token-for-ref-key-1', expiresAt: '' }); + expect(d.secretsGet).toHaveBeenCalledWith('ref-key-1'); + expect(d.bumpGeneration).not.toHaveBeenCalled(); + }); + + it('serves the key whatever its lifetime says, because lifetime is not its rule', async () => { + const d = deps(); + const expired = { ...ROW, config: { ...ROW.config, expiresAt: new Date('2000-01-01T00:00:00.000Z') } }; + await expect(credentialFor(expired, d.overrides)).resolves.toMatchObject({ token: 'token-for-ref-key-1' }); + }); + + it('refuses with reconnect_required when no key is stored', async () => { + const d = deps(); + const noKey = { ...ROW, config: { entryId: 'latchkey', agentId: 'a1' } }; + await expect(credentialFor(noKey, d.overrides)).rejects.toMatchObject({ + code: 'reconnect_required', + }); + await expect(credentialFor(noKey, d.overrides)).rejects.toBeInstanceOf(HostedMcpCredentialError); + expect(d.secretsGet).not.toHaveBeenCalled(); + }); + + it('refuses a removed connection before it reads the key', async () => { + const d = deps(); + await expect(credentialFor({ ...ROW, revokedAt: new Date() }, d.overrides)).rejects.toMatchObject({ + code: 'connection_mismatch', + }); + expect(d.secretsGet).not.toHaveBeenCalled(); + }); +}); diff --git a/backend/__tests__/unit/services/hostedMcpOperatorKey.test.js b/backend/__tests__/unit/services/hostedMcpOperatorKey.test.js new file mode 100644 index 000000000..f8a960cb6 --- /dev/null +++ b/backend/__tests__/unit/services/hostedMcpOperatorKey.test.js @@ -0,0 +1,38 @@ +/** + * The operator-key mint binding (TASK-330, Vera's read §2): a key is one + * agent's, so a grant on its row may name that agent alone. Pure function, so + * these arms need no database. + */ +const { operatorKeyAudienceRefusal } = require('../../../services/hostedMcpOperatorKey'); + +const KEY_ENTRY = { credentialMode: 'operator_key' }; +const OAUTH_ENTRY = { credentialMode: undefined }; +const bound = (agentId) => ({ config: { agentId } }); + +describe('operatorKeyAudienceRefusal', () => { + it('leaves OAuth rows alone, so the member default still applies to them', () => { + expect(operatorKeyAudienceRefusal(OAUTH_ENTRY, bound('a1'), undefined)).toBeNull(); + expect(operatorKeyAudienceRefusal(undefined, bound('a1'), undefined)).toBeNull(); + }); + + it('names exactly the bound agent', () => { + expect(operatorKeyAudienceRefusal(KEY_ENTRY, bound('a1'), ['a1'])).toBeNull(); + }); + + it('refuses an omitted audience, which would otherwise default to every member', () => { + expect(operatorKeyAudienceRefusal(KEY_ENTRY, bound('a1'), undefined)).toMatch(/omitted audience/); + }); + + it('refuses another agent, and refuses the bound agent alongside another', () => { + expect(operatorKeyAudienceRefusal(KEY_ENTRY, bound('a1'), ['a2'])).toBe('audience must be exactly [a1]'); + expect(operatorKeyAudienceRefusal(KEY_ENTRY, bound('a1'), ['a1', 'a2'])).toBe('audience must be exactly [a1]'); + }); + + it('refuses an empty audience, which names nobody', () => { + expect(operatorKeyAudienceRefusal(KEY_ENTRY, bound('a1'), [])).toBe('audience must be exactly [a1]'); + }); + + it('refuses a row with no bound agent, since the key has no owner to name', () => { + expect(operatorKeyAudienceRefusal(KEY_ENTRY, { config: {} }, ['a1'])).toBe('this key is not bound to an agent'); + }); +}); diff --git a/backend/routes/grants.ts b/backend/routes/grants.ts index 414cc7128..e33befd68 100644 --- a/backend/routes/grants.ts +++ b/backend/routes/grants.ts @@ -6,6 +6,8 @@ import { createHash } from 'crypto'; import { Types } from 'mongoose'; import Integration from '../models/Integration'; import { isHostedMcpEntryHidden } from '../integrations/hostedMcp/visibility'; +import { HOSTED_MCP_ENTRIES, findHostedMcpEntry } from '../integrations/hostedMcp/entries'; +import { operatorKeyAudienceRefusal } from '../services/hostedMcpOperatorKey'; import Pod from '../models/Pod'; import RoomGrant from '../models/RoomGrant'; import ToolCall from '../models/ToolCall'; @@ -357,6 +359,16 @@ router.post('/', grantRateLimit, auth, async (req: AuthenticatedRequest, res: ex return res.status(400).json({ error: 'invalid_target', message: 'target.kind must be pod' }); } const members = await ensureTargetAccess(target, userId); + // An operator-key connection names exactly its bound agent. This runs before + // the member default below, which is what would otherwise hand the key to + // every member (TASK-330). + if (hosted) { + const boundEntry = findHostedMcpEntry(HOSTED_MCP_ENTRIES, String(connection.config?.entryId || '')); + const boundRefusal = operatorKeyAudienceRefusal(boundEntry, connection, body.audience); + if (boundRefusal) { + return res.status(400).json({ error: 'invalid_audience', message: boundRefusal }); + } + } // Pod grants default to the current member snapshot. const audience = body.audience === undefined ? members : body.audience; const audienceValues = Array.isArray(audience) ? audience.map(String) : audience; diff --git a/backend/services/hostedMcpCredentialService.ts b/backend/services/hostedMcpCredentialService.ts index 25fc2acef..fffd96a7c 100644 --- a/backend/services/hostedMcpCredentialService.ts +++ b/backend/services/hostedMcpCredentialService.ts @@ -394,6 +394,17 @@ export const credentialFor = async ( const id = rowId(row); const entry = deps.entryFor(row); + if (entry.credentialMode === 'operator_key') { + // A pasted key does not expire by a lifetime and cannot be refreshed, so + // the mode decides here, never a missing `expiresAt` (which would make an + // OAuth row with a lost lifetime serve a dead token). No fence, no lease. + assertUsable(row); + const keyRef = row.config?.credentialRef; + if (!keyRef) { + throw new HostedMcpCredentialError('reconnect_required', 'no key is stored for this connection'); + } + return { token: await deps.secrets.get(keyRef), expiresAt: '' }; + } const recordedClientId = row.config?.clientId; let clientId: string; let clientSecret: string | undefined; diff --git a/backend/services/hostedMcpEntryService.ts b/backend/services/hostedMcpEntryService.ts index abd1c06c1..0b3d2cf3b 100644 --- a/backend/services/hostedMcpEntryService.ts +++ b/backend/services/hostedMcpEntryService.ts @@ -64,6 +64,13 @@ export interface HostedMcpPinnedTool { export interface HostedMcpEntry { /** Namespaces every tool name (`.`) and names the row's app. */ id: string; + /** + * How the row's credential is obtained. Absent means OAuth, which every + * shipped entry uses. `operator_key` is a static bearer the operator pastes + * for one agent (Latchkey, TASK-330): it has no refresh, so `credentialFor` + * serves it without the freshness rule or the fence. + */ + credentialMode?: 'oauth' | 'operator_key'; title: string; /** * The line the Tools page draws under the title. Ours, like a tool's diff --git a/backend/services/hostedMcpOperatorKey.ts b/backend/services/hostedMcpOperatorKey.ts new file mode 100644 index 000000000..5c58b7cfc --- /dev/null +++ b/backend/services/hostedMcpOperatorKey.ts @@ -0,0 +1,29 @@ +import type { HostedMcpEntry } from './hostedMcpEntryService'; + +/** + * Why a grant on an operator-key connection may not name this audience, or + * null when it may (TASK-330, Vera's read §2). + * + * The key is one agent's bearer. The mint's default audience is every current + * pod member (grants.ts), so an omitted audience here would hand one key to + * all of them. Only the agent the row is bound to may be named, and only alone. + * This runs before the default is applied, so an omitted audience is refused + * rather than defaulted. + */ +export const operatorKeyAudienceRefusal = ( + entry: Pick | undefined, + connection: { config?: { agentId?: unknown } } | undefined, + audience: unknown, +): string | null => { + if (entry?.credentialMode !== 'operator_key') return null; + const agentId = String(connection?.config?.agentId || '').trim(); + if (!agentId) return 'this key is not bound to an agent'; + if (audience === undefined) { + return `name the bound agent in audience: an omitted audience reaches every pod member, ` + + `and this key is ${agentId}'s alone`; + } + if (!Array.isArray(audience) || audience.length !== 1 || String(audience[0]) !== agentId) { + return `audience must be exactly [${agentId}]`; + } + return null; +}; From 1d3560f55c8bf008f4ca03a2666290c21e24061b Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 30 Oct 2026 23:47:31 -0700 Subject: [PATCH 02/11] feat(connectors): operator-key intake probe and scope refusal (TASK-330) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice 2 of TASK-330 (Vera §4-§5): - listHostedMcpTools is extracted from the broker's drift listing, so intake proves a pasted key through the same MCP client the calls use. - validateOperatorKey refuses secrets:write, jobs:approve and migration:apply before any vendor call, probes tools/list with the key, and returns fixed copy only. The key never appears in a result. - scopeCheck reports 'self_report' when the vendor cannot report a key's scopes, so a refused scope is never presented as a server guarantee. Not in this slice: the paste-key route (owner auth, rate limit, row write, no body logging) and the discriminated entry union. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- .../hostedMcpOperatorKeyIntake.test.js | 85 +++++++++++++++++++ .../services/hostedMcpOperatorKeyIntake.ts | 74 ++++++++++++++++ backend/services/hostedMcpToolDefinitions.ts | 16 +++- 3 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 backend/__tests__/unit/services/hostedMcpOperatorKeyIntake.test.js create mode 100644 backend/services/hostedMcpOperatorKeyIntake.ts diff --git a/backend/__tests__/unit/services/hostedMcpOperatorKeyIntake.test.js b/backend/__tests__/unit/services/hostedMcpOperatorKeyIntake.test.js new file mode 100644 index 000000000..04c4b5df1 --- /dev/null +++ b/backend/__tests__/unit/services/hostedMcpOperatorKeyIntake.test.js @@ -0,0 +1,85 @@ +/** + * Operator-key intake (TASK-330, Vera §4-§5): a pasted key is proved against + * the vendor before its row can go connected, a forbidden scope is refused, + * and the key never comes back in any result. The vendor client is injected, + * so these arms need no network. + */ +const { RoomGrantError } = require('../../../services/roomGrantService'); +const { + validateOperatorKey, + refusedOperatorKeyScopes, + OPERATOR_KEY_REFUSED_SCOPES, +} = require('../../../services/hostedMcpOperatorKeyIntake'); + +const KEY_ENTRY = { id: 'latchkey', credentialMode: 'operator_key', resource: 'https://latchkey.dev/mcp' }; +const SECRET = 'lk_live_THE_SECRET_VALUE'; +const TOOLS = [{ name: 'list_jobs' }]; +const fetcher = jest.fn(); + +const run = (overrides = {}) => validateOperatorKey({ + entry: KEY_ENTRY, + key: SECRET, + reportedScopes: ['read', 'jobs:run', 'dispatch'], + fetcher, + listTools: jest.fn(async () => TOOLS), + ...overrides, +}); + +describe('refusedOperatorKeyScopes', () => { + it('names exactly the three scopes a key may never carry', () => { + expect([...OPERATOR_KEY_REFUSED_SCOPES].sort()).toEqual(['jobs:approve', 'migration:apply', 'secrets:write']); + }); + + it('returns only the forbidden ones, and nothing for the walk scopes', () => { + expect(refusedOperatorKeyScopes(['read', 'jobs:run', 'dispatch'])).toEqual([]); + expect(refusedOperatorKeyScopes(['read', 'secrets:write', 'jobs:approve'])).toEqual(['secrets:write', 'jobs:approve']); + }); +}); + +describe('validateOperatorKey', () => { + it('accepts a key whose probe lists tools, and reports the read check', async () => { + const out = await run(); + expect(out).toEqual({ ok: true, tools: TOOLS, scopeCheck: 'read' }); + }); + + it('marks the check as a self-report when the vendor cannot report scopes', async () => { + const out = await run({ reportedScopes: null }); + expect(out).toMatchObject({ ok: true, scopeCheck: 'self_report' }); + }); + + it('refuses a forbidden scope before it probes the vendor', async () => { + const listTools = jest.fn(); + const out = await run({ reportedScopes: ['read', 'secrets:write'], listTools }); + expect(out).toMatchObject({ ok: false, code: 'scope_refused' }); + expect(out.message).toContain('secrets:write'); + expect(listTools).not.toHaveBeenCalled(); + }); + + it('refuses a missing key and a non-key entry', async () => { + await expect(run({ key: ' ' })).resolves.toMatchObject({ ok: false, code: 'key_missing' }); + await expect(run({ entry: { id: 'linear', resource: 'https://x' } })) + .resolves.toMatchObject({ ok: false, code: 'not_operator_key' }); + }); + + it('maps a vendor rejection to fixed copy', async () => { + const listTools = jest.fn(async () => { + throw new RoomGrantError('credential_rejected', 'the vendor rejected this connection’s credential', 403); + }); + const out = await run({ listTools }); + expect(out).toEqual({ ok: false, code: 'vendor_rejected', message: 'the vendor rejected this key' }); + }); + + it('never returns the key, even when the vendor error repeats it', async () => { + const listTools = jest.fn(async () => { + throw new RoomGrantError('provider_error', `vendor said ${SECRET} is wrong`, 502); + }); + const out = await run({ listTools }); + expect(out).toMatchObject({ ok: false, code: 'vendor_unreachable' }); + expect(JSON.stringify(out)).not.toContain(SECRET); + }); + + it('never returns the key on success or refusal', async () => { + expect(JSON.stringify(await run())).not.toContain(SECRET); + expect(JSON.stringify(await run({ reportedScopes: ['jobs:approve'] }))).not.toContain(SECRET); + }); +}); diff --git a/backend/services/hostedMcpOperatorKeyIntake.ts b/backend/services/hostedMcpOperatorKeyIntake.ts new file mode 100644 index 000000000..c9938273b --- /dev/null +++ b/backend/services/hostedMcpOperatorKeyIntake.ts @@ -0,0 +1,74 @@ +import type { HostedMcpEntry, HostedMcpUpstreamTool } from './hostedMcpEntryService'; +import { listHostedMcpTools } from './hostedMcpToolDefinitions'; + +/** + * Scopes an operator key may never carry (TASK-330 Decision 2). A key with any + * of them could approve its own jobs, rotate secrets or apply migrations, which + * is more authority than a seat should hold through a connector. + */ +export const OPERATOR_KEY_REFUSED_SCOPES = ['secrets:write', 'jobs:approve', 'migration:apply'] as const; + +export const refusedOperatorKeyScopes = (scopes: readonly string[]): string[] => ( + scopes.filter((scope) => (OPERATOR_KEY_REFUSED_SCOPES as readonly string[]).includes(scope)) +); + +export type OperatorKeyIntakeRefusal = + | { ok: false; code: 'not_operator_key' | 'key_missing' | 'scope_refused' | 'vendor_rejected' | 'vendor_unreachable'; message: string }; + +export type OperatorKeyIntakeResult = + | { ok: true; tools: HostedMcpUpstreamTool[]; scopeCheck: 'read' | 'self_report' } + | OperatorKeyIntakeRefusal; + +export interface OperatorKeyIntakeInput { + entry: HostedMcpEntry; + /** The pasted key. It is used for the probe and never returned, logged or echoed. */ + key: string; + /** + * The scopes Latchkey reports for this key, or null when it cannot report + * them. Null means the check is a self-report, which the caller must surface + * as such: a refused scope is then not a server guarantee (Vera §4). + */ + reportedScopes: readonly string[] | null; + fetcher: typeof fetch; + listTools?: typeof listHostedMcpTools; +} + +/** + * Prove a pasted key before its row can go `connected` (TASK-330, Vera §5). + * + * The probe is the same tools/list client the broker uses for calls, so a key + * that passes here is one the calls can use. Every refusal carries fixed copy: + * the vendor's own text and the key never leave this function, so a caller + * cannot relay the secret by mistake. + */ +export const validateOperatorKey = async ( + input: OperatorKeyIntakeInput, +): Promise => { + const { entry, key } = input; + if (entry.credentialMode !== 'operator_key') { + return { ok: false, code: 'not_operator_key', message: 'this connector does not take a pasted key' }; + } + const trimmed = typeof key === 'string' ? key.trim() : ''; + if (!trimmed) { + return { ok: false, code: 'key_missing', message: 'paste the key to continue' }; + } + const refused = input.reportedScopes ? refusedOperatorKeyScopes(input.reportedScopes) : []; + if (refused.length > 0) { + return { + ok: false, + code: 'scope_refused', + message: `this key carries ${refused.join(', ')}; create a key without those scopes`, + }; + } + const listTools = input.listTools ?? listHostedMcpTools; + try { + const tools = await listTools(entry, trimmed, input.fetcher); + return { ok: true, tools, scopeCheck: input.reportedScopes ? 'read' : 'self_report' }; + } catch (error) { + const status = (error as { code?: string })?.code; + if (status === 'credential_rejected') { + return { ok: false, code: 'vendor_rejected', message: 'the vendor rejected this key' }; + } + return { ok: false, code: 'vendor_unreachable', message: 'the vendor could not be reached to check this key' }; + } +}; diff --git a/backend/services/hostedMcpToolDefinitions.ts b/backend/services/hostedMcpToolDefinitions.ts index 223108525..ce1b95e3d 100644 --- a/backend/services/hostedMcpToolDefinitions.ts +++ b/backend/services/hostedMcpToolDefinitions.ts @@ -426,11 +426,25 @@ const listUpstreamTools = async ( const entry = findHostedMcpEntry(entries, String(row.config?.entryId || '')); if (!entry) throw new RoomGrantError('connection_mismatch', 'hosted-MCP entry is unavailable', 403); const { token } = await deps.credentialFor(row); + return listHostedMcpTools(entry, token, deps.fetcher); +}; + +/** + * The vendor's full tools/list for an entry under one bearer. Shared by the + * broker's drift check and by operator-key intake (TASK-330), which must prove + * a pasted key against the same client the calls will use. Errors are the + * broker's RoomGrantErrors, with any token already redacted from vendor text. + */ +export const listHostedMcpTools = async ( + entry: HostedMcpEntry, + token: string, + fetcher: typeof fetch, +): Promise => { const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), HOSTED_MCP_CALL_TIMEOUT_MS); let session: Awaited> | undefined; try { - session = await openHostedMcpSession(entry.resource, token, deps.fetcher, controller.signal); + session = await openHostedMcpSession(entry.resource, token, fetcher, controller.signal); const allTools: HostedMcpUpstreamTool[] = []; const seenCursors = new Set(); let cursor: string | undefined; From 0c02a1a700fbc592ab9de2790901d12c4a51909c Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 30 Oct 2026 23:53:40 -0700 Subject: [PATCH 03/11] fix(connectors): agent-scoped hosted index, declared agentId, route-level operator-key mint test (TASK-330) The unique index (createdBy, config.entryId) rejected a second agent's key for the same entry (E11000). It is now (createdBy, config.entryId, config.agentId), and config.agentId is declared and server-owned so it is not silently dropped. scripts/migrate-hosted-mcp-agent-index.js drops the old index and must run before any key is issued. The mint test runs the grants route: an operator-key grant with an omitted audience, a foreign agent, or the bound agent plus another member is 400; exactly the bound agent passes. Removing the grants.ts guard turns the refusal arms red. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- .../hostedMcpAgentIndexMigration.test.js | 78 +++++++++++++++++++ .../service/hostedMcpConnectionRow.test.js | 25 ++++++ .../__tests__/unit/routes/grants.mint.test.js | 50 ++++++++++++ backend/models/Integration.ts | 27 +++++-- .../scripts/migrate-hosted-mcp-agent-index.js | 74 ++++++++++++++++++ backend/utils/serverOwnedConfigKeys.ts | 2 +- 6 files changed, 248 insertions(+), 8 deletions(-) create mode 100644 backend/__tests__/service/hostedMcpAgentIndexMigration.test.js create mode 100644 backend/scripts/migrate-hosted-mcp-agent-index.js diff --git a/backend/__tests__/service/hostedMcpAgentIndexMigration.test.js b/backend/__tests__/service/hostedMcpAgentIndexMigration.test.js new file mode 100644 index 000000000..ecabcac87 --- /dev/null +++ b/backend/__tests__/service/hostedMcpAgentIndexMigration.test.js @@ -0,0 +1,78 @@ +/** + * The TASK-330 index migration (scripts/migrate-hosted-mcp-agent-index.js). + * Before it runs, the old `(createdBy, entryId)` index refuses a second agent's + * key with E11000. After it runs, that key saves, and the old index is gone. + * Run against a real mongod, because the failure is an index, not a mock. + */ +process.env.PG_HOST = ''; + +const mongoose = require('mongoose'); +const Integration = require('../../models/Integration'); +const { setupMongoDb, closeMongoDb, clearMongoDb } = require('../utils/testUtils'); +const { run, OLD_INDEX } = require('../../scripts/migrate-hosted-mcp-agent-index'); + +const OWNER = new mongoose.Types.ObjectId(); +const keyFor = (agentId) => ({ + scope: 'user', + type: 'hosted-mcp', + status: 'connected', + createdBy: OWNER, + config: { entryId: 'latchkey', intake: 'operator_key', agentId }, +}); + +const indexNames = async () => (await Integration.collection.indexes()).map((index) => index.name); + +describe('the TASK-330 agent-scoped index migration', () => { + beforeAll(async () => { + await setupMongoDb(); + await Integration.init(); + }); + + beforeEach(async () => { + await Integration.deleteMany({}); + }); + + afterAll(async () => { + await clearMongoDb(); + await closeMongoDb(); + }); + + test('before the migration, the old index refuses a second agent key for the same person and entry', async () => { + // Put the database in the production state: the old index present, the new + // one absent. The old index is what this migration exists to remove. + const agentIndex = (await Integration.collection.indexes()) + .find((index) => index.key.createdBy === 1 && index.key['config.agentId'] === 1); + await Integration.collection.dropIndex(agentIndex.name); + await Integration.collection.createIndex( + { createdBy: 1, 'config.entryId': 1 }, + { unique: true, partialFilterExpression: { type: 'hosted-mcp' }, name: OLD_INDEX }, + ); + + await Integration.create(keyFor('agent-a')); + await expect(Integration.create(keyFor('agent-b'))).rejects.toMatchObject({ code: 11000 }); + }); + + test('the migration drops the old index, builds the new one, and lets a second agent key save', async () => { + await run({ apply: true }); + + const names = await indexNames(); + expect(names).not.toContain(OLD_INDEX); + expect(names.some((name) => name.includes('config.agentId'))).toBe(true); + await expect(Integration.create(keyFor('agent-b'))).resolves.toBeDefined(); + }); + + test('a second run is a no-op rather than an error', async () => { + await run({ apply: true }); + await expect(run({ apply: true })).resolves.toBeUndefined(); + expect(await indexNames()).not.toContain(OLD_INDEX); + }); + + test('read-only by default: a dry run leaves the old index in place', async () => { + await Integration.collection.createIndex( + { createdBy: 1, 'config.entryId': 1 }, + { unique: true, partialFilterExpression: { type: 'hosted-mcp' }, name: OLD_INDEX }, + ).catch(() => {}); + await run({ apply: false }); + expect(await indexNames()).toContain(OLD_INDEX); + }); +}); diff --git a/backend/__tests__/service/hostedMcpConnectionRow.test.js b/backend/__tests__/service/hostedMcpConnectionRow.test.js index 5777f8768..340ac2be6 100644 --- a/backend/__tests__/service/hostedMcpConnectionRow.test.js +++ b/backend/__tests__/service/hostedMcpConnectionRow.test.js @@ -43,6 +43,9 @@ const shape = (overrides = {}) => ({ // failure names the key that was dropped rather than the object that lost it. const RECORD_KEYS = { 'config.entryId': 'linear', + // The agent an operator key is bound to (TASK-330). Declared, so a $set the + // schema drops is caught here and not by a mint that refuses "not bound". + 'config.agentId': 'agent-a', 'config.intake': 'oauth', 'config.clientId': 'https://api.commonly.me/api/integrations/connect/hosted-mcp/linear/client-metadata', 'config.providerSubject': 'subject-1', @@ -158,6 +161,28 @@ describe('the hosted-mcp connection row', () => { await expect(Integration.create(shape({ createdBy: OWNER_B }))).resolves.toBeDefined(); }); + test('one owner holds one key per bound agent, and an OAuth row co-exists with them', async () => { + // TASK-330: an operator key is one agent's bearer, so the second agent's + // key for the same vendor is a second row, not an E11000. A second key for + // the SAME agent is still one row, and an OAuth row (no agentId) is its own + // row beside them. Each is per (person, entry, agent). + const keyFor = (agentId) => shape({ config: { entryId: ENTRY_A, intake: 'operator_key', agentId } }); + + await expect(Integration.create(keyFor('agent-a'))).resolves.toBeDefined(); + await expect(Integration.create(keyFor('agent-b'))).resolves.toBeDefined(); + await expect(Integration.create(keyFor('agent-a'))).rejects.toMatchObject({ code: 11000 }); + await expect(Integration.create(shape())).resolves.toBeDefined(); + await expect(Integration.create(shape())).rejects.toMatchObject({ code: 11000 }); + }); + + test('the declared index keys on the bound agent as well as the person and entry', () => { + const declared = Integration.schema.indexes().find(([keys, options]) => ( + keys.createdBy === 1 && options.partialFilterExpression?.type === 'hosted-mcp' + )); + expect(declared[0]).toEqual({ createdBy: 1, 'config.entryId': 1, 'config.agentId': 1 }); + expect(declared[1].unique).toBe(true); + }); + test('positive control: the index is PARTIAL, not a unique index on createdBy', async () => { // If the declaration lost `partialFilterExpression`, the unique key would // still be enforced on hosted-mcp rows and the arm above would keep diff --git a/backend/__tests__/unit/routes/grants.mint.test.js b/backend/__tests__/unit/routes/grants.mint.test.js index a50b49e02..5c38b6dd5 100644 --- a/backend/__tests__/unit/routes/grants.mint.test.js +++ b/backend/__tests__/unit/routes/grants.mint.test.js @@ -409,3 +409,53 @@ describe('POST /api/grants on a hosted-mcp connection', () => { expect(await RoomGrant.countDocuments({})).toBe(0); }); }); + +/** + * TASK-330: an operator-key row is one agent's bearer, so its grant may name + * only that agent, and only alone. The helper has its own arms; these run the + * mint, so the test goes red if the grants.ts call is removed. + */ +describe('POST /api/grants on an operator-key hosted connection', () => { + const KEY_ENTRY = { ...ENTRY, id: 'latchkey', title: 'Latchkey', credentialMode: 'operator_key' }; + const KEY_ID = 'eeeeeeeeeeeeeeeeeeeeee02'; + const keyRow = (config = {}) => ({ + _id: KEY_ID, type: 'hosted-mcp', status: 'connected', createdBy: OWNER, + config: { entryId: 'latchkey', agentId: SEAT, credentialRef: 'secret-ref-key', ...config }, + }); + const keyMint = (body = {}) => mint({ connectionId: KEY_ID, tools: ['latchkey.list_issues'], ...body }); + + beforeEach(() => { + Integration.findById.mockResolvedValue(keyRow()); + }); + + test('an omitted audience is refused, not defaulted to every pod member', async () => { + const res = await withCatalog([KEY_ENTRY], () => keyMint()); + expect(res.status).toBe(400); + expect(res.body).toMatchObject({ error: 'invalid_audience' }); + expect(res.body.message).toMatch(/omitted audience/); + expect(await RoomGrant.countDocuments({})).toBe(0); + }); + + test('an audience naming another agent is refused', async () => { + const res = await withCatalog([KEY_ENTRY], () => keyMint({ audience: [OWNER] })); + expect(res.status).toBe(400); + expect(res.body).toEqual({ error: 'invalid_audience', message: `audience must be exactly [${SEAT}]` }); + expect(await RoomGrant.countDocuments({})).toBe(0); + }); + + test('an audience of the bound agent plus another member is refused', async () => { + const res = await withCatalog([KEY_ENTRY], () => keyMint({ audience: [SEAT, OWNER] })); + expect(res.status).toBe(400); + expect(res.body).toMatchObject({ error: 'invalid_audience' }); + expect(await RoomGrant.countDocuments({})).toBe(0); + }); + + test('an audience of exactly the bound agent passes the audience check', async () => { + const res = await withCatalog([KEY_ENTRY], () => keyMint({ audience: [SEAT] })); + // The mint then reaches the broker, which this harness does not stand up + // (see the 503 arm above), so the arm asserts what it names: not refused + // for its audience. + expect(res.body.error).not.toBe('invalid_audience'); + expect(res.status).not.toBe(400); + }); +}); diff --git a/backend/models/Integration.ts b/backend/models/Integration.ts index adb47d3c9..ad8e2ecb2 100644 --- a/backend/models/Integration.ts +++ b/backend/models/Integration.ts @@ -175,6 +175,8 @@ export interface IIntegration extends Document { // this half is what stops a typed write from needing a cast to state the // same thing — a cast being the same silence one layer up (#1976 gate). entryId?: string; + /** The agent an operator key is bound to (TASK-330). Server-owned; see the schema. */ + agentId?: string; intake?: 'oauth'; clientId?: string; providerSubject?: string; @@ -378,6 +380,11 @@ const IntegrationSchema = new Schema( return this.type === 'hosted-mcp'; }, }, + // The agent an operator key is bound to (TASK-330). Set server-side at + // paste intake and never by a body. Absent on every OAuth row, which is + // why the unique index below can treat it as one more key without + // splitting an OAuth person's single row. + agentId: { type: String, trim: true }, intake: String, // 'oauth' is the only intake for this type clientId: String, // id that minted this pair; DCR rows resolve this against held registrations providerSubject: String, // the authorization server's stable id for the account @@ -470,14 +477,20 @@ IntegrationSchema.index({ podId: 1, type: 1 }); IntegrationSchema.index({ status: 1 }); IntegrationSchema.index({ createdBy: 1 }); IntegrationSchema.index({ installationId: 1 }, { unique: true, sparse: true }); -// One `hosted-mcp` row per (person, catalogue entry). The page cannot say which -// row a grant uses if a person holds two for one vendor, and the two would go -// through removal separately (scope §2). Partial, not sparse: `config.entryId` -// is absent on every other type, and every other type has no such key to -// collide on. A second connect by the same person REUSES the row through the -// §10.3 fence rather than inserting one. +// One `hosted-mcp` row per (person, catalogue entry, bound agent). An operator +// key is one agent's bearer (TASK-330), so one person may hold one row per +// agent for the same vendor. OAuth rows have no `config.agentId`, index as +// null, and so still get one row per person per entry, as before. Partial, not +// sparse: `config.entryId` is absent on every other type. A second OAuth connect +// by the same person REUSES the row through the §10.3 fence rather than +// inserting one. +// +// This replaces `createdBy_1_config.entryId_1`, which would refuse a second +// agent's key with E11000. Mongo does not swap an index in place: the old one +// must be dropped by scripts/migrate-hosted-mcp-agent-index.js before the new +// key is used on a database that already holds hosted rows. IntegrationSchema.index( - { createdBy: 1, 'config.entryId': 1 }, + { createdBy: 1, 'config.entryId': 1, 'config.agentId': 1 }, { unique: true, partialFilterExpression: { type: 'hosted-mcp' } }, ); IntegrationSchema.index({ 'ingestTokens.tokenHash': 1 }); diff --git a/backend/scripts/migrate-hosted-mcp-agent-index.js b/backend/scripts/migrate-hosted-mcp-agent-index.js new file mode 100644 index 000000000..a33211399 --- /dev/null +++ b/backend/scripts/migrate-hosted-mcp-agent-index.js @@ -0,0 +1,74 @@ +/* eslint-disable no-console, import/extensions, import/no-unresolved */ +/** + * TASK-330 rollout step. The hosted-mcp unique index moves from + * `(createdBy, config.entryId)` to `(createdBy, config.entryId, config.agentId)` + * so one person may hold one operator key per agent. Mongo does not replace an + * index in place, so the old one must go first, or the second agent's key + * still fails with E11000. + * + * Read-only by default. --apply drops the old index if present and then builds + * the declared one. It deletes no rows, so it cannot change which person holds + * which connection. If the old index is still present after --apply, stop: the + * new key is not safe to use yet. + */ +const mongoose = require('mongoose'); + +const Integration = require('../models/Integration'); + +const OLD_INDEX = 'createdBy_1_config.entryId_1'; +const declared = Integration.schema.indexes().find(([keys, options]) => ( + keys.createdBy === 1 && keys['config.agentId'] === 1 && options.unique && options.partialFilterExpression +)); +if (!declared) throw new Error('the agent-scoped hosted-mcp index is not declared on Integration'); +const [keys, options] = declared; + +async function listIndexes() { + return Integration.collection.indexes().catch((error) => { + if (error.code === 26) return []; + throw error; + }); +} + +async function run({ apply }) { + const before = await listIndexes(); + const oldPresent = before.some((index) => index.name === OLD_INDEX); + const newPresent = before.some((index) => index.name === options.name + || JSON.stringify(index.key) === JSON.stringify(keys)); + console.log(JSON.stringify({ oldIndexPresent: oldPresent, newIndexPresent: newPresent, apply })); + if (!apply) return; + + if (oldPresent) { + await Integration.collection.dropIndex(OLD_INDEX); + console.log(`dropped ${OLD_INDEX}`); + } + if (!newPresent) { + await Integration.collection.createIndex(keys, { ...options }); + console.log('created the agent-scoped hosted-mcp index'); + } + const after = await listIndexes(); + const stillOld = after.some((index) => index.name === OLD_INDEX); + if (stillOld) throw new Error(`${OLD_INDEX} is still present; do not issue operator keys`); + console.log('ok: old index gone, agent-scoped index present'); +} + +if (require.main === module) { + const apply = process.argv.includes('--apply'); + const uri = process.env.MONGO_URI; + if (!uri) { + console.error('MONGO_URI is required'); + process.exit(1); + } + // Only the CLI turns autoIndex off: a module-level set would leak into any + // process that requires this file, the tests included. + mongoose.set('autoIndex', false); + mongoose.connect(uri) + .then(() => run({ apply })) + .then(() => mongoose.disconnect()) + .catch(async (error) => { + console.error(error.message); + await mongoose.disconnect(); + process.exit(1); + }); +} + +module.exports = { run, OLD_INDEX }; diff --git a/backend/utils/serverOwnedConfigKeys.ts b/backend/utils/serverOwnedConfigKeys.ts index 976f9b89c..8b1beedf8 100644 --- a/backend/utils/serverOwnedConfigKeys.ts +++ b/backend/utils/serverOwnedConfigKeys.ts @@ -92,7 +92,7 @@ export const SERVER_OWNED_CONFIG_KEYS = [ // is a single-use nonce — a supplied one is a state the callback would accept // from whoever wrote it. `intake` and `credentialHint` are here for the same // reason at lower stakes: no browser decides how a credential is obtained. - 'entryId', 'intake', 'clientId', 'clientSecret', 'providerSubject', 'grantedScope', 'expiresAt', + 'entryId', 'agentId', 'intake', 'clientId', 'clientSecret', 'providerSubject', 'grantedScope', 'expiresAt', 'credentialRef', 'refreshTokenRef', 'refreshGeneration', 'authGeneration', 'credentialHint', 'pendingAuth', // The mark step 3b of the removal sequence records (`connectionRemovalService`) // after the provider revoke TAKES. A body that could write it would make a From a7279a3dc4202eda8c60b822aa6ced1361cfa4de Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 30 Oct 2026 23:57:47 -0700 Subject: [PATCH 04/11] feat(connectors): paste-key intake route for operator-key connections (TASK-330) POST /connect/hosted-mcp/:entryId/operator-key takes one agent's key and makes it a connected row bound to that agent (config.agentId, checked against a bot User). The route refuses refused scopes among the operator's picks before any vendor call, proves the key with a live tools/list, then upserts the row pending, stores the key as a ConnectorSecret on the row (never on a grant), and only then marks it connected. scopeCheck is always self_report here: reportedScopes is passed as null until a vendor readback exists, so the operator's picks cannot make a refusal look like a server guarantee. The key is never echoed in a response, and a failed secret write leaves the row pending. Tests pin the scope refusal ordering, the bot check, the self_report ignoring a body claim, the pending-then-connected write order, and that the key appears in no response or row write. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- .../routes/hostedMcpOperatorKey.route.test.js | 153 ++++++++++++++++++ backend/routes/hostedMcpConnect.ts | 3 + backend/routes/hostedMcpOperatorKey.ts | 105 ++++++++++++ backend/services/hostedMcpToolDefinitions.ts | 2 +- 4 files changed, 262 insertions(+), 1 deletion(-) create mode 100644 backend/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js create mode 100644 backend/routes/hostedMcpOperatorKey.ts diff --git a/backend/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js b/backend/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js new file mode 100644 index 000000000..6b35a58d6 --- /dev/null +++ b/backend/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js @@ -0,0 +1,153 @@ +/** + * TASK-330 paste-key intake route. The row, the agent check, the secret store + * and the vendor probe are mocked at their module edges; the handler, the + * scope refusal and the intake probe are real. The key must never come back in + * a response or reach the row, and the scope the route reports must be a + * self-report whatever the body claims (Vera's review of 2d4e37fc7). + */ +const express = require('express'); +const request = require('supertest'); + +jest.mock('../../../models/Integration', () => ({ findOneAndUpdate: jest.fn(), updateOne: jest.fn() })); +jest.mock('../../../models/User', () => ({ findOne: jest.fn() })); +jest.mock('../../../services/connectorSecrets', () => ({ put: jest.fn() })); +jest.mock('../../../services/hostedMcpToolDefinitions', () => ({ + guardedFetch: jest.fn(), + listHostedMcpTools: jest.fn(), +})); + +const Integration = require('../../../models/Integration'); +const User = require('../../../models/User'); +const connectorSecrets = require('../../../services/connectorSecrets'); +const { listHostedMcpTools } = require('../../../services/hostedMcpToolDefinitions'); +const { HOSTED_MCP_ENTRIES } = require('../../../integrations/hostedMcp/entries'); +const { submitOperatorKey } = require('../../../routes/hostedMcpOperatorKey'); + +const OWNER = 'bbbbbbbbbbbbbbbbbbbbbb01'; +const AGENT = 'cccccccccccccccccccccc01'; +const ROW_ID = 'eeeeeeeeeeeeeeeeeeeeee02'; +const KEY = 'lk_live_SECRET_VALUE_DO_NOT_ECHO'; + +const KEY_ENTRY = { + id: 'latchkey', + title: 'Latchkey', + resource: 'https://latchkey.example/mcp', + credentialMode: 'operator_key', + scopes: [], + revoke: 'https://latchkey.example/keys', + tools: [{ + name: 'list_jobs', + upstreamName: 'list_jobs', + description: 'List jobs', + class: 'read', + inputSchema: { type: 'object' }, + }], +}; + +const app = express(); +app.use(express.json()); +app.post('/connect/hosted-mcp/:entryId/operator-key', (req, res, next) => { + const id = req.get('x-test-user'); + if (!id) return res.status(401).json({ error: 'unauthorized' }); + req.user = { id }; + return next(); +}, submitOperatorKey); + +const submit = (body = {}) => request(app) + .post('/connect/hosted-mcp/latchkey/operator-key') + .set('x-test-user', OWNER) + .send({ agentId: AGENT, key: KEY, ...body }); + +let shipped; +beforeEach(() => { + jest.clearAllMocks(); + shipped = HOSTED_MCP_ENTRIES.splice(0, HOSTED_MCP_ENTRIES.length, KEY_ENTRY); + User.findOne.mockReturnValue({ select: () => ({ lean: async () => ({ _id: AGENT }) }) }); + listHostedMcpTools.mockResolvedValue([{ name: 'list_jobs' }]); + Integration.findOneAndUpdate.mockResolvedValue({ _id: ROW_ID, config: { authGeneration: 1 } }); + Integration.updateOne.mockResolvedValue({ acknowledged: true }); + connectorSecrets.put.mockResolvedValue('secret-ref-1'); +}); +afterEach(() => { + HOSTED_MCP_ENTRIES.splice(0, HOSTED_MCP_ENTRIES.length, ...shipped); +}); + +/** Every value the route handed to a model write, so a leak anywhere shows. */ +const everyWrittenValue = () => JSON.stringify([ + Integration.findOneAndUpdate.mock.calls, + Integration.updateOne.mock.calls, +]); + +describe('POST /connect/hosted-mcp/:entryId/operator-key', () => { + test('a refused scope among the operator picks is refused before any vendor call or row write', async () => { + const res = await submit({ scopePicks: ['read', 'jobs:approve'] }); + expect(res.status).toBe(400); + expect(res.body).toMatchObject({ error: 'scope_refused' }); + expect(res.body.message).toMatch(/jobs:approve/); + expect(listHostedMcpTools).not.toHaveBeenCalled(); + expect(Integration.findOneAndUpdate).not.toHaveBeenCalled(); + expect(connectorSecrets.put).not.toHaveBeenCalled(); + }); + + test('a vendor that rejects the key is refused, and no row or secret is written', async () => { + listHostedMcpTools.mockRejectedValue(Object.assign(new Error('nope'), { code: 'credential_rejected' })); + const res = await submit(); + expect(res.status).toBe(400); + expect(res.body).toEqual({ error: 'vendor_rejected', message: 'the vendor rejected this key' }); + expect(Integration.findOneAndUpdate).not.toHaveBeenCalled(); + expect(connectorSecrets.put).not.toHaveBeenCalled(); + }); + + test('an agentId that is not a bot is refused before the probe', async () => { + User.findOne.mockReturnValue({ select: () => ({ lean: async () => null }) }); + const res = await submit(); + expect(res.status).toBe(400); + expect(res.body).toMatchObject({ error: 'invalid_agent' }); + expect(listHostedMcpTools).not.toHaveBeenCalled(); + }); + + test('a valid key writes a pending row, stores the secret on it, then connects it', async () => { + const res = await submit(); + expect(res.status).toBe(201); + expect(res.body).toEqual({ + integrationId: ROW_ID, status: 'connected', scopeCheck: 'self_report', toolCount: 1, + }); + expect(Integration.findOneAndUpdate).toHaveBeenCalledWith( + { + type: 'hosted-mcp', createdBy: OWNER, 'config.entryId': 'latchkey', 'config.agentId': AGENT, + }, + expect.objectContaining({ $set: expect.objectContaining({ status: 'pending' }) }), + expect.objectContaining({ upsert: true }), + ); + expect(connectorSecrets.put).toHaveBeenCalledWith(ROW_ID, expect.anything(), KEY, expect.any(Number)); + expect(Integration.updateOne).toHaveBeenCalledWith( + { _id: ROW_ID }, + { $set: { status: 'connected', 'config.credentialRef': 'secret-ref-1' } }, + ); + }); + + test('scopeCheck is a self-report even when the body claims a readback', async () => { + const res = await submit({ scopeCheck: 'read', reportedScopes: ['read'] }); + expect(res.status).toBe(201); + expect(res.body.scopeCheck).toBe('self_report'); + }); + + test('the key appears in no response, no row write, and no thrown error text', async () => { + const ok = await submit(); + expect(JSON.stringify(ok.body)).not.toContain(KEY); + expect(everyWrittenValue()).not.toContain(KEY); + + listHostedMcpTools.mockRejectedValue(new Error(`upstream echoed ${KEY}`)); + const unreachable = await submit(); + expect(JSON.stringify(unreachable.body)).not.toContain(KEY); + }); + + test('a failed secret write leaves the row pending, never connected, and says so without the key', async () => { + connectorSecrets.put.mockRejectedValue(new Error(`store failed for ${KEY}`)); + const res = await submit(); + expect(res.status).toBe(500); + expect(res.body).toEqual({ error: 'key_store_failed', message: 'the key could not be stored; try again' }); + expect(Integration.updateOne).not.toHaveBeenCalled(); + expect(JSON.stringify(res.body)).not.toContain(KEY); + }); +}); diff --git a/backend/routes/hostedMcpConnect.ts b/backend/routes/hostedMcpConnect.ts index 21a195804..55e69f73b 100644 --- a/backend/routes/hostedMcpConnect.ts +++ b/backend/routes/hostedMcpConnect.ts @@ -26,6 +26,7 @@ import { upstreamFetch } from '../services/upstreamFetch'; import { dcrClientForRow, ensureDcrClient, refuseDcrClient } from '../services/hostedMcpDcrRegistry'; import { forgetHostedMcpDrift } from '../services/hostedMcpToolDefinitions'; import { publicAppUrl } from '../utils/publicAppUrl'; +import { submitOperatorKey } from './hostedMcpOperatorKey'; // eslint-disable-next-line global-require const Integration = require('../models/Integration'); // eslint-disable-next-line global-require @@ -343,6 +344,8 @@ const startConnect = async (req: AuthedRequest, res: Response) => { }); }; +router.post('/:entryId/operator-key', writeIntegrationsRateLimit, auth, submitOperatorKey); + router.post('/:entryId/start', writeIntegrationsRateLimit, auth, async (req: AuthedRequest, res: Response) => { try { await startConnect(req, res); diff --git a/backend/routes/hostedMcpOperatorKey.ts b/backend/routes/hostedMcpOperatorKey.ts new file mode 100644 index 000000000..fc45aafb7 --- /dev/null +++ b/backend/routes/hostedMcpOperatorKey.ts @@ -0,0 +1,105 @@ +import type { Request, Response } from 'express'; +import { HOSTED_MCP_ENTRIES, findHostedMcpEntry } from '../integrations/hostedMcp/entries'; +import { isHostedMcpEntryHidden } from '../integrations/hostedMcp/visibility'; +import { refusedOperatorKeyScopes, validateOperatorKey } from '../services/hostedMcpOperatorKeyIntake'; +import { guardedFetch } from '../services/hostedMcpToolDefinitions'; +import { secretWriteGeneration } from '../services/hostedMcpSecretGeneration'; + +const Integration = require('../models/Integration'); +const User = require('../models/User'); +const connectorSecrets = require('../services/connectorSecrets'); +const { HOSTED_MCP_ACCESS_TOKEN } = require('../services/connectorSecretKinds'); + +type OperatorKeyRequest = Request & { user?: { id?: string } }; + +/** + * POST /connect/hosted-mcp/:entryId/operator-key (TASK-330, Vera §5). + * + * Takes one agent's pasted key and makes it a connected row bound to that + * agent. The key is proven by a live tools/list before the row can go + * `connected`, is stored as a ConnectorSecret on the row (never on a grant), + * and is never echoed in a response or written to a log. + * + * `scopeCheck` is always `self_report` here. Only a vendor readback of the + * key's scopes may set `reportedScopes`, and no readback exists yet, so the + * operator's scope picks are checked for refused scopes and nothing more. + */ +export const submitOperatorKey = async (req: OperatorKeyRequest, res: Response) => { + const userId = req.user?.id; + if (!userId) return res.status(401).json({ error: 'unauthorized' }); + + const entryId = String(req.params.entryId || ''); + const entry = findHostedMcpEntry(HOSTED_MCP_ENTRIES, entryId); + if (!entry || isHostedMcpEntryHidden(entryId, process.env, userId)) { + return res.status(404).json({ error: 'unknown_entry' }); + } + + const body = (req.body || {}) as Record; + const agentId = typeof body.agentId === 'string' ? body.agentId.trim() : ''; + if (!/^[a-f\d]{24}$/i.test(agentId)) { + return res.status(400).json({ error: 'invalid_agent', message: 'agentId must name an agent' }); + } + const agent = await User.findOne({ _id: agentId, isBot: true }).select('_id').lean(); + if (!agent) { + return res.status(400).json({ error: 'invalid_agent', message: 'agentId must name an agent' }); + } + + const picks = Array.isArray(body.scopePicks) ? body.scopePicks.map(String) : []; + const refused = refusedOperatorKeyScopes(picks); + if (refused.length > 0) { + return res.status(400).json({ + error: 'scope_refused', + message: `this key carries ${refused.join(', ')}; create a key without those scopes`, + }); + } + + const key = typeof body.key === 'string' ? body.key : ''; + const intake = await validateOperatorKey({ + entry, + key, + reportedScopes: null, + fetcher: guardedFetch, + }); + if (!intake.ok) { + const status = intake.code === 'vendor_unreachable' ? 502 : 400; + return res.status(status).json({ error: intake.code, message: intake.message }); + } + + // The row is written only after the probe passes. It is upserted pending, + // then the secret lands on it, then it goes connected: a failed write leaves + // a pending row that grants refuse, never a connected row with no key. + const row = await Integration.findOneAndUpdate( + { type: 'hosted-mcp', createdBy: userId, 'config.entryId': entry.id, 'config.agentId': agentId }, + { + $inc: { 'config.authGeneration': 1 }, + $set: { status: 'pending', 'config.intake': 'operator_key' }, + $setOnInsert: { + type: 'hosted-mcp', + scope: 'user', + createdBy: userId, + 'config.entryId': entry.id, + 'config.agentId': agentId, + }, + }, + { upsert: true, new: true, setDefaultsOnInsert: true }, + ); + const writeGeneration = secretWriteGeneration(row.config?.authGeneration ?? 0, 0); + let credentialRef: string; + try { + credentialRef = await connectorSecrets.put(String(row._id), HOSTED_MCP_ACCESS_TOKEN, key, writeGeneration); + } catch { + // Fixed copy only: the store's error text stays out of the response. + return res.status(500).json({ error: 'key_store_failed', message: 'the key could not be stored; try again' }); + } + await Integration.updateOne( + { _id: row._id }, + { $set: { status: 'connected', 'config.credentialRef': credentialRef } }, + ); + + return res.status(201).json({ + integrationId: String(row._id), + status: 'connected', + scopeCheck: intake.scopeCheck, + toolCount: intake.tools.length, + }); +}; diff --git a/backend/services/hostedMcpToolDefinitions.ts b/backend/services/hostedMcpToolDefinitions.ts index ce1b95e3d..e7335fab8 100644 --- a/backend/services/hostedMcpToolDefinitions.ts +++ b/backend/services/hostedMcpToolDefinitions.ts @@ -75,7 +75,7 @@ const messageOf = (error: unknown): string => ( // Loaded lazily, like the model and the credential service below: the frontend // glyph-catalogue test imports this module for its pure projection, and must // not pull the guard's node:dns and node:http transport in with it. -const guardedFetch: typeof fetch = (input, init) => { +export const guardedFetch: typeof fetch = (input, init) => { // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports const { hostedVendorFetch } = require('./hostedVendorFetch'); return hostedVendorFetch(input, init); From 5ba96a8fb6f702bd732343bad000f82442b104aa Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 31 Oct 2026 00:00:13 -0700 Subject: [PATCH 05/11] fix(connectors): build the agent-scoped index before dropping the old one (TASK-330) Dropping first left the collection with no unique index for the window between the drop and a failed build. The migration now builds the declared index, then drops the old one, so a failed build keeps uniqueness. Each test arm seeds its own pre-migration state instead of relying on an earlier arm to create the old index. A new arm pins the order: a rejected build leaves the old index in place and the second agent's key still refused. The header gives the compiled-output command, as the other backend scripts do. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- .../hostedMcpAgentIndexMigration.test.js | 51 +++++++++++++------ .../scripts/migrate-hosted-mcp-agent-index.js | 22 +++++--- 2 files changed, 50 insertions(+), 23 deletions(-) diff --git a/backend/__tests__/service/hostedMcpAgentIndexMigration.test.js b/backend/__tests__/service/hostedMcpAgentIndexMigration.test.js index ecabcac87..b0f437230 100644 --- a/backend/__tests__/service/hostedMcpAgentIndexMigration.test.js +++ b/backend/__tests__/service/hostedMcpAgentIndexMigration.test.js @@ -3,6 +3,9 @@ * Before it runs, the old `(createdBy, entryId)` index refuses a second agent's * key with E11000. After it runs, that key saves, and the old index is gone. * Run against a real mongod, because the failure is an index, not a mock. + * + * Every arm seeds its own pre-migration state (old index present, new index + * absent, agent-a's key saved) so no arm depends on another having run first. */ process.env.PG_HOST = ''; @@ -22,6 +25,19 @@ const keyFor = (agentId) => ({ const indexNames = async () => (await Integration.collection.indexes()).map((index) => index.name); +/** Put the database in the production state: the old index present, the agent-scoped one absent. */ +const seedPreMigration = async () => { + const agentIndex = (await Integration.collection.indexes()) + .find((index) => index.key.createdBy === 1 && index.key['config.agentId'] === 1); + if (agentIndex) await Integration.collection.dropIndex(agentIndex.name); + if (!(await indexNames()).includes(OLD_INDEX)) { + await Integration.collection.createIndex( + { createdBy: 1, 'config.entryId': 1 }, + { unique: true, partialFilterExpression: { type: 'hosted-mcp' }, name: OLD_INDEX }, + ); + } +}; + describe('the TASK-330 agent-scoped index migration', () => { beforeAll(async () => { await setupMongoDb(); @@ -30,6 +46,11 @@ describe('the TASK-330 agent-scoped index migration', () => { beforeEach(async () => { await Integration.deleteMany({}); + await seedPreMigration(); + }); + + afterEach(() => { + jest.restoreAllMocks(); }); afterAll(async () => { @@ -38,27 +59,31 @@ describe('the TASK-330 agent-scoped index migration', () => { }); test('before the migration, the old index refuses a second agent key for the same person and entry', async () => { - // Put the database in the production state: the old index present, the new - // one absent. The old index is what this migration exists to remove. - const agentIndex = (await Integration.collection.indexes()) - .find((index) => index.key.createdBy === 1 && index.key['config.agentId'] === 1); - await Integration.collection.dropIndex(agentIndex.name); - await Integration.collection.createIndex( - { createdBy: 1, 'config.entryId': 1 }, - { unique: true, partialFilterExpression: { type: 'hosted-mcp' }, name: OLD_INDEX }, - ); - await Integration.create(keyFor('agent-a')); await expect(Integration.create(keyFor('agent-b'))).rejects.toMatchObject({ code: 11000 }); }); - test('the migration drops the old index, builds the new one, and lets a second agent key save', async () => { + test('the migration builds the new index, drops the old one, and lets a second agent key save', async () => { + await Integration.create(keyFor('agent-a')); + await run({ apply: true }); const names = await indexNames(); expect(names).not.toContain(OLD_INDEX); expect(names.some((name) => name.includes('config.agentId'))).toBe(true); await expect(Integration.create(keyFor('agent-b'))).resolves.toBeDefined(); + // The migration deletes no rows: agent-a's key is still there. + expect(await Integration.countDocuments({ 'config.agentId': 'agent-a' })).toBe(1); + }); + + test('a failed build leaves the old index in place, so uniqueness is never lost', async () => { + jest.spyOn(Integration.collection, 'createIndex').mockRejectedValueOnce(new Error('build failed')); + + await expect(run({ apply: true })).rejects.toThrow('build failed'); + + expect(await indexNames()).toContain(OLD_INDEX); + await Integration.create(keyFor('agent-a')); + await expect(Integration.create(keyFor('agent-b'))).rejects.toMatchObject({ code: 11000 }); }); test('a second run is a no-op rather than an error', async () => { @@ -68,10 +93,6 @@ describe('the TASK-330 agent-scoped index migration', () => { }); test('read-only by default: a dry run leaves the old index in place', async () => { - await Integration.collection.createIndex( - { createdBy: 1, 'config.entryId': 1 }, - { unique: true, partialFilterExpression: { type: 'hosted-mcp' }, name: OLD_INDEX }, - ).catch(() => {}); await run({ apply: false }); expect(await indexNames()).toContain(OLD_INDEX); }); diff --git a/backend/scripts/migrate-hosted-mcp-agent-index.js b/backend/scripts/migrate-hosted-mcp-agent-index.js index a33211399..f22ae67a7 100644 --- a/backend/scripts/migrate-hosted-mcp-agent-index.js +++ b/backend/scripts/migrate-hosted-mcp-agent-index.js @@ -6,10 +6,15 @@ * index in place, so the old one must go first, or the second agent's key * still fails with E11000. * - * Read-only by default. --apply drops the old index if present and then builds - * the declared one. It deletes no rows, so it cannot change which person holds - * which connection. If the old index is still present after --apply, stop: the - * new key is not safe to use yet. + * Read-only by default. --apply builds the declared index first, then drops the + * old one. The order matters: dropping first would leave the collection with no + * uniqueness at all if the build then failed. It deletes no rows, so it cannot + * change which person holds which connection. If the old index is still present + * after --apply, stop: the new key is not safe to use yet. + * + * Run from the compiled output, like the other backend scripts: + * cd backend && node dist/scripts/migrate-hosted-mcp-agent-index.js # dry run + * cd backend && node dist/scripts/migrate-hosted-mcp-agent-index.js --apply */ const mongoose = require('mongoose'); @@ -37,14 +42,15 @@ async function run({ apply }) { console.log(JSON.stringify({ oldIndexPresent: oldPresent, newIndexPresent: newPresent, apply })); if (!apply) return; - if (oldPresent) { - await Integration.collection.dropIndex(OLD_INDEX); - console.log(`dropped ${OLD_INDEX}`); - } + // Build first: if the build fails, the old index still enforces uniqueness. if (!newPresent) { await Integration.collection.createIndex(keys, { ...options }); console.log('created the agent-scoped hosted-mcp index'); } + if (oldPresent) { + await Integration.collection.dropIndex(OLD_INDEX); + console.log(`dropped ${OLD_INDEX}`); + } const after = await listIndexes(); const stillOld = after.some((index) => index.name === OLD_INDEX); if (stillOld) throw new Error(`${OLD_INDEX} is still present; do not issue operator keys`); From 8c4074221cb3a6fd6d9f092df7d3ebbb7838dd1d Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 31 Oct 2026 00:44:29 -0700 Subject: [PATCH 06/11] fix(connectors): sole-installer ownership check on the paste-key route (TASK-330) The operator key is a credential the agent presents, so it takes the same rule as a runtime token: ownsRuntimeIdentity(caller, agentName, instanceId) from services/runtimeIdentityOwnership.ts. An agent the caller did not solely install is refused 403 before any probe or row write. Admins are not exempt. The agent identity now comes from the bot's botMetadata, not the body. Tests cover a bot with no identity, a shared identity, and that the check runs for every caller. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- .../routes/hostedMcpOperatorKey.route.test.js | 34 ++++++++++++++++++- backend/routes/hostedMcpOperatorKey.ts | 13 +++++-- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/backend/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js b/backend/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js index 6b35a58d6..46c747b0c 100644 --- a/backend/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js +++ b/backend/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js @@ -15,11 +15,13 @@ jest.mock('../../../services/hostedMcpToolDefinitions', () => ({ guardedFetch: jest.fn(), listHostedMcpTools: jest.fn(), })); +jest.mock('../../../services/runtimeIdentityOwnership', () => ({ ownsRuntimeIdentity: jest.fn() })); const Integration = require('../../../models/Integration'); const User = require('../../../models/User'); const connectorSecrets = require('../../../services/connectorSecrets'); const { listHostedMcpTools } = require('../../../services/hostedMcpToolDefinitions'); +const { ownsRuntimeIdentity } = require('../../../services/runtimeIdentityOwnership'); const { HOSTED_MCP_ENTRIES } = require('../../../integrations/hostedMcp/entries'); const { submitOperatorKey } = require('../../../routes/hostedMcpOperatorKey'); @@ -62,7 +64,10 @@ let shipped; beforeEach(() => { jest.clearAllMocks(); shipped = HOSTED_MCP_ENTRIES.splice(0, HOSTED_MCP_ENTRIES.length, KEY_ENTRY); - User.findOne.mockReturnValue({ select: () => ({ lean: async () => ({ _id: AGENT }) }) }); + User.findOne.mockReturnValue({ + select: () => ({ lean: async () => ({ _id: AGENT, botMetadata: { agentName: 'openclaw', instanceId: 'latch' } }) }), + }); + ownsRuntimeIdentity.mockResolvedValue({ owned: true }); listHostedMcpTools.mockResolvedValue([{ name: 'list_jobs' }]); Integration.findOneAndUpdate.mockResolvedValue({ _id: ROW_ID, config: { authGeneration: 1 } }); Integration.updateOne.mockResolvedValue({ acknowledged: true }); @@ -106,6 +111,33 @@ describe('POST /connect/hosted-mcp/:entryId/operator-key', () => { expect(listHostedMcpTools).not.toHaveBeenCalled(); }); + test('a bot with no agent identity is refused before the probe', async () => { + User.findOne.mockReturnValue({ select: () => ({ lean: async () => ({ _id: AGENT, botMetadata: {} }) }) }); + const res = await submit(); + expect(res.status).toBe(400); + expect(res.body).toMatchObject({ error: 'invalid_agent' }); + expect(ownsRuntimeIdentity).not.toHaveBeenCalled(); + expect(listHostedMcpTools).not.toHaveBeenCalled(); + }); + + test('an agent the caller did not solely install is refused, and nothing is probed or written', async () => { + ownsRuntimeIdentity.mockResolvedValue({ owned: false, failure: 'another_installer' }); + const res = await submit(); + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: 'agent_not_owned', message: 'you must be the sole installer of this agent' }); + expect(ownsRuntimeIdentity).toHaveBeenCalledWith(OWNER, 'openclaw', 'latch'); + expect(listHostedMcpTools).not.toHaveBeenCalled(); + expect(Integration.findOneAndUpdate).not.toHaveBeenCalled(); + expect(connectorSecrets.put).not.toHaveBeenCalled(); + }); + + test('the sole-installer rule has no admin exemption: the check runs whatever the caller role', async () => { + ownsRuntimeIdentity.mockResolvedValue({ owned: false, failure: 'owner_installation_missing' }); + const res = await submit(); + expect(res.status).toBe(403); + expect(ownsRuntimeIdentity).toHaveBeenCalledTimes(1); + }); + test('a valid key writes a pending row, stores the secret on it, then connects it', async () => { const res = await submit(); expect(res.status).toBe(201); diff --git a/backend/routes/hostedMcpOperatorKey.ts b/backend/routes/hostedMcpOperatorKey.ts index fc45aafb7..8ac27ee27 100644 --- a/backend/routes/hostedMcpOperatorKey.ts +++ b/backend/routes/hostedMcpOperatorKey.ts @@ -4,6 +4,7 @@ import { isHostedMcpEntryHidden } from '../integrations/hostedMcp/visibility'; import { refusedOperatorKeyScopes, validateOperatorKey } from '../services/hostedMcpOperatorKeyIntake'; import { guardedFetch } from '../services/hostedMcpToolDefinitions'; import { secretWriteGeneration } from '../services/hostedMcpSecretGeneration'; +import { ownsRuntimeIdentity } from '../services/runtimeIdentityOwnership'; const Integration = require('../models/Integration'); const User = require('../models/User'); @@ -39,10 +40,18 @@ export const submitOperatorKey = async (req: OperatorKeyRequest, res: Response) if (!/^[a-f\d]{24}$/i.test(agentId)) { return res.status(400).json({ error: 'invalid_agent', message: 'agentId must name an agent' }); } - const agent = await User.findOne({ _id: agentId, isBot: true }).select('_id').lean(); - if (!agent) { + const agent = await User.findOne({ _id: agentId, isBot: true }).select('_id botMetadata').lean(); + const agentName = agent?.botMetadata?.agentName; + const instanceId = agent?.botMetadata?.instanceId; + if (!agent || !agentName || !instanceId) { return res.status(400).json({ error: 'invalid_agent', message: 'agentId must name an agent' }); } + // The key is a credential the agent presents, so it takes the same sole-installer + // rule as a runtime token. Admins are not exempt. + const ownership = await ownsRuntimeIdentity(userId, agentName, instanceId); + if (!ownership.owned) { + return res.status(403).json({ error: 'agent_not_owned', message: 'you must be the sole installer of this agent' }); + } const picks = Array.isArray(body.scopePicks) ? body.scopePicks.map(String) : []; const refused = refusedOperatorKeyScopes(picks); From 88239595a3ad95b5afdd6871ae2d7a99df82e614 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 31 Oct 2026 00:47:34 -0700 Subject: [PATCH 07/11] fix(connectors): paste-key ownership arms run the real sole-installer rule (TASK-330) The route's 403 now carries the ownership failure code, and the agent name is lowercased as credentials.ts does. The route test mocks the installation and pod data instead of the helper, so ownsRuntimeIdentity runs for real. Arms: the sole owner passes; a caller with no installation, a second installer, and an agent-dm installation are each refused 403 before any probe or write. Disabling the guard reddens the three refusal arms. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- .../routes/hostedMcpOperatorKey.route.test.js | 91 +++++++++++++------ backend/routes/hostedMcpOperatorKey.ts | 13 ++- 2 files changed, 75 insertions(+), 29 deletions(-) diff --git a/backend/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js b/backend/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js index 46c747b0c..60b9fefbd 100644 --- a/backend/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js +++ b/backend/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js @@ -1,8 +1,9 @@ /** - * TASK-330 paste-key intake route. The row, the agent check, the secret store - * and the vendor probe are mocked at their module edges; the handler, the - * scope refusal and the intake probe are real. The key must never come back in - * a response or reach the row, and the scope the route reports must be a + * TASK-330 paste-key intake route. The row, the secret store, the vendor probe + * and the installation data are mocked at their module edges; the handler, the + * scope refusal, the intake probe and the sole-installer rule + * (services/runtimeIdentityOwnership.ts) are real. The key must never come back + * in a response or reach the row, and the scope the route reports must be a * self-report whatever the body claims (Vera's review of 2d4e37fc7). */ const express = require('express'); @@ -10,24 +11,30 @@ const request = require('supertest'); jest.mock('../../../models/Integration', () => ({ findOneAndUpdate: jest.fn(), updateOne: jest.fn() })); jest.mock('../../../models/User', () => ({ findOne: jest.fn() })); +jest.mock('../../../models/AgentRegistry', () => ({ AgentInstallation: { find: jest.fn() } })); +jest.mock('../../../models/Pod', () => ({ find: jest.fn() })); jest.mock('../../../services/connectorSecrets', () => ({ put: jest.fn() })); jest.mock('../../../services/hostedMcpToolDefinitions', () => ({ guardedFetch: jest.fn(), listHostedMcpTools: jest.fn(), })); -jest.mock('../../../services/runtimeIdentityOwnership', () => ({ ownsRuntimeIdentity: jest.fn() })); const Integration = require('../../../models/Integration'); const User = require('../../../models/User'); +const { AgentInstallation } = require('../../../models/AgentRegistry'); +const Pod = require('../../../models/Pod'); const connectorSecrets = require('../../../services/connectorSecrets'); const { listHostedMcpTools } = require('../../../services/hostedMcpToolDefinitions'); -const { ownsRuntimeIdentity } = require('../../../services/runtimeIdentityOwnership'); const { HOSTED_MCP_ENTRIES } = require('../../../integrations/hostedMcp/entries'); const { submitOperatorKey } = require('../../../routes/hostedMcpOperatorKey'); const OWNER = 'bbbbbbbbbbbbbbbbbbbbbb01'; +const OTHER = 'bbbbbbbbbbbbbbbbbbbbbb02'; const AGENT = 'cccccccccccccccccccccc01'; const ROW_ID = 'eeeeeeeeeeeeeeeeeeeeee02'; +const POD = 'aaaaaaaaaaaaaaaaaaaaaa01'; +const OTHER_POD = 'aaaaaaaaaaaaaaaaaaaaaa03'; +const DM_POD = 'aaaaaaaaaaaaaaaaaaaaaa02'; const KEY = 'lk_live_SECRET_VALUE_DO_NOT_ECHO'; const KEY_ENTRY = { @@ -60,14 +67,23 @@ const submit = (body = {}) => request(app) .set('x-test-user', OWNER) .send({ agentId: AGENT, key: KEY, ...body }); +// The installations of the agent identity, and which of their pods are agent-dm. +let installations; +let dmPods; + let shipped; beforeEach(() => { jest.clearAllMocks(); shipped = HOSTED_MCP_ENTRIES.splice(0, HOSTED_MCP_ENTRIES.length, KEY_ENTRY); User.findOne.mockReturnValue({ - select: () => ({ lean: async () => ({ _id: AGENT, botMetadata: { agentName: 'openclaw', instanceId: 'latch' } }) }), + select: () => ({ lean: async () => ({ _id: AGENT, botMetadata: { agentName: 'OpenClaw', instanceId: 'latch' } }) }), }); - ownsRuntimeIdentity.mockResolvedValue({ owned: true }); + installations = [{ installedBy: OWNER, podId: POD }]; + dmPods = []; + AgentInstallation.find.mockImplementation(() => ({ + collation: () => ({ select: () => ({ lean: async () => installations }) }), + })); + Pod.find.mockImplementation(() => ({ select: () => ({ lean: async () => dmPods }) })); listHostedMcpTools.mockResolvedValue([{ name: 'list_jobs' }]); Integration.findOneAndUpdate.mockResolvedValue({ _id: ROW_ID, config: { authGeneration: 1 } }); Integration.updateOne.mockResolvedValue({ acknowledged: true }); @@ -116,37 +132,66 @@ describe('POST /connect/hosted-mcp/:entryId/operator-key', () => { const res = await submit(); expect(res.status).toBe(400); expect(res.body).toMatchObject({ error: 'invalid_agent' }); - expect(ownsRuntimeIdentity).not.toHaveBeenCalled(); + expect(AgentInstallation.find).not.toHaveBeenCalled(); expect(listHostedMcpTools).not.toHaveBeenCalled(); }); - test('an agent the caller did not solely install is refused, and nothing is probed or written', async () => { - ownsRuntimeIdentity.mockResolvedValue({ owned: false, failure: 'another_installer' }); + test('the sole owner of the agent passes, and the identity is read from the bot row, lowercased', async () => { + const res = await submit(); + expect(res.status).toBe(201); + expect(res.body).toEqual({ + integrationId: ROW_ID, status: 'connected', scopeCheck: 'self_report', toolCount: 1, + }); + expect(AgentInstallation.find).toHaveBeenCalledWith(expect.objectContaining({ agentName: 'openclaw', instanceId: 'latch' })); + }); + + test('a caller with no installation of the agent is refused 403 before any probe or write', async () => { + installations = []; const res = await submit(); expect(res.status).toBe(403); - expect(res.body).toEqual({ error: 'agent_not_owned', message: 'you must be the sole installer of this agent' }); - expect(ownsRuntimeIdentity).toHaveBeenCalledWith(OWNER, 'openclaw', 'latch'); + expect(res.body).toEqual({ + error: 'agent_not_owned', + failure: 'owner_installation_missing', + message: 'you must be the sole installer of this agent', + }); expect(listHostedMcpTools).not.toHaveBeenCalled(); expect(Integration.findOneAndUpdate).not.toHaveBeenCalled(); expect(connectorSecrets.put).not.toHaveBeenCalled(); }); - test('the sole-installer rule has no admin exemption: the check runs whatever the caller role', async () => { - ownsRuntimeIdentity.mockResolvedValue({ owned: false, failure: 'owner_installation_missing' }); + test('an agent with a second installer is refused 403, even when the caller also installed it', async () => { + installations = [ + { installedBy: OWNER, podId: POD }, + { installedBy: OTHER, podId: OTHER_POD }, + ]; + const res = await submit(); + expect(res.status).toBe(403); + expect(res.body).toMatchObject({ error: 'agent_not_owned', failure: 'another_installer' }); + expect(listHostedMcpTools).not.toHaveBeenCalled(); + expect(Integration.findOneAndUpdate).not.toHaveBeenCalled(); + }); + + test('an agent-dm installation is a conversation, not ownership: the caller is refused', async () => { + installations = [{ installedBy: OWNER, podId: DM_POD }]; + dmPods = [{ _id: DM_POD }]; const res = await submit(); expect(res.status).toBe(403); - expect(ownsRuntimeIdentity).toHaveBeenCalledTimes(1); + expect(res.body).toMatchObject({ error: 'agent_not_owned', failure: 'owner_installation_missing' }); + expect(listHostedMcpTools).not.toHaveBeenCalled(); + }); + + test('scopeCheck is a self-report even when the body claims a readback', async () => { + const res = await submit({ scopeCheck: 'read', reportedScopes: ['read'] }); + expect(res.status).toBe(201); + expect(res.body.scopeCheck).toBe('self_report'); }); test('a valid key writes a pending row, stores the secret on it, then connects it', async () => { const res = await submit(); expect(res.status).toBe(201); - expect(res.body).toEqual({ - integrationId: ROW_ID, status: 'connected', scopeCheck: 'self_report', toolCount: 1, - }); expect(Integration.findOneAndUpdate).toHaveBeenCalledWith( { - type: 'hosted-mcp', createdBy: OWNER, 'config.entryId': 'latchkey', 'config.agentId': AGENT, + type: 'hosted-mcp', createdBy: OWNER, 'config.entryId': 'latchkey', 'config.agentId': AGENT, }, expect.objectContaining({ $set: expect.objectContaining({ status: 'pending' }) }), expect.objectContaining({ upsert: true }), @@ -158,12 +203,6 @@ describe('POST /connect/hosted-mcp/:entryId/operator-key', () => { ); }); - test('scopeCheck is a self-report even when the body claims a readback', async () => { - const res = await submit({ scopeCheck: 'read', reportedScopes: ['read'] }); - expect(res.status).toBe(201); - expect(res.body.scopeCheck).toBe('self_report'); - }); - test('the key appears in no response, no row write, and no thrown error text', async () => { const ok = await submit(); expect(JSON.stringify(ok.body)).not.toContain(KEY); diff --git a/backend/routes/hostedMcpOperatorKey.ts b/backend/routes/hostedMcpOperatorKey.ts index 8ac27ee27..87f20d5d6 100644 --- a/backend/routes/hostedMcpOperatorKey.ts +++ b/backend/routes/hostedMcpOperatorKey.ts @@ -41,8 +41,11 @@ export const submitOperatorKey = async (req: OperatorKeyRequest, res: Response) return res.status(400).json({ error: 'invalid_agent', message: 'agentId must name an agent' }); } const agent = await User.findOne({ _id: agentId, isBot: true }).select('_id botMetadata').lean(); - const agentName = agent?.botMetadata?.agentName; - const instanceId = agent?.botMetadata?.instanceId; + // The identity comes from the bot's own row, never the body, and is lowercased + // as credentials.ts does. Refused before the probe, so a refused caller's key + // never reaches the vendor. + const agentName = String(agent?.botMetadata?.agentName || '').toLowerCase(); + const instanceId = String(agent?.botMetadata?.instanceId || ''); if (!agent || !agentName || !instanceId) { return res.status(400).json({ error: 'invalid_agent', message: 'agentId must name an agent' }); } @@ -50,7 +53,11 @@ export const submitOperatorKey = async (req: OperatorKeyRequest, res: Response) // rule as a runtime token. Admins are not exempt. const ownership = await ownsRuntimeIdentity(userId, agentName, instanceId); if (!ownership.owned) { - return res.status(403).json({ error: 'agent_not_owned', message: 'you must be the sole installer of this agent' }); + return res.status(403).json({ + error: 'agent_not_owned', + failure: ownership.failure, + message: 'you must be the sole installer of this agent', + }); } const picks = Array.isArray(body.scopePicks) ? body.scopePicks.map(String) : []; From 7f591c20a6243a51b2ac1b3478946a272e58fece Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 31 Oct 2026 01:08:00 -0700 Subject: [PATCH 08/11] refactor(hosted-mcp): split HostedMcpEntry into an OAuth/operator-key union HostedMcpEntry is now HostedMcpOAuthEntry | HostedMcpOperatorKeyEntry. OAuth fields (issuer, client, scopes, authorizationParams) exist only on the OAuth member, so a reader of an operator-key entry can no longer read them by accident. isOAuthEntry narrows the union. OAuth-only paths take HostedMcpOAuthEntry: token refresh, revoke, the DCR registry, the client-id backfill. Removal's vendor revoke runs only for OAuth entries, so an operator key is never sent to an RFC 7009 endpoint. isHostedMcpClientConfigured accepts any entry; an operator-key entry has no instance client to preconfigure. The compiler is the census: tsconfig.build.json has no new errors against the 19-error baseline. Related suites pass (37 files, 732 tests). Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- backend/integrations/hostedMcp/entries.ts | 3 +- .../scripts/backfill-hosted-mcp-client-id.ts | 3 +- backend/services/connectionRemovalService.ts | 15 +++--- backend/services/hostedMcpClientIdBackfill.ts | 7 +-- .../services/hostedMcpCredentialService.ts | 8 +-- backend/services/hostedMcpDcrRegistry.ts | 12 ++--- backend/services/hostedMcpEntryService.ts | 52 +++++++++++++------ backend/services/hostedMcpIntakeService.ts | 24 +++++---- 8 files changed, 77 insertions(+), 47 deletions(-) diff --git a/backend/integrations/hostedMcp/entries.ts b/backend/integrations/hostedMcp/entries.ts index b969b774d..a6b01a529 100644 --- a/backend/integrations/hostedMcp/entries.ts +++ b/backend/integrations/hostedMcp/entries.ts @@ -11,6 +11,7 @@ * verify the URL, transport, auth mode, token lifetime, refresh and revocation, * and their pinned read tools claim `readOnlyHint: true` (§3, §11). */ +import { isOAuthEntry } from '../../services/hostedMcpEntryService'; import type { HostedMcpEntry } from '../../services/hostedMcpEntryService'; import { AIRTABLE_ENTRY } from './airtable'; import { ATLASSIAN_ENTRY } from './atlassian'; @@ -168,7 +169,7 @@ export const assertHostedMcpEntries = ( if (reservedIds.has(entry.id)) { throw new Error(`hosted-mcp entry id collides with a builtin tool installable: ${entry.id}`); } - if (entry.authorizationParams !== undefined) { + if (isOAuthEntry(entry) && entry.authorizationParams !== undefined) { if (!entry.authorizationParams || typeof entry.authorizationParams !== 'object' || Array.isArray(entry.authorizationParams)) { throw new Error(`hosted-mcp entry authorization parameters are not an object: ${entry.id}`); diff --git a/backend/scripts/backfill-hosted-mcp-client-id.ts b/backend/scripts/backfill-hosted-mcp-client-id.ts index a60365be9..ae6eef676 100644 --- a/backend/scripts/backfill-hosted-mcp-client-id.ts +++ b/backend/scripts/backfill-hosted-mcp-client-id.ts @@ -3,6 +3,7 @@ import mongoose from 'mongoose'; import Integration from '../models/Integration'; import { HOSTED_MCP_ENTRIES } from '../integrations/hostedMcp/entries'; +import { isOAuthEntry } from '../services/hostedMcpEntryService'; import { resolvedClientId } from '../services/hostedMcpIntakeService'; import { backfillHostedMcpClientIds, @@ -36,7 +37,7 @@ const main = async (): Promise => { .select('_id config.entryId config.clientId config.credentialRef config.refreshTokenRef') .lean(); - const entries = new Map(HOSTED_MCP_ENTRIES.map((entry) => [entry.id, entry])); + const entries = new Map(HOSTED_MCP_ENTRIES.filter(isOAuthEntry).map((entry) => [entry.id, entry])); const result = await backfillHostedMcpClientIds({ findLegacyRows: async () => rows as unknown as HostedMcpClientIdBackfillRow[], entryFor: (entryId) => (LEGACY_ENTRY_IDS.has(entryId) ? entries.get(entryId) : undefined), diff --git a/backend/services/connectionRemovalService.ts b/backend/services/connectionRemovalService.ts index b70bccbb7..035e29795 100644 --- a/backend/services/connectionRemovalService.ts +++ b/backend/services/connectionRemovalService.ts @@ -49,7 +49,8 @@ import { findHostedMcpEntry, hostedMcpRevokeTarget, } from '../integrations/hostedMcp/entries'; -import type { HostedMcpEntry } from './hostedMcpEntryService'; +import { isOAuthEntry } from './hostedMcpEntryService'; +import type { HostedMcpEntry, HostedMcpOAuthEntry } from './hostedMcpEntryService'; import { buildRevocationRequest, discoverAuthorizationServer, @@ -103,12 +104,12 @@ export interface RemovalDeps { /** Step 3b, between the provider revoke and any material going. Idempotent. */ markProviderRevoked: (id: unknown, at: Date) => Promise; entryFor: (connection: RemovableConnection) => HostedMcpEntry | null; - clientIdFor: (entry: HostedMcpEntry) => string; - clientSecretFor: (entry: HostedMcpEntry) => string | undefined; + clientIdFor: (entry: HostedMcpOAuthEntry) => string; + clientSecretFor: (entry: HostedMcpOAuthEntry) => string | undefined; dcrClientForRow: typeof dcrClientForRow; /** Step 3. Resolves when the authorization is gone; throws when it may still be live. */ revokeAtVendor: (input: { - entry: HostedMcpEntry; + entry: HostedMcpOAuthEntry; clientId: string; clientSecret?: string; /** A DCR confidential client's registered method (TASK-248); sent as recorded. */ @@ -165,7 +166,7 @@ export type RemovalResult = * endpoint that refused. */ export const revokeTokenAtVendor = async (input: { - entry: HostedMcpEntry; + entry: HostedMcpOAuthEntry; clientId: string; clientSecret?: string; tokenEndpointAuthMethod?: HostedMcpClientConfiguration['tokenEndpointAuthMethod']; @@ -318,7 +319,9 @@ export const removeConnection = async (options: { // the material may be half-swept, and the token is already dead. const alreadyRevokedAtVendor = Boolean(connection?.config?.[PROVIDER_REVOKED_MARK]); - if (entry && target?.endpoint && !alreadyRevokedAtVendor) { + // Only OAuth entries have a vendor revoke. An operator key is a static bearer + // with no refresh token, so it is never sent to an RFC 7009 endpoint. + if (entry && isOAuthEntry(entry) && target?.endpoint && !alreadyRevokedAtVendor) { // The row records which client minted this pair; the instance's current // configuration remains the source for client credentials. A recorded id // that differs from the current one must never send a held token under the diff --git a/backend/services/hostedMcpClientIdBackfill.ts b/backend/services/hostedMcpClientIdBackfill.ts index f6230b3d9..dd1c55284 100644 --- a/backend/services/hostedMcpClientIdBackfill.ts +++ b/backend/services/hostedMcpClientIdBackfill.ts @@ -1,4 +1,4 @@ -import type { HostedMcpEntry } from './hostedMcpEntryService'; +import type { HostedMcpOAuthEntry } from './hostedMcpEntryService'; export interface HostedMcpClientIdBackfillRow { _id: unknown; @@ -12,8 +12,9 @@ export interface HostedMcpClientIdBackfillRow { export interface HostedMcpClientIdBackfillDeps { findLegacyRows: () => Promise; - entryFor: (entryId: string) => HostedMcpEntry | undefined; - clientIdFor: (entry: HostedMcpEntry) => string; + /** Legacy rows hold a token pair, so only OAuth entries can be backfilled. */ + entryFor: (entryId: string) => HostedMcpOAuthEntry | undefined; + clientIdFor: (entry: HostedMcpOAuthEntry) => string; /** Must update only while `config.clientId` is still absent or blank. */ updateIfMissing: (row: HostedMcpClientIdBackfillRow, clientId: string) => Promise; } diff --git a/backend/services/hostedMcpCredentialService.ts b/backend/services/hostedMcpCredentialService.ts index fffd96a7c..ea0056e73 100644 --- a/backend/services/hostedMcpCredentialService.ts +++ b/backend/services/hostedMcpCredentialService.ts @@ -48,7 +48,7 @@ import Integration from '../models/Integration'; import { HOSTED_MCP_ENTRIES, findHostedMcpEntry } from '../integrations/hostedMcp/entries'; -import type { HostedMcpEntry } from './hostedMcpEntryService'; +import type { HostedMcpEntry, HostedMcpOAuthEntry } from './hostedMcpEntryService'; import { buildRefreshRequest, discoverAuthorizationServer, @@ -116,13 +116,13 @@ interface RefreshResult { interface CredentialDeps { now: () => Date; entryFor: (row: HostedMcpRow) => HostedMcpEntry; - clientIdFor: (entry: HostedMcpEntry) => string; - clientSecretFor: (entry: HostedMcpEntry) => string | undefined; + clientIdFor: (entry: HostedMcpOAuthEntry) => string; + clientSecretFor: (entry: HostedMcpOAuthEntry) => string | undefined; dcrClientForRow: typeof dcrClientForRow; /** Conditional on the client-id snapshot, so a concurrent reconnect wins. */ markClientMismatch: (id: unknown, recordedClientId: string | undefined, message: string) => Promise; refreshAtVendor: (input: { - entry: HostedMcpEntry; + entry: HostedMcpOAuthEntry; clientId: string; clientSecret?: string; /** A DCR confidential client's registered method (TASK-248); sent as recorded. */ diff --git a/backend/services/hostedMcpDcrRegistry.ts b/backend/services/hostedMcpDcrRegistry.ts index 9ee39d64b..3a59ee38f 100644 --- a/backend/services/hostedMcpDcrRegistry.ts +++ b/backend/services/hostedMcpDcrRegistry.ts @@ -14,7 +14,7 @@ */ import { Types } from 'mongoose'; import HostedMcpDcrRegistration, { IHostedMcpDcrRegistration } from '../models/HostedMcpDcrRegistration'; -import type { HostedMcpEntry } from './hostedMcpEntryService'; +import type { HostedMcpOAuthEntry } from './hostedMcpEntryService'; import { CONFIDENTIAL_CLIENT_METHODS, HostedMcpClientError, @@ -37,7 +37,7 @@ type RegistrationMethod = 'none' | ConfidentialMethod; */ export const CLIENT_SECRET_RENEW_MARGIN_MS = 24 * 60 * 60 * 1000; -const currentFilter = (entry: HostedMcpEntry) => ({ +const currentFilter = (entry: HostedMcpOAuthEntry) => ({ entryId: entry.id, issuer: entry.issuer, current: true, @@ -69,7 +69,7 @@ const clientFromRow = async (row: RegistrationRow): Promise { +const registrationMethod = (entry: HostedMcpOAuthEntry, metadata: HostedMcpAuthorizationServer): RegistrationMethod => { const supported = metadata.token_endpoint_auth_methods_supported; // RFC 8414 defaults an omitted list to client_secret_basic, but this registry // registers only a method the vendor names: never POST hoping it accepts one. @@ -90,7 +90,7 @@ const registrationMethod = (entry: HostedMcpEntry, metadata: HostedMcpAuthorizat * client for new starts; rows already minted by it continue to use it. */ export const ensureDcrClient = async ( - entry: HostedMcpEntry, + entry: HostedMcpOAuthEntry, metadata: HostedMcpAuthorizationServer, redirectUri: string, fetchImpl: typeof fetch = hostedVendorFetch, @@ -249,7 +249,7 @@ export const ensureDcrClient = async ( /** A row may use a retired registration, but never one the vendor refused. */ export const dcrClientForRow = async ( - entry: HostedMcpEntry, + entry: HostedMcpOAuthEntry, recordedClientId: string | undefined, ): Promise => { if (entry.client !== 'dcr' || !recordedClientId) return null; @@ -263,7 +263,7 @@ export const dcrClientForRow = async ( }; /** Invalid client on token/revoke poisons this registration, not every row. */ -export const refuseDcrClient = async (entry: HostedMcpEntry, clientId: string): Promise => { +export const refuseDcrClient = async (entry: HostedMcpOAuthEntry, clientId: string): Promise => { if (entry.client !== 'dcr') return; const now = new Date(); const refused = await HostedMcpDcrRegistration.findOneAndUpdate( diff --git a/backend/services/hostedMcpEntryService.ts b/backend/services/hostedMcpEntryService.ts index 0b3d2cf3b..4a8082539 100644 --- a/backend/services/hostedMcpEntryService.ts +++ b/backend/services/hostedMcpEntryService.ts @@ -61,16 +61,10 @@ export interface HostedMcpPinnedTool { annotations?: HostedMcpAnnotations; } -export interface HostedMcpEntry { +/** Fields every hosted entry carries, whatever its credential. */ +export interface HostedMcpEntryCommon { /** Namespaces every tool name (`.`) and names the row's app. */ id: string; - /** - * How the row's credential is obtained. Absent means OAuth, which every - * shipped entry uses. `operator_key` is a static bearer the operator pastes - * for one agent (Latchkey, TASK-330): it has no refresh, so `credentialFor` - * serves it without the freshness rule or the fence. - */ - credentialMode?: 'oauth' | 'operator_key'; title: string; /** * The line the Tools page draws under the title. Ours, like a tool's @@ -84,6 +78,22 @@ export interface HostedMcpEntry { resource: string; /** The PRM resource identifier when it differs from the tools/call endpoint. */ audience?: string; + /** + * Where the authorization dies, naming its kind because the URL cannot (§3). + * Exactly one key: an `endpoint` is the RFC 7009 revocation endpoint the AS's + * metadata advertises, a `page` is where a person revokes by hand. Both are + * https, and only the kind decides whether removal calls the vendor. + */ + revoke: { page: string; endpoint?: string }; + tools: HostedMcpPinnedTool[]; +} + +/** + * OAuth entries: every shipped one. The authorization server and the client + * shape belong to the OAuth flow, so they live only on this member. + */ +export interface HostedMcpOAuthEntry extends HostedMcpEntryCommon { + credentialMode?: 'oauth'; /** The authorization server the protected-resource metadata names. */ issuer: string; client: 'pre-registered' | 'cimd' | 'dcr'; @@ -93,16 +103,21 @@ export interface HostedMcpEntry { access_type?: string; prompt?: string; }; - /** - * Where the authorization dies, naming its kind because the URL cannot (§3). - * Exactly one key: an `endpoint` is the RFC 7009 revocation endpoint the AS's - * metadata advertises, a `page` is where a person revokes by hand. Both are - * https, and only the kind decides whether removal calls the vendor. - */ - revoke: { page: string; endpoint?: string }; - tools: HostedMcpPinnedTool[]; } +/** + * Operator-key entries (TASK-330): a static bearer the operator pastes for one + * agent. It has no refresh, so `credentialFor` serves it without the freshness + * rule or the fence. No issuer, client or scopes exist for it, so those fields + * are absent from this member: reading one off the union is a compile error, + * which is the census check. + */ +export interface HostedMcpOperatorKeyEntry extends HostedMcpEntryCommon { + credentialMode: 'operator_key'; +} + +export type HostedMcpEntry = HostedMcpOAuthEntry | HostedMcpOperatorKeyEntry; + /** One tool as the vendor's `tools/list` reports it. */ export interface HostedMcpUpstreamTool { name: string; @@ -459,3 +474,8 @@ export const createHostedMcpDriftCache = (options: HostedMcpDriftCacheOptions): }, }; }; + +/** True for the OAuth members of the union. The OAuth flow and its readers narrow with this, not a cast. */ +export const isOAuthEntry = (entry: HostedMcpEntry): entry is HostedMcpOAuthEntry => ( + entry.credentialMode !== 'operator_key' +); diff --git a/backend/services/hostedMcpIntakeService.ts b/backend/services/hostedMcpIntakeService.ts index 57d247db5..e64dcc834 100644 --- a/backend/services/hostedMcpIntakeService.ts +++ b/backend/services/hostedMcpIntakeService.ts @@ -24,7 +24,8 @@ * mix-up (§4). */ import { createHash, randomBytes, timingSafeEqual } from 'crypto'; -import type { HostedMcpEntry } from './hostedMcpEntryService'; +import { isOAuthEntry } from './hostedMcpEntryService'; +import type { HostedMcpEntry, HostedMcpOAuthEntry } from './hostedMcpEntryService'; import { upstreamFetch } from './upstreamFetch'; import { hostedVendorFetch } from './hostedVendorFetch'; @@ -106,7 +107,7 @@ export interface HostedMcpClientMetadataDocument { } export const buildClientMetadataDocument = ( - entry: HostedMcpEntry, + entry: HostedMcpOAuthEntry, apiBase: string = hostedMcpApiBase(), ): HostedMcpClientMetadataDocument => { const clientId = entryPath(apiBase, entry.id, CLIENT_METADATA_LEAF); @@ -173,7 +174,7 @@ export const hostedMcpClientConfigKeys = (entryId: string) => { * the authorization boundary, which is where it should be loud. */ export const resolvedClientId = ( - entry: HostedMcpEntry, + entry: HostedMcpOAuthEntry, env: NodeJS.ProcessEnv = process.env, ): string => { if (entry.client === 'cimd') return hostedMcpClientMetadataUrl(entry.id); @@ -196,7 +197,7 @@ export const resolvedClientId = ( /** A client secret exists only for the pre-registered confidential-client kind. */ export const resolvedClientSecret = ( - entry: HostedMcpEntry, + entry: HostedMcpOAuthEntry, env: NodeJS.ProcessEnv = process.env, ): string | undefined => { if (entry.client !== 'pre-registered') return undefined; @@ -213,7 +214,7 @@ export const resolvedClientSecret = ( /** Resolve all instance-held client material before offering or using the entry. */ export const resolvedHostedMcpClient = ( - entry: HostedMcpEntry, + entry: HostedMcpOAuthEntry, env: NodeJS.ProcessEnv = process.env, ): HostedMcpClientConfiguration => { const clientId = resolvedClientId(entry, env); @@ -225,6 +226,9 @@ export const isHostedMcpClientConfigured = ( entry: HostedMcpEntry, env: NodeJS.ProcessEnv = process.env, ): boolean => { + // An operator-key entry has no instance client: its key is per agent, and is + // proven at connect time, so there is nothing to preconfigure here. + if (!isOAuthEntry(entry)) return true; // DCR has no instance credential to preconfigure. Vendor support is checked // against discovered metadata on the first start, before any pending row. if (entry.client === 'dcr') return true; @@ -348,7 +352,7 @@ export const discoverAuthorizationServer = async ( * one function pair rather than by two call sites that could drift. */ export const buildAuthorizeUrl = ( - entry: HostedMcpEntry, + entry: HostedMcpOAuthEntry, authorizationEndpoint: string, params: { clientId: string; redirectUri: string; state: string; codeChallenge: string }, ): string => { @@ -421,7 +425,7 @@ const confidentialAuthentication = ( }; const endpointAuthentication = ( - entry: HostedMcpEntry, + entry: HostedMcpOAuthEntry, metadata: Partial, client: HostedMcpClientConfiguration, endpoint: HostedMcpEndpointKind, @@ -470,7 +474,7 @@ const endpointAuthentication = ( /** Authorization-code exchange: PKCE and resource survive every client kind. */ export const buildTokenExchangeRequest = ( - entry: HostedMcpEntry, + entry: HostedMcpOAuthEntry, metadata: Partial, client: HostedMcpClientConfiguration, params: { redirectUri: string; code: string; codeVerifier: string }, @@ -491,7 +495,7 @@ export const buildTokenExchangeRequest = ( /** Refresh uses the same client and endpoint-authentication rule as the exchange. */ export const buildRefreshRequest = ( - entry: HostedMcpEntry, + entry: HostedMcpOAuthEntry, metadata: Partial, client: HostedMcpClientConfiguration, params: { refreshToken: string }, @@ -510,7 +514,7 @@ export const buildRefreshRequest = ( /** RFC 7009 request: the same client must authenticate the revocation. */ export const buildRevocationRequest = ( - entry: HostedMcpEntry, + entry: HostedMcpOAuthEntry, metadata: Partial, client: HostedMcpClientConfiguration, params: { token: string; tokenTypeHint: 'refresh_token' | 'access_token' }, From d050ee1ccef2a9d93911c7ca0976c1ac62cd6827 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 31 Oct 2026 01:11:32 -0700 Subject: [PATCH 09/11] fix(hosted-mcp): type the OAuth connect route and refuse operator-key entries there The connect route loaded the entry registry, intake service and entry guard through untyped require(), so tsc saw every entry as any and the OAuth-only reads (client, issuer, scopes) in /start, the callback and client-metadata went unchecked. An operator-key entry was refused only by accident. Typed requires make the route visible to tsc. Each handler now refuses a non-OAuth entry before reading an OAuth field: client-metadata returns 404 client_metadata_not_used, /start returns 404 unknown_entry, and the callback redirects with unknown_entry. Operator keys connect through their own paste route. tsc: no new errors against the baseline. Lint: 0 errors. Related route and service suites pass (15 files, 284 tests). Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- backend/routes/hostedMcpConnect.ts | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/backend/routes/hostedMcpConnect.ts b/backend/routes/hostedMcpConnect.ts index 55e69f73b..924f6b952 100644 --- a/backend/routes/hostedMcpConnect.ts +++ b/backend/routes/hostedMcpConnect.ts @@ -39,7 +39,14 @@ const { revokeConnectionGrants } = require('../services/roomGrantService'); // eslint-disable-next-line global-require const { revokeTokenAtVendor } = require('../services/connectionRemovalService'); // eslint-disable-next-line global-require -const { HOSTED_MCP_ENTRIES, findHostedMcpEntry, hostedMcpRevokeTarget } = require('../integrations/hostedMcp/entries'); +// Typed requires: tsc checks this route against the real module shapes, so an +// OAuth-only field read on the HostedMcpEntry union fails the build. +const { + HOSTED_MCP_ENTRIES, findHostedMcpEntry, hostedMcpRevokeTarget, +}: typeof import('../integrations/hostedMcp/entries') = require('../integrations/hostedMcp/entries'); +const { + isOAuthEntry, +}: typeof import('../services/hostedMcpEntryService') = require('../services/hostedMcpEntryService'); const { isHostedMcpEntryHidden } = require('../integrations/hostedMcp/visibility'); // eslint-disable-next-line global-require const { @@ -57,7 +64,7 @@ const { hostedMcpNonceCookiePath, readCookie, resolvedHostedMcpClient, -} = require('../services/hostedMcpIntakeService'); +}: typeof import('../services/hostedMcpIntakeService') = require('../services/hostedMcpIntakeService'); type AuthedRequest = Request & { user?: { id?: string; role?: string } }; @@ -218,7 +225,7 @@ router.get('/:entryId/client-metadata', clientMetadataRateLimit, (req: Request, // still accept as a client with nowhere to send a code. return res.status(404).json({ error: 'unknown_entry' }); } - if (entry.client !== 'cimd') { + if (!isOAuthEntry(entry) || entry.client !== 'cimd') { return res.status(404).json({ error: 'client_metadata_not_used' }); } return res.json(buildClientMetadataDocument(entry)); @@ -241,7 +248,8 @@ const startConnect = async (req: AuthedRequest, res: Response) => { return res.status(401).json({ error: 'authentication_required' }); } const entry = findHostedMcpEntry(HOSTED_MCP_ENTRIES, req.params.entryId); - if (!entry) { + // An operator-key entry connects through its own paste route, never this flow. + if (!entry || !isOAuthEntry(entry)) { return res.status(404).json({ error: 'unknown_entry' }); } if (isHostedMcpEntryHidden(entry.id, process.env, userId)) return res.status(404).json({ error: 'unknown_entry' }); @@ -382,7 +390,7 @@ interface CallbackFlow { const finishCallback = async (req: Request, res: Response, flow: CallbackFlow) => { const entry = findHostedMcpEntry(HOSTED_MCP_ENTRIES, req.params.entryId); - if (!entry) { + if (!entry || !isOAuthEntry(entry)) { return callbackRedirect(res, 'error', 'unknown_entry'); } const state = typeof req.query.state === 'string' ? req.query.state : ''; From dc4a7ad61713016d256afadf38ba5cd6ecb7de44 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 31 Oct 2026 01:13:08 -0700 Subject: [PATCH 10/11] test(hosted-mcp): pin the /start refusal for an operator-key entry; type the catalog requires installableCatalogService loaded the entry registry and intake guard through untyped require(), so tsc could not check its entry reads. Typed requires make it visible to tsc, and it adds no new errors. hostedMcpConnect.test.js gains an arm for /start on an operator-key entry. It asserts 404 unknown_entry, and that discovery, the row lookup and the row write never run. Removing the isOAuthEntry guard from /start reddens the arm (503). Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- .../__tests__/unit/routes/hostedMcpConnect.test.js | 14 ++++++++++++++ .../installable/installableCatalogService.ts | 8 ++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/backend/__tests__/unit/routes/hostedMcpConnect.test.js b/backend/__tests__/unit/routes/hostedMcpConnect.test.js index 06f4b57cf..fe2f6c603 100644 --- a/backend/__tests__/unit/routes/hostedMcpConnect.test.js +++ b/backend/__tests__/unit/routes/hostedMcpConnect.test.js @@ -368,6 +368,20 @@ describe('hosted-mcp connect: start', () => { entries.HOSTED_MCP_ENTRIES.pop(); }); + it('refuses an operator-key entry before any client, discovery or row read', async () => { + const entries = jest.requireMock('../../../integrations/hostedMcp/entries'); + entries.HOSTED_MCP_ENTRIES.push({ + ...FIXTURE_ENTRY, id: 'latchkey', credentialMode: 'operator_key', client: undefined, + }); + const res = await start('latchkey'); + expect(res.status).toBe(404); + expect(res.body).toEqual({ error: 'unknown_entry' }); + expect(intake.discoverAuthorizationServer).not.toHaveBeenCalled(); + expect(Integration.findOne).not.toHaveBeenCalled(); + expect(Integration.findOneAndUpdate).not.toHaveBeenCalled(); + entries.HOSTED_MCP_ENTRIES.pop(); + }); + it('requires a person, because it is the person who will be consenting', async () => { mockAuthedUser = undefined; const res = await start(); diff --git a/backend/services/installable/installableCatalogService.ts b/backend/services/installable/installableCatalogService.ts index e26e6b403..d8ad1becb 100644 --- a/backend/services/installable/installableCatalogService.ts +++ b/backend/services/installable/installableCatalogService.ts @@ -11,9 +11,13 @@ const { manifests } = require('../../integrations/manifests'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require const { mcpComponentOf, projectTools, toolInstallableMetas } = require('./toolInstallables'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require -const { HOSTED_MCP_ENTRIES, findHostedMcpEntry } = require('../../integrations/hostedMcp/entries'); +const { + HOSTED_MCP_ENTRIES, findHostedMcpEntry, +}: typeof import('../../integrations/hostedMcp/entries') = require('../../integrations/hostedMcp/entries'); const { isHostedMcpEntryHidden } = require('../../integrations/hostedMcp/visibility'); -const { isHostedMcpClientConfigured } = require('../hostedMcpIntakeService'); +const { + isHostedMcpClientConfigured, +}: typeof import('../hostedMcpIntakeService') = require('../hostedMcpIntakeService'); type ProviderReadiness = { available: boolean; reason?: 'not_configured' }; From 77dc4f54ffb508dd484e38cf26ad8df6a3ee9b9a Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 31 Oct 2026 01:16:49 -0700 Subject: [PATCH 11/11] fix(hosted-mcp): type the vendor-revoke require and narrow revokeRefusedGrant revokeTokenAtVendor came through an untyped require(), so revokeRefusedGrant took HostedMcpEntry and still compiled against a callee that takes the OAuth entry. Its parameter is now HostedMcpOAuthEntry, and the require is typed, so tsc checks the call. Both callers already pass the narrowed OAuth entry. tsc: no new errors against the baseline. Connect and removal suites pass. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- backend/routes/hostedMcpConnect.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/backend/routes/hostedMcpConnect.ts b/backend/routes/hostedMcpConnect.ts index 924f6b952..0e0309a16 100644 --- a/backend/routes/hostedMcpConnect.ts +++ b/backend/routes/hostedMcpConnect.ts @@ -16,7 +16,7 @@ const express = require('express'); import type { Request, Response } from 'express'; import type { HostedMcpAuthorizationServer } from '../services/hostedMcpIntakeService'; -import type { HostedMcpEntry } from '../services/hostedMcpEntryService'; +import type { HostedMcpOAuthEntry } from '../services/hostedMcpEntryService'; import rateLimit from 'express-rate-limit'; // eslint-disable-next-line global-require const auth = require('../middleware/auth'); @@ -37,7 +37,9 @@ const { HOSTED_MCP_ACCESS_TOKEN, HOSTED_MCP_REFRESH_TOKEN } = require('../servic // eslint-disable-next-line global-require const { revokeConnectionGrants } = require('../services/roomGrantService'); // eslint-disable-next-line global-require -const { revokeTokenAtVendor } = require('../services/connectionRemovalService'); +const { + revokeTokenAtVendor, +}: typeof import('../services/connectionRemovalService') = require('../services/connectionRemovalService'); // eslint-disable-next-line global-require // Typed requires: tsc checks this route against the real module shapes, so an // OAuth-only field read on the HostedMcpEntry union fails the build. @@ -105,7 +107,7 @@ const parseScopeSet = (scope: string): Set => new Set( /** A refused grant is never stored; revoke its refresh token when the entry can. */ const revokeRefusedGrant = async ( - entry: HostedMcpEntry, + entry: HostedMcpOAuthEntry, client: { clientId: string; clientSecret?: string }, tokens: TokenResponse, ): Promise => {