mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-06 14:39:10 +00:00
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:
parent
5ff2c9d4a8
commit
32d5986923
2 changed files with 41 additions and 1 deletions
|
|
@ -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));
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue