diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index e15d802973..747e2401bb 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -12,6 +12,7 @@ const { const { sendEvent, getToolkitKey, + createSafeUser, getUserMCPAuthMap, loadToolDefinitions, GenerationJobManager, @@ -1252,7 +1253,35 @@ async function loadToolsForExecution({ }) { const appConfig = req.config; const allLoadedTools = []; + /** + * Re-inject user identity into the configurable returned to the + * `ON_TOOL_EXECUTE` handler. The handler merges this onto whatever + * configurable arrived with the dispatched event: + * `{ ...configurable, ...toolConfigurable }` + * For the parent agent the outer stream config (set in AgentClient / + * Responses / OpenAI controllers) already carries `user` and `user_id`; + * re-injecting the same values from `req.user` is a no-op there. + * + * For SUBAGENT tool calls the SDK's `SubagentExecutor` invokes the child + * workflow with a fresh configurable of `{ thread_id }` only — the + * parent's `user`, `user_id`, and `userMCPAuthMap` are dropped on the way + * into the child graph. Without this re-injection, downstream MCP tools + * read `config.configurable.user?.id || user_id === undefined` and + * `MCPManager.getConnection` throws "No connection found for server X" + * (it can't fall through to the user-connection path without a userId). + * + * Only set the keys when `req.user.id` is present. Because + * `toolConfigurable` wins the merge, writing `user_id: undefined` + * unconditionally would clobber the outer config's `'api-user'` + * fallback (set by Responses/OpenAI controllers when `req.user` is + * absent). Omitting the keys lets the merge preserve whatever the + * outer configurable already had. + */ const configurable = { userMCPAuthMap }; + if (req.user?.id) { + configurable.user = createSafeUser(req.user); + configurable.user_id = req.user.id; + } if (actionsEnabled === undefined) { const enabledCapabilities = await resolveAgentCapabilities(req, appConfig, agent?.id); diff --git a/api/server/services/__tests__/ToolService.spec.js b/api/server/services/__tests__/ToolService.spec.js index 63b2287684..86f00f7562 100644 --- a/api/server/services/__tests__/ToolService.spec.js +++ b/api/server/services/__tests__/ToolService.spec.js @@ -363,6 +363,88 @@ describe('ToolService - Action Capability Gating', () => { }); }); + describe('loadToolsForExecution — configurable user injection', () => { + /** + * Subagents invoked via `@librechat/agents`' SubagentExecutor receive + * a fresh `configurable` of `{ thread_id }` only — the parent's + * `user`/`user_id` are dropped on the way into the child workflow. + * The handler in `packages/api/src/agents/handlers.ts` merges + * `{ ...configurable, ...toolConfigurable }`, so the configurable + * returned here MUST carry the user identity for MCP connection + * lookup to succeed in subagent tool calls. + * + * Conversely, when `req.user` is absent (API-key path) we must NOT + * write `user`/`user_id` keys, otherwise the merge would clobber the + * outer config's `'api-user'` fallback set by Responses/OpenAI + * controllers. + */ + it('returns configurable with user and user_id from req.user', async () => { + const req = createMockReq([]); + req.config = {}; + req.user = { id: 'user_abc', email: 'a@b.c', role: 'USER' }; + + const result = await loadToolsForExecution({ + req, + res: {}, + agent: { id: 'agent_123' }, + toolNames: [], + }); + + expect(result.configurable.user_id).toBe('user_abc'); + expect(result.configurable.user).toEqual( + expect.objectContaining({ id: 'user_abc', email: 'a@b.c', role: 'USER' }), + ); + }); + + it('omits user keys when req.user is undefined so outer config fallback is preserved', async () => { + const req = createMockReq([]); + req.config = {}; + req.user = undefined; + + const result = await loadToolsForExecution({ + req, + res: {}, + agent: { id: 'agent_123' }, + toolNames: [], + }); + + expect('user_id' in result.configurable).toBe(false); + expect('user' in result.configurable).toBe(false); + }); + + it('omits user keys when req.user is null so outer config fallback is preserved', async () => { + const req = createMockReq([]); + req.config = {}; + req.user = null; + + const result = await loadToolsForExecution({ + req, + res: {}, + agent: { id: 'agent_123' }, + toolNames: [], + }); + + expect('user_id' in result.configurable).toBe(false); + expect('user' in result.configurable).toBe(false); + }); + + it('omits user keys when req.user is present but has no id', async () => { + const req = createMockReq([]); + req.config = {}; + req.user = { email: 'a@b.c' }; + + const result = await loadToolsForExecution({ + req, + res: {}, + agent: { id: 'agent_123' }, + toolNames: [], + }); + + expect('user_id' in result.configurable).toBe(false); + expect('user' in result.configurable).toBe(false); + }); + }); + describe('checkCapability logic', () => { const createCheckCapability = (enabledCapabilities, logger = { warn: jest.fn() }) => { return (capability) => { diff --git a/packages/api/src/endpoints/google/initialize.ts b/packages/api/src/endpoints/google/initialize.ts index 812769e030..dcf750bb87 100644 --- a/packages/api/src/endpoints/google/initialize.ts +++ b/packages/api/src/endpoints/google/initialize.ts @@ -32,7 +32,7 @@ export async function initializeGoogle({ let userKey = null; if (expiresAt && isUserProvided) { checkUserKeyExpiry(expiresAt, EModelEndpoint.google); - userKey = await db.getUserKey({ userId: req.user?.id, name: EModelEndpoint.google }); + userKey = await db.getUserKey({ userId: req.user?.id ?? '', name: EModelEndpoint.google }); } let serviceKey: Record = {}; diff --git a/packages/api/src/middleware/remoteAgentAuth.ts b/packages/api/src/middleware/remoteAgentAuth.ts index ab06583c41..0471352744 100644 --- a/packages/api/src/middleware/remoteAgentAuth.ts +++ b/packages/api/src/middleware/remoteAgentAuth.ts @@ -462,7 +462,15 @@ export function createRemoteAgentAuth({ updateUser, getAppConfig, }: RemoteAgentAuthDeps): RequestHandler { - return async (req: Request, res: Response, next: NextFunction) => { + /** + * Annotated as `express.Request` (and helpers below take the same type) + * so the local `Request.user` augmentation in `src/types/express.d.ts` + * applies inside the closure. The closure is then cast to + * `RequestHandler` at the return — `RequestHandler`'s internal + * `Request` resolves through `express-serve-static-core` and lacks the + * augmentation, so a direct return would mismatch on `user`. + */ + const handler = async (req: Request, res: Response, next: NextFunction) => { try { const initialConfigOptions = getConfigOptions(req); const config = await getAppConfig(initialConfigOptions); @@ -559,4 +567,5 @@ export function createRemoteAgentAuth({ return; } }; + return handler as RequestHandler; } diff --git a/packages/api/src/utils/env.ts b/packages/api/src/utils/env.ts index f71a131c09..b5220f1ae8 100644 --- a/packages/api/src/utils/env.ts +++ b/packages/api/src/utils/env.ts @@ -92,7 +92,14 @@ export function createSafeUser( const safeUser: Partial & { federatedTokens?: IUser['federatedTokens'] } = {}; for (const field of ALLOWED_USER_FIELDS) { if (field in user) { - safeUser[field] = user[field]; + /** + * Indexed write through a union-typed key would otherwise fail strict + * checking — TS computes the LHS type as the *intersection* of all + * field write types (which collapses to `undefined` when fields have + * mixed types). `Object.assign` widens the assignment so each field + * preserves its concrete type at runtime. + */ + Object.assign(safeUser, { [field]: user[field] }); } } diff --git a/packages/data-schemas/src/types/user.ts b/packages/data-schemas/src/types/user.ts index d4ab3356f9..86d382a711 100644 --- a/packages/data-schemas/src/types/user.ts +++ b/packages/data-schemas/src/types/user.ts @@ -4,6 +4,14 @@ import { CursorPaginationParams } from '~/common'; export interface IUser extends Document { _id: Types.ObjectId; + /** + * Mongoose's `Document.id` virtual is typed `id?: any`. At runtime it's + * always `_id.toString()` for a hydrated doc, so narrow to a required + * string. This also lets `IUser` satisfy Express.User augmentations + * (the OIDC remote-agent middleware assigns `req.user = IUser` where + * the project's local `Express.User` requires `id: string`). + */ + id: string; name?: string; username?: string; email: string;