fix: Reject placeholder-bearing admin keys from shared backfill

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SxKWxwqxAGckYpRsYTqx3F
This commit is contained in:
Claude 2026-08-30 19:31:22 +00:00
parent 5ff2c9d4a8
commit 32d5986923
No known key found for this signature in database
2 changed files with 41 additions and 1 deletions

View file

@ -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));

View file

@ -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)
);
}