diff --git a/control-api/test/services.rpcHostAccessAuthorizer.test.ts b/control-api/test/services.rpcHostAccessAuthorizer.test.ts index 64abbc801f..8f04c1723f 100644 --- a/control-api/test/services.rpcHostAccessAuthorizer.test.ts +++ b/control-api/test/services.rpcHostAccessAuthorizer.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import type { K8sGateway } from '../src/k8s.js' import type { RpcAccessClaims } from '../src/profileTypes.js' import { authorizeRpcHostAccess } from '../src/services/access/rpcHostAccessAuthorizer.js' +import { signRpcAccessToken, verifyRpcAccessToken } from '../src/utils/auth/rpcAuthToken.js' const CLAIMS: RpcAccessClaims = { sub: 'user-1', @@ -48,6 +49,81 @@ function dependencies( } describe('authorizeRpcHostAccess', () => { + function issuedClaims(hostRefs: string[]): RpcAccessClaims { + const token = signRpcAccessToken({ + sub: CLAIMS.sub, + typ: CLAIMS.typ, + teamId: CLAIMS.teamId, + role: CLAIMS.role, + scopes: CLAIMS.scopes, + hostRefs, + jti: `issued-${hostRefs.join('-') || 'empty'}`, + }) + const verified = verifyRpcAccessToken(token) + if (!verified) throw new Error('Control API rejected its own signed RPC token') + return verified + } + + it('intersects signed Host refs with live direct grants across grants and revocation', async () => { + const hosts = [ + { metadata: { name: 'host-a' }, spec: { enabled: true } }, + { metadata: { name: 'host-b' }, spec: { enabled: true } }, + ] + const tokenA = issuedClaims(['host-a']) + const liveBoth = dependencies({ userAgents: ['host-a', 'host-b'], hosts }) + await expect( + authorizeRpcHostAccess(liveBoth.gateway, tokenA, 'user-1', 'host-a', liveBoth.directory) + ).resolves.toMatchObject({ authorized: true }) + await expect( + authorizeRpcHostAccess(liveBoth.gateway, tokenA, 'user-1', 'host-b', liveBoth.directory) + ).resolves.toEqual({ authorized: false, reason: 'host_claim_missing' }) + + const reminted = issuedClaims(['host-a', 'host-b']) + await expect( + authorizeRpcHostAccess(liveBoth.gateway, reminted, 'user-1', 'host-b', liveBoth.directory) + ).resolves.toMatchObject({ authorized: true }) + const revokedB = dependencies({ userAgents: ['host-a'], hosts }) + await expect( + authorizeRpcHostAccess(revokedB.gateway, reminted, 'user-1', 'host-b', revokedB.directory) + ).resolves.toEqual({ authorized: false, reason: 'directory_grant_missing' }) + await expect( + authorizeRpcHostAccess(revokedB.gateway, reminted, 'user-1', 'host-a', revokedB.directory) + ).resolves.toMatchObject({ authorized: true }) + }) + + it('applies the signed ceiling to team grants and rejects subject substitution', async () => { + const claims = issuedClaims(['host-a', 'host-b']) + const liveTeam = dependencies({ + userAgents: [], + teamAgents: ['host-b'], + hosts: [{ metadata: { name: 'host-b' }, spec: { enabled: true } }], + }) + await expect( + authorizeRpcHostAccess(liveTeam.gateway, claims, 'user-1', 'host-b', liveTeam.directory) + ).resolves.toMatchObject({ authorized: true }) + await expect( + authorizeRpcHostAccess( + liveTeam.gateway, + claims, + 'different-user', + 'host-b', + liveTeam.directory + ) + ).resolves.toEqual({ authorized: false, reason: 'subject_mismatch' }) + expect( + verifyRpcAccessToken( + signRpcAccessToken({ + sub: CLAIMS.sub, + typ: CLAIMS.typ, + teamId: CLAIMS.teamId, + role: CLAIMS.role, + scopes: CLAIMS.scopes, + hostRefs: [], + jti: 'issued-empty', + }) + ) + ).toBeNull() + }) it('returns a typed subject mismatch before directory or Kubernetes access', async () => { const { gateway, directory } = dependencies() await expect( diff --git a/desktop-app/src/__tests__/rpcProxyClient.test.ts b/desktop-app/src/__tests__/rpcProxyClient.test.ts index 9fdee5cbd3..6bec508741 100644 --- a/desktop-app/src/__tests__/rpcProxyClient.test.ts +++ b/desktop-app/src/__tests__/rpcProxyClient.test.ts @@ -69,6 +69,26 @@ describe('RpcProxyClient.listSessions', () => { vi.restoreAllMocks() }) + it('surfaces W1 400 instead of treating it as absent session or available Host', async () => { + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ + ok: false, + status: 400, + text: async () => 'Invalid hostRef', + }) + ) + await expect(client.listSessions('token', 'invalid_host')).rejects.toMatchObject({ + status: 400, + }) + await expect( + client.loadSessionMessages('token', 'invalid_host', 'agent', 'chat') + ).rejects.toMatchObject({ status: 400 }) + await expect( + client.getContextBreakdown('token', 'invalid_host', 'agent', 'chat') + ).rejects.toMatchObject({ status: 400 }) + }) + it.each(['.', '..', 'host/name', 'host\\name', 'host\nforged', 'x'.repeat(501)])( 'rejects an unsafe host path before issuing a request: %s', async hostRef => { @@ -874,6 +894,16 @@ describe('RpcProxyClient.getHostModels', () => { await expect(client.getHostModels('t', 'h', 'c')).resolves.toBeNull() }) + it('does not treat W1 Host-ref 400 as an unavailable model-list feature', async () => { + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ ok: false, status: 400, text: async () => 'Invalid hostRef' }) + ) + await expect(client.getHostModels('t', 'invalid_host', 'c')).rejects.toMatchObject({ + status: 400, + }) + }) + it('throws on a genuine server error (500)', async () => { vi.stubGlobal( 'fetch', diff --git a/external-rest-api/src/releaseManifest.ts b/external-rest-api/src/releaseManifest.ts index d17a613bfc..4362678f2f 100644 --- a/external-rest-api/src/releaseManifest.ts +++ b/external-rest-api/src/releaseManifest.ts @@ -9,7 +9,7 @@ export type ReleaseManifest = { export const releaseManifest: ReleaseManifest = { releaseId: 'v0.10.0', externalRestApiVersion: '0.1.113', - rpcProxyVersion: '0.1.121', + rpcProxyVersion: '0.1.123', desktopVersion: '0.10.0', minimumDesktopVersion: '0.1.252', } diff --git a/rpc-proxy/package-lock.json b/rpc-proxy/package-lock.json index dd87f9cc68..36ef5f620c 100644 --- a/rpc-proxy/package-lock.json +++ b/rpc-proxy/package-lock.json @@ -1,12 +1,12 @@ { "name": "clerum-rpc-proxy", - "version": "0.1.121", + "version": "0.1.123", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "clerum-rpc-proxy", - "version": "0.1.121", + "version": "0.1.123", "dependencies": { "@clerum/action-context-contracts": "file:../packages/action-context-contracts", "cors": "^2.8.5", diff --git a/rpc-proxy/package.json b/rpc-proxy/package.json index 7ac5763a04..baba9b46a3 100644 --- a/rpc-proxy/package.json +++ b/rpc-proxy/package.json @@ -1,6 +1,6 @@ { "name": "clerum-rpc-proxy", - "version": "0.1.121", + "version": "0.1.123", "description": "External-facing RPC proxy for tenant-scoped MCP access", "main": "dist/main.js", "engines": { diff --git a/rpc-proxy/src/__tests__/host-ref-validation.test.ts b/rpc-proxy/src/__tests__/host-ref-validation.test.ts new file mode 100644 index 0000000000..6a1c6b1138 --- /dev/null +++ b/rpc-proxy/src/__tests__/host-ref-validation.test.ts @@ -0,0 +1,580 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import express from 'express' +import { execFileSync } from 'node:child_process' +import { readFileSync } from 'node:fs' +import { resolve } from 'node:path' +import request from 'supertest' +import { createRpcRouter } from '../routes/rpc.js' +import { createRpcHostActivityStreamRouter } from '../routes/rpcHostActivityStream.js' +import { createRpcHostProgressStreamRouter } from '../routes/rpcHostProgressStream.js' +import { createRpcHostStatusStreamRouter } from '../routes/rpcHostStatusStream.js' + +const authMock = vi.hoisted(() => ({ verifyRpcToken: vi.fn() })) +const hostMock = vi.hoisted(() => ({ + resolveHostConnectionForUser: vi.fn(), + resolveArtifactReadHostConnectionForUser: vi.fn(), + requestHostWakeFromControlApi: vi.fn(), +})) +const admissionMock = vi.hoisted(() => ({ requestHostRpcAdmission: vi.fn() })) + +const repositoryRoot = resolve(process.cwd(), '..') +const tsx = resolve(repositoryRoot, 'rpc-proxy', 'node_modules', '.bin', 'tsx') +const hostWakeDelegationProducer = resolve( + repositoryRoot, + 'control-api', + 'test', + 'fixtures', + 'emitUserDelegationV2Fixture.ts' +) +const bodyBoundDelegationProducer = resolve( + repositoryRoot, + 'control-api', + 'test', + 'fixtures', + 'emitBodyBoundRouteDelegationV2Fixture.ts' +) + +vi.mock('../authToken.js', async importOriginal => ({ + ...(await importOriginal()), + verifyRpcToken: authMock.verifyRpcToken, +})) +vi.mock('../services/mcpProxyService.js', async importOriginal => ({ + ...(await importOriginal()), + resolveHostConnectionForUser: hostMock.resolveHostConnectionForUser, + resolveArtifactReadHostConnectionForUser: hostMock.resolveArtifactReadHostConnectionForUser, +})) +vi.mock('../services/controlApiRestService.js', async importOriginal => ({ + ...(await importOriginal()), + requestHostWakeFromControlApi: hostMock.requestHostWakeFromControlApi, + requestHostRpcAdmission: admissionMock.requestHostRpcAdmission, +})) + +const legacyClaims = { + sub: 'user-1', + typ: 'user' as const, + accessScope: 'team' as const, + teamId: 'team-1', + scopes: [ + 'host:message:invoke', + 'host:wake:write', + 'host:approval:write', + 'host:session:read', + 'host:session:write', + 'host:model:write', + 'host:task:read', + 'host:activity:read', + 'host:status:read', + 'host:health:read', + ], + hostRefs: ['host-a'], + jti: 'jti-1', + iat: 1, + exp: 9_999_999_999, +} + +function mountedApp() { + const app = express() + app.use(createRpcRouter()) + app.use(createRpcHostStatusStreamRouter()) + app.use(createRpcHostActivityStreamRouter()) + app.use(createRpcHostProgressStreamRouter()) + app.use((_req, res) => res.status(404).json({ error: 'Not Found' })) + return app +} + +type Route = { method: 'get' | 'post' | 'patch'; suffix: string; body?: object } +const w1PayloadRoutes: Route[] = [ + { + method: 'post', + suffix: '/approvals/approve', + body: { taskId: 'task-1', toolCallId: 'approval-1' }, + }, + { + method: 'post', + suffix: '/approvals/deny', + body: { taskId: 'task-1', toolCallId: 'approval-1' }, + }, + { + method: 'post', + suffix: '/model', + body: { agent: 'agent', chatId: 'chat', provider: 'openai', model: 'model' }, + }, + { method: 'patch', suffix: '/sessions/agent/chat/name', body: { title: 'Valid title' } }, +] +const legacyRoutes: Route[] = [ + { method: 'post', suffix: '/messages', body: { content: 'hello' } }, + { method: 'post', suffix: '/wake', body: {} }, + ...w1PayloadRoutes.slice(0, 2), + w1PayloadRoutes[2]!, + { method: 'post', suffix: '/tasks/task-1/cancel', body: {} }, + w1PayloadRoutes[3]!, + { method: 'get', suffix: '/sessions' }, + { method: 'get', suffix: '/sessions/agent/chat/messages' }, + { method: 'get', suffix: '/sessions/agent/chat/context-breakdown' }, + { method: 'get', suffix: '/models' }, + { method: 'get', suffix: '/tasks/task-1/result' }, + { method: 'get', suffix: '/artifacts' }, + { method: 'get', suffix: '/artifacts/file.txt/download' }, + { method: 'get', suffix: '/activity' }, + { method: 'get', suffix: '/status' }, + { method: 'get', suffix: '/health' }, + { method: 'get', suffix: '/status/stream' }, + { method: 'get', suffix: '/activity/stream' }, + { method: 'get', suffix: '/tasks/task-1/progress/stream' }, +] +const invalidCapturedRefs = [ + '*', + '%2A', + '%252A', + 'Host', + '-host', + 'host-', + 'bad.name', + 'bad%20name', + 'bad_host', + 'bad%2Fhost', + 'bad%0Ahost', + 'bad%00host', + 'h'.repeat(64), +] + +async function callRoute(app: express.Express, route: Route, encodedHostRef: string) { + const path = `/rpc/hosts/${encodedHostRef}${route.suffix}` + const call = request(app)[route.method](path).set('authorization', 'Bearer legacy-token') + return route.body === undefined ? call : call.send(route.body) +} + +beforeEach(() => { + vi.clearAllMocks() + authMock.verifyRpcToken.mockReturnValue(legacyClaims) + admissionMock.requestHostRpcAdmission.mockResolvedValue(undefined) + hostMock.resolveHostConnectionForUser.mockResolvedValue(null) + hostMock.resolveArtifactReadHostConnectionForUser.mockResolvedValue(null) +}) + +afterEach(() => { + vi.unstubAllGlobals() +}) + +describe('public Host-ref validation', () => { + it('uses the Control API Host-create grammar exactly', async () => { + const { HOST_REF_RE } = await import('../middleware/hostRefValidation.js') + const authority = readFileSync( + resolve(process.cwd(), '../control-api/src/http/rfc1123.ts'), + 'utf8' + ) + const authoritativePattern = authority.match(/export const RFC1123_RE\s*=\s*(\/[^\n]+\/)/) + expect(authoritativePattern?.[1]).toBe(HOST_REF_RE.toString()) + }) + + it.each(legacyRoutes)( + '$method $suffix rejects invalid captured Host refs before resolution', + async route => { + const response = await callRoute(mountedApp(), route, 'invalid_host') + expect(response.status).toBe(400) + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + expect(hostMock.resolveArtifactReadHostConnectionForUser).not.toHaveBeenCalled() + expect(hostMock.requestHostWakeFromControlApi).not.toHaveBeenCalled() + expect(admissionMock.requestHostRpcAdmission).not.toHaveBeenCalled() + } + ) + + it.each(legacyRoutes)( + '$method $suffix rejects every captured invalid Host before effects', + async route => { + for (const encodedHostRef of invalidCapturedRefs) { + vi.clearAllMocks() + const response = await callRoute(mountedApp(), route, encodedHostRef) + expect(response.status).toBe(400) + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + expect(hostMock.resolveArtifactReadHostConnectionForUser).not.toHaveBeenCalled() + expect(hostMock.requestHostWakeFromControlApi).not.toHaveBeenCalled() + expect(admissionMock.requestHostRpcAdmission).not.toHaveBeenCalled() + } + } + ) + + it.each(w1PayloadRoutes)( + 'rejects invalid Host at W1 for valid $method $suffix input before admission or effects', + async route => { + const downstreamEffect = vi.fn() + vi.stubGlobal('fetch', downstreamEffect) + const response = await callRoute(mountedApp(), route, 'invalid_host') + + expect(response.status).toBe(400) + expect(response.body).toEqual({ error: 'Invalid hostRef' }) + expect(admissionMock.requestHostRpcAdmission).not.toHaveBeenCalled() + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + expect(hostMock.resolveArtifactReadHostConnectionForUser).not.toHaveBeenCalled() + expect(hostMock.requestHostWakeFromControlApi).not.toHaveBeenCalled() + expect(downstreamEffect).not.toHaveBeenCalled() + } + ) + + it.each(w1PayloadRoutes)( + 'valid Host control reaches live authority after preflight for $method $suffix', + async route => { + authMock.verifyRpcToken.mockReturnValue({ ...legacyClaims, hostRefs: ['valid-host'] }) + const downstreamEffect = vi.fn() + vi.stubGlobal('fetch', downstreamEffect) + const response = await callRoute(mountedApp(), route, 'valid-host') + + expect(response.status).toBe(403) + expect(response.body).toEqual({ error: 'Forbidden: user cannot access this host' }) + expect(admissionMock.requestHostRpcAdmission).toHaveBeenCalledTimes(1) + expect(hostMock.resolveHostConnectionForUser).toHaveBeenCalledWith( + 'user-1', + 'valid-host', + 'legacy-token', + expect.anything() + ) + expect(downstreamEffect).not.toHaveBeenCalled() + } + ) + + it.each([ + { + name: 'approval approve', + operationId: 'task.manage', + resourceType: 'runtime_session', + resourceId: 'session-a', + target: { + hostRef: 'mcp-host/chatllm', + taskId: '60000000-0000-4000-8000-000000000006', + action: 'approve', + approvalRequestId: '70000000-0000-4000-8000-000000000007', + }, + suffix: '/approvals/approve', + body: { + taskId: '60000000-0000-4000-8000-000000000006', + toolCallId: '70000000-0000-4000-8000-000000000007', + }, + }, + { + name: 'approval deny', + operationId: 'task.manage', + resourceType: 'runtime_session', + resourceId: 'session-a', + target: { + hostRef: 'mcp-host/chatllm', + taskId: '60000000-0000-4000-8000-000000000006', + action: 'deny', + approvalRequestId: '70000000-0000-4000-8000-000000000007', + }, + suffix: '/approvals/deny', + body: { + taskId: '60000000-0000-4000-8000-000000000006', + toolCallId: '70000000-0000-4000-8000-000000000007', + }, + }, + { + name: 'model write', + operationId: 'model.select', + resourceType: 'runtime_session', + resourceId: 'session-a', + target: { + hostRef: 'mcp-host/chatllm', + agent: 'agent', + chatId: 'chat', + provider: 'openai', + model: 'model', + }, + suffix: '/model', + body: { agent: 'agent', chatId: 'chat', provider: 'openai', model: 'model' }, + }, + ])('rejects producer-bound v2 $name at W1 before checkpoint or effects', async route => { + const producerOutput = execFileSync( + tsx, + [ + bodyBoundDelegationProducer, + JSON.stringify({ + operationId: route.operationId, + resourceType: route.resourceType, + resourceId: route.resourceId, + target: route.target, + }), + ], + { cwd: repositoryRoot, encoding: 'utf8', env: process.env } + ) + const { token } = JSON.parse(producerOutput) as { token: string } + const checkpoint = vi.fn( + async () => + new Response(JSON.stringify({ error: 'checkpoint_should_not_run' }), { + status: 503, + headers: { 'content-type': 'application/json' }, + }) + ) + vi.stubGlobal('fetch', checkpoint) + + const response = await request(mountedApp()) + .post(`/rpc/hosts/%20chatllm%20${route.suffix}`) + .set('authorization', `Bearer ${token}`) + .send(route.body) + + expect(response.status).toBe(400) + expect(response.body).toEqual({ error: 'Invalid hostRef' }) + expect(checkpoint).not.toHaveBeenCalled() + expect(admissionMock.requestHostRpcAdmission).not.toHaveBeenCalled() + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + expect(hostMock.requestHostWakeFromControlApi).not.toHaveBeenCalled() + }) + + it.each(['a', 'a'.repeat(63), 'valid-host-1'])( + 'allows valid boundary %s to reach the existing live authority path', + async hostRef => { + authMock.verifyRpcToken.mockReturnValue({ ...legacyClaims, hostRefs: [hostRef] }) + const response = await callRoute(mountedApp(), { method: 'get', suffix: '/status' }, hostRef) + expect(response.status).toBe(403) + expect(hostMock.resolveHostConnectionForUser).toHaveBeenCalledWith( + 'user-1', + hostRef, + 'legacy-token', + expect.anything() + ) + expect(admissionMock.requestHostRpcAdmission).toHaveBeenCalledTimes(1) + } + ) + + it.each([legacyRoutes[5], legacyRoutes[11]])( + 'retains 404 for a valid but inaccessible $suffix task Host', + async route => { + const response = await callRoute(mountedApp(), route, 'valid-host') + expect(response.status).toBe(404) + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + } + ) + + it('keeps the v2-only session-search denial before W1 for a legacy token', async () => { + const response = await request(mountedApp()) + .get('/rpc/hosts/invalid_host/sessions/search?q=x') + .set('authorization', 'Bearer legacy-token') + expect(response.status).toBe(403) + expect(response.body).toEqual({ error: 'User delegation v2 required for session search' }) + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + }) + + it('keeps authentication and scope denial ahead of W1', async () => { + await request(mountedApp()).get('/rpc/hosts/invalid_host/status').expect(401) + authMock.verifyRpcToken.mockReturnValue({ ...legacyClaims, scopes: [] }) + await request(mountedApp()) + .get('/rpc/hosts/invalid_host/status') + .set('authorization', 'Bearer legacy-token') + .expect(403) + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + }) + + it('rejects a whitespace Host ref after real v2 binding but before the remote checkpoint', async () => { + const producerOutput = execFileSync(tsx, [hostWakeDelegationProducer], { + cwd: repositoryRoot, + encoding: 'utf8', + env: process.env, + }) + const { token } = JSON.parse(producerOutput) as { token: string } + const remoteCheckpoint = vi.fn( + async () => + new Response(JSON.stringify({ error: 'checkpoint_should_not_run' }), { + status: 503, + headers: { 'content-type': 'application/json' }, + }) + ) + vi.stubGlobal('fetch', remoteCheckpoint) + + // The real producer signs mcp-host/chatllm. The local route binder's + // existing trim makes this exact target match, while W1 must inspect and + // reject the captured bare segment without normalization. + const response = await request(mountedApp()) + .post('/rpc/hosts/%20chatllm%20/wake') + .set('authorization', `Bearer ${token}`) + .send({ wakeReason: 'explicit' }) + + expect(response.status).toBe(400) + expect(response.body).toEqual({ error: 'Invalid hostRef' }) + expect(remoteCheckpoint).not.toHaveBeenCalled() + expect(hostMock.requestHostWakeFromControlApi).not.toHaveBeenCalled() + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + }) + + it('rejects real v2 target substitutions during local binding before W1 or checkpoint I/O', async () => { + const producerOutput = execFileSync(tsx, [hostWakeDelegationProducer], { + cwd: repositoryRoot, + encoding: 'utf8', + env: process.env, + }) + const { token } = JSON.parse(producerOutput) as { token: string } + const remoteCheckpoint = vi.fn() + vi.stubGlobal('fetch', remoteCheckpoint) + + for (const routeHost of ['other-host', 'invalid_host']) { + const response = await request(mountedApp()) + .post(`/rpc/hosts/${routeHost}/wake`) + .set('authorization', `Bearer ${token}`) + .send({ wakeReason: 'explicit' }) + + expect(response.status).toBe(400) + expect(response.body).toEqual({ error: 'invalid_binding' }) + } + expect(remoteCheckpoint).not.toHaveBeenCalled() + expect(hostMock.requestHostWakeFromControlApi).not.toHaveBeenCalled() + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + }) + + it('runs message preflight and W1 after exact binding but before the remote checkpoint', async () => { + const producerOutput = execFileSync( + tsx, + [ + bodyBoundDelegationProducer, + JSON.stringify({ + operationId: 'chat.message.invoke', + resourceType: 'host', + resourceId: 'mcp-host/chatllm', + target: { + hostRef: 'mcp-host/chatllm', + channelType: 'rpc', + channelId: 'chatllm', + }, + }), + ], + { cwd: repositoryRoot, encoding: 'utf8', env: process.env } + ) + const fixture = JSON.parse(producerOutput) as { token: string; messageId: string } + const checkpoint = vi.fn() + vi.stubGlobal('fetch', checkpoint) + + const invalidRef = await request(mountedApp()) + .post('/rpc/hosts/%20chatllm%20/messages') + .set('authorization', `Bearer ${fixture.token}`) + .send({ content: 'hello', messageId: fixture.messageId }) + expect(invalidRef.status).toBe(400) + expect(invalidRef.body).toEqual({ error: 'Invalid hostRef' }) + expect(checkpoint).not.toHaveBeenCalled() + + const invalidMessageAndRef = await request(mountedApp()) + .post('/rpc/hosts/%20chatllm%20/messages') + .set('authorization', `Bearer ${fixture.token}`) + .send({ content: '', messageId: fixture.messageId }) + expect(invalidMessageAndRef.status).toBe(400) + expect(invalidMessageAndRef.body).toEqual({ error: 'Invalid host message request payload' }) + expect(checkpoint).not.toHaveBeenCalled() + + const invalidMessage = await request(mountedApp()) + .post('/rpc/hosts/chatllm/messages') + .set('authorization', `Bearer ${fixture.token}`) + .send({ content: '', messageId: fixture.messageId }) + expect(invalidMessage.status).toBe(400) + expect(invalidMessage.body).toEqual({ error: 'Invalid host message request payload' }) + expect(checkpoint).not.toHaveBeenCalled() + }) + + it('does not convert a missing segment into a captured invalid Host', async () => { + await request(mountedApp()) + .get('/rpc/hosts//status') + .set('authorization', 'Bearer legacy-token') + .expect(404) + }) + + it('preserves malformed JSON precedence for a legacy session rename', async () => { + const response = await request(mountedApp()) + .patch('/rpc/hosts/invalid_host/sessions/agent/chat/name') + .set('authorization', 'Bearer legacy-token') + .set('content-type', 'application/json') + .send('{') + expect(response.status).toBe(400) + expect(response.text).not.toContain('Invalid hostRef') + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + }) + + it('returns W1 400 after valid JSON on a legacy session rename', async () => { + const response = await request(mountedApp()) + .patch('/rpc/hosts/invalid_host/sessions/agent/chat/name') + .set('authorization', 'Bearer legacy-token') + .send({ title: 'Valid title' }) + expect(response.status).toBe(400) + expect(response.body).toEqual({ error: 'Invalid hostRef' }) + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + }) + + it.each([ + { method: 'get' as const, suffix: '/tasks/bad.id/result' }, + { method: 'post' as const, suffix: '/tasks/bad.id/cancel' }, + ])('keeps task-ID denial before W1 on $method $suffix', async route => { + const response = await callRoute(mountedApp(), route, 'invalid_host') + expect(response.status).toBe(400) + expect(response.body).toEqual({ error: 'Invalid taskId' }) + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + }) + + it('keeps progress task-ID denial before W1', async () => { + const response = await callRoute( + mountedApp(), + { method: 'get', suffix: '/tasks/bad.id/progress/stream' }, + 'invalid_host' + ) + expect(response.status).toBe(400) + expect(response.body).toEqual({ + error: 'taskId is required and must be alphanumeric (max 128 chars)', + }) + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + }) + + it.each([ + { + suffix: '/status/stream', + error: 'Status stream is read-only and does not accept request bodies', + }, + { + suffix: '/activity/stream', + error: 'Activity stream is read-only and does not accept request bodies', + }, + { + suffix: '/tasks/task-1/progress/stream', + error: 'Progress stream is read-only and does not accept request bodies', + }, + ])('keeps read-only body denial before W1 on $suffix', async route => { + const response = await request(mountedApp()) + .get(`/rpc/hosts/invalid_host${route.suffix}`) + .set('authorization', 'Bearer legacy-token') + .set('content-length', '1') + expect(response.status).toBe(400) + expect(response.body).toEqual({ error: route.error }) + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + }) + + it.each([ + { suffix: '/sessions?limit=0', error: 'Invalid session pagination query' }, + { suffix: '/sessions/bad:agent/chat/messages', error: 'Invalid hostRef, agent, or chatId' }, + { + suffix: '/sessions/agent/bad%2Fchat/context-breakdown', + error: 'Invalid hostRef, agent, or chatId', + }, + ])('keeps pre-resolution session validation before W1 on $suffix', async route => { + const response = await request(mountedApp()) + .get(`/rpc/hosts/invalid_host${route.suffix}`) + .set('authorization', 'Bearer legacy-token') + expect(response.status).toBe(400) + expect(response.body).toEqual({ error: route.error }) + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + }) + + it('keeps message payload denial before W1', async () => { + const response = await request(mountedApp()) + .post('/rpc/hosts/invalid_host/messages') + .set('authorization', 'Bearer legacy-token') + .send({ content: '' }) + expect(response.status).toBe(400) + expect(response.body).toEqual({ error: 'Invalid host message request payload' }) + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + }) + + it('keeps message attachment denial before W1', async () => { + const response = await request(mountedApp()) + .post('/rpc/hosts/invalid_host/messages') + .set('authorization', 'Bearer legacy-token') + .send({ content: 'hello', attachments: 'invalid' }) + expect(response.status).toBe(400) + expect(response.body).toEqual({ + error: 'invalid_attachments', + message: 'Attachments must be a list.', + }) + expect(hostMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + }) +}) diff --git a/rpc-proxy/src/__tests__/session-rename-passthrough.test.ts b/rpc-proxy/src/__tests__/session-rename-passthrough.test.ts index 1c7161c19a..72f996a905 100644 --- a/rpc-proxy/src/__tests__/session-rename-passthrough.test.ts +++ b/rpc-proxy/src/__tests__/session-rename-passthrough.test.ts @@ -242,6 +242,26 @@ describe('PATCH /rpc/hosts/:hostRef/sessions/:agent/:chatId/name — rename pass expect(upstreamHeaders['x-clerum-edge-user-id']).toBeUndefined() }) + it('keeps malformed JSON ahead of v2 binding and W1 for a sibling Host', async () => { + delegationMock.tokenDeclaresV2.mockImplementation(token => token === 'v2-token') + const fetchMock = vi.fn() + globalThis.fetch = fetchMock as typeof fetch + + const orderedApp = express() + orderedApp.use(createRpcRouter()) + const response = await request(orderedApp) + .patch('/rpc/hosts/invalid_host/sessions/chatllm/c1/name') + .set('authorization', 'Bearer v2-token') + .set('content-type', 'application/json') + .send('{') + + expect(response.status).toBe(400) + expect(response.body).not.toMatchObject({ error: 'invalid_binding' }) + expect(fetchMock).not.toHaveBeenCalled() + expect(serviceMock.resolveHostConnectionForUser).not.toHaveBeenCalled() + expect(authTokenMock.verifyRpcToken).not.toHaveBeenCalled() + }) + it('returns 401 when no auth token is presented', async () => { const fetchMock = vi.fn() globalThis.fetch = fetchMock as unknown as typeof fetch diff --git a/rpc-proxy/src/__tests__/session-search-v2.test.ts b/rpc-proxy/src/__tests__/session-search-v2.test.ts index e8e38580c0..b1beb537c6 100644 --- a/rpc-proxy/src/__tests__/session-search-v2.test.ts +++ b/rpc-proxy/src/__tests__/session-search-v2.test.ts @@ -178,4 +178,17 @@ describe('GET /rpc/hosts/:hostRef/sessions/search v2 transport', () => { expect(fetchMock).not.toHaveBeenCalled() }) + + it('keeps v2 Host binding ahead of W1 without legacy fallback', async () => { + const fetchMock = vi.fn() + globalThis.fetch = fetchMock as typeof fetch + const response = await request(makeApp()) + .get('/rpc/hosts/invalid_host/sessions/search?q=budget') + .set('authorization', 'Bearer v2-token') + + expect(response.status).toBe(400) + expect(response.body).toMatchObject({ error: 'invalid_binding' }) + expect(fetchMock).not.toHaveBeenCalled() + expect(legacyAuthMock.verifyRpcToken).not.toHaveBeenCalled() + }) }) diff --git a/rpc-proxy/src/hostRpcPreflight.ts b/rpc-proxy/src/hostRpcPreflight.ts index 3c7d7b6fb4..258a9a3621 100644 --- a/rpc-proxy/src/hostRpcPreflight.ts +++ b/rpc-proxy/src/hostRpcPreflight.ts @@ -59,6 +59,10 @@ function fail(error: string): ParseResult { return { status: 400, body: { error } } } +function failBody(body: unknown): ParseResult { + return { status: 400, body } +} + export function hostStreamBodyError(req: AuthedRequest): string | undefined { if ( Number(req.headers['content-length'] || 0) <= 0 && @@ -80,6 +84,25 @@ function parseRoute(req: AuthedRequest): ParseResult { const hostRef = String(req.params.hostRef || '').trim() const base: HostRpcPreflight = { route, hostRef } + if (route === 'POST /rpc/hosts/:hostRef/messages') { + if (!safePathSegment(hostRef)) return fail('Invalid hostRef') + const body = req.body as unknown + if (!body || typeof body !== 'object' || Array.isArray(body)) { + return fail('Invalid host message request payload') + } + const message = body as Record + if (typeof message.content !== 'string' || !message.content.trim()) { + return fail('Invalid host message request payload') + } + if (message.attachments != null && !Array.isArray(message.attachments)) { + return failBody({ + error: 'invalid_attachments', + message: 'Attachments must be a list.', + }) + } + return { value: { ...base, body: message } } + } + if (route === 'GET /rpc/hosts/:hostRef/sessions/search') { const rawQuery = req.query.q const rawScope = req.query.scope diff --git a/rpc-proxy/src/middleware/auth.ts b/rpc-proxy/src/middleware/auth.ts index 463db3b7bf..e6daa15f2c 100644 --- a/rpc-proxy/src/middleware/auth.ts +++ b/rpc-proxy/src/middleware/auth.ts @@ -19,6 +19,8 @@ import { tokenDeclaresV2, verifyUserDelegationV2, } from '../userDelegationV2.js' +import { validateHostRef } from './hostRefValidation.js' +import { jsonBody } from './jsonBody.js' export type AuthedRequest = Request & { auth?: RpcAccessClaims @@ -28,6 +30,7 @@ export type AuthedRequest = Request & { /** Canonically parsed Spec 65 request, retained across remote checkpoint. */ hostRpcPreflight?: HostRpcPreflight authorizedActionV2?: AuthorizedActionV2 + deferHostV2Checkpoint?: boolean /** Per-inbound-request receipt; never pass this into shared wake coordination. */ hostMessageAdmissionRetryContext?: HostMessageAdmissionRetryContext } @@ -86,9 +89,16 @@ export function requireRpcAuth(req: AuthedRequest, res: Response, next: NextFunc next() } -export function requireScope(scope: RpcScope) { +export function requireScope(scope: RpcScope, options: { deferHostV2Checkpoint?: boolean } = {}) { return (req: AuthedRequest, res: Response, next: NextFunction): void => { if (req.userDelegationV2) { + if (options.deferHostV2Checkpoint) { + // Spec 61 Host-ref routes complete their exact local binding and W1 + // inside the route's established validation point before checkpoint I/O. + req.deferHostV2Checkpoint = true + next() + return + } void authorizeBoundRequestV2(req, res, next) return } @@ -138,6 +148,54 @@ export function requireHostRpcPreflightScope(scope: RpcScope) { } } +/** + * Preserve legacy scope-before-body precedence while v2 keeps parser-before-binding precedence. + * Both variants run the same canonical preflight, W1, and checkpoint stages afterward. + */ +export function requireHostRpcJsonBodyPreflightScope(scope: RpcScope) { + const bindScope = bindHostRpcScope(scope) + return (req: AuthedRequest, res: Response, next: NextFunction): void => { + const bindThenPreflight = (): void => { + bindScope(req, res, error => { + if (error) return next(error) + runHostRpcPreflightCheckpoint(req, res, next) + }) + } + if (req.userDelegationV2) { + jsonBody(req, res, error => { + if (error) return next(error) + bindThenPreflight() + }) + return + } + bindScope(req, res, error => { + if (error) return next(error) + jsonBody(req, res, parseError => { + if (parseError) return next(parseError) + runHostRpcPreflightCheckpoint(req, res, next) + }) + }) + } +} + +/** W1 path for Spec 61 routes outside the Spec 65 admission class (artifacts). */ +export function requireHostRefCheckpointScope(scope: RpcScope) { + const bindScope = bindHostRpcScope(scope) + return (req: AuthedRequest, res: Response, next: NextFunction): void => { + bindScope(req, res, error => { + if (error) return next(error) + if (!validateHostRef(req, res)) return + checkpointBoundHostRpcAction(req, res, next) + }) + } +} + +/** W1 after a route-specific deterministic validator, before the remote checkpoint. */ +export function runHostRefCheckpoint(req: AuthedRequest, res: Response, next: NextFunction): void { + if (!validateHostRef(req, res)) return + checkpointBoundHostRpcAction(req, res, next) +} + export function runHostRpcPreflightCheckpoint( req: AuthedRequest, res: Response, @@ -145,6 +203,7 @@ export function runHostRpcPreflightCheckpoint( ): void { hostRpcRoutePreflight(req, res, error => { if (error) return next(error) + if (!validateHostRef(req, res)) return checkpointBoundHostRpcAction(req, res, next) }) } diff --git a/rpc-proxy/src/middleware/hostRefValidation.ts b/rpc-proxy/src/middleware/hostRefValidation.ts new file mode 100644 index 0000000000..a7e7999036 --- /dev/null +++ b/rpc-proxy/src/middleware/hostRefValidation.ts @@ -0,0 +1,38 @@ +import type { Request, RequestHandler, Response } from 'express' +import { + authorizeDeferredHostRouteActionV2, + bindDeferredHostRouteActionV2, +} from '../routeActionBindingV2.js' +import type { AuthedRequest } from './auth.js' + +// Mirrors the authoritative Control API Host-create metadata.name contract. +// Express has already decoded the captured segment; do not normalize it. +export const HOST_REF_RE = /^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$/ + +export function validateHostRef(req: Request, res: Response): boolean { + const hostRef = req.params.hostRef + if (typeof hostRef !== 'string' || !HOST_REF_RE.test(hostRef)) { + res.status(400).json({ error: 'Invalid hostRef' }) + return false + } + return true +} + +/** Completes the approved auth/local-bind → W1 → remote-checkpoint sequence. */ +export async function validateHostRefAndAuthorize( + req: AuthedRequest, + res: Response +): Promise { + if (req.deferHostV2Checkpoint && !bindDeferredHostRouteActionV2(req, res)) return false + if (!validateHostRef(req, res)) return false + if (req.deferHostV2Checkpoint) { + return authorizeDeferredHostRouteActionV2(req, res) + } + return true +} + +export const requireValidHostRef: RequestHandler = (req, res, next) => { + void validateHostRefAndAuthorize(req as AuthedRequest, res).then(valid => { + if (valid) next() + }) +} diff --git a/rpc-proxy/src/routeActionBindingV2.ts b/rpc-proxy/src/routeActionBindingV2.ts index 29970e85c5..51067f0334 100644 --- a/rpc-proxy/src/routeActionBindingV2.ts +++ b/rpc-proxy/src/routeActionBindingV2.ts @@ -430,37 +430,99 @@ export async function authorizePreparedBoundRequestV2( res.status(400).json({ error: 'invalid_binding' }) return } - const hostMessageAdmission = - bound.operationId === 'chat.message.invoke' - ? { - sendNonce: randomBytes(32).toString('base64url'), - delegationExpiresAt: claims.exp, - } - : undefined - if (hostMessageAdmission) req.hostMessageAdmissionRetryContext = hostMessageAdmission + req.boundActionV2 = bound + if (bound.operationId === 'chat.message.invoke' && !req.hostMessageAdmissionRetryContext) { + req.hostMessageAdmissionRetryContext = { + sendNonce: randomBytes(32).toString('base64url'), + delegationExpiresAt: claims.exp, + } + } + const authorized = await authorizeBoundActionV2(req, res, options.authorize) + if (authorized) next() +} + +/** Performs only the pure exact local route/delegation binding for a staged Host-ref route. */ +export function bindDeferredHostRouteActionV2(req: AuthedRequest, res: Response): boolean { + const claims = req.userDelegationV2 + if (!claims) return true + if (req.boundActionV2) return true + + let bound: BoundActionV2 try { - req.authorizedActionV2 = await (options.authorize ?? authorizeActionV2)( + if (!req.deferHostV2Checkpoint) throw new RouteActionBindingError('invalid_binding') + bound = bindRouteActionV2(req, claims) + } catch (error) { + if (error instanceof RouteActionBindingError) { + res.status(400).json({ error: 'invalid_binding' }) + } else { + res.status(503).json({ error: 'authority_unavailable' }) + } + return false + } + + req.boundActionV2 = bound + return true +} + +/** Runs the existing remote checkpoint only after the route's local gates pass. */ +export async function authorizeDeferredHostRouteActionV2( + req: AuthedRequest, + res: Response +): Promise { + if (!req.userDelegationV2) return true + if (!req.boundActionV2 && !bindDeferredHostRouteActionV2(req, res)) return false + return authorizeBoundActionV2(req, res) +} + +async function authorizeBoundActionV2( + req: AuthedRequest, + res: Response, + authorize: ( + claims: UserDelegationV2Claims, + bound: BoundActionV2, + options?: { + hostMessageAdmission?: HostMessageAdmissionCheckpointContext + onHostMessageAdmissionReceipt?: (receipt: string) => void + } + ) => Promise = authorizeActionV2 +): Promise { + const claims = req.userDelegationV2 + const bound = req.boundActionV2 + if (!claims || !bound) { + res.status(401).json({ error: 'Unauthorized' }) + return false + } + if (bound.operationId === 'chat.message.invoke' && !req.hostMessageAdmissionRetryContext) { + req.hostMessageAdmissionRetryContext = { + sendNonce: randomBytes(32).toString('base64url'), + delegationExpiresAt: claims.exp, + } + } + const admissionContext = req.hostMessageAdmissionRetryContext + try { + req.authorizedActionV2 = await authorize( claims, bound, - hostMessageAdmission + admissionContext ? { - hostMessageAdmission, + hostMessageAdmission: admissionContext, onHostMessageAdmissionReceipt: receipt => { req.hostMessageAdmissionRetryContext = Object.freeze({ - ...hostMessageAdmission, + ...admissionContext, receipt, }) }, } : undefined ) - next() + return true } catch (error) { if (error instanceof ActionAuthorityCheckpointError) { sendCheckpointError(res, error) - return + } else { + res.status(503).json({ error: 'authority_unavailable' }) } - res.status(503).json({ error: 'authority_unavailable' }) + return false } } diff --git a/rpc-proxy/src/routes/rpc.ts b/rpc-proxy/src/routes/rpc.ts index 07f207a3e1..0b99ef7d9f 100644 --- a/rpc-proxy/src/routes/rpc.ts +++ b/rpc-proxy/src/routes/rpc.ts @@ -13,10 +13,13 @@ import { type AuthedRequest, bindHostRpcScope, extractAuthToken, + requireHostRefCheckpointScope, + requireHostRpcJsonBodyPreflightScope, requireHostRpcPreflightScope, requireRpcAuth, requireScope, requireV2SessionSearch, + runHostRefCheckpoint, runHostRpcPreflightCheckpoint, } from '../middleware/auth.js' import { chatJsonBody } from '../middleware/chatJsonBody.js' @@ -457,7 +460,8 @@ export function createRpcRouter(): Router { '/rpc/hosts/:hostRef/messages', requireRpcAuth, chatJsonBody, - requireScope('host:message:invoke'), + bindHostRpcScope('host:message:invoke'), + runHostRpcPreflightCheckpoint, async (req: AuthedRequest, res) => { // Host runtime write path (REST-oriented): // - separate from read-only status stream @@ -465,21 +469,8 @@ export function createRpcRouter(): Router { try { const auth = req.auth! const rpcAccessToken = extractAuthToken(req) - const hostRef = String(req.params.hostRef || '').trim() - if (!isSafeUpstreamPathSegment(hostRef)) { - res.status(400).json({ error: 'Invalid hostRef' }) - return - } - - const body = req.body as HostRuntimeMessageRequest - if (!body || typeof body !== 'object' || Array.isArray(body)) { - res.status(400).json({ error: 'Invalid host message request payload' }) - return - } - if (typeof body.content !== 'string' || !body.content.trim()) { - res.status(400).json({ error: 'Invalid host message request payload' }) - return - } + const { hostRef, body: preflightBody } = getHostRpcPreflight(req) + const body = preflightBody as HostRuntimeMessageRequest // Identity invariant: rpc-proxy is the sole authority on the Desktop // RPC envelope. Ignore client-supplied channel, sender, host, and @@ -524,12 +515,6 @@ export function createRpcRouter(): Router { requestId, origin: 'direct_chat', }) - if (body.attachments != null && !Array.isArray(body.attachments)) { - res - .status(400) - .json({ error: 'invalid_attachments', message: 'Attachments must be a list.' }) - return - } const forwardedBody: HostRuntimeMessageRequest = { content: body.content, channelType: 'rpc', @@ -1220,9 +1205,7 @@ export function createRpcRouter(): Router { router.patch( '/rpc/hosts/:hostRef/sessions/:agent/:chatId/name', requireRpcAuth, - bindHostRpcScope('host:session:write'), - jsonBody, - runHostRpcPreflightCheckpoint, + requireHostRpcJsonBodyPreflightScope('host:session:write'), async (req: AuthedRequest, res, next) => { try { const auth = req.auth! @@ -1546,7 +1529,7 @@ export function createRpcRouter(): Router { router.get( '/rpc/hosts/:hostRef/artifacts', requireRpcAuth, - requireScope('host:task:read'), + requireHostRefCheckpointScope('host:task:read'), resolveArtifactReadHost, artifactReadEdgeRateLimit, async (req: AuthedRequest, res, next) => { @@ -1591,7 +1574,7 @@ export function createRpcRouter(): Router { router.get( '/rpc/hosts/:hostRef/artifacts/:filename/download', requireRpcAuth, - requireScope('host:task:read'), + bindHostRpcScope('host:task:read'), (req: AuthedRequest, res, next) => { const filename = String(req.params.filename || '').trim() if ( @@ -1606,6 +1589,7 @@ export function createRpcRouter(): Router { } next() }, + runHostRefCheckpoint, resolveArtifactReadHost, artifactReadEdgeRateLimit, async (req: AuthedRequest, res, next) => { diff --git a/rpc-proxy/test/routes.rpc.v2-session-retry.test.ts b/rpc-proxy/test/routes.rpc.v2-session-retry.test.ts index ca36f9aee5..dbf3b65754 100644 --- a/rpc-proxy/test/routes.rpc.v2-session-retry.test.ts +++ b/rpc-proxy/test/routes.rpc.v2-session-retry.test.ts @@ -41,7 +41,11 @@ vi.mock('../src/middleware/auth.js', () => ({ }, requireScope: () => (_req: unknown, _res: unknown, next: () => void) => next(), requireHostRpcPreflightScope: () => (_req: unknown, _res: unknown, next: () => void) => next(), + requireHostRpcJsonBodyPreflightScope: () => (_req: unknown, _res: unknown, next: () => void) => + next(), + requireHostRefCheckpointScope: () => (_req: unknown, _res: unknown, next: () => void) => next(), bindHostRpcScope: () => (_req: unknown, _res: unknown, next: () => void) => next(), + runHostRefCheckpoint: (_req: unknown, _res: unknown, next: () => void) => next(), runHostRpcPreflightCheckpoint: (_req: unknown, _res: unknown, next: () => void) => next(), requireV2SessionSearch: (_req: unknown, _res: unknown, next: () => void) => next(), }))