From 32d59869237405104da0afb5ffefae96e0190bc7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 19:31:22 +0000 Subject: [PATCH] fix: Reject placeholder-bearing admin keys from shared backfill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex round three P1, verified end-to-end before fixing: processMCPEnv injects an admin `apiKey.key` into the request headers (env.ts:448) BEFORE header values get per-user placeholder resolution (env.ts:478), so a key like `{{LIBRECHAT_OPENID_ACCESS_TOKEN}}` makes the connection identity-scoped — while `placeholderBearingFields` never inspects `apiKey.key` and the gate rejected only `source: 'user'`. Instructions fetched under one user's identity could then be stored for everyone. The gate now scans the admin key value with the same runtime-placeholder predicate. Kept narrow deliberately: widening `placeholderBearingFields` itself would change `requiresUserScopedConnection` for every caller — connection pooling included — which is its own decision. Static admin keys still backfill (positive control test); both new refusal tests verified red without the gate change. Suite 39/39. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SxKWxwqxAGckYpRsYTqx3F --- .../resolvedInstructionsBackfill.test.ts | 34 +++++++++++++++++++ packages/api/src/mcp/utils.ts | 8 ++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/packages/api/src/mcp/registry/__tests__/resolvedInstructionsBackfill.test.ts b/packages/api/src/mcp/registry/__tests__/resolvedInstructionsBackfill.test.ts index 049a4cd799..e23f9a8f35 100644 --- a/packages/api/src/mcp/registry/__tests__/resolvedInstructionsBackfill.test.ts +++ b/packages/api/src/mcp/registry/__tests__/resolvedInstructionsBackfill.test.ts @@ -156,6 +156,13 @@ describe('MCPServersRegistry.setResolvedInstructions', () => { oauth: { authorization_url: 'https://idp.example.com/authorize' }, } as unknown as t.ParsedServerConfig, ], + [ + 'placeholder-bearing-admin-key', + { + ...startupDeferredYamlEntry, + apiKey: { source: 'admin', authorization_type: 'bearer', key: '{{LIBRECHAT_USER_ID}}' }, + } as unknown as t.ParsedServerConfig, + ], ])('refuses a %s-deferred server at the shared-registry boundary', async (_reason, config) => { await registry['cacheConfigsRepo'].add('deferred_server', config); @@ -415,11 +422,38 @@ describe('UserConnectionManager.backfillResolvedInstructions', () => { oauth_headers: { 'X-Tenant': 'per-user' }, } as unknown as t.ParsedServerConfig, ], + [ + 'an admin API key whose value is a runtime identity placeholder', + { + ...startupDeferredYamlEntry, + apiKey: { + source: 'admin', + authorization_type: 'bearer', + key: '{{LIBRECHAT_OPENID_ACCESS_TOKEN}}', + }, + } as unknown as t.ParsedServerConfig, + ], ])('does not persist instructions for %s context', async (_reason, config) => { await backfill(config, connectionWith(INSTRUCTIONS)); expect(setResolvedInstructions).not.toHaveBeenCalled(); }); + it('still persists instructions for a static admin API key', async () => { + const config = { + ...startupDeferredYamlEntry, + apiKey: { source: 'admin', authorization_type: 'bearer', key: 'static-shared-secret' }, + } as unknown as t.ParsedServerConfig; + + await backfill(config, connectionWith(INSTRUCTIONS)); + + expect(setResolvedInstructions).toHaveBeenCalledWith( + 'deferred_server', + INSTRUCTIONS, + 'user-1', + config, + ); + }); + it('still backfills when the stored config carries no source stamp', async () => { const { source: _source, ...unstamped } = startupDeferredYamlEntry; await backfill(unstamped as t.ParsedServerConfig, connectionWith(INSTRUCTIONS)); diff --git a/packages/api/src/mcp/utils.ts b/packages/api/src/mcp/utils.ts index 41457e41e9..385d7fa61e 100644 --- a/packages/api/src/mcp/utils.ts +++ b/packages/api/src/mcp/utils.ts @@ -437,7 +437,13 @@ export function canBackfillSharedServerInstructions(config: UserScopedConnection * `isOAuthServer` still arms the OAuth machinery for the unstamped case. */ config.oauth == null && config.oauth_headers == null && - config.apiKey?.source !== 'user' + config.apiKey?.source !== 'user' && + /** An admin key value can itself carry a runtime identity placeholder + * ({{LIBRECHAT_USER_ID}}, {{LIBRECHAT_OPENID_ACCESS_TOKEN}}, ...): + * processMCPEnv copies it into the request headers before per-user + * placeholder resolution, so the connection is identity-scoped even + * though `placeholderBearingFields` never sees `apiKey.key`. */ + !hasRuntimeContextPlaceholder(config.apiKey?.key) ); }