diff --git a/backend/__tests__/service/hostedMcpAgentIndexMigration.test.js b/backend/__tests__/service/hostedMcpAgentIndexMigration.test.js new file mode 100644 index 0000000000..b0f4372301 --- /dev/null +++ b/backend/__tests__/service/hostedMcpAgentIndexMigration.test.js @@ -0,0 +1,99 @@ +/** + * 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. + * + * 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 = ''; + +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); + +/** 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(); + await Integration.init(); + }); + + beforeEach(async () => { + await Integration.deleteMany({}); + await seedPreMigration(); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + 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 () => { + await Integration.create(keyFor('agent-a')); + await expect(Integration.create(keyFor('agent-b'))).rejects.toMatchObject({ code: 11000 }); + }); + + 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 () => { + 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 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 5777f87685..340ac2be66 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__/service/hostedMcpOperatorKey.perAgent.test.js b/backend/__tests__/service/hostedMcpOperatorKey.perAgent.test.js new file mode 100644 index 0000000000..bacd9270ce --- /dev/null +++ b/backend/__tests__/service/hostedMcpOperatorKey.perAgent.test.js @@ -0,0 +1,306 @@ +/** + * TASK-330: an operator key is ONE agent's bearer. These arms run the real + * broker, the real native hosted turn, the real credential service and the real + * ConnectorSecret store on a real Mongo; only the model (LiteLLM, via + * axios.post) and the vendor's network edge (hostedVendorFetch) are stubbed. + * + * 1. per-agent offer — only the seat the key is bound to and granted for is + * offered the vendor's tool, read off its own model call. + * 2. per-agent bearer — each seat's call reaches the vendor carrying its OWN + * key, and never the other seat's. + * 3. leave empties it — after the bound seat leaves the pod (the product's + * removeAgentFromPod), its run offers no vendor tool. + */ +const { setupMongoDb, closeMongoDb } = require('../utils/testUtils'); + +process.env.CONNECTOR_SECRET_KEYS = `opkey-test:${Buffer.alloc(32, 5).toString('base64')}`; +process.env.CONNECTOR_SECRET_ACTIVE_KEY = 'opkey-test'; + +jest.mock('axios', () => ({ __esModule: true, default: { post: jest.fn() } })); +jest.mock('../../models/pg/Message', () => ({ findByPodId: jest.fn().mockResolvedValue([]) })); +jest.mock('../../services/agentTypingService', () => ({ + emitAgentTypingStart: jest.fn(), emitAgentTypingStop: jest.fn(), +})); +jest.mock('../../services/agentMessageService', () => ({ postMessage: jest.fn().mockResolvedValue({ success: true }) })); +jest.mock('../../services/agentComputerBroker', () => ({ agentComputerBroker: {}, agentComputerEnabled: false })); +// The ToolCall ledger lives in PostgreSQL, which this tier does not run; the +// same in-memory stand-in teammate-approval.test.js uses. It decides nothing: +// the budget always admits, and the rows are only read back as a trail. +const mockLedger = []; +jest.mock('../../models/ToolCall', () => ({ + create: jest.fn(async (row) => { mockLedger.push(row); }), + digestArgs: (args) => require('crypto').createHash('sha256').update(JSON.stringify(args || {})).digest('hex'), + reserveBudgetLineage: jest.fn(async () => true), +})); +// The vendor's network edge. Every hosted-MCP request (tools/list for drift, +// tools/call for the call) goes through this one module. +const mockVendorRequests = []; +jest.mock('../../services/hostedVendorFetch', () => ({ + hostedVendorFetch: (...args) => mockVendorServer(...args), + HOSTED_VENDOR_MAX_RESPONSE_BYTES: 4 * 1024 * 1024, +})); + +const User = require('../../models/User'); +const Pod = require('../../models/Pod'); +const RoomGrant = require('../../models/RoomGrant'); +const AgentRun = require('../../models/AgentRun'); +const { AgentInstallation } = require('../../models/AgentRegistry'); +const Integration = require('../../models/Integration'); +const ConnectorSecret = require('../../models/ConnectorSecret'); +const connectorSecrets = require('../../services/connectorSecrets'); +const { HOSTED_MCP_ACCESS_TOKEN } = require('../../services/connectorSecretKinds'); +const { secretWriteGeneration } = require('../../services/hostedMcpSecretGeneration'); +const { HOSTED_MCP_ENTRIES, assertHostedMcpEntries } = require('../../integrations/hostedMcp/entries'); +const { runAgent } = require('../../services/nativeRuntimeService'); +const { GRANT_BROKER_ID } = require('../../services/installable/toolInstallables'); +const AgentIdentityService = require('../../services/agentIdentityService'); +const axios = require('axios').default; + +jest.setTimeout(60000); + +const READ_TOOL = { + name: 'list_jobs', + upstreamName: 'list_jobs', + description: 'List the jobs this key can see', + class: 'read', + annotations: { readOnlyHint: true }, + inputSchema: { type: 'object', properties: { limit: { type: 'number' } } }, +}; +const WRITE_TOOL = { + name: 'cancel_job', + upstreamName: 'cancel_job', + description: 'Cancel a job', + class: 'write', + annotations: { readOnlyHint: false, destructiveHint: true }, + inputSchema: { type: 'object', properties: { jobId: { type: 'string' } }, required: ['jobId'] }, +}; +const ENTRY = { + id: 'keyvendor', + title: 'Key Vendor', + resource: 'https://mcp.keyvendor.example/mcp', + credentialMode: 'operator_key', + revoke: { page: 'https://keyvendor.example/settings/keys' }, + tools: [READ_TOOL, WRITE_TOOL], +}; +const READ_NAME = 'keyvendor.list_jobs'; +const READ_FUNCTION = 'keyvendor_list_jobs'; + +const json = (body, headers = {}) => new Response(JSON.stringify(body), { + status: 200, headers: { 'content-type': 'application/json', ...headers }, +}); +// A Streamable-HTTP MCP server that answers the four messages the broker sends +// and records the bearer on every request, whatever its method. +async function mockVendorServer(url, init = {}) { + const auth = new Headers(init.headers).get('authorization'); + if (init.method === 'DELETE') { + mockVendorRequests.push({ auth, method: 'DELETE' }); + return new Response(null, { status: 204 }); + } + if (init.method === 'GET') return new Response(null, { status: 405 }); + const body = JSON.parse(String(init.body)); + mockVendorRequests.push({ auth, method: body.method }); + if (body.method === 'initialize') { + return json({ jsonrpc: '2.0', id: body.id, result: { + protocolVersion: '2025-11-25', capabilities: { tools: {} }, serverInfo: { name: 'keyvendor', version: '1' }, + } }, { 'mcp-session-id': 'keyvendor-session' }); + } + if (body.method === 'notifications/initialized') return new Response(null, { status: 202 }); + if (body.method === 'tools/list') { + return json({ jsonrpc: '2.0', id: body.id, result: { tools: [READ_TOOL, WRITE_TOOL].map((tool) => ({ + name: tool.upstreamName, description: tool.description, inputSchema: tool.inputSchema, annotations: tool.annotations, + })) } }); + } + if (body.method === 'tools/call') { + // Name the account by label, not by echoing the key: the broker redacts a + // bearer it finds in vendor text, so an echoed key would read [redacted]. + const account = { 'Bearer key-A': 'account-A', 'Bearer key-B': 'account-B' }[auth] || 'unknown-account'; + return json({ jsonrpc: '2.0', id: body.id, result: { content: [{ type: 'text', text: `jobs of ${account}` }] } }); + } + return json({ jsonrpc: '2.0', id: body.id, error: { code: -32601, message: 'method not found' } }); +} + +let shipped; +beforeAll(async () => { + assertHostedMcpEntries([ENTRY]); // the fixture is one the catalogue would load + shipped = HOSTED_MCP_ENTRIES.splice(0, HOSTED_MCP_ENTRIES.length, ENTRY); + await setupMongoDb(); +}); +afterAll(async () => { + HOSTED_MCP_ENTRIES.splice(0, HOSTED_MCP_ENTRIES.length, ...shipped); + await closeMongoDb(); +}); + +let owner; +let seatA; +let seatB; +let pod; +let previousEnv; + +const makeSeat = (agentName) => User.create({ + username: AgentIdentityService.buildAgentUsername(agentName, 'default'), + email: `${agentName}@agents.commonly.local`, + password: 'test-password', + isBot: true, + botType: 'agent', + botMetadata: { agentName, instanceId: 'default' }, +}); + +const HOSTED_NATIVE = { runtime: { runtimeType: 'hosted' }, hosted: { engine: 'native', provisionedAt: new Date() } }; + +beforeEach(async () => { + previousEnv = { ...process.env }; + process.env.LITELLM_BASE_URL = 'http://litellm.test'; + process.env.LITELLM_MASTER_KEY = 'test-only'; + delete process.env.HOSTED_TURNS_PER_DAY; + await Promise.all([User.deleteMany({}), Pod.deleteMany({}), RoomGrant.deleteMany({}), AgentRun.deleteMany({}), + Integration.deleteMany({}), ConnectorSecret.deleteMany({}), AgentInstallation.deleteMany({})]); + mockLedger.length = 0; + mockVendorRequests.length = 0; + owner = await User.create({ username: 'key-owner', email: 'key-owner@example.test', password: 'test-password' }); + seatA = await makeSeat('seat-a'); + seatB = await makeSeat('seat-b'); + pod = await Pod.create({ name: 'Operator key pod', type: 'team', createdBy: owner._id, + members: [owner._id, seatA._id, seatB._id], publicRead: false, joinPolicy: 'invite-only' }); + // Both seats are installed hosted-native, so the pod passes the broker's + // outsider-pod eligibility check for real (outsiderGrantPodEligibility.ts). + await AgentInstallation.create(['seat-a', 'seat-b'].map((agentName) => ({ + agentName, instanceId: 'default', podId: pod._id, version: '1', installedBy: owner._id, + status: 'active', config: HOSTED_NATIVE, + }))); + axios.post.mockReset(); +}); +afterEach(() => { process.env = previousEnv; }); + +/** + * The row the paste-key route writes (routes/hostedMcpOperatorKey.ts): bound to + * one agent, its key in the real ConnectorSecret store, then `connected`. + */ +const connectKey = async (seat, key) => { + const row = await Integration.create({ + type: 'hosted-mcp', scope: 'user', status: 'pending', createdBy: owner._id, + config: { entryId: ENTRY.id, agentId: String(seat._id), intake: 'operator_key', authGeneration: 1 }, + }); + const credentialRef = await connectorSecrets.put( + String(row._id), HOSTED_MCP_ACCESS_TOKEN, key, secretWriteGeneration(1, 0), + ); + await Integration.updateOne({ _id: row._id }, { $set: { status: 'connected', 'config.credentialRef': credentialRef } }); + return row; +}; + +const grantFor = (seat, row) => RoomGrant.create({ + grantId: `keyvendor-grant-${seat.botMetadata.agentName}`, + connectionId: String(row._id), installationId: String(row._id), + target: { kind: 'pod', id: String(pod._id) }, + audience: [String(seat._id)], tools: [READ_NAME], writeMode: 'read', + expiresAt: new Date(Date.now() + 86400000), brokerId: GRANT_BROKER_ID, +}); + +const run = (seat) => runAgent({ podId: pod._id, agentName: seat.botMetadata.agentName, instanceId: 'default', + installedBy: owner._id, displayName: seat.botMetadata.agentName, + config: HOSTED_NATIVE, +}, { type: 'chat.mention', payload: { content: 'List my jobs.' } }); + +const finalReply = { data: { + choices: [{ message: { role: 'assistant', content: 'Done.' } }], + usage: { prompt_tokens: 1, completion_tokens: 1, total_tokens: 2 }, +} }; +// The model calls the vendor's read tool when it is offered, then finishes. +const modelCallsReadToolWhenOffered = () => axios.post.mockImplementation(async (_url, body) => { + const offered = (body.tools || []).map((tool) => tool.function.name); + const answered = (body.messages || []).some((message) => message.role === 'tool'); + if (offered.includes(READ_FUNCTION) && !answered) { + return { data: { + choices: [{ message: { role: 'assistant', content: null, tool_calls: [{ + id: 'call-1', type: 'function', function: { name: READ_FUNCTION, arguments: '{"limit":5}' }, + }] } }], + usage: { prompt_tokens: 1, completion_tokens: 1, total_tokens: 2 }, + } }; + } + return finalReply; +}); +/** Function names offered on the run's FIRST model request. */ +const offeredOnFirstCall = () => axios.post.mock.calls[0][1].tools.map((tool) => tool.function.name); +/** The tool message the run fed back to the model after its broker call. */ +const toolResultSent = () => { + const last = axios.post.mock.calls[axios.post.mock.calls.length - 1][1]; + return last.messages.filter((message) => message.role === 'tool').map((message) => message.content).join('\n'); +}; + +describe('operator-key hosted MCP is per agent (TASK-330)', () => { + test('arm 1 per-agent offer: only the bound, granted seat is offered the vendor tool', async () => { + const rowA = await connectKey(seatA, 'key-A'); + await grantFor(seatA, rowA); + axios.post.mockResolvedValue(finalReply); + + expect(await run(seatA)).toMatchObject({ status: 'succeeded' }); + expect(offeredOnFirstCall()).toContain(READ_FUNCTION); + // Reads only on a hosted turn: the write pin is never offered. + expect(offeredOnFirstCall()).not.toContain('keyvendor_cancel_job'); + + axios.post.mockClear(); + expect(await run(seatB)).toMatchObject({ status: 'succeeded' }); + expect(offeredOnFirstCall().filter((name) => name.startsWith('keyvendor'))).toEqual([]); + }); + + test('arm 2 per-agent bearer: each seat reaches the vendor with its own key, never the other', async () => { + const rowA = await connectKey(seatA, 'key-A'); + const rowB = await connectKey(seatB, 'key-B'); + await grantFor(seatA, rowA); + await grantFor(seatB, rowB); + modelCallsReadToolWhenOffered(); + + expect(await run(seatA)).toMatchObject({ status: 'succeeded' }); + expect(toolResultSent()).toContain('jobs of account-A'); + const asA = mockVendorRequests.splice(0); + + axios.post.mockClear(); + expect(await run(seatB)).toMatchObject({ status: 'succeeded' }); + expect(toolResultSent()).toContain('jobs of account-B'); + const asB = mockVendorRequests.splice(0); + + for (const [requests, own, other] of [[asA, 'Bearer key-A', 'Bearer key-B'], [asB, 'Bearer key-B', 'Bearer key-A']]) { + expect(requests.map((request) => request.method)).toEqual(expect.arrayContaining(['initialize', 'tools/call'])); + expect(requests.filter((request) => request.method === 'tools/call')).toHaveLength(1); + expect(requests.every((request) => request.auth === own)).toBe(true); + expect(requests.some((request) => request.auth === other)).toBe(false); + } + }); + + test('arm 3 leave empties the audience: after the bound seat leaves, its run offers no vendor tool', async () => { + const rowA = await connectKey(seatA, 'key-A'); + await grantFor(seatA, rowA); + axios.post.mockResolvedValue(finalReply); + + expect(await run(seatA)).toMatchObject({ status: 'succeeded' }); + expect(offeredOnFirstCall()).toContain(READ_FUNCTION); // the positive control for this arm + + await AgentIdentityService.removeAgentFromPod('seat-a', pod._id, 'default'); + const after = await Pod.findById(pod._id).select('members').lean(); + expect(after.members.map(String)).not.toContain(String(seatA._id)); + + axios.post.mockClear(); + mockVendorRequests.length = 0; + expect(await run(seatA)).toMatchObject({ status: 'succeeded' }); + expect(offeredOnFirstCall().filter((name) => name.startsWith('keyvendor'))).toEqual([]); + expect(mockVendorRequests).toEqual([]); + }); + + // The mint refuses this grant (grants.ts, operatorKeyAudienceRefusal), so it + // is written straight to the store: a grant from any other path, or one that + // predates the mint rule, must still never spend A's key on B's turn. + test('arm 4 call-time binding: a grant on A\'s row that names B is refused, and A\'s key never leaves', async () => { + const rowA = await connectKey(seatA, 'key-A'); + await RoomGrant.create({ + grantId: 'keyvendor-grant-stray', connectionId: String(rowA._id), installationId: String(rowA._id), + target: { kind: 'pod', id: String(pod._id) }, + audience: [String(seatB._id)], tools: [READ_NAME], writeMode: 'read', + expiresAt: new Date(Date.now() + 86400000), brokerId: GRANT_BROKER_ID, + }); + modelCallsReadToolWhenOffered(); + + expect(await run(seatB)).toMatchObject({ status: 'succeeded' }); + expect(toolResultSent()).not.toContain('account-A'); + expect(toolResultSent()).toContain('connection_mismatch'); + expect(mockVendorRequests.some((request) => request.auth === 'Bearer key-A')).toBe(false); + }); +}); diff --git a/backend/__tests__/unit/routes/grants.mint.test.js b/backend/__tests__/unit/routes/grants.mint.test.js index 9976ecd969..1306af3c6f 100644 --- a/backend/__tests__/unit/routes/grants.mint.test.js +++ b/backend/__tests__/unit/routes/grants.mint.test.js @@ -500,3 +500,53 @@ describe('task-bound mint', () => { 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/__tests__/unit/routes/hostedMcpConnect.test.js b/backend/__tests__/unit/routes/hostedMcpConnect.test.js index 06f4b57cfc..fe2f6c6036 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/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js b/backend/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js new file mode 100644 index 0000000000..1e0e1a9488 --- /dev/null +++ b/backend/__tests__/unit/routes/hostedMcpOperatorKey.route.test.js @@ -0,0 +1,328 @@ +/** + * 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'); +const request = require('supertest'); + +jest.mock('../../../models/Integration', () => ({ findOneAndUpdate: jest.fn(), updateOne: jest.fn(), exists: 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(), revoke: jest.fn(), revokeOwnWrite: jest.fn() })); +jest.mock('../../../services/roomGrantService', () => ({ revokeConnectionGrants: jest.fn() })); +jest.mock('../../../services/hostedMcpToolDefinitions', () => ({ + guardedFetch: jest.fn(), + listHostedMcpTools: jest.fn(), + forgetHostedMcpDrift: jest.fn(), +})); +// The route is reached through the production router, and auth and the write +// limiter are observable, so dropping either from the registration fails a test. +jest.mock('../../../middleware/auth', () => jest.fn((req, res, next) => { + const id = req.get('x-test-user'); + if (!id) return res.status(401).json({ error: 'unauthorized' }); + req.user = { id }; + return next(); +})); +jest.mock('../../../middleware/integrationRateLimit', () => ({ + writeIntegrationsRateLimit: jest.fn((_req, _res, next) => next()), + listIntegrationsRateLimit: (_req, _res, next) => next(), + ingestIngressRateLimit: (_req, _res, next) => next(), + ingestIntegrationsRateLimit: (_req, _res, next) => next(), +})); + +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, forgetHostedMcpDrift } = require('../../../services/hostedMcpToolDefinitions'); +const { revokeConnectionGrants } = require('../../../services/roomGrantService'); +const { HOSTED_MCP_ENTRIES } = require('../../../integrations/hostedMcp/entries'); +const auth = require('../../../middleware/auth'); +const { writeIntegrationsRateLimit } = require('../../../middleware/integrationRateLimit'); +const connectRoutes = require('../../../routes/hostedMcpConnect'); + +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 = { + 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.use('/connect/hosted-mcp', connectRoutes); + +const submit = (body = {}) => request(app) + .post('/connect/hosted-mcp/latchkey/operator-key') + .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' } }) }), + }); + 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, matchedCount: 1 }); + 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('the write limiter runs, then auth, and a caller with no session stops there', async () => { + const res = await request(app) + .post('/connect/hosted-mcp/latchkey/operator-key') + .send({ agentId: AGENT, key: KEY }); + + expect(res.status).toBe(401); + expect(writeIntegrationsRateLimit).toHaveBeenCalledTimes(1); + expect(auth).toHaveBeenCalledTimes(1); + expect(writeIntegrationsRateLimit.mock.invocationCallOrder[0]).toBeLessThan(auth.mock.invocationCallOrder[0]); + expect(User.findOne).not.toHaveBeenCalled(); + expect(listHostedMcpTools).not.toHaveBeenCalled(); + expect(connectorSecrets.put).not.toHaveBeenCalled(); + }); + + 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 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(AgentInstallation.find).not.toHaveBeenCalled(); + expect(listHostedMcpTools).not.toHaveBeenCalled(); + }); + + 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', + 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('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(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(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, 'config.authGeneration': 1, status: 'pending', revokedAt: null, + }, + { $set: { status: 'connected', 'config.credentialRef': 'secret-ref-1' } }, + ); + // A first paste has no grants to end. + expect(revokeConnectionGrants).not.toHaveBeenCalled(); + }); + + // Both F07 reviewers: fetch strips header whitespace, so a key pasted with a + // newline authenticates while the stored copy no longer matches what the + // vendor echoes, and redaction (an exact match) lets it through. + test('a key pasted with surrounding whitespace is stored as the trimmed value the probe proved', async () => { + const res = await submit({ key: ` ${KEY}\n` }); + expect(res.status).toBe(201); + expect(listHostedMcpTools).toHaveBeenCalledWith(expect.anything(), KEY, expect.anything()); + expect(connectorSecrets.put).toHaveBeenCalledWith(ROW_ID, expect.anything(), KEY, expect.any(Number)); + }); + + // A grant was decided against the key that was connected then; a pasted key + // cannot prove it is the same account, so a re-paste ends them, as the OAuth + // callback does for a subject it cannot compare (hostedMcpConnect.ts). + test('a re-paste on an existing row revokes its grants and forgets its drift before storing the new key', async () => { + const existing = { _id: ROW_ID, config: { authGeneration: 2 } }; + Integration.findOneAndUpdate.mockResolvedValue(existing); + const res = await submit(); + expect(res.status).toBe(201); + expect(revokeConnectionGrants).toHaveBeenCalledWith({ connection: existing, revokedBy: OWNER }); + expect(forgetHostedMcpDrift).toHaveBeenCalledWith(ROW_ID); + expect(revokeConnectionGrants.mock.invocationCallOrder[0]) + .toBeLessThan(connectorSecrets.put.mock.invocationCallOrder[0]); + }); + + test('a paste clears the row\'s revocation, error and provider-revoked mark', async () => { + await submit(); + const [, update] = Integration.findOneAndUpdate.mock.calls[0]; + expect(update.$set).toEqual(expect.objectContaining({ revokedAt: null, errorMessage: null })); + expect(update.$unset).toEqual({ 'config.providerRevokedAt': '' }); + }); + + // The paste's own write is generation 1 (authGeneration 1, refresh 0). + const OWN_WRITE = 1_000_000; + + test('a row removed while the key was being saved: the new secret is revoked and the paste refused', async () => { + Integration.updateOne.mockResolvedValue({ acknowledged: true, matchedCount: 0 }); + const res = await submit(); + expect(res.status).toBe(409); + expect(res.body).toEqual({ error: 'connection_changed', message: 'the connection changed while the key was saved; paste it again' }); + expect(connectorSecrets.revokeOwnWrite).toHaveBeenCalledWith('secret-ref-1', OWN_WRITE); + expect(connectorSecrets.revoke).not.toHaveBeenCalled(); + }); + + // Removal marks the row disconnected and revoked before it deletes it + // (connectionRemovalService), so a paste can finish between the two. That + // row must not go back to connected, and nothing will use the new secret. + test('a row marked by a removal mid-paste stays unpublished, and its new secret is revoked', async () => { + const marked = { + _id: ROW_ID, 'config.authGeneration': 1, status: 'disconnected', revokedAt: new Date(), + }; + const matches = (filter) => Object.entries(filter) + .every(([key, value]) => (value === null ? marked[key] == null : marked[key] === value)); + Integration.updateOne.mockImplementation(async (filter) => ({ + acknowledged: true, matchedCount: matches(filter) ? 1 : 0, + })); + const res = await submit(); + expect(res.status).toBe(409); + expect(res.body.error).toBe('connection_changed'); + expect(connectorSecrets.revokeOwnWrite).toHaveBeenCalledWith('secret-ref-1', OWN_WRITE); + }); + + // The ref is reused, so a newer paste may own it by now. The delete is fenced + // on this paste's own write generation, which a newer write has replaced + // (the store's own test pins that), never a delete by ref alone. + test('a newer paste that won the row keeps its secret: the older paste deletes only its own write', async () => { + Integration.updateOne.mockResolvedValue({ acknowledged: true, matchedCount: 0 }); + const res = await submit(); + expect(res.status).toBe(409); + expect(connectorSecrets.revoke).not.toHaveBeenCalled(); + expect(connectorSecrets.revokeOwnWrite).toHaveBeenCalledWith('secret-ref-1', OWN_WRITE); + }); + + 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/__tests__/unit/services/connectionRemoval.test.js b/backend/__tests__/unit/services/connectionRemoval.test.js index 2992e1196f..3e3c60eb72 100644 --- a/backend/__tests__/unit/services/connectionRemoval.test.js +++ b/backend/__tests__/unit/services/connectionRemoval.test.js @@ -527,6 +527,35 @@ describe('an Atlassian removal (TASK-339)', () => { }); }); +describe('an operator-key removal (TASK-330)', () => { + // A pasted key has no refresh token and no RFC 7009 client, so removal never + // sends it anywhere: it ends our grants, deletes the stored key, and points + // the person at the vendor's key page. The entry here even names an endpoint, + // so it is the credential mode, not a missing URL, that keeps the vendor out. + const KEY_PAGE = 'https://latchkey.example/settings/api-keys'; + const KEY_ENTRY = { + id: 'keyvendor', + credentialMode: 'operator_key', + revoke: { page: KEY_PAGE, endpoint: 'https://latchkey.example/oauth/revoke' }, + resource: 'https://latchkey.example/mcp', + }; + + it('never calls the vendor, deletes the stored key, and hands over the key page', async () => { + const { calls, deps, revokeGrants } = recorder({ entryFor: () => KEY_ENTRY }); + const row = hostedRow({ config: { entryId: 'keyvendor', agentId: 'agent-1', intake: 'operator_key', credentialRef: 'key-ref' } }); + + const result = await remove(row, deps, revokeGrants); + + expect(result).toEqual({ removed: true, grantsRevoked: 2, revokeAt: KEY_PAGE, providerRevoked: false }); + expect(deps.revokeAtVendor).not.toHaveBeenCalled(); + expect(deps.secrets.get).not.toHaveBeenCalled(); + expect(deps.secrets.revoke).toHaveBeenCalledWith('key-ref'); + // The refresh slot is swept too and is empty (`connectorSecrets.revoke` + // returns on a missing ref), as on any row without a refresh token. + expect(calls).toEqual(['grants', 'row', 'material:undefined', 'material:key-ref', 'delete']); + }); +}); + describe('a row whose entry is gone', () => { const COPIED_PAGE = 'https://linear.app/settings/security/legacy'; diff --git a/backend/__tests__/unit/services/connectorSecrets.test.js b/backend/__tests__/unit/services/connectorSecrets.test.js index 690a5fec62..6ad0b30cca 100644 --- a/backend/__tests__/unit/services/connectorSecrets.test.js +++ b/backend/__tests__/unit/services/connectorSecrets.test.js @@ -9,6 +9,7 @@ const { putIfAbsent, rewrap, revoke, + revokeOwnWrite, } = require('../../../services/connectorSecrets'); const { DISCORD_WEBHOOK_URL, @@ -124,6 +125,21 @@ describe('connectorSecrets', () => { // TASK-539: the import path. Insert-only, so a secret a live edit stored // first is never replaced with an older copy. + // A writer cleaning up after itself: the ref is stable per (integration, + // kind), so a newer writer may already own it by the time the older one + // deletes, and a delete by ref alone would destroy that writer's key. + it('revokeOwnWrite deletes a secret only while it still holds that write', async () => { + const integrationId = new mongoose.Types.ObjectId().toString(); + const ref = await put(integrationId, SLACK_BOT_TOKEN, 'older', 1_000_000); + expect(await put(integrationId, SLACK_BOT_TOKEN, 'newer', 2_000_000)).toBe(ref); + + await revokeOwnWrite(ref, 1_000_000); + await expect(get(ref)).resolves.toBe('newer'); + + await revokeOwnWrite(ref, 2_000_000); + expect(await ConnectorSecret.findById(ref).lean()).toBeNull(); + }); + it('putIfAbsent inserts when the pair is free and reports it created', async () => { const integrationId = new mongoose.Types.ObjectId().toString(); const { ref, created } = await putIfAbsent(integrationId, GROUPME_ACCESS_TOKEN, 'imported'); 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 0000000000..4b93e9dc16 --- /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 0000000000..f8a960cb6a --- /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/__tests__/unit/services/hostedMcpOperatorKeyIntake.test.js b/backend/__tests__/unit/services/hostedMcpOperatorKeyIntake.test.js new file mode 100644 index 0000000000..04c4b5df14 --- /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/integrations/hostedMcp/entries.ts b/backend/integrations/hostedMcp/entries.ts index fe2db399bc..3f2e9b85db 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'; @@ -170,7 +171,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/models/Integration.ts b/backend/models/Integration.ts index adb47d3c91..ad8e2ecb25 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/routes/grants.ts b/backend/routes/grants.ts index fc22be280a..cf29ea4820 100644 --- a/backend/routes/grants.ts +++ b/backend/routes/grants.ts @@ -7,6 +7,7 @@ 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'; @@ -397,6 +398,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/routes/hostedMcpConnect.ts b/backend/routes/hostedMcpConnect.ts index 21a1958048..0e0309a160 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'); @@ -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 @@ -36,9 +37,18 @@ 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 -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 { @@ -56,7 +66,7 @@ const { hostedMcpNonceCookiePath, readCookie, resolvedHostedMcpClient, -} = require('../services/hostedMcpIntakeService'); +}: typeof import('../services/hostedMcpIntakeService') = require('../services/hostedMcpIntakeService'); type AuthedRequest = Request & { user?: { id?: string; role?: string } }; @@ -97,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 => { @@ -217,7 +227,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)); @@ -240,7 +250,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' }); @@ -343,6 +354,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); @@ -379,7 +392,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 : ''; diff --git a/backend/routes/hostedMcpOperatorKey.ts b/backend/routes/hostedMcpOperatorKey.ts new file mode 100644 index 0000000000..04af9b783d --- /dev/null +++ b/backend/routes/hostedMcpOperatorKey.ts @@ -0,0 +1,157 @@ +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 { forgetHostedMcpDrift, guardedFetch } from '../services/hostedMcpToolDefinitions'; +import { secretWriteGeneration } from '../services/hostedMcpSecretGeneration'; +import { ownsRuntimeIdentity } from '../services/runtimeIdentityOwnership'; + +const Integration = require('../models/Integration'); +const User = require('../models/User'); +const connectorSecrets = require('../services/connectorSecrets'); +const { HOSTED_MCP_ACCESS_TOKEN } = require('../services/connectorSecretKinds'); +const { revokeConnectionGrants } = require('../services/roomGrantService'); + +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 botMetadata').lean(); + // 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' }); + } + // 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', + failure: ownership.failure, + message: 'you must be the sole installer of this 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`, + }); + } + + // Trimmed once, here: fetch strips header whitespace, so a pasted newline + // would still authenticate while the stored copy no longer matched what a + // vendor echoes, and the result redaction is an exact match. + const key = typeof body.key === 'string' ? body.key.trim() : ''; + 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 }, + // A paste is a fresh connection: whatever a half-finished removal left on + // the row (its revocation, error, provider-revoked mark) is not about + // this key, and a surviving mark would let the next removal skip a step. + $set: { status: 'pending', 'config.intake': 'operator_key', revokedAt: null, errorMessage: null }, + $unset: { 'config.providerRevokedAt': '' }, + $setOnInsert: { + type: 'hosted-mcp', + scope: 'user', + createdBy: userId, + 'config.entryId': entry.id, + 'config.agentId': agentId, + }, + }, + { upsert: true, new: true, setDefaultsOnInsert: true }, + ); + const generation = row.config?.authGeneration ?? 0; + if (generation > 1) { + // A grant was decided against the key connected then, and a pasted key + // cannot prove it is the same account, so a re-paste ends them, as the OAuth + // callback does for a subject it cannot compare (hostedMcpConnect.ts). + await revokeConnectionGrants({ connection: row, revokedBy: userId }); + forgetHostedMcpDrift(String(row._id)); + } + const writeGeneration = secretWriteGeneration(generation, 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' }); + } + // Published only while the row is still the pending row this paste wrote. A + // removal marks the row disconnected and revoked before it deletes it, so + // the generation alone would let a paste that lands in between put that row + // back to connected. The OAuth callback fences on its own interim status the + // same way (hostedMcpConnect.ts). + const published = await Integration.updateOne( + { + _id: row._id, 'config.authGeneration': generation, status: 'pending', revokedAt: null, + }, + { $set: { status: 'connected', 'config.credentialRef': credentialRef } }, + ); + if (published?.matchedCount === 0) { + // A removal took the row mid-paste (deleted it, or marked it on the way), + // or a newer paste took it. Either way nothing will use this paste's key. + // The ref is reused per row, so a newer paste may own it by now: delete + // only while it still holds this paste's own write. A row lookup here + // would race that newer paste, which can clear the mark and overwrite the + // ref between the lookup and the delete. + await connectorSecrets.revokeOwnWrite(credentialRef, writeGeneration); + return res.status(409).json({ + error: 'connection_changed', + message: 'the connection changed while the key was saved; paste it again', + }); + } + + return res.status(201).json({ + integrationId: String(row._id), + status: 'connected', + scopeCheck: intake.scopeCheck, + toolCount: intake.tools.length, + }); +}; diff --git a/backend/scripts/backfill-hosted-mcp-client-id.ts b/backend/scripts/backfill-hosted-mcp-client-id.ts index a60365be99..ae6eef676e 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/scripts/migrate-hosted-mcp-agent-index.js b/backend/scripts/migrate-hosted-mcp-agent-index.js new file mode 100644 index 0000000000..f22ae67a7c --- /dev/null +++ b/backend/scripts/migrate-hosted-mcp-agent-index.js @@ -0,0 +1,80 @@ +/* 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 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'); + +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; + + // 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`); + 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/services/connectionRemovalService.ts b/backend/services/connectionRemovalService.ts index 5244396bc1..dd8bae8923 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. */ @@ -177,7 +178,7 @@ export type RemovalResult = * endpoint that refused. */ export const revokeTokenAtVendor = async (input: { - entry: HostedMcpEntry; + entry: HostedMcpOAuthEntry; clientId: string; clientSecret?: string; tokenEndpointAuthMethod?: HostedMcpClientConfiguration['tokenEndpointAuthMethod']; @@ -342,7 +343,9 @@ export const removeConnection = async (options: { const alreadyRevokedAtVendor = Boolean(connection?.config?.[PROVIDER_REVOKED_MARK]); let providerRevoked = alreadyRevokedAtVendor; - 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/connectorSecrets.ts b/backend/services/connectorSecrets.ts index 68322515c3..c41dd4ecdf 100644 --- a/backend/services/connectorSecrets.ts +++ b/backend/services/connectorSecrets.ts @@ -213,6 +213,17 @@ export const revoke = async (ref: string | undefined): Promise => { await ConnectorSecret.deleteOne({ _id: ref }); }; +/** + * Delete a secret only while it still holds the material `writeGeneration` + * wrote. For a writer cleaning up after itself: the ref is stable per + * (integration, kind), so a newer writer may already own it, and a delete by + * ref alone would destroy that writer's key. + */ +export const revokeOwnWrite = async (ref: string | undefined, writeGeneration: number): Promise => { + if (!ref) return; + await ConnectorSecret.deleteOne({ _id: ref, writeGeneration }); +}; + export const rewrap = async (ref: string): Promise => { const secret = await ConnectorSecret.findById(ref) as IConnectorSecret | null; if (!secret) throw new ConnectorSecretNotFoundError(ref); @@ -237,6 +248,7 @@ module.exports = { putIfAbsent, get, revoke, + revokeOwnWrite, rewrap, listWithUnavailableKey, }; diff --git a/backend/services/hostedMcpClientIdBackfill.ts b/backend/services/hostedMcpClientIdBackfill.ts index f6230b3d90..dd1c552843 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 6b53e42b5f..3b81a23f98 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. */ @@ -396,6 +396,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/hostedMcpDcrRegistry.ts b/backend/services/hostedMcpDcrRegistry.ts index 9ee39d64b8..3a59ee38f2 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 b30892aa1d..eeefe2a568 100644 --- a/backend/services/hostedMcpEntryService.ts +++ b/backend/services/hostedMcpEntryService.ts @@ -61,7 +61,8 @@ 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; title: string; @@ -77,17 +78,6 @@ export interface HostedMcpEntry { resource: string; /** The PRM resource identifier when it differs from the tools/call endpoint. */ audience?: string; - /** The authorization server the protected-resource metadata names. */ - issuer: string; - client: 'pre-registered' | 'cimd' | 'dcr'; - /** Entry-pinned exception when an AS omits its token auth-method list. */ - tokenEndpointAuthMethod?: 'client_secret_basic' | 'client_secret_post'; - scopes: string[]; - /** Only the vendor-specific authorization parameters supported by this code. */ - authorizationParams?: { - 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 @@ -105,6 +95,38 @@ export interface HostedMcpEntry { 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'; + /** Entry-pinned exception when an AS omits its token auth-method list. */ + tokenEndpointAuthMethod?: 'client_secret_basic' | 'client_secret_post'; + scopes: string[]; + /** Only the vendor-specific authorization parameters supported by this code. */ + authorizationParams?: { + access_type?: string; + prompt?: string; + }; +} + +/** + * 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; @@ -461,3 +483,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 fac8e1ad69..3011755ec2 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, @@ -473,7 +477,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 }, @@ -494,7 +498,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 }, @@ -513,7 +517,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' }, diff --git a/backend/services/hostedMcpOperatorKey.ts b/backend/services/hostedMcpOperatorKey.ts new file mode 100644 index 0000000000..5c58b7cfc3 --- /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; +}; diff --git a/backend/services/hostedMcpOperatorKeyIntake.ts b/backend/services/hostedMcpOperatorKeyIntake.ts new file mode 100644 index 0000000000..c9938273b5 --- /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 223108525c..e7335fab8e 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); @@ -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; diff --git a/backend/services/installable/installableCatalogService.ts b/backend/services/installable/installableCatalogService.ts index 43a6541c94..cc31bc9d2e 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' }; diff --git a/backend/services/toolBrokerService.ts b/backend/services/toolBrokerService.ts index 6f3cd817ea..32cfd7c9c8 100644 --- a/backend/services/toolBrokerService.ts +++ b/backend/services/toolBrokerService.ts @@ -605,7 +605,7 @@ const resolveHostedConnection = async ( revokedAt?: Date | null; createdBy?: unknown; createdAt?: unknown; - config?: { entryId?: unknown }; + config?: { entryId?: unknown; agentId?: unknown; intake?: unknown }; } | null; if (!row || row.type !== 'hosted-mcp' || row.status !== 'connected' || row.revokedAt) { throw new RoomGrantError( @@ -625,6 +625,22 @@ const resolveHostedConnection = async ( 403, ); } + // An operator-key row is ONE agent's bearer (TASK-330). The mint holds its + // grants to exactly that agent; this holds every grant whatever made it, so a + // stray or older grant can never spend the key on another agent's turn. The + // caller is already inside the grant's audience (`assertGrantUsable`), so an + // audience of only the bound agent means the caller is that agent. + const boundAgentId = row.config?.agentId ? String(row.config.agentId) : ''; + if (boundAgentId || row.config?.intake === 'operator_key') { + const audience = ((grant as { audience?: unknown }).audience || []) as unknown[]; + if (!boundAgentId || audience.length === 0 || audience.some((id) => String(id) !== boundAgentId)) { + throw new RoomGrantError( + 'connection_mismatch', + 'this connection\'s key belongs to one agent, and the grant names another', + 403, + ); + } + } const grantCreatedAt = createdAtMs((grant as { createdAt?: unknown }).createdAt); const rowCreatedAt = createdAtMs(row.createdAt); if (grantCreatedAt === null || rowCreatedAt === null) { diff --git a/backend/utils/serverOwnedConfigKeys.ts b/backend/utils/serverOwnedConfigKeys.ts index 976f9b89c0..8b1beedf86 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