When joining a PENDING flow, reusedStoredClient was only set on the
success return but not before the await. If createFlow throws (e.g.
invalid_client during token exchange), the outer catch returns the
local variable which was still false, skipping stale-client cleanup.
Client registration reuse without cleanup capability creates a
permanent failure loop: if the reused client is stale, the code
detects the rejection but cannot clear the stored registration
because deleteTokens is missing, so every retry reuses the same
broken client_id.
- MCPConnectionFactory: only pass findToken to initiateOAuthFlow
when deleteTokens is also available, ensuring reuse is only
enabled when recovery is possible
- api/server/services/MCP.js: add deleteTokens to the tokenMethods
object (was the only MCP call site missing it)
The previous restructuring moved oauthError and missing-code checks
behind CSRF validation, breaking tests that expect those redirects
without cookies. The redirect itself is harmless (just shows an error
page). Only the failFlow call needs CSRF gating to prevent DoS.
Restructure: oauthError check stays early (redirects immediately),
but failFlow inside it runs the full CSRF/session/active-flow
validation before marking the flow as FAILED.
- OAuth callback: move failFlow call to after CSRF/session/active-flow
validation so an attacker with only a leaked state parameter cannot
force-fail a flow without passing the same integrity checks required
for legitimate callbacks
- PENDING join path: propagate reusedStoredClient from flow metadata
into the return object so joiners can trigger stale-client cleanup
if the joined flow later fails with a client rejection
- Issuer check: re-register when storedIssuer is absent or non-string
instead of silently reusing. Narrows unknown type with typeof guard
and inverts condition so missing issuer → fresh DCR (safer default).
- OAuth callback route: call failFlow with the OAuth error when the
authorization server redirects back with error= parameter, so the
waiting flow receives the actual rejection instead of timing out.
This lets isClientRejection match stale-client errors correctly.
- Extract duplicated cleanup block to clearStaleClientIfRejected()
private method, called from both returnOnOAuth and blocking paths.
- Test fixes: add issuer to stored metadata in reuse tests, reset
server to undefined in afterEach to prevent double-close.
- Add isClientRejection unit tests: invalid_client, unauthorized_client,
client_id mismatch, client not found, unknown client, and negative
cases (timeout, flow state not found, user denied, null, undefined)
- Enhance OAuth test server with enforceClientId option: binds auth
codes to the client_id that initiated /authorize, rejects token
exchange with mismatched or unregistered client_id (401 invalid_client)
- Add integration tests proving the test server correctly rejects
stale client_ids and accepts matching ones at /token
The returnOnOAuth cleanup was unreliable: it depended on reading
FAILED flow state, but FlowStateManager.monitorFlow() deletes FAILED
state before rejecting. Move cleanup into createFlow's catch handler
where flowMetadata.reusedStoredClient is still in scope.
Make cleanup selective in both paths: add isClientRejection() helper
that only matches errors indicating the OAuth server rejected the
client_id (invalid_client, unauthorized_client, client not found).
Timeouts, user-cancelled flows, and other transient failures no
longer wipe valid stored registrations.
Thread the error from handleOAuthRequired() through the return type
so the blocking path can also check isClientRejection().
FlowStateManager.createFlow() deletes FAILED flow state before
rejecting, so getFlowState() after handleOAuthRequired() returns null
would find nothing — making the stale-client cleanup dead code.
Fix: hoist reusedStoredClient flag from flowMetadata into a local
variable, include it in handleOAuthRequired()'s return type (both
success and catch paths), and use result.reusedStoredClient directly
in the caller instead of a second getFlowState() round-trip.
- Blocking path: remove result?.clientInfo guard that made cleanup
unreachable (handleOAuthRequired returns null on failure, so
result?.clientInfo was always false in the failure branch)
- returnOnOAuth path: only clear stored client when the prior flow
status is FAILED, not on COMPLETED or PENDING flows, to avoid
deleting valid registrations during normal flow replacement
- N3: Type deleteClientRegistration param as TokenMethods['deleteTokens']
instead of Promise<unknown>
- N5: Elevate deletion failure logging from debug to warn for operator
visibility when stale client cleanup fails
- N6: Use getLogPrefix() instead of hardcoded log prefix to respect
system-user privacy convention
- Gate client reuse on authorization server identity: compare stored
issuer against freshly discovered metadata before reusing, preventing
wrong-client reuse when the MCP server switches auth providers
- Add reusedStoredClient flag to MCPOAuthFlowMetadata so cleanup only
runs when the failed flow actually reused a stored registration,
not on unrelated failures (timeouts, user-denied consent, etc.)
- Add cleanup in returnOnOAuth path: when a prior flow that reused a
stored client is detected as failed, clear the stale registration
before re-initiating
- Add tests for issuer mismatch and reusedStoredClient flag assertions
When a stored client_id is no longer recognized by the OAuth server,
the flow fails but the stale client stays in MongoDB, causing every
retry to reuse the same invalid registration in an infinite loop.
On OAuth failure, clear the stored client registration so the next
attempt falls through to fresh Dynamic Client Registration.
- Add MCPTokenStorage.deleteClientRegistration() for targeted cleanup
- Call it from MCPConnectionFactory's OAuth failure path
- Add integration test proving recovery from stale client reuse
- R1: Move `import type { TokenMethods }` to the type-imports section,
before local types, per CLAUDE.md import order rules
- R2: Add unit test for empty redirect_uris in handler.test.ts to
verify the inverted condition triggers re-registration
- R3: Use delete for process.env.DOMAIN_SERVER restoration when the
original value was undefined to avoid coercion to string "undefined"
The SDK's OAuthClientInformation type lacks redirect_uris (only on
OAuthClientInformationFull). Cast to the local OAuthClientInformation
type in handler.ts when accessing deserialized client info from DB,
and use intersection types in tests for clientInfo with redirect_uris.
- Fix empty redirect_uris bug: invert condition so missing/empty
redirect_uris triggers re-registration instead of silent reuse
- Revert undocumented config?.redirect_uri in auto-discovery path
- Change DB error logging from debug to warn for operator visibility
- Fix import order: move package type import to correct section
- Remove redundant type cast and misleading JSDoc comment
- Test file: remove dead imports, restore process.env.DOMAIN_SERVER,
rename describe blocks, add empty redirect_uris edge case test,
add concurrent reconnection test with pre-seeded token,
scope documentation to reconnection stabilization
Reproduces the client_id mismatch bug that occurs in multi-replica deployments
where concurrent initiateOAuthFlow calls each register a new OAuth client.
Tests verify that the findToken-based client reuse prevents re-registration.
When using auto-discovered OAuth (DCR), LibreChat calls /register on every
flow initiation, getting a new client_id each time. When concurrent
connections or reconnections happen, the client_id used during /authorize
differs from the one used during /token, causing the server to reject the
exchange.
Before registering a new client, check if a valid client registration
already exists in the database and reuse it.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: pass explicit primaryKey to Meilisearch addDocuments/updateDocuments calls
Meilisearch v1.0+ refuses to auto-infer the primary key when a document
contains multiple fields ending with 'id'. The messages index has both
conversationId and messageId, causing addDocuments to silently fail with
index_primary_key_multiple_candidates_found, leaving message search empty.
Pass { primaryKey } to addDocumentsInBatches, addDocuments, and
updateDocuments — the variable was already in scope.
Also replace raw this.collection.updateMany with Mongoose Model.updateMany
to satisfy the no-restricted-syntax ESLint rule (tenant isolation guard).
Closes#12538
* fix: resolve additional Meilisearch plugin bugs found in review
Address review findings from PR #12542:
- Fix deleteObjectFromMeili using MongoDB _id instead of the Meilisearch
primary key (conversationId/messageId), causing post-remove cleanup to
silently no-op and leave orphaned documents in the index.
- Pass options.primaryKey explicitly to createMeiliMongooseModel factory
instead of deriving it from attributesToIndex[0] (schema field order),
eliminating a fragile implicit contract.
- Fix updateObjectToMeili skipping preprocessObjectForIndex, which meant
updates bypassed content array-to-text conversion and conversationId
pipe character escaping.
- Change collection.updateMany to collection.updateOne in addObjectToMeili
since _id is unique (semantic correctness).
- Add primaryKey to validateOptions required keys.
- Strengthen test assertions to verify { primaryKey } argument is passed
to addDocuments, addDocumentsInBatches, and updateDocuments. Add tests
for the update path including preprocessObjectForIndex pipe escaping.
* fix: add regression tests for delete and message update paths
Address follow-up review findings:
- Add test for deleteObjectFromMeili verifying it uses messageId (not
MongoDB _id) when calling index.deleteDocument, guarding against
regression of the silent orphaned-document bug.
- Add test for message model update path asserting { primaryKey:
'messageId' } is passed to updateDocuments (previously only the
conversation model update path was tested).
- Add @param config.primaryKey to createMeiliMongooseModel JSDoc.
* 🔧 chore: Update `mongodb-memory-server` to v11.0.1
- Bump `mongodb-memory-server` version in `package-lock.json`, `api/package.json`, and `packages/data-schemas/package.json` from 10.1.4 to 11.0.1.
- Update related dependencies in `mongodb-memory-server` and `mongodb-memory-server-core` to ensure compatibility with the new version.
- Adjust `tslib` version in `mongodb-memory-server` to 2.8.1 and `debug` to 4.4.3 for consistency.
* chore: npm audit fix
* chore: Update `mermaid` dependency to version 11.14.0 in `package-lock.json` and `client/package.json`
* fix: use deterministic timestamps in convoStructure test
MongoDB 8.x (from mongodb-memory-server v11) no longer guarantees
insertion-order return for documents with identical timestamps.
Use sequential timestamps with overrideTimestamp to ensure buildTree
processes parents before children.
* directly returns the translation function without managing language state in client package
* chore: remove unused langAtom from packages/client store
* fix: add useCallback to match canonical useLocalize, add guard comment
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
Right now, if you have draft text in conversation A, but no draft text in
conversation B, then switching from A -> B inserts the draft from A into B
(oops).
This was caused by a bug in the `restoreText()` logic which did not
restore *blank* text as the saved draft.
Now, it'll always restore whatever is found as a draft (or set to blank if
there is no draft).
* fix: Resolve custom role permissions not loading in frontend
Users assigned to custom roles (non-USER/ADMIN) had all permission
checks fail because AuthContext only fetched system role permissions.
The roles map keyed by USER/ADMIN never contained the custom role name,
so useHasAccess returned false for every feature gate.
- Fetch the user's custom role in AuthContext and include it in the
roles map so useHasAccess can resolve permissions correctly
- Use encodeURIComponent instead of toLowerCase for role name URLs
to preserve custom role casing through the API roundtrip
- Only uppercase system role names on the backend GET route; pass
custom role names through as-is for exact DB lookup
- Allow users to fetch their own assigned role without READ_ROLES
capability
* refactor: Normalize all role names to uppercase
Custom role names were stored in original casing, causing case-sensitivity
bugs across the stack — URL lowercasing, route uppercasing, and
case-sensitive DB lookups all conflicted for mixed-case custom roles.
Enforce uppercase normalization at every boundary:
- createRoleByName trims and uppercases the name before storage
- createRoleHandler uppercases before passing to createRoleByName
- All admin route handlers (get, update, delete, members, permissions)
uppercase the :name URL param before DB lookups
- addRoleMemberHandler uppercases before setting user.role
- Startup migration (normalizeRoleNames) finds non-uppercase custom
roles, renames them, and updates affected user.role values with
collision detection
Legacy GET /api/roles/:roleName retains always-uppercase behavior.
Tests updated to expect uppercase role names throughout.
* fix: Use case-preserved role names with strict equality
Remove uppercase normalization — custom role names are stored and
compared exactly as the user sets them, with only trimming applied.
USER and ADMIN remain reserved case-insensitively via isSystemRoleName.
- Remove toUpperCase from createRoleByName, createRoleHandler, and
all admin route handlers (get, update, delete, members, permissions)
- Remove toUpperCase from legacy GET and PUT routes in roles.js;
the frontend now sends exact casing via encodeURIComponent
- Remove normalizeRoleNames startup migration
- Revert test expectations to original casing
* fix: Format useMemo dependency array for Prettier
* feat: Add custom role support to admin settings + review fixes
- Add backend tests for isOwnRole authorization gate on GET /api/roles/:roleName
- Add frontend tests for custom role detection and fetching in AuthContext
- Fix transient null permission flash by only spreading custom role once loaded
- Add isSystemRoleName helper to data-provider for case-insensitive system role detection
- Use sentinel value in useGetRole to avoid ghost cache entry from empty string
- Add useListRoles hook and listRoles data service for fetching all roles
- Update AdminSettingsDialog and PeoplePickerAdminSettings to dynamically
list custom roles in the role dropdown, with proper fallback defaults
* fix: Address review findings for custom role permissions
- Add assertions to AuthContext test verifying custom role in roles map
- Fix empty array bypassing nullish coalescing fallback in role dropdowns
- Add null/undefined guard to isSystemRoleName helper
- Memoize role dropdown items to avoid unnecessary re-renders
- Apply sentinel pattern to useGetRole in admin settings for consistency
- Mark ListRolesResponse description as required to match schema
* fix: Prevent prototype pollution in role authorization gate
- Replace roleDefaults[roleName] with Object.hasOwn to prevent
prototype chain bypass for names like constructor or __proto__
- Add dedicated rolesList query key to avoid cache collision when
a custom role is named 'list'
- Add regression test for prototype property name authorization
* fix: Resolve Prettier formatting and unused variable lint errors
* fix: Address review findings for custom role permissions
- Add ADMIN self-read test documenting isOwnRole bypass behavior
- Guard save button while custom role data loads to prevent data loss
- Extract useRoleSelector hook eliminating ~55 lines of duplication
- Unify defaultValues/useEffect permission resolution (fixes inconsistency)
- Make ListRolesResponse.description and _id optional to match schema
- Fix vacuous test assertions to verify sentinel calls exist
- Only fetch userRole when user.role === USER (avoid unnecessary requests)
- Remove redundant empty string guard in custom role detection
* fix: Revert USER role fetch restriction to preserve admin settings
Admins need the USER role loaded in AuthContext.roles so the admin
settings dialog shows persisted USER permissions instead of defaults.
* fix: Remove unused useEffect import from useRoleSelector
* fix: Clean up useRoleSelector hook
- Use existing isCustom variable instead of re-calling isSystemRoleName
- Remove unused roles and availableRoleNames from return object
* fix: Address review findings for custom role permissions
- Use Set-based isSystemRoleName to auto-expand with future SystemRoles
- Add isCustomRoleError handling: guard useEffect reset and disable Save
- Remove resolvePermissions from hook return; use defaultValues in useEffect
to eliminate redundant computation and stale-closure reset race
- Rename customRoleName to userRoleName in AuthContext for clarity
* fix: Request server-max roles for admin dropdown
listRoles now passes limit=200 (the server's MAX_PAGE_LIMIT) so the
admin role selector shows all roles instead of silently truncating
at the default page size of 50.
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
* chore: remove unused `interface.endpointsMenu` config field
* chore: address review — restore JSDoc UI-only example, add Zod strip test
* chore: remove unused `interface.sidePanel` config field
* chore: restrict fileStrategy/fileStrategies schema to valid storage backends
* fix: use valid FileStorage value in AppService test
* chore: address review — version bump, exhaustiveness guard, JSDoc, configSchema test
* chore: remove debug logger.log from MessageIcon render path
* fix: rewrite MessageIcon render tests to use render counting instead of logger spying
* chore: bump librechat-data-provider to 0.8.407
* chore: sync example YAML version to 1.3.7
* debug: add instrumentation to MessageIcon arePropsEqual + render cycle tests
Temporary debug commit to identify which field triggers MessageIcon
re-renders during message creation and streaming.
arePropsEqual now logs 'icon_memo_diff' with the exact field name and
prev/next values whenever it returns false. Filter browser console for
'icon_memo_diff' to see the trigger.
Also adds render-level integration tests that simulate the message
lifecycle (initial mount, streaming chunks, context updates) and
assert render counts via logger spy.
* perf: stabilize MultiMessage key to prevent unmount/remount during SSE lifecycle
messageId changes 3 times during the SSE message lifecycle:
1. useChatFunctions creates initialResponse with client-generated UUID
2. createdHandler replaces it with userMessageId + '_'
3. finalHandler replaces it with server-assigned messageId
Since MultiMessage used key={message.messageId}, each change caused
React to destroy and recreate the entire message component subtree,
unmounting MessageIcon and all memoized children. This produced visible
icon/image flickering that no memo comparator could prevent.
Switch to key={parentMessageId + '_' + siblingIdx}:
- parentMessageId is stable from creation through final response
- siblingIdx ensures sibling switches still get clean remounts
- Eliminates 2 unnecessary unmount/remount cycles per message
Add key stability tests verifying:
- Current key={messageId} causes 3 mounts / 2 unmounts per lifecycle
- Stable key causes 1 mount / 0 unmounts per lifecycle
- Sibling switches still trigger clean remounts with stable key
* perf: stabilize root MultiMessage key across new conversation lifecycle
When a user sends their first message in a new conversation,
conversationId transitions from null/'new' to the server-assigned
UUID. MessagesView used key={conversationId} on the root MultiMessage,
so this transition destroyed the entire message tree and rebuilt it
from scratch — causing all MessageIcons to unmount/remount (visible
as image flickering).
Use a ref-based stable key that captures the first real conversationId
and only changes on genuine conversation switches (navigating to a
different conversation), not on the null→UUID transition within the
same conversation.
* debug: add mount/unmount lifecycle tracking to MessageIcon
Adds icon_lifecycle logs (MOUNT/UNMOUNT) and render count to
distinguish between fresh mounts (memo comparator not called)
and internal re-renders (hook bypassing memo).
Enable: localStorage.setItem('DEBUG_LOGGING', 'icon_lifecycle,icon_data,icon_memo_diff')
* debug: add key and root tracking to MultiMessage and MessagesView
Logs multi_message_key (stableKey, messageId, parentMessageId, route)
and messages_view_key (rootKey, conversationId) to trace which key
changes trigger unmount/remount cycles.
Enable: localStorage.setItem('DEBUG_LOGGING', 'icon_lifecycle,icon_data,icon_memo_diff,multi_message_key,messages_view_key')
* perf: remove key from root MultiMessage to prevent tree destruction
The ref-based stable key still changed during 'new' → real UUID
transition, destroying the entire tree. The root MultiMessage is the
sole child at its position, so React reuses the instance via
positional reconciliation without any key. The messageId prop
(conversationId) naturally resets Recoil siblingIdxFamily state on
conversation switches.
* perf: remove unstable keys from MultiMessage to prevent SSE lifecycle remounts
Both messageId and parentMessageId change during the SSE lifecycle
(client UUID → CREATED server ID → FINAL server ID), making neither
viable as a stable React key. Each key change caused React to destroy
and recreate the entire message component subtree, including all
memoized children — visible as icon/image flickering.
Remove explicit keys entirely and rely on React's positional
reconciliation. MultiMessage always renders exactly one child at
the same position, so React reuses the component instance and
updates props in place. The existing memo comparators on
ContentRender/MessageRender handle field-level diffing correctly.
Update tests to verify: key={messageId} causes 3 mounts/2 unmounts
per lifecycle, while no key causes 1 mount/0 unmounts.
* perf: remove unstable keys from child MultiMessage in message wrappers
Message.tsx, MessageContent.tsx, and MessageParts.tsx each render a
child MultiMessage with key={messageId} for the current message's
children. Since messageId changes during the SSE lifecycle (CREATED
event replaces the user message ID), the child MultiMessage gets
destroyed and recreated, unmounting the entire agent response subtree
including its MessageIcon.
Remove these keys for the same reason as the parent MultiMessage:
each child MultiMessage renders exactly one child at a fixed position,
so positional reconciliation correctly reuses the instance.
* chore: remove MultiMessage key tests — they test React behavior, not our code
The tests verified that key={messageId} causes remounts while no key
doesn't, but this is React's own reconciliation behavior. No unit test
can prevent someone from re-adding a key prop to JSX. The JSDoc comments
on MultiMessage document the decision and rationale.
* 🔐 fix: Strip code_challenge from admin OAuth requests before Passport
openid-client v6's Passport Strategy uses `currentUrl.searchParams.size === 0`
to distinguish initial authorization requests from OAuth callbacks. The
admin-panel-specific `code_challenge` query parameter caused the strategy to
misclassify the request as a callback and return 401 Unauthorized.
* 🔐 fix: Strip code_challenge from admin OAuth requests before Passport
openid-client v6's Passport Strategy uses `currentUrl.searchParams.size === 0`
to distinguish initial authorization requests from OAuth callbacks. The
admin-panel-specific `code_challenge` query parameter caused the strategy to
misclassify the request as a callback and return 401 Unauthorized.
- Fix regex to handle `code_challenge` in any query position without producing
malformed URLs, and handle empty `code_challenge=` values (`[^&]*` vs `[^&]+`)
- Combine `storePkceChallenge` + `stripCodeChallenge` into a single
`storeAndStripChallenge` helper to enforce read-store-strip ordering
- Apply defensively to all 7 admin OAuth providers
- Add 12 unit tests covering stripCodeChallenge and storeAndStripChallenge
* refactor: Extract PKCE helpers to utility file, harden tests
- Move stripCodeChallenge and storeAndStripChallenge to
api/server/utils/adminPkce.js — eliminates _test production export
and avoids loading the full auth.js module tree in tests
- Add missing req.originalUrl/req.url assertions to invalid-challenge
and no-challenge test branches (regression blind spots)
- Hoist cache reference to module scope in tests (was redundantly
re-acquired from mock factory on every beforeEach)
* chore: Address review NITs — imports, exports, naming, assertions
- Fix import order in auth.js (longest-to-shortest per CLAUDE.md)
- Remove unused PKCE_CHALLENGE_TTL/PKCE_CHALLENGE_PATTERN exports
- Hoist strip arrow to module-scope stripChallengeFromUrl
- Rename auth.test.js → auth.spec.js (project convention)
- Tighten cache-failure test: toBe instead of toContain, add req.url
* refactor: Move PKCE helpers to packages/api with dependency injection
Move stripCodeChallenge and storeAndStripChallenge from api/server/utils
into packages/api/src/auth/exchange.ts alongside the existing PKCE
verification logic. Cache is now injected as a Keyv parameter, matching
the dependency-injection pattern used throughout packages/api/.
- Add PkceStrippableRequest interface for minimal req typing
- auth.js imports storeAndStripChallenge from @librechat/api
- Delete api/server/utils/adminPkce.js
- Move tests to packages/api/src/auth/adminPkce.spec.ts (TypeScript,
real Keyv instances, no getLogStores mock needed)
* fix: check supportedMimeTypes before routing unrecognized file types
In processAttachments, files not matching the hardcoded mime type
categories (image, PDF, video, audio) were silently dropped. Now
resolves the endpoint's file config and checks the file type against
supportedMimeTypes before routing to the documents pipeline. Files
not matching any config are still skipped (original behavior).
Closes#12482
* feat: encode generic document types for supported providers
Remove restrictive mime type filter in encodeAndFormatDocuments that
only allowed PDFs and application/* types. Add a generic encoding
path for non-PDF, non-Bedrock files using the provider's native
format (Anthropic base64 document, OpenAI file block, Google media
block). Files are already validated upstream by supportedMimeTypes.
* fix: guard file.type and cache file config in processAttachments
- Add file.type truthiness check before checkType to prevent
coercion of null/undefined to string 'null'/'undefined'
- Cache mergedFileConfig and endpointFileConfig on the instance
so addPreviousAttachments doesn't recompute per message
* refactor: harden generic document encoding with validation and tests
- Extract formatDocumentBlock helper to eliminate ~30 lines of
duplicate provider-dispatch code between PDF and generic paths
- Add size validation in generic encoding path using
configuredFileSizeLimit (was fetched but unused)
- Guard Bedrock from generic path — non-bedrockDocumentFormats
types are now skipped instead of silently tracking metadata
- Only push metadata to result.files when a document block was
actually created, preventing silent inconsistent state
- Enable Anthropic citations for text/plain, text/html,
text/markdown (supported by Anthropic's document API)
- Fix != to !== for Providers.AZURE comparison
- Add 9 tests covering all four provider branches, Bedrock
exclusion, size limit enforcement, and unhandled provider
* fix: resolve filename type mismatch in formatDocumentBlock
filename parameter is string | undefined but OpenAIFileBlock and
OpenAIInputFileBlock require string. Default to 'document' when
filename is undefined.
* fix: use endpoint name for file config lookup in processAttachments
Agent runs can have agent.provider set to a base provider (e.g.,
openAI) while agent.endpoint is a custom endpoint name. Using
provider for the getEndpointFileConfig lookup bypassed custom
endpoint supportedMimeTypes config. Now uses agent.endpoint,
matching the pattern in addDocuments.
* perf: filter non-Bedrock files before fetching streams
Bedrock only supports types in bedrockDocumentFormats. Previously,
getFileStream was called for all files and unsupported types were
discarded after download. Now pre-filters the file list for Bedrock
to avoid unnecessary network and memory overhead for large
unsupported attachments.
* refactor: clean up processAttachments file config handling
- Remove redundant ?? null intermediaries; use instance properties
directly in the else-if condition
- Add JSDoc @type annotations for _mergedFileConfig and
_endpointFileConfig in the constructor
* refactor: harden document encoding and add routing tests
- Hoist configuredFileSizeLimit above the loop to avoid recomputing
mergeFileConfig per file
- Replace Buffer.from decode with base64 length formula in the
generic size check to avoid unnecessary heap allocation
- Use nullish coalescing (??) for filename fallback
- Clean up test: remove unnecessary type cast, use createMockRequest
helper for size-limit test
- Add 14 tests for processAttachments categorization logic covering
supportedMimeTypes routing, null/undefined guards, standard type
passthrough, and edge cases
* fix: use optional chaining for checkType in routing tests
FileConfig.checkType is typed as optional. Use optional chaining
to satisfy strict type checking.
* fix: skip stream fetches for unsupported providers, block Bedrock generic routing
- Return early from encodeAndFormatDocuments when the provider is
neither document-supported nor Bedrock, avoiding unnecessary
getFileStream calls for providers that would discard all results
- Add !isBedrock guard to the supportedMimeTypes fallback branch in
processAttachments so permissive patterns like '.*' don't route
non-Bedrock types into documents that would be silently dropped
- Add test for Bedrock + non-Bedrock-document-type skipping
* fix: respect supportedMimeTypes config for Bedrock endpoints
Remove !isBedrock guard from the generic supportedMimeTypes routing
branch. If a user configures permissive supportedMimeTypes for a
Bedrock endpoint, the upload validation already accepted the file.
The encoding layer pre-filters to Bedrock-supported types before
fetching streams, so unsupported types are handled there without
silently dropping files the user explicitly allowed.
* fix: prevent MCP tools with `_action` in name from being misclassified as OpenAPI action tools
Add `isActionTool()` helper that checks for the `_action_` delimiter
while guarding against cross-delimiter collision with `_mcp_`. Replace
all `includes(actionDelimiter)` classification checks with the new
helper across backend and frontend.
* test: add coverage for MCP/action cross-delimiter collision
Verify that `isActionTool` correctly rejects MCP tool names containing
`_action` and that `loadAgentTools` does not filter them based on
`actionsEnabled`. Add ToolIcon and definitions test cases.
* fix: simplify isActionTool to handle all MCP name patterns
- Use `!toolName.includes('_mcp_')` instead of checking only after the
first `_action_` occurrence, which missed MCP tools with `_action_` in
the middle of their name (e.g. `get_action_data_mcp_myserver`).
- Reference `Constants.mcp_delimiter` value via a local const to avoid
circular import from config.ts, with a comment explaining why.
- Remove dead `actionDelimiter` import from definitions.ts.
- Replace double-filter with single-pass partition in loadToolsForExecution.
- Add test for mid-name `_action_` collision case.
* fix: narrow MCP exclusion to delimiter position in isActionTool
Only reject when `_mcp_` appears after `_action_` (the MCP suffix
position). `_mcp_` before `_action_` is part of the operationId and
is valid — e.g. `sync_mcp_state_action_api---example---com` is a
legitimate action tool whose operationId happens to contain `_mcp_`.
* fix: document positional _mcp_ guard and known RFC-invalid domain limitation
Expand JSDoc on isActionTool to explain the action/MCP format
disambiguation and the theoretical false negative for non-RFC-compliant
domains containing `_mcp_`. Add test documenting this known edge case.
* fix: omit externalResources for static Sandpack previews
The Tailwind CDN URL lacks a file extension, causing Sandpack's
static template to throw a runtime injection error. Static previews
already load Tailwind via a script tag in the shared index.html,
so externalResources is unnecessary for them.
Closes#12507
* refactor: extract buildSandpackOptions and add tests
- Surgically omit only externalResources for static templates
instead of discarding all sharedOptions, preventing future
regression if new template-agnostic options are added.
- Extract options logic into a pure, testable helper function.
- Add unit tests covering all template/config combinations.
* chore: fix import order and pin test assertions
* fix: use URL fragment hint instead of omitting externalResources
Sandpack's static template regex detects resource type from the URL's
last file extension. The versioned CDN path (/3.4.17) matched ".17"
instead of ".js", throwing "Unable to determine file type". Rather
than omitting externalResources for static templates (which would
remove the only Tailwind injection path for HTML artifacts that don't
embed their own script tag), append a #tailwind.js fragment hint so
the regex matches ".js". Fragments are not sent to the server, so
the CDN response is unchanged.
* fix: pass recursionLimit to processStream in OpenAI-compatible agents API
The OpenAI-compatible endpoint never passed recursionLimit to LangGraph's
processStream(), silently capping all API-based agent calls at the default
25 steps. Mirror the 3-step cascade already used by the UI path (client.js):
yaml config default → per-agent DB override → max cap.
* refactor: extract resolveRecursionLimit into shared utility
Extract the 3-step recursion limit cascade into a shared
resolveRecursionLimit() function in @librechat/api. Both openai.js and
client.js now call this single source of truth.
Also fixes falsy-guard edge cases where recursion_limit=0 or
maxRecursionLimit=0 would silently misbehave, by using explicit
typeof + positive checks.
Includes unit tests covering all cascade branches and edge cases.
* refactor: use resolveRecursionLimit in openai.js and client.js
Replace duplicated cascade logic in both controllers with the shared
resolveRecursionLimit() utility from @librechat/api.
In openai.js: hoist agentsEConfig to avoid double property walk,
remove displaced comment, add integration test assertions.
In client.js: remove inline cascade that was overriding config
after initial assignment.
* fix: hoist processStream mock for test accessibility
The processStream mock was created inline inside mockResolvedValue,
making it inaccessible via createRun.mock.results (which returns
the Promise, not the resolved value). Hoist it to a module-level
variable so tests can assert on it directly.
* test: improve test isolation and boundary coverage
Use mockReturnValueOnce instead of mockReturnValue to prevent mock
leaking across test boundaries. Add boundary tests for downward
agent override and exact-match maxRecursionLimit.
* refactor: self-healing tenant isolation update guard
Replace the strict throw-on-any-tenantId guard with a
strip-or-throw approach:
- $set/$setOnInsert: strip when value matches current tenant
or no context is active; throw only on cross-tenant mutations
- $unset/$rename: always strip (unsetting/renaming tenantId
is never valid)
- Top-level tenantId: same logic as $set
This eliminates the entire class of "tenantId in update payload"
bugs at the plugin level while preserving the cross-tenant
security invariant.
* test: update mutation guard tests for self-healing behavior
- Convert same-tenant $set/$setOnInsert tests to expect silent
stripping instead of throws
- Convert $unset test to expect silent stripping
- Add cross-tenant throw tests for $set, $setOnInsert, top-level
- Add same-tenant stripping tests for $set, $setOnInsert, top-level
- Add $rename stripping test
- Add no-context stripping test
- Update error message assertions to match new cross-tenant message
* revert: remove call-site tenantId stripping patches
Revert the per-call-site tenantId stripping from #12498 and
the excludedKeys patch from #12501. These are no longer needed
since the self-healing guard handles tenantId in update payloads
at the plugin level.
Reverted patches:
- conversation.ts: delete update.tenantId in saveConvo(),
tenantId destructuring in bulkSaveConvos()
- message.ts: delete update.tenantId in saveMessage() and
recordMessage(), tenantId destructuring in bulkSaveMessages()
and updateMessage()
- config.ts: tenantId in excludedKeys Set
- config.spec.ts: tenantId in excludedKeys test assertion
* fix: strip tenantId from update documents in tenantSafeBulkWrite
Mongoose middleware does not fire for bulkWrite, so the plugin-level
guard never sees update payloads in bulk operations. Extend
injectTenantId() to strip tenantId from update documents for
updateOne/updateMany operations, preventing cross-tenant overwrites.
* refactor: rename guard, add empty-op cleanup and strict-mode warning
- Rename assertNoTenantIdMutation to sanitizeTenantIdMutation
- Remove empty operator objects after stripping to avoid MongoDB errors
- Log warning in strict mode when stripping tenantId without context
- Fix $setOnInsert test to use upsert:true with non-matching filter
* test: fix bulk-save tests and add negative excludedKeys assertion
- Wrap bulkSaveConvos/bulkSaveMessages tests in tenantStorage.run()
to exercise the actual multi-tenant stripping path
- Assert tenantId equals the real tenant, not undefined
- Add negative assertion: excludedKeys must NOT contain tenantId
* fix: type-safe tenantId stripping in tenantSafeBulkWrite
- Fix TS2345 error: replace conditional type inference with
UpdateQuery<Record<string, unknown>> for stripTenantIdFromUpdate
- Handle empty updates after stripping (e.g., $set: { tenantId } as
sole field) by filtering null ops from the bulk array
- Add 4 tests for bulk update tenantId stripping: plain-object update,
$set stripping, $unset stripping, and sole-field-in-$set edge case
* fix: resolve TS2345 in stripTenantIdFromUpdate parameter type
Use Record<string, unknown> instead of UpdateQuery<> to avoid
type incompatibility with Mongoose's AnyObject-based UpdateQuery
resolution in CI.
* fix: strip tenantId from bulk updates unconditionally
Separate sanitization from injection in tenantSafeBulkWrite:
tenantId is now stripped from all update documents before any
tenant-context checks, closing the gap where no-context and
system-context paths passed caller-supplied tenantId through
to MongoDB unmodified.
* refactor: address review findings in tenant isolation
- Fix early-return gap in stripTenantIdFromUpdate that skipped
operator-level tenantId when top-level was also present
- Lazy-allocate copy in stripTenantIdFromUpdate (no allocation
when no tenantId is present)
- Document behavioral asymmetry: plugin throws on cross-tenant,
bulkWrite strips silently (intentional, documented in JSDoc)
- Remove double JSDoc on injectTenantId
- Remove redundant cast in stripTenantIdFromUpdate
- Use shared frozen EMPTY_BULK_RESULT constant
- Remove Record<string, unknown> annotation in recordMessage
- Isolate bulkSave* tests: pre-create docs then update with
cross-tenant payload, read via runAsSystem to prove stripping
is independent of filter injection
* fix: no-op empty updates after tenantId sanitization
When tenantId is the sole field in an update (e.g., { $set: { tenantId } }),
sanitization leaves an empty update object that would fail with
"Update document requires atomic operators." The updateGuard now
detects this and short-circuits the query by adding an unmatchable
filter condition and disabling upsert, matching the bulk-write
handling that filters out null ops.
* refactor: remove dead logger.warn branches, add mixed-case test
- Remove unreachable logger.warn calls in sanitizeTenantIdMutation:
queryMiddleware throws before updateGuard in strict+no-context,
and isStrict() is false in non-strict+no-context
- Add test for combined top-level + operator-level tenantId stripping
to lock in the early-return fix
* feat: ESLint rule to ban raw bulkWrite and collection.* in data-schemas
Add no-restricted-syntax rules to the data-schemas ESLint config that
flag direct Model.bulkWrite() and Model.collection.* calls. These
bypass Mongoose middleware and the tenant isolation plugin — all bulk
writes must use tenantSafeBulkWrite() instead.
Test files are excluded since they intentionally use raw driver calls
for fixture setup.
Also migrate the one remaining raw bulkWrite in seedSystemGrants() to
use tenantSafeBulkWrite() for consistency.
* test: add findByIdAndUpdate coverage to mutation guard tests
* fix: keep tenantSafeBulkWrite in seedSystemGrants, fix ESLint config
- Revert to tenantSafeBulkWrite in seedSystemGrants (always runs
under runAsSystem, so the wrapper passes through correctly)
- Split data-schemas ESLint config: shared TS rules for all files,
no-restricted-syntax only for production non-wrapper files
- Fix unused destructure vars to use _tenantId pattern
* fix: auth-aware config caching for fresh sessions
- Add auth state to startup config query key via shared `startupConfigKey`
builder so login (unauthenticated) and chat (authenticated) configs are
cached independently
- Disable queries during login onMutate to prevent premature unauthenticated
refetches after cache clear
- Re-enable queries in setUserContext only after setTokenHeader runs, with
positive-only guard to avoid redundant disable on logout
- Update all getQueryData call sites to use the shared key builder
- Fall back to getConfigDefaults().interface in useEndpoints, hoisted to
module-level constant to avoid per-render recomputation
* fix: address review findings for auth-aware config caching
- Move defaultInterface const after all imports in ModelSelector.tsx
- Remove dead QueryKeys import, use import type for TStartupConfig
in ImportConversations.tsx
- Spread real exports in useQueryParams.spec.ts mock to preserve
startupConfigKey, fixing TypeError in all 6 tests
* chore: import order
* fix: re-enable queries on login failure
When login fails, onSuccess never fires so queriesEnabled stays
false. Re-enable in onError so the login page can re-fetch config
(needed for LDAP username validation and social login options).
`BaseClient.js` iterates existing conversation keys to build `unsetFields`
for removal when `endpointOptions` doesn't include them. When tenant
isolation stamps `tenantId` on the document, it gets swept into `$unset`,
triggering `assertNoTenantIdMutation`. Adding `tenantId` to `excludedKeys`
prevents this — it's a system field, not an endpoint option.
* fix: Exclude field from conversation and message updates
* fix: Remove tenantId from conversation and message update objects to prevent unintended data exposure.
* refactor: Adjust update logic in createConversationMethods and createMessageMethods to ensure tenantId is not included in the updates, maintaining data integrity.
* fix: Strip tenantId from all write paths in conversation and message methods
Extends the existing tenantId stripping to bulkSaveConvos, bulkSaveMessages,
recordMessage, and updateMessage — all of which previously passed caller-supplied
tenantId straight through to the update document. Renames discard alias from _t
to _tenantId for clarity. Adds regression tests for all six write paths.
* fix: Eliminate double-copy overhead and strengthen test assertions
Replace destructure-then-spread with spread-once-then-delete for saveConvo,
saveMessage, and recordMessage — removes one O(n) copy per call on hot paths.
Add missing not-null and positive data assertions to tenantId stripping tests.
* test: Add positive data assertions to bulkSaveMessages and recordMessage tests
* fix: Allow empty-overrides scope creation when priority is provided
The upsertConfigOverrides handler short-circuited when overrides was
empty, returning a plain message instead of creating the config document.
This broke the admin panel's "create blank scope" flow which sends
`{ overrides: {}, priority: N }` — the missing `config` property in the
response caused an `_id` error on the client.
The early return now only triggers when both overrides are empty and no
priority is provided. Per-section permission checks are scoped to cases
where override sections are actually present.
* test: Add tests for empty-overrides scope creation with priority
* test: Address review nits for empty-overrides scope tests
- Add res.statusCode/res.body assertions to capability-check test
- Add 403/401 tests for empty overrides + priority path
- Use mockResolvedValue(null) for consistency on bare jest.fn()
- Remove narrating comment; fold intent into test name
* 🛡️ fix: restrict system grants to role principals only
Narrows GrantPrincipalType to PrincipalType.ROLE, rejecting GROUP and
USER with 400. Removes grant cascade cleanup from group/user deletion
handlers and their route wiring since only roles can hold grants.
* 🛡️ fix: address review findings for grants roles-only restriction
Add missing GROUP rejection test for revokeGrant (symmetric with
getPrincipalGrants and assignGrant coverage), add extensibility comment
to GrantPrincipalType, and document the checkRoleExists guard.
* refactor: split /api/config into unauthenticated and authenticated response paths
- Replace preAuthTenantMiddleware with optionalJwtAuth on the /api/config
route so the handler can detect whether the request is authenticated
- When unauthenticated: call getAppConfig({ baseOnly: true }) for zero DB
queries, return only login-relevant fields (social logins, turnstile,
privacy policy / terms of service from interface config)
- When authenticated: call getAppConfig({ role, userId, tenantId }) to
resolve per-user DB overrides (USER + ROLE + GROUP + PUBLIC principals),
return full payload including modelSpecs, balance, webSearch, etc.
- Extract buildSharedPayload() and addWebSearchConfig() helpers to avoid
duplication between the two code paths
- Fixes per-user balance overrides not appearing in the frontend because
userId was never passed to getAppConfig (follow-up to #12474)
* test: rewrite config route tests for unauthenticated vs authenticated paths
- Replace the previously-skipped supertest tests with proper mocked tests
- Cover unauthenticated path: baseOnly config call, minimal payload,
interface subset (privacyPolicy/termsOfService only), exclusion of
authenticated-only fields
- Cover authenticated path: getAppConfig called with userId, full payload
including modelSpecs/balance/webSearch, per-user balance override merging
* fix: address review findings — restore multi-tenant support, improve tests
- Chain preAuthTenantMiddleware back before optionalJwtAuth on /api/config
so unauthenticated requests in multi-tenant deployments still get
tenant-scoped config via X-Tenant-Id header (Finding #1)
- Use getAppConfig({ tenantId }) instead of getAppConfig({ baseOnly: true })
when a tenant context is present; fall back to baseOnly for single-tenant
- Fix @type annotation: unauthenticated payload is Partial<TStartupConfig>
- Refactor addWebSearchConfig into pure buildWebSearchConfig that returns a
value instead of mutating the payload argument
- Hoist isBirthday() to module level
- Remove inline narration comments
- Assert tenantId propagation in tests, including getTenantId fallback and
user.tenantId preference
- Add error-path tests for both unauthenticated and authenticated branches
- Expand afterEach env var cleanup for proper test isolation
* test: fix mock isolation and add tenant-scoped response test
- Replace jest.clearAllMocks() with jest.resetAllMocks() so
mockReturnValue implementations don't leak between tests
- Add test verifying tenant-scoped socialLogins and turnstile are
correctly mapped in the unauthenticated response
* fix: add optionalJwtAuth to /api/config in experimental.js
Without this middleware, req.user is never populated in the experimental
cluster entrypoint, so authenticated users always receive the minimal
unauthenticated config payload.
* perf: add custom memo comparator to MessageIcon for stable re-render gating
MessageIcon receives full `agent` and `assistant` objects as props from
useMessageActions, which recomputes them when AgentsMapContext or
AssistantsMapContext update (e.g., react-query refetch on window focus).
These context-triggered re-renders bypass MessageRender's React.memo,
producing new object references for agent/assistant even when the
underlying data is unchanged. The default shallow comparison in
MessageIcon's memo then fails, causing unnecessary re-renders that
manifest as visible icon flickering.
Add arePropsEqual comparator that checks only the fields MessageIcon
actually uses (name, avatar filepath, metadata avatar) instead of
object identity, so the component correctly bails out when icon-relevant
data hasn't changed.
* refactor: export arePropsEqual, drop redundant useMemos, add JSDoc
- Export arePropsEqual so it can be tested in isolation
- Add JSDoc documenting which fields are intentionally omitted (id)
and why iconData uses reference equality
- Replace five trivial useMemo calls (agent?.name ?? '', etc.) with
direct computed values — the custom comparator already gates
re-renders, so these memos only add closure/dep-array overhead
without ever providing cache hits
- Remove unused React import
* test: add unit tests for MessageIcon arePropsEqual comparator
Exercise the custom memo comparator to ensure:
- New object references with same display fields are treated as equal
- Changed name or avatar filepath triggers re-render
- iconData reference change triggers re-render
- undefined→defined agent with undefined fields is treated as equal
* fix: replace nested ternary with if-else for eslint compliance
* test: add comment on subtle equality invariant and defined→undefined case
* perf: compare iconData by field values instead of reference
iconData is a useMemo'd object from the parent, but comparing by
reference still causes unnecessary re-renders when the parent
recomputes the memo with identical primitive values. Compare all
five fields individually (endpoint, model, iconURL, modelLabel,
isCreatedByUser) for consistency with how agent/assistant are handled.
* 🔧 chore: bump @librechat/agents to v3.1.63
* 🔧 chore: update axios dependency to exact version 1.13.6
* 🔧 chore: update @aws-sdk/client-bedrock-runtime to version 3.1013.0 in package.json and package-lock.json
- Bump the version of @aws-sdk/client-bedrock-runtime across package.json files in api and packages/api to ensure compatibility with the latest features and fixes.
- Reflect the updated version in package-lock.json to maintain consistency in dependency resolution.
* 🔧 chore: update axios dependency to version 1.13.6 across multiple package.json and package-lock.json files
- Bump axios version from ^1.13.5 to 1.13.6 in package.json and package-lock.json for improved performance and security.
- Ensure consistency in dependency resolution across the project by updating all relevant files.
* chore: Update Handlebars and package versions in package-lock.json and package.json
- Upgrade Handlebars from version 4.7.7 to 4.7.9 in both package-lock.json and package.json for improved performance and security.
- Update librechat-data-provider version from 0.8.401 to 0.8.406 in package-lock.json.
- Update @librechat/data-schemas version from 0.0.40 to 0.0.48 in package-lock.json.
* chore: Upgrade @happy-dom/jest-environment and happy-dom versions in package-lock.json and package.json
- Update @happy-dom/jest-environment from version 20.8.3 to 20.8.9 for improved compatibility.
- Upgrade happy-dom from version 20.8.3 to 20.8.9 to ensure consistency across dependencies.
* chore: Upgrade @rollup/plugin-terser to version 1.0.0 in package-lock.json and package.json
- Update @rollup/plugin-terser from version 0.4.4 to 1.0.0 in both package-lock.json and package.json for improved performance and compatibility.
- Reflect the new version in the dependencies of data-provider and data-schemas packages.
* chore: Upgrade rollup-plugin-typescript2 to version 0.37.0 in package-lock.json and package.json
- Update rollup-plugin-typescript2 from version 0.35.0 to 0.37.0 in package-lock.json and all relevant package.json files for improved compatibility and performance.
- Adjust dependencies for semver and tslib to their latest versions in line with the rollup-plugin-typescript2 upgrade.
* chore: Upgrade nodemailer to version 8.0.4 in package-lock.json and package.json
- Update nodemailer from version 7.0.11 to 8.0.4 in both package-lock.json and package.json to enhance functionality and security.
* chore: Upgrade picomatch, yaml, brace-expansion versions in package-lock.json
- Update picomatch from version 4.0.3 to 4.0.4 across multiple dependencies for improved functionality.
- Upgrade brace-expansion from version 2.0.2 to 2.0.3 and from 5.0.3 to 5.0.5 to enhance compatibility and performance.
- Update yaml from version 1.10.2 to 1.10.3 for better stability.
Add INTERFACE_PERMISSION_FIELDS set defining the interface fields that
seed role permissions at startup (prompts, agents, marketplace, etc.).
These fields are now stripped from DB config overrides in the merge
layer because updateInterfacePermissions() only runs at boot — DB
overrides for these fields create a client/server permission mismatch.
Pure UI fields (endpointsMenu, modelSelect, parameters, presets,
sidePanel, customWelcome, etc.) continue to work in overrides as
before.
YAML startup path is completely unaffected.
* feat: add admin user management endpoints
Add /api/admin/users with list, search, and delete handlers gated by
ACCESS_ADMIN + READ_USERS/MANAGE_USERS system grants. Handler factory
in packages/api uses findUsers, countUsers, and deleteUserById from
data-schemas.
* fix: address convention violations in admin users handlers
* fix: add pagination, self-deletion guard, and DB-level search limit
- listUsers now uses parsePagination + countUsers for proper pagination
matching the roles/groups pattern
- findUsers extended with optional limit/offset options
- deleteUser returns 403 when caller tries to delete own account
- searchUsers passes limit to DB query instead of fetching all and
slicing in JS
- Fix import ordering per CLAUDE.md, complete logger mock
- Replace fabricated date fallback with undefined
* fix: deterministic sort, null-safe pagination, consistent search filter
- Add sort option to findUsers; listUsers sorts by createdAt desc for
deterministic pagination
- Use != null guards for offset/limit to handle zero values correctly
- Remove username from search filter since it is not in the projection
or AdminUserSearchResult response type
* fix: last-admin deletion guard and search query max-length
- Prevent deleting the last admin user (look up target role, count
admins, reject with 400 if count <= 1)
- Cap search query at 200 characters to prevent regex DoS
- Add tests for both guards
* fix: include missing capability name in 403 Forbidden response
* fix: cascade user deletion cleanup, search username, parallel capability checks
- Cascade Config, AclEntry, and SystemGrant cleanup on user deletion
(matching the pattern in roles/groups handlers)
- Add username to admin search $or filter for parity with searchUsers
- Parallelize READ_* capability checks in listAllGrants with Promise.all
* fix: TOCTOU safety net, capability info leak, DRY/style cleanup, data-layer tests
- Add post-delete admin recount with CRITICAL log if race leaves 0 admins
- Revert capability name from 403 response to server-side log only
- Document thin deleteUserById limitation (full cascade is a future task)
- DRY: extract query.trim() to local variable in searchUsersHandler
- Add username to search projection, response type, and AdminUserSearchResult
- Functional filter/map in grants.ts parallel capability check
- Consistent null guards and limit>0 guard in findUsers options
- Fallback for empty result.message on delete response
- Fix mockUser() to generate unique _id per call
- Break long destructuring across multiple lines
- Assert countUsers filter and non-admin skip in delete tests
- Add data-layer tests for findUsers limit, offset, sort, and pagination
* chore: comment out admin delete user endpoint (out of scope)
* fix: cast USER principalId to ObjectId for ACL entry cleanup
ACL entries store USER principalId as ObjectId (via grantPermission casting),
but deleteAclEntries is a raw deleteMany that passes the filter through.
Passing a string won't match stored ObjectIds, leaving orphaned entries.
* chore: comment out unused requireManageUsers alongside disabled delete route
* fix: add missing logger.warn mock in capabilities test
* fix: harden admin users handlers — type safety, response consistency, test coverage
- Unify response shape: AdminUserSearchResult.userId → id, add AdminUserListItem type
- Fix unsafe req.query type assertion in searchUsersHandler (typeof guards)
- Anchor search regex with ^ for prefix matching (enables index usage)
- Add total/capped to search response for truncation signaling
- Add parseInt radix, remove redundant new Date() wrap
- Add tests: countUsers throw, countUsers call args, array query param, capped flag
* fix: scope deleteGrantsForPrincipal to tenant, deterministic search sort, align test mocks
- Add tenantId option to AdminUsersDeps.deleteGrantsForPrincipal and
pass req.user.tenantId at the call site, matching the pattern already
used by the roles and groups handlers
- Add sort: { name: 1 } to searchUsersHandler for deterministic results
- Align test mock deleteUserById messages with production output
('User was deleted successfully.')
- Make capped-results test explicitly set limit: '20' instead of
relying on the implicit default
* test: add tenantId propagation test for deleteGrantsForPrincipal
Add tenantId to createReqRes user type and test that a non-undefined
tenantId is threaded through to deleteGrantsForPrincipal.
* test: remove redundant deleteUserById override in tenantId test
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
* fix: Lazy-initialize balance record when missing at check time
When balance is configured via admin panel DB overrides, users with
existing sessions never pass through the login middleware that creates
their balance record. This causes checkBalanceRecord to find no record
and return balance: 0, blocking the user.
Add optional balanceConfig and upsertBalanceFields deps to
CheckBalanceDeps. When no balance record exists but startBalance is
configured, lazily create the record instead of returning canSpend: false.
Pass the new deps from BaseClient, chatV1, and chatV2 callers.
* test: Add checkBalance lazy initialization tests
Cover lazy balance init scenarios: successful init with startBalance,
insufficient startBalance, missing config fallback, undefined
startBalance, missing upsertBalanceFields dep, and startBalance of 0.
* fix: Address review findings for lazy balance initialization
- Use canonical BalanceConfig and IBalanceUpdate types from
@librechat/data-schemas instead of inline type definitions
- Include auto-refill fields (autoRefillEnabled, refillIntervalValue,
refillIntervalUnit, refillAmount, lastRefill) during lazy init,
mirroring the login middleware's buildUpdateFields logic
- Add try/catch around upsertBalanceFields with graceful fallback to
canSpend: false on DB errors
- Read balance from DB return value instead of raw startBalance constant
- Fix misleading test names to describe observable throw behavior
- Add tests: upsertBalanceFields rejection, auto-refill field inclusion,
DB-returned balance value, and logViolation assertions
* fix: Address second review pass findings
- Fix import ordering: package type imports before local type imports
- Remove misleading comment on DB-fallback test, rename for clarity
- Add logViolation assertion to insufficient-balance lazy-init test
- Add test for partial auto-refill config (autoRefillEnabled without
required dependent fields)
* refactor: Replace createMockReqRes factory with describe-scoped consts
Replace zero-argument factory with plain const declarations using
direct type casts instead of double-cast through unknown.
* fix: Sort local type imports longest-first, add missing logViolation assertion
- Reorder local type imports in spec file per AGENTS.md (longest to
shortest within sub-group)
- Add logViolation assertion to startBalance: 0 test for consistent
violation payload coverage across all throw paths
* refactor: Add existingUsersOnly support to social and SAML auth callbacks
- Add `existingUsersOnly` option to the `socialLogin` handler factory to
reject unknown users instead of creating new accounts
- Refactor SAML strategy callback into `createSamlCallback(existingUsersOnly)`
factory function, mirroring the OpenID `createOpenIDCallback` pattern
- Extract shared SAML config into `getBaseSamlConfig()` helper
- Register `samlAdmin` passport strategy with `existingUsersOnly: true` and
admin-specific callback URL, called automatically from `setupSaml()`
* feat: Register admin OAuth strategy variants for all social providers
- Add admin strategy exports to Google, GitHub, Discord, Facebook, and
Apple strategy files with admin callback URLs and existingUsersOnly
- Extract provider configs into reusable helpers to avoid duplication
between regular and admin strategy constructors
- Re-export all admin strategy factories from strategies/index.js
- Register admin passport strategies (googleAdmin, githubAdmin, etc.)
alongside regular ones in socialLogins.js when env vars are present
* feat: Add admin auth routes for SAML and social OAuth providers
- Add initiation and callback routes for SAML, Google, GitHub, Discord,
Facebook, and Apple to the admin auth router
- Each provider follows the exchange code + PKCE pattern established by
OpenID admin auth: store PKCE challenge on initiation, retrieve on
callback, generate exchange code for the admin panel
- SAML and Apple use POST callbacks with state extracted from
req.body.RelayState and req.body.state respectively
- Extract storePkceChallenge(), retrievePkceChallenge(), and
generateState() helpers; refactor existing OpenID routes to use them
- All callback chains enforce requireAdminAccess, setBalanceConfig,
checkDomainAllowed, and the shared createOAuthHandler
- No changes needed to the generic POST /oauth/exchange endpoint
* fix: Update SAML strategy test to handle dual strategy registration
setupSaml() now registers both 'saml' and 'samlAdmin' strategies,
causing the SamlStrategy mock to be called twice. The verifyCallback
variable was getting overwritten with the admin callback (which has
existingUsersOnly: true), making all new-user tests fail.
Fix: capture only the first callback per setupSaml() call and reset
between tests.
* fix: Address review findings for admin OAuth strategy changes
- Fix existingUsersOnly rejection in socialLogin.js to use
cb(null, false, { message }) instead of cb(error), ensuring
passport's failureRedirect fires correctly for admin flows
- Consolidate duplicate require() calls in strategies/index.js by
destructuring admin exports from the already-imported default export
- Pass pre-parsed baseConfig to setupSamlAdmin() to avoid redundant
certificate file I/O at startup
- Extract getGoogleConfig() helper in googleStrategy.js for consistency
with all other provider strategy files
- Replace randomState() (openid-client) with generateState() (crypto)
in the OpenID admin route for consistency with all other providers,
and remove the now-unused openid-client import
* Reorder import statements in auth.js