diff --git a/.env.example b/.env.example index 40b0d16..9a47ca4 100644 --- a/.env.example +++ b/.env.example @@ -11,6 +11,10 @@ COVAL_API_KEY=your_staging_api_key_here # PORT=8080 # CLERK_PUBLISHABLE_KEY= # CLERK_SECRET_KEY= +# Dedicated credential for the hosted managed-key exchange. Keep this distinct +# from the shared compatibility credential below. +# COVAL_MCP_INTERNAL_API_KEY= +# Temporary rotation fallback only; remove after the dedicated-key cutover has drained. # COVAL_INTERNAL_API_KEY= # Optional: Log level (default: info) diff --git a/README.md b/README.md index ac4a299..9f36b3e 100644 --- a/README.md +++ b/README.md @@ -170,6 +170,30 @@ transport: The hosted connector can access only the Coval organization selected during OAuth consent. Remove the connector from the client or revoke its Coval access when it is no longer needed. +### Hosted managed-key credential rotation + +Hosted deployments use `COVAL_MCP_INTERNAL_API_KEY` as the dedicated credential for managed-key +exchange. It must be distinct from `COVAL_INTERNAL_API_KEY`, must match the exact immutable secret +version selected by the API deployment, and must never be exposed to MCP clients. + +During a coordinated cutover, `COVAL_INTERNAL_API_KEY` may remain configured as a temporary +compatibility credential. The server tries the dedicated credential first and retries the +compatibility credential only once, only after an explicit `401` response. It does not fall back +for timeouts, network errors, `403` responses, or other failures. + +Use this order for rotation: + +1. Provision a new dedicated secret version and pin the API deployment to that exact version. +2. Deploy the MCP server with the new value in `COVAL_MCP_INTERNAL_API_KEY` and the prior shared + value temporarily retained in `COVAL_INTERNAL_API_KEY`. +3. Activate dedicated-credential validation at the API only after both deployments are healthy. +4. Confirm old instances and in-flight requests have drained, then remove + `COVAL_INTERNAL_API_KEY` from the MCP deployment. + +The overlap is bounded migration state, not a permanent dual-key configuration. If the dedicated +exchange is rolled back, disable its API activation before removing or changing the pinned secret +version. + ## Development ```bash @@ -196,6 +220,8 @@ npm run check:remote | `COVAL_API_KEY` | Stdio | - | Coval API key for the local stdio transport | | `COVAL_API_BASE_URL` | No | `https://api.coval.dev/v1` | API base URL | | `SOFIA_DELEGATION_ORIGIN` | No | Derived from `COVAL_API_BASE_URL` | Overrides the expected Sofia origin used to validate delegation URLs | +| `COVAL_MCP_INTERNAL_API_KEY` | Hosted remote | - | Dedicated managed-key exchange credential; must be distinct from the shared compatibility credential | +| `COVAL_INTERNAL_API_KEY` | Rotation only | - | Temporary hosted exchange fallback during a bounded dedicated-credential cutover; retried once only after `401` | | `LOG_LEVEL` | No | `info` | Logging level | | `MCP_ALLOWED_ORIGINS` | No | Claude and OpenAI web origins | Comma-separated exact browser origins allowed to call `/mcp` or `/claude/mcp`; clients that omit `Origin` remain supported | | `OPENAI_APPS_CHALLENGE` | No | - | OpenAI plugin-portal domain verification token served as plain text from `/.well-known/openai-apps-challenge` | diff --git a/src/managed-api-key.ts b/src/managed-api-key.ts index 3794a42..81bd23b 100644 --- a/src/managed-api-key.ts +++ b/src/managed-api-key.ts @@ -12,6 +12,23 @@ interface CacheEntry { expiresAt: number; } +export interface ManagedApiKeyCredentials { + primary: string; + fallback?: string; +} + +export function managedApiKeyCredentialsFromEnvironment( + environment: NodeJS.ProcessEnv = process.env, +): ManagedApiKeyCredentials { + const dedicated = environment.COVAL_MCP_INTERNAL_API_KEY?.trim() || ''; + const previous = environment.COVAL_INTERNAL_API_KEY?.trim() || ''; + if (!dedicated) return { primary: previous }; + return { + primary: dedicated, + ...(previous && previous !== dedicated ? { fallback: previous } : {}), + }; +} + export class ManagedApiKeyError extends Error { constructor( message: string, @@ -29,6 +46,7 @@ export class ManagedApiKeyProvider { constructor( private readonly internalApiKey: string, private readonly apiBaseUrl = process.env.COVAL_API_BASE_URL || DEFAULT_API_BASE_URL, + private readonly fallbackInternalApiKey = '', ) {} isConfigured(): boolean { @@ -60,18 +78,27 @@ export class ManagedApiKeyProvider { clerkOrganizationId: string, clerkUserId: string, ): Promise { - const response = await fetch(`${this.apiBaseUrl.replace(/\/$/, '')}/internal/mcp/api-key`, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'X-Coval-Internal-Api-Key': this.internalApiKey, - }, - body: JSON.stringify({ - clerk_organization_id: clerkOrganizationId, - user_id: clerkUserId, - }), - signal: AbortSignal.timeout(15_000), - }); + let response = await this.requestApiKey( + clerkOrganizationId, + clerkUserId, + this.internalApiKey, + ); + const fallbackInternalApiKey = this.fallbackInternalApiKey.trim(); + if ( + response.status === 401 && + fallbackInternalApiKey && + fallbackInternalApiKey !== this.internalApiKey.trim() + ) { + // The initial credential cutover spans independently deployed services. Retry the previous + // credential once, and only for an explicit authentication rejection during that overlap. + if (response.body) await response.body.cancel().catch(() => undefined); + response = await this.requestApiKey( + clerkOrganizationId, + clerkUserId, + fallbackInternalApiKey, + ); + } + const payload = (await response.json().catch(() => ({}))) as Partial & { error?: string; }; @@ -93,4 +120,23 @@ export class ManagedApiKeyProvider { } return payload.api_key; } + + private requestApiKey( + clerkOrganizationId: string, + clerkUserId: string, + internalApiKey: string, + ): Promise { + return fetch(`${this.apiBaseUrl.replace(/\/$/, '')}/internal/mcp/api-key`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Coval-Internal-Api-Key': internalApiKey, + }, + body: JSON.stringify({ + clerk_organization_id: clerkOrganizationId, + user_id: clerkUserId, + }), + signal: AbortSignal.timeout(15_000), + }); + } } diff --git a/src/remote.ts b/src/remote.ts index 4d94d1f..fe814ff 100644 --- a/src/remote.ts +++ b/src/remote.ts @@ -15,7 +15,11 @@ import { rewriteLegacyToolCalls, rewriteOpenAiToolCalls, } from './compatibility.js'; -import { ManagedApiKeyError, ManagedApiKeyProvider } from './managed-api-key.js'; +import { + ManagedApiKeyError, + managedApiKeyCredentialsFromEnvironment, + ManagedApiKeyProvider, +} from './managed-api-key.js'; import { COVAL_MCP_SERVER_VERSION, createMcpServer } from './server.js'; import type { ToolAnnotationProfile } from './tools/annotations.js'; @@ -148,7 +152,12 @@ export async function createRemoteApp(): Promise { }); app.use(clerkMiddleware()); - const managedKeys = new ManagedApiKeyProvider(process.env.COVAL_INTERNAL_API_KEY || ''); + const managedKeyCredentials = managedApiKeyCredentialsFromEnvironment(); + const managedKeys = new ManagedApiKeyProvider( + managedKeyCredentials.primary, + undefined, + managedKeyCredentials.fallback, + ); const oauth = await mcpAuth(async (token, req): Promise => { const auth = getAuth(req, { acceptsToken: 'oauth_token' }); if (!auth.isAuthenticated || !auth.scopes?.includes(REQUIRED_ORG_SCOPE)) { diff --git a/tests/unit/managed-api-key.test.ts b/tests/unit/managed-api-key.test.ts index 516a75c..70e09e2 100644 --- a/tests/unit/managed-api-key.test.ts +++ b/tests/unit/managed-api-key.test.ts @@ -1,9 +1,46 @@ import { jest } from '@jest/globals'; -import { ManagedApiKeyError, ManagedApiKeyProvider } from '../../src/managed-api-key.js'; +import { + ManagedApiKeyError, + managedApiKeyCredentialsFromEnvironment, + ManagedApiKeyProvider, +} from '../../src/managed-api-key.js'; describe('ManagedApiKeyProvider', () => { afterEach(() => jest.restoreAllMocks()); + it('prefers the dedicated credential and retains the previous credential for rollout overlap', () => { + expect( + managedApiKeyCredentialsFromEnvironment({ + COVAL_MCP_INTERNAL_API_KEY: ' dedicated-service-key ', + COVAL_INTERNAL_API_KEY: ' previous-service-key ', + }), + ).toEqual({ + primary: 'dedicated-service-key', + fallback: 'previous-service-key', + }); + }); + + it('uses the previous credential directly when no dedicated credential is configured', () => { + expect( + managedApiKeyCredentialsFromEnvironment({ + COVAL_INTERNAL_API_KEY: 'previous-service-key', + }), + ).toEqual({ + primary: 'previous-service-key', + }); + }); + + it('does not configure a fallback when both environment variables contain the same credential', () => { + expect( + managedApiKeyCredentialsFromEnvironment({ + COVAL_MCP_INTERNAL_API_KEY: 'same-service-key', + COVAL_INTERNAL_API_KEY: 'same-service-key', + }), + ).toEqual({ + primary: 'same-service-key', + }); + }); + it('exchanges verified Clerk identity without forwarding an OAuth token', async () => { const fetchMock = jest.spyOn(global, 'fetch').mockResolvedValue( new Response(JSON.stringify({ api_key: 'managed-user-key', organization_id: 'org_123' }), { @@ -32,6 +69,53 @@ describe('ManagedApiKeyProvider', () => { expect(JSON.stringify(fetchMock.mock.calls)).not.toContain('oauth'); }); + it('retries the previous credential once after an authentication rejection', async () => { + const fetchMock = jest + .spyOn(global, 'fetch') + .mockResolvedValueOnce(new Response('{}', { status: 401 })) + .mockResolvedValueOnce( + new Response(JSON.stringify({ api_key: 'managed-user-key', organization_id: 'org_123' }), { + status: 200, + }), + ); + const provider = new ManagedApiKeyProvider( + 'dedicated-service-key', + 'https://api.example.com/v1', + 'previous-service-key', + ); + + await expect(provider.getApiKey('clerk_org_123', 'clerk_user_123')).resolves.toBe( + 'managed-user-key', + ); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(fetchMock.mock.calls[0][1]?.headers).toEqual( + expect.objectContaining({ 'X-Coval-Internal-Api-Key': 'dedicated-service-key' }), + ); + expect(fetchMock.mock.calls[1][1]?.headers).toEqual( + expect.objectContaining({ 'X-Coval-Internal-Api-Key': 'previous-service-key' }), + ); + }); + + it('does not use the rollout fallback for non-authentication failures', async () => { + const fetchMock = jest + .spyOn(global, 'fetch') + .mockResolvedValue(new Response('{}', { status: 503 })); + const provider = new ManagedApiKeyProvider( + 'dedicated-service-key', + 'https://api.example.com/v1', + 'previous-service-key', + ); + + await expect(provider.getApiKey('clerk_org_123', 'clerk_user_123')).rejects.toEqual( + expect.objectContaining>({ + message: 'Unable to establish a Coval MCP session', + status: 502, + }), + ); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + it('reuses a bounded short-lived managed key cache', async () => { const fetchMock = jest.spyOn(global, 'fetch').mockResolvedValue( new Response(JSON.stringify({ api_key: 'managed-user-key', organization_id: 'org_123' }), {