mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-06-09 17:31:19 +00:00
3920 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Marco Beretta
|
dca258f36a
|
Merge remote-tracking branch 'origin/dev-staging' into feat/context-window-ui
# Conflicts: # api/server/controllers/assistants/chatV1.js # api/server/controllers/assistants/chatV2.js # api/utils/tokens.spec.js # packages/api/src/agents/usage.ts # packages/api/src/types/tokens.ts # packages/api/src/utils/tokens.ts # packages/data-schemas/src/methods/tx.spec.ts # packages/data-schemas/src/methods/tx.ts |
||
Danny Avila
|
981b9b7e0e
|
🗑️ chore: Remove Action Test Suite and Update Mock Implementations (#12268)
- Deleted the Action test suite located in `api/models/Action.spec.js` to streamline the codebase. - Updated various test files to reflect changes in model mocks, consolidating mock implementations for user-related actions and enhancing clarity. - Improved consistency in test setups by aligning with the latest model updates and removing redundant mock definitions. |
||
|
Danny Avila
|
480fd8005d
|
🔄 chore: Consolidate agent model imports across middleware and tests from rebase
- Updated imports for `createAgent` and `getAgent` to streamline access from a unified `~/models` path. - Enhanced test files to reflect the new import structure, ensuring consistency and maintainability across the codebase. - Improved clarity by removing redundant imports and aligning with the latest model updates. |
||
Atef Bellaaj
|
b1bcbe0f6e
|
🗂️ refactor: Migrate S3 Storage to TypeScript in packages/api (#11947)
* Migrate S3 storage module with unit and integration tests - Migrate S3 CRUD and image operations to packages/api/src/storage/s3/ - Add S3ImageService class with dependency injection - Add unit tests using aws-sdk-client-mock - Add integration tests with real s3 bucket (condition presence of AWS_TEST_BUCKET_NAME) * AI Review Findings Fixes * chore: tests and refactor S3 storage types - Added mock implementations for the 'sharp' library in various test files to improve image processing testing. - Updated type references in S3 storage files from MongoFile to TFile for consistency and type safety. - Refactored S3 CRUD operations to ensure proper handling of file types and improve code clarity. - Enhanced integration tests to validate S3 file operations and error handling more effectively. * chore: rename test file * Remove duplicate import of refreshS3Url * chore: imports order * fix: remove duplicate imports for S3 URL handling in UserController * fix: remove duplicate import of refreshS3FileUrls in files.js * test: Add mock implementations for 'sharp' and '@librechat/api' in UserController tests - Introduced mock functions for the 'sharp' library to facilitate image processing tests, including metadata retrieval and buffer conversion. - Enhanced mocking for '@librechat/api' to ensure consistent behavior in tests, particularly for the needsRefresh and getNewS3URL functions. --------- Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
Danny Avila
|
96ad61b750
|
🏢 feat: Multi-Tenant Data Isolation Infrastructure (#12091)
* chore: imports
* chore: optional chaining in `spendTokens.spec.ts`
* feat: Add tenantId field to all MongoDB schemas for multi-tenant isolation
- Add AsyncLocalStorage-based tenant context (`tenantContext.ts`) for
request-scoped tenantId propagation without modifying method signatures
- Add Mongoose `applyTenantIsolation` plugin that injects `{ tenantId }`
into all query filters when tenant context is present, with
`TENANT_ISOLATION_STRICT` env var for fail-closed production mode
- Add optional `tenantId` field to all 28 collection schemas
- Update all compound unique indexes to include tenantId (email, OAuth IDs,
role names, serverName, conversationId+user, messageId+user, etc.)
- Apply tenant isolation plugin in all 28 model factories
- Add `tenantId?: string` to all TypeScript document interfaces
Behaviorally inert — transitional mode (default) passes through all queries
unchanged. No migration required for existing deployments.
* refactor: Update tenant context and enhance tenant isolation plugin
- Changed `tenantId` in `TenantContext` to be optional, allowing for more flexible usage.
- Refactored `runAsSystem` function to accept synchronous functions, improving usability.
- Introduced comprehensive tests for the `applyTenantIsolation` plugin, ensuring correct tenant filtering in various query scenarios.
- Enhanced the plugin to handle aggregate queries and save operations with tenant context, improving data isolation capabilities.
* docs: tenant context documentation and improve tenant isolation tests
- Added detailed documentation for the `tenantStorage` AsyncLocalStorage instance in `tenantContext.ts`, clarifying its usage for async tenant context propagation.
- Updated tests in `tenantIsolation.spec.ts` to improve clarity and coverage, including new tests for strict mode behavior and tenant context propagation through await boundaries.
- Refactored existing test cases for better readability and consistency, ensuring robust validation of tenant isolation functionality.
* feat: Enhance tenant isolation by preventing tenantId mutations in update operations
- Added a new function to assert that tenantId cannot be modified through update operators in Mongoose queries.
- Implemented middleware to enforce this restriction during findOneAndUpdate, updateOne, and updateMany operations.
- Updated documentation to reflect the new behavior regarding tenantId modifications, ensuring clarity on tenant isolation rules.
* feat: Enhance tenant isolation tests and enforce tenantId restrictions
- Updated existing tests to clarify behavior regarding tenantId preservation during save and insertMany operations.
- Introduced new tests to validate that tenantId cannot be modified through update operations, ensuring strict adherence to tenant isolation rules.
- Added checks for mismatched tenantId scenarios, reinforcing the integrity of tenant context propagation.
- Enhanced test coverage for async context propagation and mutation guards, improving overall robustness of tenant isolation functionality.
* fix: Remove duplicate re-exports in utils/index.ts
Merge artifact caused `string` and `tempChatRetention` to be exported
twice, which produces TypeScript compile errors for duplicate bindings.
* fix: Resolve admin capability gap in multi-tenant mode (TODO #12091)
- hasCapabilityForPrincipals now queries both tenant-scoped AND
platform-level grants when tenantId is set, so seeded ADMIN grants
remain effective in tenant mode.
- Add applyTenantIsolation to SystemGrant model factory.
* fix: Harden tenant isolation plugin
- Add replaceGuard for replaceOne/findOneAndReplace to prevent
cross-tenant document reassignment via replacement documents.
- Cache isStrict() result to avoid process.env reads on every query.
Export _resetStrictCache() for test teardown.
- Replace console.warn with project logger (winston).
- Add 5 new tests for replace guard behavior (46 total).
* style: Fix import ordering in convo.ts and message.ts
Move type imports after value imports per project style guide.
* fix: Remove tenant isolation from SystemGrant, stamp tenantId in replaceGuard
- SystemGrant is a cross-tenant control plane whose methods handle
tenantId conditions explicitly. Applying the isolation plugin
injects a hard equality filter that overrides the $and/$or logic
in hasCapabilityForPrincipals, making platform-level ADMIN grants
invisible in tenant mode.
- replaceGuard now stamps tenantId into replacement documents when
absent, preventing replaceOne from silently stripping tenant
context. Replacements with a matching tenantId are allowed;
mismatched tenantId still throws.
* test: Add multi-tenant unique constraint and replace stamping tests
- Verify same name/email can exist in different tenants (compound
unique index allows it).
- Verify duplicate within same tenant is rejected (E11000).
- Verify tenant-scoped query returns only the correct document.
- Update replaceOne test to assert tenantId is stamped into
replacement document.
- Add test for replacement with matching tenantId.
* style: Reorder imports in message.ts to align with project style guide
* feat: Add migration to drop superseded unique indexes for multi-tenancy
Existing deployments have single-field unique indexes (e.g. { email: 1 })
that block multi-tenant operation — same email in different tenants
triggers E11000. Mongoose autoIndex creates the new compound indexes
but never drops the old ones.
dropSupersededTenantIndexes() drops all 19 superseded indexes across 11
collections. It is idempotent, skips missing indexes/collections, and
is a no-op on fresh databases.
Must be called before enabling multi-tenant middleware on an existing
deployment. Single-tenant deployments are unaffected (old indexes
coexist harmlessly until migration runs).
Includes 11 tests covering:
- Full upgrade simulation (create old indexes, drop them, verify gone)
- Multi-tenant writes work after migration (same email, different tenant)
- Intra-tenant uniqueness preserved (duplicate within tenant rejected)
- Fresh database (no-op, no errors)
- Partial migration (some collections exist, some don't)
- SUPERSEDED_INDEXES coverage validation
* fix: Update systemGrant test — platform grants now satisfy tenant queries
The TODO #12091 fix intentionally changed hasCapabilityForPrincipals to
match both tenant-scoped AND platform-level grants. The test expected
the old behavior (platform grant invisible to tenant query). Updated
test name and expectation to match the new semantics.
* fix: Align getCapabilitiesForPrincipal with hasCapabilityForPrincipals tenant query
getCapabilitiesForPrincipal used a hard tenantId equality filter while
hasCapabilityForPrincipals uses $and/$or to match both tenant-scoped
and platform-level grants. This caused the two functions to disagree
on what grants a principal holds in tenant mode.
Apply the same $or pattern: when tenantId is provided, match both
{ tenantId } and { tenantId: { $exists: false } }.
Adds test verifying platform-level ADMIN grants appear in
getCapabilitiesForPrincipal when called with a tenantId.
* fix: Remove categories from tenant index migration
categoriesSchema is exported but never used to create a Mongoose model.
No Category model factory exists, no code constructs a model from it,
and no categories collection exists in production databases. Including
it in the migration would attempt to drop indexes from a non-existent
collection (harmlessly skipped) but implies the collection is managed.
* fix: Restrict runAsSystem to async callbacks only
Sync callbacks returning Mongoose thenables silently lose ALS context —
the system bypass does nothing and strict mode throws with no indication
runAsSystem was involved. Narrowing to () => Promise<T> makes the wrong
pattern a compile error. All existing call sites already use async.
* fix: Use next(err) consistently in insertMany pre-hook
The hook accepted a next callback but used throw for errors. Standardize
on next(err) for all error paths so the hook speaks one language —
callback-style throughout.
* fix: Replace optional chaining with explicit null assertions in spendTokens tests
Optional chaining on test assertions masks failures with unintelligible
error messages. Add expect(result).not.toBeNull() before accessing
properties, so a null result produces a clear diagnosis instead of
"received value must be a number".
|
||
|
Danny Avila
|
f539b8df28
|
📜 feat: Implement System Grants for Capability-Based Authorization (#11896)
* feat: Implement System Grants for Role-Based Capabilities
- Added a new `systemGrant` model and associated methods to manage role-based capabilities within the application.
- Introduced middleware functions `hasCapability` and `requireCapability` to check user permissions based on their roles.
- Updated the database seeding process to include system grants for the ADMIN role, ensuring all necessary capabilities are assigned on startup.
- Enhanced type definitions and schemas to support the new system grant functionality, improving overall type safety and clarity in the codebase.
* test: Add unit tests for capabilities middleware and system grant methods
- Introduced comprehensive unit tests for the capabilities middleware, including `hasCapability` and `requireCapability`, ensuring proper permission checks based on user roles.
- Added tests for the `SystemGrant` methods, verifying the seeding of system grants, capability granting, and revocation processes.
- Enhanced test coverage for edge cases, including idempotency of grant operations and handling of unexpected errors in middleware.
- Utilized mocks for database interactions to isolate tests and improve reliability.
* refactor: Transition to Capability-Based Access Control
- Replaced role-based access checks with capability-based checks across various middleware and routes, enhancing permission management.
- Introduced `hasCapability` and `requireCapability` functions to streamline capability verification for user actions.
- Updated relevant routes and middleware to utilize the new capability system, ensuring consistent permission enforcement.
- Enhanced type definitions and added tests for the new capability functions, improving overall code reliability and maintainability.
* test: Enhance capability-based access tests for ADMIN role
- Updated tests to reflect the new capability-based access control, specifically for the ADMIN role.
- Modified test descriptions to clarify that users with the MANAGE_AGENTS capability can bypass permission checks.
- Seeded capabilities for the ADMIN role in multiple test files to ensure consistent permission checks across different routes and middleware.
- Improved overall test coverage for capability verification, ensuring robust permission management.
* test: Update capability tests for MCP server access
- Renamed test to reflect the correct capability for bypassing permission checks, changing from MANAGE_AGENTS to MANAGE_MCP_SERVERS.
- Updated seeding of capabilities for the ADMIN role to align with the new capability structure.
- Ensured consistency in capability definitions across tests and middleware for improved permission management.
* feat: Add hasConfigCapability for enhanced config access control
- Introduced `hasConfigCapability` function to check user permissions for managing or reading specific config sections.
- Updated middleware to export the new capability function, ensuring consistent access control across the application.
- Enhanced unit tests to cover various scenarios for the new capability, improving overall test coverage and reliability.
* fix: Update tenantId filter in createSystemGrantMethods
- Added a condition to set tenantId filter to { $exists: false } when tenantId is null, ensuring proper handling of cases where tenantId is not provided.
- This change improves the robustness of the system grant methods by explicitly managing the absence of tenantId in the filter logic.
* fix: account deletion capability check
- Updated the `canDeleteAccount` middleware to ensure that the `hasManageUsers` capability check only occurs if a user is present, preventing potential errors when the user object is undefined.
- This change improves the robustness of the account deletion logic by ensuring proper handling of user permissions.
* refactor: Optimize seeding of system grants for ADMIN role
- Replaced sequential capability granting with parallel execution using Promise.all in the seedSystemGrants function.
- This change improves performance and efficiency during the initialization of system grants, ensuring all capabilities are granted concurrently.
* refactor: Simplify systemGrantSchema index definition
- Removed the sparse option from the unique index on principalType, principalId, capability, and tenantId in the systemGrantSchema.
- This change streamlines the index definition, potentially improving query performance and clarity in the schema design.
* refactor: Reorganize role capability check in roles route
- Moved the capability check for reading roles to occur after parsing the roleName, improving code clarity and structure.
- This change ensures that the authorization logic is consistently applied before fetching role details, enhancing overall permission management.
* refactor: Remove unused ISystemGrant interface from systemCapabilities.ts
- Deleted the ISystemGrant interface as it was no longer needed, streamlining the code and improving clarity.
- This change helps reduce clutter in the file and focuses on relevant capabilities for the system.
* refactor: Migrate SystemCapabilities to data-schemas
- Replaced imports of SystemCapabilities from 'librechat-data-provider' with imports from '@librechat/data-schemas' across multiple files.
- This change centralizes the management of system capabilities, improving code organization and maintainability.
* refactor: Update account deletion middleware and capability checks
- Modified the `canDeleteAccount` middleware to ensure that the account deletion permission is only granted to users with the `MANAGE_USERS` capability, improving security and clarity in permission management.
- Enhanced error logging for unauthorized account deletion attempts, providing better insights into permission issues.
- Updated the `capabilities.ts` file to ensure consistent handling of user authentication checks, improving robustness in capability verification.
- Refined type definitions in `systemGrant.ts` and `systemGrantMethods.ts` to utilize the `PrincipalType` enum, enhancing type safety and code clarity.
* refactor: Extract principal ID normalization into a separate function
- Introduced `normalizePrincipalId` function to streamline the normalization of principal IDs based on their type, enhancing code clarity and reusability.
- Updated references in `createSystemGrantMethods` to utilize the new normalization function, improving maintainability and reducing code duplication.
* test: Add unit tests for principalId normalization in systemGrant
- Introduced tests for the `grantCapability`, `revokeCapability`, and `getCapabilitiesForPrincipal` methods to verify correct handling of principalId normalization between string and ObjectId formats.
- Enhanced the `capabilities.ts` middleware to utilize the `PrincipalType` enum for improved type safety.
- Added a new utility function `normalizePrincipalId` to streamline principal ID normalization logic, ensuring consistent behavior across the application.
* feat: Introduce capability implications and enhance system grant methods
- Added `CapabilityImplications` to define relationships between broader and implied capabilities, allowing for more intuitive permission checks.
- Updated `createSystemGrantMethods` to expand capability queries to include implied capabilities, improving authorization logic.
- Enhanced `systemGrantSchema` to include an `expiresAt` field for future TTL enforcement of grants, and added validation to ensure `tenantId` is not set to null.
- Documented authorization requirements for prompt group and prompt deletion methods to clarify access control expectations.
* test: Add unit tests for canDeleteAccount middleware
- Introduced unit tests for the `canDeleteAccount` middleware to verify account deletion permissions based on user roles and capabilities.
- Covered scenarios for both allowed and blocked account deletions, including checks for ADMIN users with the `MANAGE_USERS` capability and handling of undefined user cases.
- Enhanced test structure to ensure clarity and maintainability of permission checks in the middleware.
* fix: Add principalType enum validation to SystemGrant schema
Without enum validation, any string value was accepted for principalType
and silently stored. Invalid documents would never match capability
queries, creating phantom grants impossible to diagnose without raw DB
inspection. All other ACL models in the codebase validate this field.
* fix: Replace seedSystemGrants Promise.all with bulkWrite for concurrency safety
When two server instances start simultaneously (K8s rolling deploy, PM2
cluster), both call seedSystemGrants. With Promise.all + findOneAndUpdate
upsert, both instances may attempt to insert the same documents, causing
E11000 duplicate key errors that crash server startup.
bulkWrite with ordered:false handles concurrent upserts gracefully and
reduces 17 individual round trips to a single network call. The returned
documents (previously discarded) are no longer fetched.
* perf: Add AsyncLocalStorage per-request cache for capability checks
Every hasCapability call previously required 2 DB round trips
(getUserPrincipals + SystemGrant.exists) — replacing what were O(1)
string comparisons. Routes like patchPromptGroup triggered this twice,
and hasConfigCapability's fallback path resolved principals twice.
This adds a per-request AsyncLocalStorage cache that:
- Caches resolved principals (same for all checks within one request)
- Caches capability check results (same user+cap = same answer)
- Automatically scoped to request lifetime (no stale grants)
- Falls through to DB when no store exists (background jobs, tests)
- Requires no signature changes to hasCapability
The capabilityContextMiddleware is registered at the app level before
all routes, initializing a fresh store per request.
* fix: Add error handling for inline hasCapability calls
canDeleteAccount, fetchAssistants, and validateAuthor all call
hasCapability without try-catch. These were previously O(1) string
comparisons that could never throw. Now they hit the database and can
fail on connection timeout or transient errors.
Wrap each call in try-catch, defaulting to deny (false) on error.
This ensures a DB hiccup returns a clean 403 instead of an unhandled
500 with a stack trace.
* test: Add canDeleteAccount DB-error resilience test
Tests that hasCapability rejection (e.g., DB timeout) results in a clean
403 rather than an unhandled exception. Validates the error handling
added in the previous commit.
* refactor: Use barrel import for hasCapability in validateAuthor
Import from ~/server/middleware barrel instead of directly from
~/server/middleware/roles/capabilities for consistency with other
non-middleware consumers. Files within the middleware barrel itself
must continue using direct imports to avoid circular requires.
* refactor: Remove misleading pre('save') hook from SystemGrant schema
The pre('save') hook normalized principalId for USER/GROUP principals,
but the primary write path (grantCapability) uses findOneAndUpdate —
which does not trigger save hooks. The normalization was already handled
explicitly in grantCapability itself. The hook created a false impression
of schema-level enforcement that only covered save()/create() paths.
Replace with a comment documenting that all writes must go through
grantCapability.
* feat: Add READ_ASSISTANTS capability to complete manage/read pair
Every other managed resource had a paired READ_X / MANAGE_X capability
except assistants. This adds READ_ASSISTANTS and registers the
MANAGE_ASSISTANTS → READ_ASSISTANTS implication in CapabilityImplications,
enabling future read-only assistant visibility grants.
* chore: Reorder systemGrant methods for clarity
Moved hasCapabilityForPrincipals to a more logical position in the returned object of createSystemGrantMethods, improving code readability. This change also maintains the inclusion of seedSystemGrants in the export, ensuring all necessary methods are available.
* fix: Wrap seedSystemGrants in try-catch to avoid blocking startup
Seeding capabilities is idempotent and will succeed on the next restart.
A transient DB error during seeding should not prevent the server from
starting — log the error and continue.
* refactor: Improve capability check efficiency and add audit logging
Move hasCapability calls after cheap early-exits in validateAuthor and
fetchAssistants so the DB check only runs when its result matters. Add
logger.debug on every capability bypass grant across all 7 call sites
for auditability, and log errors in catch blocks instead of silently
swallowing them.
* test: Add integration tests for AsyncLocalStorage capability caching
Exercises the full vertical — ALS context, generateCapabilityCheck,
real getUserPrincipals, real hasCapabilityForPrincipals, real MongoDB
via MongoMemoryServer. Covers per-request caching, cross-context
isolation, concurrent request isolation, negative caching, capability
implications, tenant scoping, group-based grants, and requireCapability
middleware.
* test: Add systemGrant data-layer and ALS edge-case integration tests
systemGrant.spec.ts (51 tests): Full integration tests for all
systemGrant methods against real MongoDB — grant/revoke lifecycle,
principalId normalization (string→ObjectId for USER/GROUP, string for
ROLE), capability implications (both directions), tenant scoping,
schema validation (null tenantId, invalid enum, required fields,
unique compound index).
capabilities.integration.spec.ts (27 tests): Adds ALS edge cases —
missing context degrades gracefully with no caching (background jobs,
child processes), nested middleware creates independent inner context,
optional-chaining safety when store is undefined, mid-request grant
changes are invisible due to result caching, requireCapability works
without ALS, and interleaved concurrent contexts maintain isolation.
* fix: Add worker thread guards to capability ALS usage
Detect when hasCapability or capabilityContextMiddleware is called from
a worker thread (where ALS context does not propagate from the parent).
hasCapability logs a warn-once per factory instance; the middleware logs
an error since mounting Express middleware in a worker is likely a
misconfiguration. Both continue to function correctly — the guard is
observability, not a hard block.
* fix: Include tenantId in ALS principal cache key for tenant isolation
The principal cache key was user.id:user.role, which would reuse
cached principals across tenants for the same user within a request.
When getUserPrincipals gains tenant-scoped group resolution, principals
from tenant-a would incorrectly serve tenant-b checks. Changed to
user.id:user.role:user.tenantId to prevent cross-tenant cache hits.
Adds integration test proving separate principal lookups per tenantId.
* test: Remove redundant mocked capabilities.spec.js
The JS wrapper test (7 tests, all mocked) is a strict subset of
capabilities.integration.spec.ts (28 tests, real MongoDB). Every
scenario it covered — hasCapability true/false, tenantId passthrough,
requireCapability 403/500, error handling — is tested with higher
fidelity in the integration suite.
* test: Replace mocked canDeleteAccount tests with real MongoDB integration
Remove hasCapability mock — tests now exercise the full capability
chain against real MongoDB (getUserPrincipals, hasCapabilityForPrincipals,
SystemGrant collection). Only mocks remaining are logger and cache.
Adds new coverage: admin role without grant is blocked, user-level
grant bypasses deletion restriction, null user handling.
* test: Add comprehensive tests for ACL entry management and user group methods
Introduces new tests for `deleteAclEntries`, `bulkWriteAclEntries`, and `findPublicResourceIds` in `aclEntry.spec.ts`, ensuring proper functionality for deleting and bulk managing ACL entries. Additionally, enhances `userGroup.spec.ts` with tests for finding groups by ID and name pattern, including external ID matching and source filtering. These changes improve coverage and validate the integrity of ACL and user group operations against real MongoDB interactions.
* refactor: Update capability checks and logging for better clarity and error handling
Replaced `MANAGE_USERS` with `ACCESS_ADMIN` in the `canDeleteAccount` middleware and related tests to align with updated permission structure. Enhanced logging in various middleware functions to use `logger.warn` for capability check failures, providing clearer error messages. Additionally, refactored capability checks in the `patchPromptGroup` and `validateAuthor` functions to improve readability and maintainability. This commit also includes adjustments to the `systemGrant` methods to implement retry logic for transient failures during capability seeding, ensuring robustness in the face of database errors.
* refactor: Enhance logging and retry logic in seedSystemGrants method
Updated the logging format in the seedSystemGrants method to include error messages for better clarity. Improved the retry mechanism by explicitly mocking multiple failures in tests, ensuring robust error handling during transient database issues. Additionally, refined imports in the systemGrant schema for better type management.
* refactor: Consolidate imports in canDeleteAccount middleware
Merged logger and SystemCapabilities imports from the data-schemas module into a single line for improved readability and maintainability of the code. This change streamlines the import statements in the canDeleteAccount middleware.
* test: Enhance systemGrant tests for error handling and capability validation
Added tests to the systemGrant methods to handle various error scenarios, including E11000 race conditions, invalid ObjectId strings for USER and GROUP principals, and invalid capability strings. These enhancements improve the robustness of the capability granting and revoking logic, ensuring proper error propagation and validation of inputs.
* fix: Wrap hasCapability calls in deny-by-default try-catch at remaining sites
canAccessResource, files.js, and roles.js all had hasCapability inside
outer try-catch blocks that returned 500 on DB failure instead of
falling through to the regular ACL check. This contradicts the
deny-by-default pattern used everywhere else.
Also removes raw error.message from the roles.js 500 response to
prevent internal host/connection info leaking to clients.
* fix: Normalize user ID in canDeleteAccount before passing to hasCapability
requireCapability normalizes req.user.id via _id?.toString() fallback,
but canDeleteAccount passed raw req.user directly. If req.user.id is
absent (some auth layers only populate _id), getUserPrincipals received
undefined, silently returning empty principals and blocking the bypass.
* fix: Harden systemGrant schema and type safety
- Reject empty string tenantId in schema validator (was only blocking
null; empty string silently orphaned documents)
- Fix reverseImplications to use BaseSystemCapability[] instead of
string[], preserving the narrow discriminated type
- Document READ_ASSISTANTS as reserved/unenforced
* test: Use fake timers for seedSystemGrants retry tests and add tenantId validation
- Switch retry tests to jest.useFakeTimers() to eliminate 3+ seconds
of real setTimeout delays per test run
- Add regression test for empty-string tenantId rejection
* docs: Add TODO(#12091) comments for tenant-scoped capability gaps
In multi-tenant mode, platform-level grants (no tenantId) won't match
tenant-scoped queries, breaking admin access. getUserPrincipals also
returns cross-tenant group memberships. Both need fixes in #12091.
|
||
|
Danny Avila
|
70445b951d
|
🪢 chore: Consolidate Pricing and Tx Imports After tx.js Module Removal (#12086)
* 🧹 chore: resolve imports due to rebase
* chore: Update model mocks in unit tests for consistency
- Consolidated model mock implementations across various test files to streamline setup and reduce redundancy.
- Removed duplicate mock definitions for `getMultiplier` and `getCacheMultiplier`, ensuring a unified approach in `recordCollectedUsage.spec.js`, `openai.spec.js`, `responses.unit.spec.js`, and `abortMiddleware.spec.js`.
- Enhanced clarity and maintainability of test files by aligning mock structures with the latest model updates.
* fix: Safeguard token credit checks in transaction tests
- Updated assertions in `transaction.spec.ts` to handle potential null values for `updatedBalance` by using optional chaining.
- Enhanced robustness of tests related to token credit calculations, ensuring they correctly account for scenarios where the balance may not be found.
* chore: transaction methods with bulk insert functionality
- Introduced `bulkInsertTransactions` method in `transaction.ts` to facilitate batch insertion of transaction documents.
- Updated test file `transactions.bulk-parity.spec.ts` to utilize new pricing function assignments and handle potential null values in calculations, improving test robustness.
- Refactored pricing function initialization for clarity and consistency.
* refactor: Enhance type definitions and introduce new utility functions for model matching
- Added `findMatchingPattern` and `matchModelName` utility functions to improve model name matching logic in transaction methods.
- Updated type definitions for `findMatchingPattern` to accept a more specific tokensMap structure, enhancing type safety.
- Refactored `dbMethods` initialization in `transactions.bulk-parity.spec.ts` to include the new utility functions, improving test clarity and functionality.
* refactor: Update database method imports and enhance transaction handling
- Refactored `abortMiddleware.js` to utilize centralized database methods for message handling and conversation retrieval, improving code consistency.
- Enhanced `bulkInsertTransactions` in `transaction.ts` to handle empty document arrays gracefully and added error logging for better debugging.
- Updated type definitions in `transactions.ts` to enforce stricter typing for token types, enhancing type safety across transaction methods.
- Improved test setup in `transactions.bulk-parity.spec.ts` by refining pricing function assignments and ensuring robust handling of potential null values.
* refactor: Update database method references and improve transaction multiplier handling
- Refactored `client.js` to update database method references for `bulkInsertTransactions` and `updateBalance`, ensuring consistency in method usage.
- Enhanced transaction multiplier calculations in `transaction.spec.ts` to provide fallback values for write and read multipliers, improving robustness in cost calculations across structured token spending tests.
|
||
|
Danny Avila
|
d9acb1b321
|
📦 refactor: Consolidate DB models, encapsulating Mongoose usage in data-schemas (#11830)
* chore: move database model methods to /packages/data-schemas * chore: add TypeScript ESLint rule to warn on unused variables * refactor: model imports to streamline access - Consolidated model imports across various files to improve code organization and reduce redundancy. - Updated imports for models such as Assistant, Message, Conversation, and others to a unified import path. - Adjusted middleware and service files to reflect the new import structure, ensuring functionality remains intact. - Enhanced test files to align with the new import paths, maintaining test coverage and integrity. * chore: migrate database models to packages/data-schemas and refactor all direct Mongoose Model usage outside of data-schemas * test: update agent model mocks in unit tests - Added `getAgent` mock to `client.test.js` to enhance test coverage for agent-related functionality. - Removed redundant `getAgent` and `getAgents` mocks from `openai.spec.js` and `responses.unit.spec.js` to streamline test setup and reduce duplication. - Ensured consistency in agent mock implementations across test files. * fix: update types in data-schemas * refactor: enhance type definitions in transaction and spending methods - Updated type definitions in `checkBalance.ts` to use specific request and response types. - Refined `spendTokens.ts` to utilize a new `SpendTxData` interface for better clarity and type safety. - Improved transaction handling in `transaction.ts` by introducing `TransactionResult` and `TxData` interfaces, ensuring consistent data structures across methods. - Adjusted unit tests in `transaction.spec.ts` to accommodate new type definitions and enhance robustness. * refactor: streamline model imports and enhance code organization - Consolidated model imports across various controllers and services to a unified import path, improving code clarity and reducing redundancy. - Updated multiple files to reflect the new import structure, ensuring all functionalities remain intact. - Enhanced overall code organization by removing duplicate import statements and optimizing the usage of model methods. * feat: implement loadAddedAgent and refactor agent loading logic - Introduced `loadAddedAgent` function to handle loading agents from added conversations, supporting multi-convo parallel execution. - Created a new `load.ts` file to encapsulate agent loading functionalities, including `loadEphemeralAgent` and `loadAgent`. - Updated the `index.ts` file to export the new `load` module instead of the deprecated `loadAgent`. - Enhanced type definitions and improved error handling in the agent loading process. - Adjusted unit tests to reflect changes in the agent loading structure and ensure comprehensive coverage. * refactor: enhance balance handling with new update interface - Introduced `IBalanceUpdate` interface to streamline balance update operations across the codebase. - Updated `upsertBalanceFields` method signatures in `balance.ts`, `transaction.ts`, and related tests to utilize the new interface for improved type safety. - Adjusted type imports in `balance.spec.ts` to include `IBalanceUpdate`, ensuring consistency in balance management functionalities. - Enhanced overall code clarity and maintainability by refining type definitions related to balance operations. * feat: add unit tests for loadAgent functionality and enhance agent loading logic - Introduced comprehensive unit tests for the `loadAgent` function, covering various scenarios including null and empty agent IDs, loading of ephemeral agents, and permission checks. - Enhanced the `initializeClient` function by moving `getConvoFiles` to the correct position in the database method exports, ensuring proper functionality. - Improved test coverage for agent loading, including handling of non-existent agents and user permissions. * chore: reorder memory method exports for consistency - Moved `deleteAllUserMemories` to the correct position in the exported memory methods, ensuring a consistent and logical order of method exports in `memory.ts`. |
||
|
Danny Avila
|
5ed776a052
|
🗑️ chore: Remove Deprecated Project Model and Associated Fields (#11773)
* chore: remove projects and projectIds usage * chore: empty line linting * chore: remove isCollaborative property across agent models and related tests - Removed the isCollaborative property from agent models, controllers, and tests, as it is deprecated in favor of ACL permissions. - Updated related validation schemas and data provider types to reflect this change. - Ensured all references to isCollaborative were stripped from the codebase to maintain consistency and clarity. |
||
|
Danny Avila
|
b90a1efee3
|
🐘 feat: FerretDB Compatibility (#11769)
* feat: replace unsupported MongoDB aggregation operators for FerretDB compatibility Replace $lookup, $unwind, $sample, $replaceRoot, and $addFields aggregation stages which are unsupported on FerretDB v2.x (postgres-documentdb backend). - Prompt.js: Replace $lookup/$unwind/$project pipelines with find().select().lean() + attachProductionPrompts() batch helper. Replace $group/$replaceRoot/$sample in getRandomPromptGroups with distinct() + Fisher-Yates shuffle. - Agent/Prompt migration scripts: Replace $lookup anti-join pattern with distinct() + $nin two-step queries for finding un-migrated resources. All replacement patterns verified against FerretDB v2.7.0. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: use $pullAll for simple array removals, fix memberIds type mismatches Replace $pull with $pullAll for exact-value scalar array removals. Both operators work on MongoDB and FerretDB, but $pullAll is more explicit for exact matching (no condition expressions). Fix critical type mismatch bugs where ObjectId values were used against String[] memberIds arrays in Group queries: - config/delete-user.js: use string uid instead of ObjectId user._id - e2e/setup/cleanupUser.ts: convert userId.toString() before query Harden PermissionService.bulkUpdateResourcePermissions abort handling to prevent crash when abortTransaction is called after commitTransaction. All changes verified against FerretDB v2.7.0 and MongoDB Memory Server. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: harden transaction support probe for FerretDB compatibility Commit the transaction before aborting in supportsTransactions probe, and wrap abortTransaction in try-catch to prevent crashes when abort is called after a successful commit (observed behavior on FerretDB). Co-authored-by: Cursor <cursoragent@cursor.com> * feat: add FerretDB compatibility test suite, retry utilities, and CI config Add comprehensive FerretDB integration test suite covering: - $pullAll scalar array operations - $pull with subdocument conditions - $lookup replacement (find + manual join) - $sample replacement (distinct + Fisher-Yates) - $bit and $bitsAllSet operations - Migration anti-join pattern - Multi-tenancy (useDb, scaling, write amplification) - Sharding proof-of-concept - Production operations (backup/restore, schema migration, deadlock retry) Add production retryWithBackoff utility for deadlock recovery during concurrent index creation on FerretDB/DocumentDB backends. Add UserController.spec.js tests for deleteUserController (runs in CI). Configure jest and eslint to isolate FerretDB tests from CI pipelines: - packages/data-schemas/jest.config.mjs: ignore misc/ directory - eslint.config.mjs: ignore packages/data-schemas/misc/ Include Docker Compose config for local FerretDB v2.7 + postgres-documentdb, dedicated jest/tsconfig for the test files, and multi-tenancy findings doc. Co-authored-by: Cursor <cursoragent@cursor.com> * style: brace formatting in aclEntry.ts modifyPermissionBits Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: reorganize retry utilities and update imports - Moved retryWithBackoff utility to a new file `retry.ts` for better structure. - Updated imports in `orgOperations.ferretdb.spec.ts` to reflect the new location of retry utilities. - Removed old import statement for retryWithBackoff from index.ts to streamline exports. * test: add $pullAll coverage for ConversationTag and PermissionService Add integration tests for deleteConversationTag verifying $pullAll removes tags from conversations correctly, and for syncUserEntraGroupMemberships verifying $pullAll removes user from non-matching Entra groups while preserving local group membership. --------- Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Danny Avila
|
b5a55b23a4
|
📦 chore: NPM audit packages (#12286)
* 🔧 chore: Update dependencies in package-lock.json and package.json - Bump @aws-sdk/client-bedrock-runtime from 3.980.0 to 3.1011.0 and update related dependencies. - Update fast-xml-parser version from 5.3.8 to 5.5.6 in package.json. - Adjust various @aws-sdk and @smithy packages to their latest versions for improved functionality and security. * 🔧 chore: Update @librechat/agents dependency to version 3.1.57 in package.json and package-lock.json - Bump @librechat/agents from 3.1.56 to 3.1.57 across multiple package files for consistency. - Remove axios dependency from package.json as it is no longer needed. |
||
|
Danny Avila
|
1e1a3a8f8d |
✨ v0.8.4-rc1 (#12285)
- App version: v0.8.3 → v0.8.4-rc1 - @librechat/api: 1.7.25 → 1.7.26 - @librechat/client: 0.4.54 → 0.4.55 - librechat-data-provider: 0.8.302 → 0.8.400 - @librechat/data-schemas: 0.0.38 → 0.0.39 |
||
Dustin Healy
|
5b31bb720d
|
🔧 fix: Proper MCP Menu Dismissal (#12256)
* fix: replace manual focus hack with modal menu in MCP selector
Use Ariakit's `modal={true}` instead of a manual `requestAnimationFrame`
focus-restore wrapper, which eliminates the `useRef`/`useCallback`
overhead and lets Ariakit manage focus trapping natively. Also removes
the unused `focusLoop` option from both MCP menu stores and a narrating
comment in MCPSubMenu.
* test: add MCPSelect menu interaction tests
Cover button rendering, menu open/close via click and Escape, and
server toggle keeping the menu open. Renders real MCPServerMenuItem
and StackedMCPIcons components instead of re-implementing their
logic in mocks.
* fix: add unmountOnHide to MCP menu for consistency
Matches the pattern used by MCPSubMenu, BookmarkMenu, and other
Ariakit menus in the codebase. Ensures the menu fully detaches
from the DOM and accessibility tree when closed.
* fix: restore focusLoop on MCP menu stores
Ariakit's CompositeStore (which MenuStore extends) defaults focusLoop
to false. The previous commit incorrectly removed the explicit
focusLoop: true, which silently disabled Arrow-key wraparound
(mandatory per WAI-ARIA Menu pattern). modal={true} only traps Tab
focus — it does not enable Arrow-key looping.
* test: improve MCPSelect test coverage and mock hygiene
- Add aria-modal regression guard so removing modal={true} fails a test
- Add guard branch tests: no MCP access, empty servers, unpinned+empty
- Fix TooltipAnchor mock to correctly spread array children
- Fix import ordering per project conventions
- Move component import to top with other imports
- Replace unasserted jest.fn() mocks with plain values
- Use mutable module-scoped vars for per-test mock overrides
* fix: enhance pointer event handling in FavoriteItem component
Updated the opacity and pointer events logic in the FavoriteItem component to improve user interaction. The changes ensure that the component correctly manages pointer events based on the popover state, enhancing accessibility and usability.
* test: add MCPSubMenu menu interaction tests
Cover guard branch (empty servers), submenu open/close with real
Ariakit components and real MCPServerMenuItem, toggle persistence,
pin/unpin button behavior and aria-label states. Only context
providers and cross-package UI are mocked.
* test: add focusLoop regression guard for both MCP menus
ArrowDown from the last item must wrap to the first — this fails
without focusLoop: true on the menu store, directly guarding the
keyboard accessibility regression that was silently introduced.
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
|
||
|
Danny Avila
|
2f09d29c71
|
🛂 fix: Validate types Query Param in People Picker Access Middleware (#12276)
* 🛂 fix: Validate `types` query param in people picker access middleware checkPeoplePickerAccess only inspected `req.query.type` (singular), allowing callers to bypass type-specific permission checks by using the `types` (plural) parameter accepted by the controller. Now both `type` and `types` are collected and each requested principal type is validated against the caller's role permissions. * 🛂 refactor: Hoist valid types constant, improve logging, and add edge-case tests - Hoist VALID_PRINCIPAL_TYPES to module-level Set to avoid per-request allocation - Include both `type` and `types` in error log for debuggability - Restore detailed JSDoc documenting per-type permission requirements - Add missing .json() assertion on partial-denial test - Add edge-case tests: all-invalid types, empty string types, PrincipalType.PUBLIC * 🏷️ fix: Align TPrincipalSearchParams with actual controller API The stale type used `type` (singular) but the controller and all callers use `types` (plural array). Aligns with PrincipalSearchParams in types/queries.ts. |
||
|
Danny Avila
|
68435cdcd0
|
🧯 fix: Add Pre-Parse File Size Guard to Document Parser (#12275)
Prevent memory exhaustion DoS by rejecting documents exceeding 15MB before reading them into memory, closing the gap between the 512MB upload limit and unbounded in-memory parsing. |
||
|
Danny Avila
|
0c378811f1
|
🏷️ fix: Clear ModelSpec Display Fields When Navigating via Agent Share Link (#12274)
- Extract `specDisplayFieldReset` constant and `mergeQuerySettingsWithSpec` utility to `client/src/utils/endpoints.ts` as a single source of truth for spec display fields that must be cleared on non-spec transitions. - Clear `spec`, `iconURL`, `modelLabel`, and `greeting` from the merged preset in `ChatRoute.getNewConvoPreset()` when URL query parameters override the conversation without explicitly setting a spec. - Also clear `greeting` in the parallel cleanup in `useQueryParams.newQueryConvo` using the shared `specDisplayFieldReset` constant. - Guard the field reset on `specPreset != null` so null values aren't injected when no spec is configured. - Add comprehensive test coverage for the merge-and-clear logic. |
||
|
Danny Avila
|
9a64791e3e
|
🪢 fix: Action Domain Encoding Collision for HTTPS URLs (#12271)
* fix: strip protocol from domain before encoding in `domainParser`
All https:// (and http://) domains produced the same 10-char base64
prefix due to ENCODED_DOMAIN_LENGTH truncation, causing tool name
collisions for agents with multiple actions.
Strip the protocol before encoding so the base64 key is derived from
the hostname. Add `legacyDomainEncode` to preserve the old encoding
logic for backward-compatible matching of existing stored actions.
* fix: backward-compatible tool matching in ToolService
Update `getActionToolDefinitions` to match stored tools against both
new and legacy domain encodings. Update `loadActionToolsForExecution`
to resolve model-called tool names via a `normalizedToDomain` map
that includes both encoding variants, with legacy fallback for
request builder lookup.
* fix: action route save/delete domain encoding issues
Save routes now remove old tools matching either new or legacy domain
encoding, preventing stale entries when an action's encoding changes
on update.
Delete routes no longer re-encode the already-encoded domain extracted
from the stored actions array, which was producing incorrect keys and
leaving orphaned tools.
* test: comprehensive coverage for action domain encoding
Rewrite ActionService tests to cover real matching patterns used by
ToolService and action routes. Tests verify encode/decode round-trips,
protocol stripping, backward-compatible tool name matching at both
definition and execution phases, save-route cleanup of old/new
encodings, delete-route domain extraction, and the collision fix for
multi-action agents.
* fix: add legacy domain compat to all execution paths, make legacyDomainEncode sync
CRITICAL: processRequiredActions (assistants path) was not updated with
legacy domain matching — existing assistants with https:// domain actions
would silently fail post-deployment because domainMap only had new encoding.
MAJOR: loadAgentTools definitionsOnly=false path had the same issue.
Both now use a normalizedToDomain map with legacy+new entries and extract
function names via the matched key (not the canonical domain).
Also: make legacyDomainEncode synchronous (no async operations), store
legacyNormalized in processedActionSets to eliminate recomputation in
the per-tool fallback, and hoist domainSeparatorRegex to module level.
* refactor: clarify domain variable naming and tool-filter helpers in action routes
Rename shadowed 'domain' to 'encodedDomain' to separate raw URL from
encoded key in both agent and assistant save routes.
Rename shouldRemoveTool to shouldRemoveAgentTool / shouldRemoveAssistantTool
to make the distinct data-shape guards explicit.
Remove await on now-synchronous legacyDomainEncode.
* test: expand coverage for all review findings
- Add validateAndUpdateTool tests (protocol-stripping match logic)
- Restore unicode domain encode/decode/round-trip tests
- Add processRequiredActions matching pattern tests (assistants path)
- Add legacy guard skip test for short bare hostnames
- Add pre-normalized Set test for definition-phase optimization
- Fix corrupt-cache test to assert typeof instead of toBeDefined
- Verify legacyDomainEncode is synchronous (not a Promise)
- Remove all await on legacyDomainEncode (now sync)
58 tests, up from 44.
* fix: address follow-up review findings A-E
A: Fix stale JSDoc @returns {Promise<string>} on now-synchronous
legacyDomainEncode — changed to @returns {string}.
B: Rename normalizedToDomain to domainLookupMap in processRequiredActions
and loadAgentTools where keys are raw encoded domains (not normalized),
avoiding confusion with loadActionToolsForExecution where keys ARE
normalized.
C: Pre-normalize actionToolNames into a Set<string> in
getActionToolDefinitions, replacing O(signatures × tools) per-check
.some() + .replace() with O(1) Set.has() lookups.
D: Remove stripProtocol from ActionService exports — it is a one-line
internal helper. Spec tests for it removed; behavior is fully covered
by domainParser protocol-stripping tests.
E: Fix pre-existing bug where processRequiredActions re-loaded action
sets on every missing-tool iteration. The guard !actionSets.length
always re-triggered because actionSets was reassigned to a plain
object (whose .length is undefined). Replaced with a null-check
on a dedicated actionSetsData variable.
* fix: strip path and query from domain URLs in stripProtocol
URLs like 'https://api.example.com/v1/endpoint?foo=bar' previously
retained the path after protocol stripping, contaminating the encoded
domain key. Now strips everything after the first '/' following the
host, using string indexing instead of URL parsing to avoid punycode
normalization of unicode hostnames.
Closes Copilot review comments 1, 2, and 5.
|
||
|
Danny Avila
|
c68066a636
|
🪝 fix: MCP Refresh token on OAuth Discovery Failure (#12266)
* 🔒 fix: Prevent token leaks to MCP server on OAuth discovery failure When OAuth metadata discovery fails, refresh logic was falling back to POSTing refresh tokens to /token on the MCP resource server URL instead of the authorization server. A malicious MCP server could exploit this by blocking .well-known discovery to harvest refresh tokens. Changes: - Replace unsafe /token fallback with hard error in both refresh paths - Thread stored token_endpoint (SSRF-validated during initial flow) through the refresh chain so legacy servers without .well-known still work after the first successful auth - Fix revokeOAuthToken to always SSRF-validate the revocation URL, including the /revoke fallback path - Redact refresh token and credentials from debug-level log output - Split branch 2 compound condition for consistent error messages * ✅ test: Add stored endpoint fallback tests and improve refresh coverage - Add storedTokenEndpoint fallback tests for both refresh branches - Add missing test for branch 2 metadata-without-token_endpoint case - Rename misleading test name to match actual mock behavior - Split auto-discovered throw test into undefined vs missing-endpoint - Remove redundant afterEach mockFetch.mockClear() calls (already covered by jest.clearAllMocks() in beforeEach) |
||
|
Danny Avila
|
d17ac8f06d
|
🔏 fix: Remove Federated Tokens from OpenID Refresh Response (#12264)
* 🔒 fix: Remove OpenID federated tokens from refresh endpoint response The refresh controller was attaching federatedTokens (including the refresh_token) to the user object returned in the JSON response, exposing HttpOnly-protected tokens to client-side JavaScript. The tokens are already stored server-side by setOpenIDAuthTokens and re-attached by the JWT strategy on authenticated requests. * 🔒 fix: Strip sensitive fields from OpenID refresh response user object The OpenID refresh path returned the raw findOpenIDUser result without field projection, unlike the non-OpenID path which excludes password, __v, totpSecret, and backupCodes via getUserById projection. Destructure out sensitive fields before serializing. Also strengthens the regression test: uses not.toHaveProperty for true property-absence checks (expect.anything() misses null/undefined), adds positive shape assertion, and DRYs up duplicated mock user setup. |
||
|
Danny Avila
|
381ed8539b
|
🪪 fix: Enforce Conversation Ownership Checks in Remote Agent Controllers (#12263)
* 🔒 fix: Validate conversation ownership in remote agent API endpoints Add user-scoped ownership checks for client-supplied conversation IDs in OpenAI-compatible and Open Responses controllers to prevent cross-tenant file/message loading via IDOR. * 🔒 fix: Harden ownership checks against type confusion and unhandled errors - Add typeof string validation before getConvo to block NoSQL operator injection (e.g. { "$gt": "" }) bypassing the ownership check - Move ownership checks inside try/catch so DB errors produce structured JSON error responses instead of unhandled promise rejections - Add string type validation for conversation_id and previous_response_id in the upstream TS request validators (defense-in-depth) * 🧪 test: Add coverage for conversation ownership validation in remote agent APIs - Fix broken getConvo mock in openai.spec.js (was missing entirely) - Add tests for: owned conversation, unowned (404), non-string type (400), absent conversation_id (skipped), and DB error (500) — both controllers |
||
|
Danny Avila
|
951d261f5c
|
🧯 fix: Prevent Env-Variable Exfil. via Placeholder Injection (#12260)
* 🔒 fix: Resolve env vars before body placeholder expansion to prevent secret exfiltration Body placeholders ({{LIBRECHAT_BODY_*}}) were substituted before extractEnvVariable ran, allowing user-controlled body fields containing ${SECRET} patterns to be expanded into real environment values in outbound headers. Reorder so env vars resolve first, preventing untrusted input from triggering env expansion. * 🛡️ fix: Block sensitive infrastructure env vars from placeholder resolution Add isSensitiveEnvVar blocklist to extractEnvVariable so that internal infrastructure secrets (JWT_SECRET, JWT_REFRESH_SECRET, CREDS_KEY, CREDS_IV, MEILI_MASTER_KEY, MONGO_URI, REDIS_URI, REDIS_PASSWORD) can never be resolved via ${VAR} expansion — even if an attacker manages to inject a placeholder pattern. Uses exact-match set (not substring patterns) to avoid breaking legitimate operator config that references OAuth/API secrets in MCP and custom endpoint configurations. * 🧹 test: Rename ANOTHER_SECRET test fixture to ANOTHER_VALUE Avoid using SECRET-containing names for non-sensitive test fixtures to prevent confusion with the new isSensitiveEnvVar blocklist. * 🔒 fix: Resolve env vars before all user-controlled substitutions in processSingleValue Move extractEnvVariable to run on the raw admin-authored template BEFORE customUserVars, user fields, OIDC tokens, and body placeholders. Previously env resolution ran after customUserVars, so a user setting a custom MCP variable to "${SECRET}" could still trigger env expansion. Now env vars are resolved strictly on operator config, and all subsequent user-controlled substitutions cannot introduce ${VAR} patterns that would be expanded. Gated by !dbSourced so DB-stored servers continue to skip env resolution. Adds a security-invariant comment documenting the ordering requirement. * 🧪 test: Comprehensive security regression tests for placeholder injection - Cover all three body fields (conversationId, parentMessageId, messageId) - Add user-field injection test (user.name containing ${VAR}) - Add customUserVars injection test (MY_TOKEN = "${VAR}") - Add processMCPEnv injection tests for body and customUserVars paths - Remove redundant process.env setup/teardown already handled by beforeEach/afterEach * 🧹 chore: Add REDIS_PASSWORD to blocklist integration test; document customUserVars gate |
||
github-actions[bot]
|
85e24e4c61
|
🌍 i18n: Update translation.json with latest translations (#12259)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Danny Avila
|
8271055c2d
|
📦 chore: Bump @librechat/agents to v3.1.56 (#12258)
* 📦 chore: Bump `@librechat/agents` to v3.1.56
* chore: resolve type error, URL property check in isMCPDomainAllowed function
|
||
|
Danny Avila
|
acd07e8085
|
🗝️ fix: Exempt Admin-Trusted Domains from MCP OAuth Validation (#12255)
* fix: exempt allowedDomains from MCP OAuth SSRF checks (#12254) The SSRF guard in validateOAuthUrl was context-blind — it blocked private/internal OAuth endpoints even for admin-trusted MCP servers listed in mcpSettings.allowedDomains. Add isHostnameAllowed() to domain.ts and skip SSRF checks in validateOAuthUrl when the OAuth endpoint hostname matches an allowed domain. * refactor: thread allowedDomains through MCP connection stack Pass allowedDomains from MCPServersRegistry through BasicConnectionOptions, MCPConnectionFactory, and into MCPOAuthHandler method calls so the OAuth layer can exempt admin-trusted domains from SSRF validation. * test: add allowedDomains bypass tests and fix registry mocks Add isHostnameAllowed unit tests (exact, wildcard, case-insensitive, private IPs). Add MCPOAuthSecurity tests covering the allowedDomains bypass for initiateOAuthFlow, refreshOAuthTokens, and revokeOAuthToken. Update registry mocks to include getAllowedDomains. * fix: enforce protocol/port constraints in OAuth allowedDomains bypass Replace isHostnameAllowed (hostname-only check) with isOAuthUrlAllowed which parses the full OAuth URL and matches against allowedDomains entries including protocol and explicit port constraints — mirroring isDomainAllowedCore's allowlist logic. Prevents a port-scoped entry like 'https://auth.internal:8443' from also exempting other ports. * test: cover auto-discovery and branch-3 refresh paths with allowedDomains Add three new integration tests using a real OAuth test server: - auto-discovered OAuth endpoints allowed when server IP is in allowedDomains - auto-discovered endpoints rejected when allowedDomains doesn't match - refreshOAuthTokens branch 3 (no clientInfo/config) with allowedDomains bypass Also rename describe block from ephemeral issue number to durable name. * docs: explain intentional absence of allowedDomains in completeOAuthFlow Prevents future contributors from assuming a missing parameter during security audits — URLs are pre-validated during initiateOAuthFlow. * test: update initiateOAuthFlow assertion for allowedDomains parameter * perf: avoid redundant URL parse for admin-trusted OAuth endpoints Move isOAuthUrlAllowed check before the hostname extraction so admin-trusted URLs short-circuit with a single URL parse instead of two. The hostname extraction (new URL) is now deferred to the SSRF-check path where it's actually needed. |
||
|
Danny Avila
|
8e8fb01d18
|
🧱 fix: Enforce Agent Access Control on Context and OCR File Loading (#12253)
* 🔏 fix: Apply agent access control filtering to context/OCR resource loading
The context/OCR file path in primeResources fetched files by file_id
without applying filterFilesByAgentAccess, unlike the file_search and
execute_code paths. Add filterFiles dependency injection to primeResources
and invoke it after getFiles to enforce consistent access control.
* fix: Wire filterFilesByAgentAccess into all agent initialization callers
Pass the filterFilesByAgentAccess function from the JS layer into the TS
initializeAgent → primeResources chain via dependency injection, covering
primary, handoff, added-convo, and memory agent init paths.
* test: Add access control filtering tests for primeResources
Cover filterFiles invocation with context/OCR files, verify filtering
rejects inaccessible files, and confirm graceful fallback when filterFiles,
userId, or agentId are absent.
* fix: Guard filterFilesByAgentAccess against ephemeral agent IDs
Ephemeral agents have no DB document, so getAgent returns null and the
access map defaults to all-false, silently blocking all non-owned files.
Short-circuit with isEphemeralAgentId to preserve the pass-through
behavior for inline-built agents (memory, tool agents).
* fix: Clean up resources.ts and JS caller import order
Remove redundant optional chain on req.user.role inside user-guarded
block, update primeResources JSDoc with filterFiles and agentId params,
and reorder JS imports to longest-to-shortest per project conventions.
* test: Strengthen OCR assertion and add filterFiles error-path test
Use toHaveBeenCalledWith for the OCR filtering test to verify exact
arguments after the OCR→context merge step. Add test for filterFiles
rejection to verify graceful degradation (logs error, returns original
tool_resources).
* fix: Correct import order in addedConvo.js and initialize.js
Sort by total line length descending: loadAddedAgent (91) before
filterFilesByAgentAccess (84), loadAgentTools (91) before
filterFilesByAgentAccess (84).
* test: Add unit tests for filterFilesByAgentAccess and hasAccessToFilesViaAgent
Cover every branch in permissions.js: ephemeral agent guard, missing
userId/agentId/files early returns, all-owned short-circuit, mixed
owned + non-owned with VIEW/no-VIEW, agent-not-found fail-closed,
author path scoped to attached files, EDIT gate on delete, DB error
fail-closed, and agent with no tool_resources.
* test: Cover file.user undefined/null in permissions spec
Files with no user field fall into the non-owned path and get run
through hasAccessToFilesViaAgent. Add two cases: attached file with
no user field is returned, unattached file with no user field is
excluded.
|
||
|
Danny Avila
|
6f87b49df8
|
🛂 fix: Enforce Actions Capability Gate Across All Event-Driven Tool Loading Paths (#12252)
* fix: gate action tools by actions capability in all code paths Extract resolveAgentCapabilities helper to eliminate 3x-duplicated capability resolution. Apply early action-tool filtering in both loadToolDefinitionsWrapper and loadAgentTools non-definitions path. Gate loadActionToolsForExecution in loadToolsForExecution behind an actionsEnabled parameter with a cache-based fallback. Replace the late capability guard in loadAgentTools with a hasActionTools check to avoid unnecessary loadActionSets DB calls and duplicate warnings. * fix: thread actionsEnabled through InitializedAgent type Add actionsEnabled to the loadTools callback return type, InitializedAgent, and the initializeAgent destructuring/return so callers can forward the resolved value to loadToolsForExecution without redundant getEndpointsConfig cache lookups. * fix: pass actionsEnabled from callers to loadToolsForExecution Thread actionsEnabled through the agentToolContexts map in initialize.js (primary and handoff agents) and through primaryConfig in the openai.js and responses.js controllers, avoiding per-tool-call capability re-resolution on the hot path. * test: add regression tests for action capability gating Test the real exported functions (resolveAgentCapabilities, loadAgentTools, loadToolsForExecution) with mocked dependencies instead of shadow re-implementations. Covers definition filtering, execution gating, actionsEnabled param forwarding, and fallback capability resolution. * test: use Constants.EPHEMERAL_AGENT_ID in ephemeral fallback test Replaces a string guess with the canonical constant to avoid fragility if the ephemeral detection heuristic changes. * fix: populate agentToolContexts for addedConvo parallel agents After processAddedConvo returns, backfill agentToolContexts for any agents in agentConfigs not already present, so ON_TOOL_EXECUTE for added-convo agents receives actionsEnabled instead of falling back to a per-call cache lookup. |
||
|
Danny Avila
|
a26eeea592
|
🔏 fix: Enforce MCP Server Authorization on Agent Tool Persistence (#12250)
* 🛡️ fix: Validate MCP tool authorization on agent create/update Agent creation and update accepted arbitrary MCP tool strings without verifying the user has access to the referenced MCP servers. This allowed a user to embed unauthorized server names in tool identifiers (e.g. "anything_mcp_<victimServer>"), causing mcpServerNames to be stored on the agent and granting consumeOnly access via hasAccessViaAgent(). Adds filterAuthorizedTools() that checks MCP tool strings against the user's accessible server configs (via getAllServerConfigs) before persisting. Applied to create, update, and duplicate agent paths. * 🛡️ fix: Harden MCP tool authorization and add test coverage Addresses review findings on the MCP agent tool authorization fix: - Wrap getMCPServersRegistry() in try/catch so uninitialized registry gracefully filters all MCP tools instead of causing a 500 (DoS risk) - Guard revertAgentVersionHandler: filter unauthorized MCP tools after reverting to a previous version snapshot - Preserve existing MCP tools on collaborative updates: only validate newly added tools, preventing silent stripping of tools the editing user lacks direct access to - Add audit logging (logger.warn) when MCP tools are rejected - Refactor to single-pass lazy-fetch (registry queried only on first MCP tool encountered) - Export filterAuthorizedTools for direct unit testing - Add 18 tests covering: authorized/unauthorized/mixed tools, registry unavailable fallback, create/update/duplicate/revert handler paths, collaborative update preservation, and mcpServerNames persistence * test: Add duplicate handler test, use Constants.mcp_delimiter, DB assertions - N1: Add duplicateAgentHandler integration test verifying unauthorized MCP tools are stripped from the cloned agent and mcpServerNames are correctly persisted in the database - N2: Replace all hardcoded '_mcp_' delimiter literals with Constants.mcp_delimiter to prevent silent false-positive tests if the delimiter value ever changes - N3: Add DB state assertion to the revert-with-strip test confirming persisted tools match the response after unauthorized tools are removed * fix: Enforce exact 2-segment format for MCP tool keys Reject MCP tool keys with multiple delimiters to prevent authorization/execution mismatch when `.pop()` vs `split[1]` extract different server names from the same key. * fix: Preserve existing MCP tools when registry is unavailable When the MCP registry is uninitialized (e.g. server restart), existing tools already persisted on the agent are preserved instead of silently stripped. New MCP tools are still rejected when the registry cannot verify them. Applies to duplicate and revert handlers via existingTools param; update handler already preserves existing tools via its diff logic. |
||
Airam Hernández Hernández
|
aee1ced817
|
🪙 fix: Resolve Azure AD Group Overage via OBO Token Exchange for OpenID (#12187)
When Azure AD users belong to 200+ groups, group claims are moved out of the ID token (overage). The existing resolveGroupsFromOverage() called Microsoft Graph directly with the app-audience access token, which Graph rejected (401/403). Changes: - Add exchangeTokenForOverage() dedicated OBO exchange with User.Read scope - Update resolveGroupsFromOverage() to exchange token before Graph call - Add overage handling to OPENID_ADMIN_ROLE block (was silently failing) - Share resolved overage groups between required role and admin role checks - Always resolve via Graph when overage detected (even with partial groups) - Remove debug-only bypass that forced Graph resolution - Add tests for OBO exchange, caching, and admin role overage scenarios Co-authored-by: Airam Hernández Hernández <airam.hernandez@intelequia.com> |
||
|
Danny Avila
|
ad08df4db6
|
🔏 fix: Scope Agent-Author File Access to Attached Files Only (#12251)
* 🛡️ fix: Scope agent-author file access to attached files only
The hasAccessToFilesViaAgent helper short-circuited for agent authors,
granting access to all requested file IDs without verifying they were
attached to the agent's tool_resources. This enabled an IDOR where any
agent author could delete arbitrary files by supplying their agent_id
alongside unrelated file IDs.
Now both the author and non-author paths check file IDs against the
agent's tool_resources before granting access.
* chore: Use Object.values/for...of and add JSDoc in getAttachedFileIds
* test: Add boundary cases for agent file access authorization
- Agent with no tool_resources denies all access (fail-closed)
- Files across multiple resource types are all reachable
- Author + isDelete: true still scopes to attached files only
|
||
|
Danny Avila
|
f7ab5e645a
|
🫷 fix: Validate User-Provided Base URL in Endpoint Init (#12248)
* 🛡️ fix: Block SSRF via user-provided baseURL in endpoint initialization User-provided baseURL values (when endpoint is configured with `user_provided`) were passed through to the OpenAI SDK without validation. Combined with `directEndpoint`, this allowed arbitrary server-side requests to internal/metadata URLs. Adds `validateEndpointURL` that checks against known SSRF targets and DNS-resolves hostnames to block private IPs. Applied in both custom and OpenAI endpoint initialization paths. * 🧪 test: Add validateEndpointURL SSRF tests Covers unparseable URLs, localhost, private IPs, link-local/metadata, internal Docker/K8s hostnames, DNS resolution to private IPs, and legitimate public URLs. * 🛡️ fix: Add protocol enforcement and import order fix - Reject non-HTTP/HTTPS schemes (ftp://, file://, data:, etc.) in validateEndpointURL before SSRF hostname checks - Document DNS rebinding limitation and fail-open semantics in JSDoc - Fix import order in custom/initialize.ts per project conventions * 🧪 test: Expand SSRF validation coverage and add initializer integration tests Unit tests for validateEndpointURL: - Non-HTTP/HTTPS schemes (ftp, file, data) - IPv6 loopback, link-local, and unique-local addresses - .local and .internal TLD hostnames - DNS fail-open path (lookup failure allows request) Integration tests for initializeCustom and initializeOpenAI: - Guard fires when userProvidesURL is true - Guard skipped when URL is system-defined or falsy - SSRF rejection propagates and prevents getOpenAIConfig call * 🐛 fix: Correct broken env restore in OpenAI initialize spec process.env was captured by reference, not by value, making the restore closure a no-op. Snapshot individual env keys before mutation so they can be properly restored after each test. * 🛡️ fix: Throw structured ErrorTypes for SSRF base URL validation Replace plain-string Error throws in validateEndpointURL with JSON-structured errors using type 'invalid_base_url' (matching new ErrorTypes.INVALID_BASE_URL enum value). This ensures the client-side Error component can look up a localized message instead of falling through to the raw-text default. Changes across workspaces: - data-provider: add INVALID_BASE_URL to ErrorTypes enum - packages/api: throwInvalidBaseURL helper emits structured JSON - client: add errorMessages entry and localization key - tests: add structured JSON format assertion * 🧹 refactor: Use ErrorTypes enum key in Error.tsx for consistency Replace bare string literal 'invalid_base_url' with computed property [ErrorTypes.INVALID_BASE_URL] to match every other entry in the errorMessages map. |
||
|
Danny Avila
|
f9927f0168
|
📑 fix: Sanitize Markdown Artifacts (#12249)
* 🛡️ fix: Sanitize markdown artifact rendering to prevent stored XSS
Replace marked-react with react-markdown + remark-gfm for artifact
markdown preview. react-markdown's skipHtml strips raw HTML tags,
and a urlTransform guard blocks javascript: and data: protocol links.
* fix: Update useArtifactProps test to expect react-markdown dependencies
* fix: Harden markdown artifact sanitization
- Convert isSafeUrl from denylist to allowlist (http, https, mailto, tel
plus relative/anchor URLs); unknown protocols are now fail-closed
- Add remark-breaks to restore single-newline-to-<br> behavior that was
silently dropped when replacing marked-react
- Export isSafeUrl from the host module and add 16 direct unit tests
covering allowed protocols, blocked schemes (javascript, data, blob,
vbscript, file, custom), edge cases (empty, whitespace, mixed case)
- Hoist remarkPlugins to a module-level constant to avoid per-render
array allocation in the generated Sandpack component
- Fix import order in generated template (shortest to longest per
AGENTS.md) and remove pre-existing trailing whitespace
* fix: Return null for blocked URLs, add sync-guard comments and test
- urlTransform returns null (not '') for blocked URLs so react-markdown
omits the href/src attribute entirely instead of producing <a href="">
- Hoist urlTransform to module-level constant alongside remarkPlugins
- Add JSDoc sync-guard comments tying the exported isSafeUrl to its
template-string mirror, so future maintainers know to update both
- Add synchronization test asserting the embedded isSafeUrl contains the
same allowlist set, URL parsing, and relative-path checks as the export
|
||
|
Danny Avila
|
bcf45519bd
|
🪪 fix: Enforce VIEW ACL on Agent Edge References at Write and Runtime (#12246)
* 🛡️ fix: Enforce ACL checks on handoff edge and added-convo agent loading Edge-linked agents and added-convo agents were fetched by ID via getAgent without verifying the requesting user's access permissions. This allowed an authenticated user to reference another user's private agent in edges or addedConvo and have it initialized at runtime. Add checkPermission(VIEW) gate in processAgent before initializing any handoff agent, and in processAddedConvo for non-ephemeral added agents. Unauthorized agents are logged and added to skippedAgentIds so orphaned-edge filtering removes them cleanly. * 🛡️ fix: Validate edge agent access at agent create/update time Reject agent create/update requests that reference agents in edges the requesting user cannot VIEW. This provides early feedback and prevents storing unauthorized agent references as defense-in-depth alongside the runtime ACL gate in processAgent. Add collectEdgeAgentIds utility to extract all unique agent IDs from an edge array, and validateEdgeAgentAccess helper in the v1 handler. * 🧪 test: Improve ACL gate test coverage and correctness - Add processAgent ACL gate tests for initializeClient (skip/allow handoff agents) - Fix addedConvo.spec.js to mock loadAddedAgent directly instead of getAgent - Seed permMap with ownedAgent VIEW bits in v1.spec.js update-403 test * 🧹 chore: Remove redundant addedConvo ACL gate (now in middleware) PR #12243 moved the addedConvo agent ACL check upstream into canAccessAgentFromBody middleware, making the runtime check in processAddedConvo and its spec redundant. * 🧪 test: Rewrite processAgent ACL test with real DB and minimal mocking Replace heavy mock-based test (12 mocks, Providers.XAI crash) with MongoMemoryServer-backed integration test that exercises real getAgent, checkPermission, and AclEntry — only external I/O (initializeAgent, ToolService, AgentClient) remains mocked. Load edge utilities directly from packages/api/src/agents/edges to sidestep the config.ts barrel. * 🧪 fix: Use requireActual spread for @librechat/agents and @librechat/api mocks The Providers.XAI crash was caused by mocking @librechat/agents with a minimal replacement object, breaking the @librechat/api initialization chain. Match the established pattern from client.test.js and recordCollectedUsage.spec.js: spread jest.requireActual for both packages, overriding only the functions under test. |
||
|
Danny Avila
|
1312cd757c
|
🛡️ fix: Validate User-provided URLs for Web Search (#12247)
* 🛡️ fix: SSRF-validate user-provided URLs in web search auth User-controlled URL fields (jinaApiUrl, firecrawlApiUrl, searxngInstanceUrl) flow from plugin auth into outbound HTTP requests without validation. Reuse existing isSSRFTarget/resolveHostnameSSRF to block private/internal targets while preserving admin-configured (env var) internal URLs. * 🛡️ fix: Harden web search SSRF validation - Reject non-HTTP(S) schemes (file://, ftp://, etc.) in isSSRFUrl - Conditional write: only assign to authResult after SSRF check passes - Move isUserProvided tracking after SSRF gate to avoid false positives - Add authenticated assertions for optional-field SSRF blocks in tests - Add file:// scheme rejection test - Wrap process.env mutation in try/finally guard - Add JSDoc + sync-obligation comment on WEB_SEARCH_URL_KEYS * 🛡️ fix: Correct auth-type reporting for SSRF-stripped optional URLs SSRF-stripped optional URL fields no longer pollute isUserProvided. Track whether the field actually contributed to authResult before crediting it as user-provided, so categories report SYSTEM_DEFINED when all surviving values match env vars. |
||
|
Danny Avila
|
8dc6d60750
|
🛡️ fix: Enforce MULTI_CONVO and agent ACL checks on addedConvo (#12243)
* 🛡️ fix: Enforce MULTI_CONVO and agent ACL checks on addedConvo
addedConvo.agent_id was passed through to loadAddedAgent without any
permission check, enabling an authenticated user to load and execute
another user's private agent via the parallel multi-convo feature.
The middleware now chains a checkAddedConvoAccess gate after the primary
agent check: when req.body.addedConvo is present it verifies the user
has MULTI_CONVO:USE role permission, and when the addedConvo agent_id is
a real (non-ephemeral) agent it runs the same canAccessResource ACL
check used for the primary agent.
* refactor: Harden addedConvo middleware and avoid duplicate agent fetch
- Convert checkAddedConvoAccess to curried factory matching Express
middleware signature: (requiredPermission) => (req, res, next)
- Call checkPermission directly for the addedConvo agent instead of
routing through canAccessResource's tempReq pattern; this avoids
orphaning the resolved agent document and enables caching it on
req.resolvedAddedAgent for downstream loadAddedAgent
- Update loadAddedAgent to use req.resolvedAddedAgent when available,
eliminating a duplicate getAgent DB call per chat request
- Validate addedConvo is a plain object and agent_id is a string
before passing to isEphemeralAgentId (prevents TypeError on object
injection, returns 400-equivalent early exit instead of 500)
- Fix JSDoc: "VIEW access" → "same permission as primary agent",
add @param/@returns to helpers, restore @example on factory
- Fix redundant return await in resolveAgentIdFromBody
* test: Add canAccessAgentFromBody spec covering IDOR fix
26 integration tests using MongoMemoryServer with real models, ACL
entries, and PermissionService — no mocks for core logic.
Covered paths:
- Factory validation (requiredPermission type check)
- Primary agent: missing agent_id, ephemeral, non-agents endpoint
- addedConvo absent / invalid shape (string, array, object injection)
- MULTI_CONVO:USE gate: denied, missing role, ADMIN bypass
- Agent resource ACL: no ACL → 403, insufficient bits → 403,
nonexistent agent → 404, valid ACL → next + cached on req
- End-to-end: both real agents, primary denied short-circuits,
ephemeral primary + real addedConvo
|
||
|
Danny Avila
|
07d0ce4ce9
|
🪤 fix: Fail-Closed MCP Domain Validation for Unparseable URLs (#12245)
* 🛡️ fix: Fail-closed MCP domain validation for unparseable URLs
`isMCPDomainAllowed` returned true (allow) when `extractMCPServerDomain`
could not parse the URL, treating it identically to a stdio transport.
A URL containing template placeholders or invalid syntax bypassed the
domain allowlist, then `processMCPEnv` resolved it to a valid—and
potentially disallowed—host at connection time.
Distinguish "no URL" (stdio, allowed) from "has URL but unparseable"
(rejected when an allowlist is active) by checking whether `config.url`
is an explicit non-empty string before falling through to the stdio path.
When no allowlist is configured the guard does not fire—unparseable URLs
fall through to connection-level SSRF protection via
`createSSRFSafeUndiciConnect`, preserving legitimate `customUserVars`
template-URL configs.
* test: Expand MCP domain validation coverage for invalid/templated URLs
Cover all branches of the fail-closed guard:
- Invalid/templated URLs rejected when allowlist is configured
- Invalid/templated URLs allowed when no allowlist (null/undefined/[])
- Whitespace-only and empty-string URLs treated as absent across all
allowedDomains configurations
- Stdio configs (no url property) remain allowed
|
||
|
Danny Avila
|
a0b4949a05
|
🛡️ fix: Cover full fe80::/10 link-local range in IPv6 check (#12244)
* 🛡️ fix: Cover full fe80::/10 link-local range in SSRF IPv6 check The `isPrivateIP` check used `startsWith('fe80')` which only matched fe80:: but missed fe90::–febf:: (the rest of the RFC 4291 fe80::/10 link-local block). Replace with a proper bitwise hextet check. * 🛡️ fix: Guard isIPv6LinkLocal against parseInt partial-parse on hostnames parseInt('fe90.example.com', 16) stops at the dot and returns 0xfe90, which passes the bitmask check and false-positives legitimate domains. Add colon-presence guard (IPv6 literals always contain ':') and a hex regex validation on the first hextet before parseInt. Also document why fc/fd use startsWith while fe80::/10 needs bitwise. * ✅ test: Harden IPv6 link-local SSRF tests with false-positive guards - Assert fe90/fea0/febf hostnames are NOT blocked (regression guard) - Add feb0::1 and bracket form [fe90::1] to isPrivateIP coverage - Extend resolveHostnameSSRF tests for fe90::1 and febf::1 |
||
|
Danny Avila
|
a01959b3d2
|
🛰️ fix: Cross-Replica Created Event Delivery (#12231)
* fix: emit created event from metadata on cross-replica subscribe In multi-instance Redis deployments, the created event (which triggers sidebar conversation creation) was lost when the SSE subscriber connected to a different instance than the one generating. The event was only in the generating instance's local earlyEventBuffer and the Redis pub/sub message was already gone by the time the subscriber's channel was active. When subscribing cross-replica (empty buffer, Redis mode, userMessage already in job metadata), reconstruct and emit the created event directly from stored metadata. * test: add skipBufferReplay regression guard for cross-replica created event Add test asserting the resume path (skipBufferReplay: true) does NOT emit a created event on cross-replica subscribe — prevents the duplication fix from PR #12225 from regressing. Add explanatory JSDoc on the cross-replica fallback branch documenting which fields are preserved from trackUserMessage() and why sender/isCreatedByUser are hardcoded. * refactor: replace as-unknown-as casts with discriminated ServerSentEvent union Split ServerSentEvent into StreamEvent | CreatedEvent | FinalEvent so event shapes are statically typed. Removes all as-unknown-as casts in GenerationJobManager and test file; narrows with proper union members where properties are accessed. * fix: await trackUserMessage before PUBLISH for structural ordering trackUserMessage was fire-and-forget — the HSET for userMessage could theoretically race with the PUBLISH. Await it so the write commits before the pub/sub fires, guaranteeing any cross-replica getJob() after the pub/sub window always finds userMessage in Redis. No-op for non-created events (early return before any async work). * refactor: type CreatedEvent.message explicitly, fix JSDoc and import Give CreatedEvent.message its full known shape instead of Record<string, unknown>. Update sendEvent JSDoc to reflect the discriminated union. Use barrel import in test file. * refactor: type FinalEvent fields with explicit message and conversation shapes Replace Record<string, unknown> on requestMessage, responseMessage, conversation, and runMessages with FinalMessageFields and a typed conversation shape. Captures the known field set used by all final event constructors (abort handler in GenerationJobManager and normal completion in request.js) while allowing extension via index signature for fields contributed by the full TMessage/TConversation schemas. * refactor: narrow trackUserMessage with discriminated union, disambiguate error fields Use 'created' in event to narrow ServerSentEvent to CreatedEvent, eliminating all Record<string, unknown> casts and manual field assertions. Add JSDoc to the two distinct error fields on FinalMessageFields and FinalEvent to prevent confusion. * fix: update cross-replica test to expect created event from metadata The cross-replica subscribe fallback now correctly emits a created event reconstructed from persisted metadata when userMessage exists in the Redis job hash. Replica B receives 4 events (created + 3 deltas) instead of 3. |
||
|
Danny Avila
|
e079fc4900
|
📎 fix: Enforce File Count and Size Limits Across All Attachment Paths (#12239)
* 🐛 fix: Enforce fileLimit and totalSizeLimit in Attached Files panel The Files side panel (PanelTable) was not checking fileLimit or totalSizeLimit from fileConfig when attaching previously uploaded files, allowing users to bypass per-endpoint file count and total size limits. * 🔧 fix: Address review findings on file limit enforcement - Fix totalSizeLimit double-counting size of already-attached files - Clarify fileLimit error message: "File limit reached: N files (endpoint)" - Replace Array.from(...).reduce with for...of loop to avoid intermediate allocation - Extract inline `type TFile` into standalone `import type` per project conventions * ✅ test: Add PanelTable handleFileClick file limit tests Cover fileLimit guard, totalSizeLimit guard, passing case, double-count prevention for re-attached files, and boundary case. * 🔧 test: Harden PanelTable test mock setup - Use explicit endpoint key matching mockConversation.endpoint instead of relying on default fallback behavior - Add supportedMimeTypes to mock config for explicit MIME coverage - Throw on missing filename cell in clickFilenameCell to prevent silent false-positive blocking assertions * ♻️ refactor: Align file validation ordering and messaging across upload paths - Reorder handleFileClick checks to match validateFiles: disabled → fileLimit → fileSizeLimit → checkType → totalSizeLimit - Change fileSizeLimit comparison from > to >= in handleFileClick to match validateFiles behavior - Align validateFiles error strings with localized key wording: "File limit reached:", "File size limit exceeded:", etc. - Remove stray console.log in validateFiles MIME-type check * ✅ test: Add validateFiles unit tests for both paths' consistency 13 tests covering disabled, empty, fileLimit (reject + boundary), fileSizeLimit (>= at limit + under limit), checkType, totalSizeLimit (reject + at limit), duplicate detection, and check ordering. Ensures both validateFiles and handleFileClick enforce the same validation rules in the same order. |
||
|
Danny Avila
|
93a628d7a2
|
📎 fix: Respect fileConfig.disabled for Agents Endpoint Upload Button (#12238)
* fix: respect fileConfig.disabled for agents endpoint upload button The isAgents check was OR'd without the !isUploadDisabled guard, bypassing the fileConfig.endpoints.agents.disabled setting and always rendering the attach file menu for agents. * test: add regression tests for fileConfig.disabled upload guard Cover the isUploadDisabled rendering gate for agents and assistants endpoints, preventing silent reintroduction of the bypass bug. * test: cover disabled fallback chain in useAgentFileConfig Verify agents-disabled propagates when no provider is set, when provider has no specific config (agents as fallback), and that provider-specific enabled overrides agents disabled. |
||
|
Danny Avila
|
0c27ad2d55
|
🛡️ refactor: Scope Action Mutations by Parent Resource Ownership (#12237)
* 🛡️ fix: Scope action mutations by parent resource ownership Prevent cross-tenant action overwrites by validating that an existing action's agent_id/assistant_id matches the URL parameter before allowing updates or deletes. Without this, a user with EDIT access on their own agent could reference a foreign action_id to hijack another agent's action record. * 🛡️ fix: Harden action ownership checks and scope write filters - Remove && short-circuit that bypassed the guard when agent_id or assistant_id was falsy (e.g. assistant-owned actions have no agent_id, so the check was skipped entirely on the agents route). - Include agent_id / assistant_id in the updateAction and deleteAction query filters so the DB write itself enforces ownership atomically. - Log a warning when deleteAction returns null (silent no-op from data-integrity mismatch). * 📝 docs: Update Action model JSDoc to reflect scoped query params * ✅ test: Add Action ownership scoping tests Cover update, delete, and cross-type protection scenarios using MongoMemoryServer to verify that scoped query filters (agent_id, assistant_id) prevent cross-tenant overwrites and deletions at the database level. * 🛡️ fix: Scope updateAction filter in agent duplication handler * 🐛 fix: Use action metadata domain instead of action_id when duplicating agent actions The duplicate handler was splitting `action.action_id` by `actionDelimiter` to extract the domain, but `action_id` is a bare nanoid that doesn't contain the delimiter. This produced malformed entries in the duplicated agent's actions array (nanoid_action_newNanoid instead of domain_action_newNanoid). The domain is available on `action.metadata.domain`. * ✅ test: Add integration tests for agent duplication action handling Uses MongoMemoryServer with real Agent and Action models to verify: - Duplicated actions use metadata.domain (not action_id) for the agent actions array entries - Sensitive metadata fields are stripped from duplicated actions - Original action documents are not modified |
||
|
Danny Avila
|
7c39a45944
|
🐍 refactor: Normalize Non-Standard Browser MIME Type Aliases in inferMimeType (#12240)
* 🐛 fix: Normalize non-standard browser MIME types in inferMimeType macOS Chrome/Firefox report .py files as text/x-python-script instead of text/x-python, causing client-side validation to reject Python file uploads. inferMimeType now normalizes known MIME type aliases before returning, so non-standard variants match the accepted regex patterns. * 🧪 test: Add tests for MIME type alias normalization in inferMimeType * 🐛 fix: Restore JSDoc params and make mimeTypeAliases immutable * 🧪 test: Add checkType integration tests, remove redundant DragDropModal tests |
||
|
Danny Avila
|
8318446704
|
💁 refactor: Better Config UX for MCP STDIO with customUserVars (#12226)
* refactor: Better UX for MCP stdio with Custom User Variables
- Updated the ConnectionsRepository to prevent connections when customUserVars are defined, improving security and access control.
- Modified the MCPServerInspector to skip capabilities fetch when customUserVars are present, streamlining server inspection.
- Added tests to validate connection restrictions with customUserVars, ensuring robust handling of various server configurations.
This change enhances the overall integrity of the connection management process by enforcing stricter rules around custom user variables.
* fix: guard against empty customUserVars and add JSDoc context
- Extract `hasCustomUserVars()` helper to guard against truthy `{}`
(Zod's `.record().optional()` yields `{}` on empty input, not `undefined`)
- Add JSDoc to `isAllowedToConnectToServer` explaining why customUserVars
servers are excluded from app-level connections
* test: improve customUserVars test coverage and fixture hygiene
- Add no-connection-provided test for MCPServerInspector (production path)
- Fix test descriptions to match actual fixture values
- Replace real package name with fictional @test/mcp-stdio-server
|
||
|
Danny Avila
|
7bc793b18d
|
🌊 fix: Prevent Buffered Event Duplication on SSE Resume Connections (#12225)
* fix: skipBufferReplay for job resume connections - Introduced a new option `skipBufferReplay` in the `subscribe` method of `GenerationJobManagerClass` to prevent duplication of events when resuming a connection. - Updated the logic to conditionally skip replaying buffered events if a sync event has already been sent, enhancing the efficiency of event handling during reconnections. - Added integration tests to verify the correct behavior of the new option, ensuring that no buffered events are replayed when `skipBufferReplay` is true, while still allowing for normal replay behavior when false. * refactor: Update GenerationJobManager to handle sync events more efficiently - Modified the `subscribe` method to utilize a new `skipBufferReplay` option, allowing for the prevention of duplicate events during resume connections. - Enhanced the logic in the `chat/stream` route to conditionally skip replaying buffered events if a sync event has already been sent, improving event handling efficiency. - Updated integration tests to verify the correct behavior of the new option, ensuring that no buffered events are replayed when `skipBufferReplay` is true, while maintaining normal replay behavior when false. * test: Enhance GenerationJobManager integration tests for Redis mode - Updated integration tests to conditionally run based on the USE_REDIS environment variable, allowing for better control over Redis-related tests. - Refactored test descriptions to utilize a dynamic `describeRedis` function, improving clarity and organization of tests related to Redis functionality. - Removed redundant checks for Redis availability within individual tests, streamlining the test logic and enhancing readability. * fix: sync handler state for new messages on resume The sync event's else branch (new response message) was missing resetContentHandler() and syncStepMessage() calls, leaving stale handler state that caused subsequent deltas to build on partial content instead of the synced aggregatedContent. * feat: atomic subscribeWithResume to close resume event gap Replaces separate getResumeState() + subscribe() calls with a single subscribeWithResume() that atomically drains earlyEventBuffer between the resume snapshot and the subscribe. In in-memory mode, drained events are returned as pendingEvents for the client to replay after sync. In Redis mode, pendingEvents is empty since chunks are already persisted. The route handler now uses the atomic method for resume connections and extracted shared SSE write helpers to reduce duplication. The client replays any pendingEvents through the existing step/content handlers after applying aggregatedContent from the sync payload. * fix: only capture gap events in subscribeWithResume, not pre-snapshot buffer The previous implementation drained the entire earlyEventBuffer into pendingEvents, but pre-snapshot events are already reflected in aggregatedContent. Replaying them re-introduced the duplication bug through a different vector. Now records buffer length before getResumeState() and slices from that index, so only events arriving during the async gap are returned as pendingEvents. Also: - Handle pendingEvents when resumeState is null (replay directly) - Hoist duplicate test helpers to shared scope - Remove redundant writableEnded guard in onDone |
||
|
Danny Avila
|
cbdc6f6060
|
📦 chore: Bump NPM Audit Packages (#12227)
* 🔧 chore: Update file-type dependency to version 21.3.2 in package-lock.json and package.json - Upgraded the "file-type" package from version 18.7.0 to 21.3.2 to ensure compatibility with the latest features and security updates. - Added new dependencies related to the updated "file-type" package, enhancing functionality and performance. * 🔧 chore: Upgrade undici dependency to version 7.24.1 in package-lock.json and package.json - Updated the "undici" package from version 7.18.2 to 7.24.1 across multiple package files to ensure compatibility with the latest features and security updates. * 🔧 chore: Upgrade yauzl dependency to version 3.2.1 in package-lock.json - Updated the "yauzl" package from version 3.2.0 to 3.2.1 to incorporate the latest features and security updates. * 🔧 chore: Upgrade hono dependency to version 4.12.7 in package-lock.json - Updated the "hono" package from version 4.12.5 to 4.12.7 to incorporate the latest features and security updates. |
||
|
Danny Avila
|
f67bbb2bc5
|
🧹 fix: Sanitize Artifact Filenames in Code Execution Output (#12222)
* fix: sanitize artifact filenames to prevent path traversal in code output * test: Mock sanitizeFilename function in process.spec.js to return the original filename - Added a mock implementation for the `sanitizeFilename` function in the `process.spec.js` test file to return the original filename, ensuring that tests can run without altering the filename during the testing process. * fix: use path.relative for traversal check, sanitize all filenames, add security logging - Replace startsWith with path.relative pattern in saveLocalBuffer, consistent with deleteLocalFile and getLocalFileStream in the same file - Hoist sanitizeFilename call before the image/non-image branch so both code paths store the sanitized name in MongoDB - Log a warning when sanitizeFilename mutates a filename (potential traversal) - Log a specific warning when saveLocalBuffer throws a traversal error, so security events are distinguishable from generic network errors in the catch * test: improve traversal test coverage and remove mock reimplementation - Remove partial sanitizeFilename reimplementation from process-traversal tests; use controlled mock returns to verify processCodeOutput wiring instead - Add test for image branch sanitization - Use mkdtempSync for test isolation in crud-traversal to avoid parallel worker collisions - Add prefix-collision bypass test case (../user10/evil vs user1 directory) * fix: use path.relative in isValidPath to prevent prefix-collision bypass Pre-existing startsWith check without path separator had the same class of prefix-collision vulnerability fixed in saveLocalBuffer. |
||
|
Danny Avila
|
35a35dc2e9
|
📏 refactor: Add File Size Limits to Conversation Imports (#12221)
* fix: add file size limits to conversation import multer instance * fix: address review findings for conversation import file size limits * fix: use local jest.mock for data-schemas instead of global moduleNameMapper The global @librechat/data-schemas mock in jest.config.js only provided logger, breaking all tests that depend on createModels from the same package. Replace with a virtual jest.mock scoped to the import spec file. * fix: move import to top of file, pre-compute upload middleware, assert logger.warn in tests * refactor: move resolveImportMaxFileSize to packages/api New backend logic belongs in packages/api as TypeScript. Delete the api/server/utils/import/limits.js wrapper and import directly from @librechat/api in convos.js and importConversations.js. Resolver unit tests move to packages/api; the api/ spec retains only multer behavior tests. * chore: rename importLimits to import * fix: stale type reference and mock isolation in import tests Update typeof import path from '../importLimits' to '../import' after the rename. Clear mockLogger.warn in beforeEach to prevent cross-test accumulation. * fix: add resolveImportMaxFileSize to @librechat/api mock in convos.spec.js * fix: resolve jest.mock hoisting issue in import tests jest.mock factories are hoisted above const declarations, so the mockLogger reference was undefined at factory evaluation time. Use a direct import of the mocked logger module instead. * fix: remove virtual flag from data-schemas mock for CI compatibility virtual: true prevents the mock from intercepting the real module in CI where @librechat/data-schemas is built, causing import.ts to use the real logger while the test asserts against the mock. |
||
|
Danny Avila
|
c6982dc180
|
🛡️ fix: Agent Permission Check on Image Upload Route (#12219)
* fix: add agent permission check to image upload route
* refactor: remove unused SystemRoles import and format test file for clarity
* fix: address review findings for image upload agent permission check
* refactor: move agent upload auth logic to TypeScript in packages/api
Extract pure authorization logic from agentPermCheck.js into
checkAgentUploadAuth() in packages/api/src/files/agentUploadAuth.ts.
The function returns a structured result ({ allowed, status, error })
instead of writing HTTP responses directly, eliminating the dual
responsibility and confusing sentinel return value. The JS wrapper
in /api is now a thin adapter that translates the result to HTTP.
* test: rewrite image upload permission tests as integration tests
Replace mock-heavy images-agent-perm.spec.js with integration tests
using MongoMemoryServer, real models, and real PermissionService.
Follows the established pattern in files.agents.test.js. Moves test
to sibling location (images.agents.test.js) matching backend convention.
Adds temp file cleanup assertions on 403/404 responses and covers
message_file exemption paths (boolean true, string "true", false).
* fix: widen AgentUploadAuthDeps types to accept ObjectId from Mongoose
The injected getAgent returns Mongoose documents where _id and author
are Types.ObjectId at runtime, not string. Widen the DI interface to
accept string | Types.ObjectId for _id, author, and resourceId so the
contract accurately reflects real callers.
* chore: move agent upload auth into files/agents/ subdirectory
* refactor: delete agentPermCheck.js wrapper, move verifyAgentUploadPermission to packages/api
The /api-only dependencies (getAgent, checkPermission) are now passed
as object-field params from the route call sites. Both images.js and
files.js import verifyAgentUploadPermission from @librechat/api and
inject the deps directly, eliminating the intermediate JS wrapper.
* style: fix import type ordering in agent upload auth
* fix: prevent token TTL race in MCPTokenStorage.storeTokens
When expires_in is provided, use it directly instead of round-tripping
through Date arithmetic. The previous code computed accessTokenExpiry
as a Date, then after an async encryptV2 call, recomputed expiresIn by
subtracting Date.now(). On loaded CI runners the elapsed time caused
Math.floor to truncate to 0, triggering the 1-year fallback and making
the token appear permanently valid — so refresh never fired.
|
||
|
Danny Avila
|
71a3b48504
|
🔑 fix: Require OTP Verification for 2FA Re-Enrollment and Backup Code Regeneration (#12223)
* fix: require OTP verification for 2FA re-enrollment and backup code regeneration * fix: require OTP verification for account deletion when 2FA is enabled * refactor: Improve code formatting and readability in TwoFactorController and UserController - Reformatted code in TwoFactorController and UserController for better readability by aligning parameters and breaking long lines. - Updated test cases in deleteUser.spec.js and TwoFactorController.spec.js to enhance clarity by formatting object parameters consistently. * refactor: Consolidate OTP and backup code verification logic in TwoFactorController and UserController - Introduced a new `verifyOTPOrBackupCode` function to streamline the verification process for TOTP tokens and backup codes across multiple controllers. - Updated the `enable2FA`, `disable2FA`, and `deleteUserController` methods to utilize the new verification function, enhancing code reusability and readability. - Adjusted related tests to reflect the changes in verification logic, ensuring consistent behavior across different scenarios. - Improved error handling and response messages for verification failures, providing clearer feedback to users. * chore: linting * refactor: Update BackupCodesItem component to enhance OTP verification logic - Consolidated OTP input handling by moving the 2FA verification UI logic to a more consistent location within the component. - Improved the state management for OTP readiness, ensuring the regenerate button is only enabled when the OTP is ready. - Cleaned up imports by removing redundant type imports, enhancing code clarity and maintainability. * chore: lint * fix: stage 2FA re-enrollment in pending fields to prevent disarmament window enable2FA now writes to pendingTotpSecret/pendingBackupCodes instead of overwriting the live fields. confirm2FA performs the atomic swap only after the new TOTP code is verified. If the user abandons mid-flow, their existing 2FA remains active and intact. |
||
|
Danny Avila
|
189cdf581d
|
🔐 fix: Add User Filter to Message Deletion (#12220)
* fix: add user filter to message deletion to prevent IDOR * refactor: streamline DELETE request syntax in messages-delete test - Simplified the DELETE request syntax in the messages-delete.spec.js test file by combining multiple lines into a single line for improved readability. This change enhances the clarity of the test code without altering its functionality. * fix: address review findings for message deletion IDOR fix * fix: add user filter to message deletion in conversation tests - Included a user filter in the message deletion test to ensure proper handling of user-specific deletions, enhancing the accuracy of the test case and preventing potential IDOR vulnerabilities. * chore: lint |
||
|
Danny Avila
|
ca79a03135
|
🚦 fix: Add Rate Limiting to Conversation Duplicate Endpoint (#12218)
* fix: add rate limiting to conversation duplicate endpoint * chore: linter * fix: address review findings for conversation duplicate rate limiting * refactor: streamline test mocks for conversation routes - Consolidated mock implementations into a dedicated `convos-route-mocks.js` file to enhance maintainability and readability of test files. - Updated tests in `convos-duplicate-ratelimit.spec.js` and `convos.spec.js` to utilize the new mock structure, improving clarity and reducing redundancy. - Enhanced the `duplicateConversation` function to accept an optional title parameter for better flexibility in conversation duplication. * chore: rename files |