Commit graph

20 commits

Author SHA1 Message Date
Dustin Healy
e55db4bfb4 fix: defensive rollback in removeRoleMember, type/style cleanup, test coverage
- Wrap removeRoleMember post-write admin rollback in try/catch so a
  transient DB failure cannot leave the system with zero administrators
- Replace double `as unknown[] as IRole[]` cast with `.lean<IRole[]>()`
- Type parsePagination param explicitly; extract DEFAULT/MAX page constants
- Preserve original error cause in updateRoleByName re-throw
- Add test for rollback failure path in removeRoleMember (returns 400)
- Add test for pre-existing roles missing description field (.lean())
2026-03-26 17:55:58 -07:00
Dustin Healy
ad47919ecd fix: address review findings — race safety, validation DRY, type accuracy, test coverage
- Add post-write admin count verification in removeRoleMember to prevent
  zero-admin race condition (TOCTOU → rollback if count hits 0)
- Make IRole.description optional; backfill in initializeRoles for
  pre-existing roles that lack the field (.lean() bypasses defaults)
- Extract parsePagination, validateNameParam, validateRoleName, and
  validateDescription helpers to eliminate duplicated validation
- Add validateNameParam guard to all 7 handlers reading req.params.name
- Catch 11000 in updateRoleByName and surface as 409 via RoleConflictError
- Add idempotent skip in addRoleMember when user already has target role
- Verify updateRolePermissions test asserts response body
- Add data-layer tests: listRoles sort/pagination/projection, countRoles,
  and createRoleByName 11000 duplicate key race
2026-03-26 17:45:16 -07:00
Dustin Healy
2506644d58 fix: paginate listRoles, null-guard permissions handler, fix export ordering
- Add limit/offset/total pagination to listRoles matching the groups pattern
- Add countRoles data-layer method
- Omit permissions from listRoles select (getRole returns full document)
- Null-guard re-fetched role in updateRolePermissionsHandler
- Move interleaved export below all imports in methods/index.ts
2026-03-26 17:24:01 -07:00
Dustin Healy
e9572854ef feat: prevent removing the last admin user
Add guard in removeRoleMember that checks countUsersByRole before
demoting an ADMIN user, returning 400 if they are the last one.
2026-03-26 17:05:43 -07:00
Dustin Healy
b9e0fa48c6 fix: validate permissions in create, RoleConflictError, rollback safety, cache consistency
- Add permissions type/array validation in createRoleHandler
- Introduce RoleConflictError class replacing fragile string-prefix matching
- Wrap rollback in !role null path with try/catch for correct 404 response
- Wrap deleteRoleByName cache.set in try/catch matching createRoleByName
- Narrow updateRoleHandler body type to { name?, description? }
- Add tests: non-string description in create, rollback failure logging,
  permissions array rejection, description max-length assertion fix
2026-03-26 17:05:01 -07:00
Dustin Healy
16678c0ece fix: guard spurious rollback, harden createRole error path, validate before DB calls
- Add migrationRan flag to prevent rollback of user migration that never ran
- Return generic message on 500 in createRoleHandler, specific only for 409
- Move description validation before DB queries in updateRoleHandler
- Return existing role early when update body has no changes
- Wrap cache.set in createRoleByName with try/catch to prevent masking DB success
- Add JSDoc on 11000 catch explaining compound unique index
- Add tests: spurious rollback guard, empty update body, description validation
  ordering, listUsersByRole pagination
2026-03-26 16:38:11 -07:00
Dustin Healy
ce526ed51a fix: rollback on rename throw, description validation, delete/DRY cleanup
- Hoist isRename/trimmedName above try block so catch can roll back user
  migration when updateRoleByName throws (not just returns null)
- Add description type + max-length (2000) validation in create and update,
  consistent with groups handler
- Remove redundant getRoleByName existence check in deleteRoleHandler —
  use deleteRoleByName return value directly
- Skip no-op name write when body.name equals current name (use isRename)
- Extract getUserModel() accessor to DRY repeated Model<IUser> casts
- Use name.trim() consistently in createRoleByName error messages
- Add tests: rename-throw rollback, description validation (create+update),
  update delete test mocks to match simplified handler
2026-03-26 16:25:14 -07:00
Dustin Healy
16bb113614 fix: add rollback on rename failure and update PR description
- Roll back user migration if updateRoleByName returns null during a
  rename (race: role deleted between existence check and update)
- Add test verifying rollback calls updateUsersByRole in reverse
- Update PR #12400 description to reflect current test counts (56
  handler tests, 40 data-layer tests) and safety features
2026-03-26 16:12:33 -07:00
Dustin Healy
94fdb3cd93 fix: address re-review findings for admin roles
- Gate deleteRoleByName on existence check — skip user reassignment and
  cache invalidation when role doesn't exist (fixes test mismatch)
- Reverse rename order: migrate users before renaming role so a migration
  failure leaves the system in a consistent state
- Add .sort({ _id: 1 }) to listUsersByRole for deterministic pagination
- Import shared AdminMember type from data-schemas instead of local copy;
  make joinedAt optional since neither groups nor roles populate it
- Change IRole.description from optional to required to match schema default
- Add data-layer tests for updateUsersByRole and countUsersByRole
- Add handler test verifying users-first rename ordering and migration
  failure safety
2026-03-26 15:54:14 -07:00
Dustin Healy
7d776de71a fix: address external review findings for admin roles
- Block renaming system roles (ADMIN/USER) and add user migration on rename
- Add input validation: name max-length, trim on update, duplicate name check
- Replace fragile String.includes error matching with prefix-based classification
- Catch MongoDB 11000 duplicate key in createRoleByName
- Add pagination (limit/offset/total) to getRoleMembersHandler
- Reverse delete order in deleteRoleByName — reassign users before deletion
- Add role existence check in removeRoleMember; drop unused createdAt select
- Add Array.isArray guard for permissions input; use consistent ?? coalescing
- Fix import ordering per AGENTS.md conventions
- Type-cast mongoose.models.User as Model<IUser> for proper TS inference
- Add comprehensive tests: rename guards, pagination, validation, 500 paths
2026-03-26 15:30:33 -07:00
Dustin Healy
88abca5d6d fix: allow system role updates when name is unchanged
The updateRoleHandler guard rejected any request where body.name matched
a system role, even when the name was not being changed. This blocked
editing a system role's description. Compare against the URL param to
only reject actual renames to reserved names.
2026-03-26 14:50:57 -07:00
Dustin Healy
9b906a85e3 test: add unit tests for admin roles and groups handlers 2026-03-26 14:50:56 -07:00
Dustin Healy
f57d90c0d7 fix: address Copilot review findings in admin roles handlers 2026-03-26 14:50:27 -07:00
Dustin Healy
afda763bcd feat: add description field to Role model
- Add description to IRole, CreateRoleRequest, UpdateRoleRequest types
- Add description field to Mongoose roleSchema (default: '')
- Wire description through createRoleHandler and updateRoleHandler
- Include description in listRoles select clause so it appears in list
2026-03-26 14:50:27 -07:00
Dustin Healy
f947546fea fix: rename createRole/deleteRole to avoid AccessRole name collision
The existing accessRole.ts already exports createRole/deleteRole for the
AccessRole model. In createMethods index.ts, these are spread after
roleMethods, overwriting them. Renamed our Role methods to
createRoleByName/deleteRoleByName to match the existing pattern
(getRoleByName, updateRoleByName) and avoid the collision.
2026-03-26 14:50:27 -07:00
Dustin Healy
69093a5d99 fix: address convention violations in admin roles handlers 2026-03-26 14:50:27 -07:00
Dustin Healy
452333ee4d feat: add admin roles handler factory and Express routes 2026-03-26 14:50:27 -07:00
Dustin Healy
2e3d66cfe2
👥 feat: Admin Groups API Endpoints (#12387)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
* feat: add listGroups and deleteGroup methods to userGroup

* feat: add admin groups handler factory and Express routes

* fix: address convention violations in admin groups handlers

* fix: address Copilot review findings in admin groups handlers

- Escape regex in listGroups to prevent injection/ReDoS
- Validate ObjectId format in all handlers accepting id/userId params
- Replace N+1 findUser loop with batched findUsers query
- Remove unused findGroupsByMemberId from dep interface
- Map Mongoose ValidationError to 400 in create/update handlers
- Validate name in updateGroupHandler (reject empty/whitespace)
- Handle null updateGroupById result (race condition)
- Tighten error message matching in add/remove member handlers

* test: add unit tests for admin groups handlers

* fix: address code review findings for admin groups

Atomic delete/update handlers (single DB trip), pass through
idOnTheSource, add removeMemberById for non-ObjectId members,
deduplicate member results, fix error message exposure, add hard
cap/sort to listGroups, replace GroupListFilter with Pick of
GroupFilterOptions, validate memberIds as array, trim name in
update, fix import order, and improve test hygiene with fresh
IDs per test.

* fix: cascade cleanup, pagination, and test coverage for admin groups

Add deleteGrantsForPrincipal to systemGrant data layer and wire cascade
cleanup (Config, AclEntry, SystemGrant) into deleteGroupHandler. Add
limit/offset pagination to getGroupMembers. Guard empty PATCH bodies with
400. Remove dead type guard and unnecessary type cast. Add 11 new tests
covering cascade delete, idempotent member removal, empty update, search
filter, 500 error paths, and pagination.

* fix: harden admin groups with cascade resilience, type safety, and fallback removal

Wrap cascade cleanup in inner try/catch so partial failure logs but still
returns 200 (group is already deleted). Replace Record<string, unknown> on
deleteAclEntries with proper typed filter. Log warning for unmapped user
ObjectIds in createGroup memberIds. Add removeMemberById fallback when
removeUserFromGroup throws User not found for ObjectId-format userId.
Extract VALID_GROUP_SOURCES constant. Add 3 new tests (60 total).

* refactor: add countGroups, pagination, and projection type to data layer

Extract buildGroupQuery helper, add countGroups method, support
limit/offset/skip in listGroups, standardize session handling to
.session(session ?? null), and tighten projection parameter from
Record<string, unknown> to Record<string, 0 | 1>.

* fix: cascade resilience, pagination, validation, and error clarity for admin groups

- Use Promise.allSettled for cascade cleanup so all steps run even if
  one fails; log individual rejections
- Echo deleted group id in delete response
- Add countGroups dep and wire limit/offset pagination for listGroups
- Deduplicate memberIds before computing total in getGroupMembers
- Use { memberIds: 1 } projection in getGroupMembers
- Cap memberIds at 500 entries in createGroup
- Reject search queries exceeding 200 characters
- Clarify addGroupMember error for non-ObjectId userId
- Document deleted-user fallback limitation in removeGroupMember

* test: extend handler and DB-layer test coverage for admin groups

Handler tests: projection assertion, dedup total, memberIds cap,
search max length, non-ObjectId memberIds passthrough, cascade partial
failure resilience, dedup scenarios, echo id in delete response.

DB-layer tests: listGroups sort/filter/pagination, countGroups,
deleteGroup, removeMemberById, deleteGrantsForPrincipal.

* fix: cast group principalId to ObjectId for ACL entry cleanup

deleteAclEntries is a thin deleteMany wrapper with no type casting,
but grantPermission stores group principalId as ObjectId. Passing the
raw string from req.params would leave orphaned ACL entries on group
deletion.

* refactor: remove redundant pagination clamping from DB listGroups

Handler already clamps limit/offset at the API boundary. The DB
method is a general-purpose building block and should not re-validate.

* fix: add source and name validation, import order, and test coverage for admin groups

- Validate source against VALID_GROUP_SOURCES in createGroupHandler
- Cap name at 500 characters in both create and update handlers
- Document total as upper bound in getGroupMembers response
- Document ObjectId requirement for deleteAclEntries in cascade
- Fix import ordering in test file (local value after type imports)
- Add tests for updateGroup with description, email, avatar fields
- Add tests for invalid source and name max-length in both handlers

* fix: add field length caps, flatten nested try/catch, and fix logger level in admin groups

Add max-length validation for description, email, avatar, and
idOnTheSource in create/update handlers. Extract removeObjectIdMember
helper to flatten nested try/catch per never-nesting convention. Downgrade
unmapped-memberIds log from error to warn. Fix type import ordering and
add missing await in removeMemberById for consistency.
2026-03-26 17:36:18 -04:00
Danny Avila
9f6d8c6e93
🧵 feat: ALS Context Middleware, Tenant Threading, and Config Cache Invalidation (#12407)
* feat: add tenant context middleware for ALS-based isolation

Introduces tenantContextMiddleware that propagates req.user.tenantId
into AsyncLocalStorage, activating the Mongoose applyTenantIsolation
plugin for all downstream DB queries within a request.

- Strict mode (TENANT_ISOLATION_STRICT=true) returns 403 if no tenantId
- Non-strict mode passes through for backward compatibility
- No-op for unauthenticated requests
- Includes 6 unit tests covering all paths

* feat: register tenant middleware and wrap startup/auth in runAsSystem()

- Register tenantContextMiddleware in Express app after capability middleware
- Wrap server startup initialization in runAsSystem() for strict mode compat
- Wrap auth strategy getAppConfig() calls in runAsSystem() since they run
  before user context is established (LDAP, SAML, OpenID, social login, AuthService)

* feat: thread tenantId through all getAppConfig callers

Pass tenantId from req.user to getAppConfig() across all callers that
have request context, ensuring correct per-tenant cache key resolution.

Also fixes getBaseConfig admin endpoint to scope to requesting admin's
tenant instead of returning the unscoped base config.

Files updated:
- Controllers: UserController, PluginController
- Middleware: checkDomainAllowed, balance
- Routes: config
- Services: loadConfigModels, loadDefaultModels, getEndpointsConfig, MCP
- Audio services: TTSService, STTService, getVoices, getCustomConfigSpeech
- Admin: getBaseConfig endpoint

* feat: add config cache invalidation on admin mutations

- Add clearOverrideCache(tenantId?) to flush per-principal override caches
  by enumerating Keyv store keys matching _OVERRIDE_: prefix
- Add invalidateConfigCaches() helper that clears base config, override
  caches, tool caches, and endpoint config cache in one call
- Wire invalidation into all 5 admin config mutation handlers
  (upsert, patch, delete field, delete overrides, toggle active)
- Add strict mode warning when __default__ tenant fallback is used
- Add 3 new tests for clearOverrideCache (all/scoped/base-preserving)

* chore: update getUserPrincipals comment to reflect ALS-based tenant filtering

The TODO(#12091) about missing tenantId filtering is resolved by the
tenant context middleware + applyTenantIsolation Mongoose plugin.
Group queries are now automatically scoped by tenantId via ALS.

* fix: replace runAsSystem with baseOnly for pre-tenant code paths

App configs are tenant-owned — runAsSystem() would bypass tenant
isolation and return cross-tenant DB overrides. Instead, add
baseOnly option to getAppConfig() that returns YAML-derived config
only, with zero DB queries.

All startup code, auth strategies, and MCP initialization now use
getAppConfig({ baseOnly: true }) to get the YAML config without
touching the Config collection.

* fix: address PR review findings — middleware ordering, types, cache safety

- Chain tenantContextMiddleware inside requireJwtAuth after passport auth
  instead of global app.use() where req.user is always undefined (Finding 1)
- Remove global tenantContextMiddleware registration from index.js
- Update BalanceMiddlewareOptions to include tenantId, remove redundant cast (Finding 4)
- Add warning log when clearOverrideCache cannot enumerate keys on Redis (Finding 3)
- Use startsWith instead of includes for cache key filtering (Finding 12)
- Use generator loop instead of Array.from for key enumeration (Finding 3)
- Selective barrel export — exclude _resetTenantMiddlewareStrictCache (Finding 5)
- Move isMainThread check to module level, remove per-request check (Finding 9)
- Move mid-file require to top of app.js (Finding 8)
- Parallelize invalidateConfigCaches with Promise.all (Finding 10)
- Remove clearOverrideCache from public app.js exports (internal only)
- Strengthen getUserPrincipals comment re: ALS dependency (Finding 2)

* fix: restore runAsSystem for startup DB ops, consolidate require, clarify baseOnly

- Restore runAsSystem() around performStartupChecks, updateInterfacePermissions,
  initializeMCPs, and initializeOAuthReconnectManager — these make Mongoose
  queries that need system context in strict tenant mode (NEW-3)
- Consolidate duplicate require('@librechat/api') in requireJwtAuth.js (NEW-1)
- Document that baseOnly ignores role/userId/tenantId in JSDoc (NEW-2)

* test: add requireJwtAuth tenant chaining + invalidateConfigCaches tests

- requireJwtAuth: 5 tests verifying ALS tenant context is set after
  passport auth, isolated between concurrent requests, and not set
  when user has no tenantId (Finding 6)
- invalidateConfigCaches: 4 tests verifying all four caches are cleared,
  tenantId is threaded through, partial failure is handled gracefully,
  and operations run in parallel via Promise.all (Finding 11)

* fix: address Copilot review — passport errors, namespaced cache keys, /base scoping

- Forward passport errors in requireJwtAuth before entering tenant
  middleware — prevents silent auth failures from reaching handlers (P1)
- Account for Keyv namespace prefix in clearOverrideCache — stored keys
  are namespaced as "APP_CONFIG:_OVERRIDE_:..." not "_OVERRIDE_:...",
  so override caches were never actually matched/cleared (P2)
- Remove role from getBaseConfig — /base should return tenant-scoped
  base config, not role-merged config that drifts per admin role (P2)
- Return tenantStorage.run() for cleaner async semantics
- Update mock cache in service.spec.ts to simulate Keyv namespacing

* fix: address second review — cache safety, code quality, test reliability

- Decouple cache invalidation from mutation response: fire-and-forget
  with logging so DB mutation success is not masked by cache failures
- Extract clearEndpointConfigCache helper from inline IIFE
- Move isMainThread check to lazy once-per-process guard (no import
  side effect)
- Memoize process.env read in overrideCacheKey to avoid per-request
  env lookups and log flooding in strict mode
- Remove flaky timer-based parallelism assertion, use structural check
- Merge orphaned double JSDoc block on getUserPrincipals
- Fix stale [getAppConfig] log prefix → [ensureBaseConfig]
- Fix import order in tenant.spec.ts (package types before local values)
- Replace "Finding 1" reference with self-contained description
- Use real tenantStorage primitives in requireJwtAuth spec mock

* fix: move JSDoc to correct function after clearEndpointConfigCache extraction

* refactor: remove Redis SCAN from clearOverrideCache, rely on TTL expiry

Redis SCAN causes 60s+ stalls under concurrent load (see #12410).
APP_CONFIG defaults to FORCED_IN_MEMORY_CACHE_NAMESPACES, so the
in-memory store.keys() path handles the standard case. When APP_CONFIG
is Redis-backed, overrides expire naturally via overrideCacheTtl (60s
default) — an acceptable window for admin config mutations.

* fix: remove return from tenantStorage.run to satisfy void middleware signature

* fix: address second review — cache safety, code quality, test reliability

- Switch invalidateConfigCaches from Promise.all to Promise.allSettled
  so partial failures are logged individually instead of producing one
  undifferentiated error (Finding 3)
- Gate overrideCacheKey strict-mode warning behind a once-per-process
  flag to prevent log flooding under load (Finding 4)
- Add test for passport error forwarding in requireJwtAuth — the
  if (err) { return next(err) } branch now has coverage (Finding 5)
- Add test for real partial failure in invalidateConfigCaches where
  clearAppConfigCache rejects (not just the swallowed endpoint error)

* chore: reorder imports in index.js and app.js for consistency

- Moved logger and runAsSystem imports to maintain a consistent import order across files.
- Improved code readability by ensuring related imports are grouped together.
2026-03-26 17:35:00 -04:00
Danny Avila
4b6d68b3b5
🎛️ feat: DB-Backed Per-Principal Config System (#12354)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
*  feat: Add Config schema, model, and methods for role-based DB config overrides

Add the database foundation for principal-based configuration overrides
(user, group, role) in data-schemas. Includes schema with tenantId and
tenant isolation, CRUD methods, and barrel exports.

* 🔧 fix: Add shebang and enforce LF line endings for git hooks

The pre-commit hook was missing #!/bin/sh, and core.autocrlf=true was
converting it to CRLF, both causing "Exec format error" on Windows.
Add .gitattributes to force LF for .husky/* and *.sh files.

*  feat: Add admin config API routes with section-level capability checks

Add /api/admin/config endpoints for managing per-principal config
overrides (user, group, role). Handlers in @librechat/api use DI pattern
with section-level hasConfigCapability checks for granular access control.

Supports full overrides replacement, per-field PATCH via dot-paths, field
deletion, toggle active, and listing.

* 🐛 fix: Move deleteConfigField fieldPath from URL param to request body

The path-to-regexp wildcard syntax (:fieldPath(*)) is not supported by
the version used in Express. Send fieldPath in the DELETE request body
instead, which also avoids URL-encoding issues with dotted paths.

*  feat: Wire config resolution into getAppConfig with override caching

Add mergeConfigOverrides utility in data-schemas for deep-merging DB
config overrides into base AppConfig by priority order.

Update getAppConfig to query DB for applicable configs when role/userId
is provided, with short-TTL caching and a hasAnyConfigs feature flag
for zero-cost when no DB configs exist.

Also: add unique compound index on Config schema, pass userId from
config middleware, and signal config changes from admin API handlers.

* 🔄 refactor: Extract getAppConfig logic into packages/api as TS service

Move override resolution, caching strategy, and signalConfigChange from
api/server/services/Config/app.js into packages/api/src/app/appConfigService.ts
using the DI factory pattern (createAppConfigService). The JS file becomes
a thin wiring layer injecting loadBaseConfig, cache, and DB dependencies.

* 🧹 chore: Rename configResolution.ts to resolution.ts

*  feat: Move admin types & capabilities to librechat-data-provider

Move SystemCapabilities, CapabilityImplications, and utility functions
(hasImpliedCapability, expandImplications) from data-schemas to
data-provider so they are available to external consumers like the
admin panel without a data-schemas dependency.

Add API-friendly admin types: TAdminConfig, TAdminSystemGrant,
TAdminAuditLogEntry, TAdminGroup, TAdminMember, TAdminUserSearchResult,
TCapabilityCategory, and CAPABILITY_CATEGORIES.

data-schemas re-exports these from data-provider and extends with
config-schema-derived types (ConfigSection, SystemCapability union).

Bump version to 0.8.500.

* feat: Add JSON-serializable admin config API response types to data-schemas

Add AdminConfig, AdminConfigListResponse, AdminConfigResponse, and
AdminConfigDeleteResponse types so both LibreChat API handlers and the
admin panel can share the same response contract. Bump version to 0.0.41.

* refactor: Move admin capabilities & types from data-provider to data-schemas

SystemCapabilities, CapabilityImplications, utility functions,
CAPABILITY_CATEGORIES, and admin API response types should not be in
data-provider as it gets compiled into the frontend bundle, exposing
the capability surface. Moved everything to data-schemas (server-only).

All consumers already import from @librechat/data-schemas, so no
import changes needed elsewhere. Consolidated duplicate AdminConfig
type (was in both config.ts and admin.ts).

* chore: Bump @librechat/data-schemas to 0.0.42

* refactor: Reorganize admin capabilities into admin/ and types/admin.ts

Split systemCapabilities.ts following data-schemas conventions:
- Types (BaseSystemCapability, SystemCapability, AdminConfig, etc.)
  → src/types/admin.ts
- Runtime code (SystemCapabilities, CapabilityImplications, utilities)
  → src/admin/capabilities.ts

Revert data-provider version to 0.8.401 (no longer modified).

* chore: Fix import ordering, rename appConfigService to service

- Rename app/appConfigService.ts → app/service.ts (directory provides context)
- Fix import order in admin/config.ts, types/admin.ts, types/config.ts
- Add naming convention to AGENTS.md

* feat: Add DB base config support (role/__base__)

- Add BASE_CONFIG_PRINCIPAL_ID constant for reserved base config doc
- getApplicableConfigs always includes __base__ in queries
- getAppConfig queries DB even without role/userId when DB configs exist
- Bump @librechat/data-schemas to 0.0.43

* fix: Address PR review issues for admin config

- Add listAllConfigs method; listConfigs endpoint returns all active
  configs instead of only __base__
- Normalize principalId to string in all config methods to prevent
  ObjectId vs string mismatch on user/group lookups
- Block __proto__ and all dunder-prefixed segments in field path
  validation to prevent prototype pollution
- Fix configVersion off-by-one: default to 0, guard pre('save') with
  !isNew, use $inc on findOneAndUpdate
- Remove unused getApplicableConfigs from admin handler deps

* fix: Enable tree-shaking for data-schemas, bump packages

- Switch data-schemas Rollup output to preserveModules so each source
  file becomes its own chunk; consumers (admin panel) can now import
  just the modules they need without pulling in winston/mongoose/etc.
- Add sideEffects: false to data-schemas package.json
- Bump data-schemas to 0.0.44, data-provider to 0.8.402

* feat: add capabilities subpath export to data-schemas

Adds `@librechat/data-schemas/capabilities` subpath export so browser
consumers can import BASE_CONFIG_PRINCIPAL_ID and capability constants
without pulling in Node.js-only modules (winston, async_hooks, etc.).

Bump version to 0.0.45.

* fix: include dist/ in data-provider npm package

Add explicit files field so npm includes dist/types/ in the published
package. Without this, the root .gitignore exclusion of dist/ causes
npm to omit type declarations, breaking TypeScript consumers.

* chore: bump librechat-data-provider to 0.8.403

* feat: add GET /api/admin/config/base for raw AppConfig

Returns the full AppConfig (YAML + DB base merged) so the admin panel
can display actual config field values and structure. The startup config
endpoint (/api/config) returns TStartupConfig which is a different shape
meant for the frontend app.

* chore: imports order

* fix: address code review findings for admin config

Critical:
- Fix clearAppConfigCache: was deleting from wrong cache store (CONFIG_STORE
  instead of APP_CONFIG), now clears BASE and HAS_DB_CONFIGS keys
- Eliminate race condition: patchConfigField and deleteConfigField now use
  atomic MongoDB $set/$unset with dot-path notation instead of
  read-modify-write cycles, removing the lost-update bug entirely
- Add patchConfigFields and unsetConfigField atomic DB methods

Major:
- Reorder cache check before principal resolution in getAppConfig so
  getUserPrincipals DB query only fires on cache miss
- Replace '' as ConfigSection with typed BROAD_CONFIG_ACCESS constant
- Parallelize capability checks with Promise.all instead of sequential
  awaits in for loops
- Use loose equality (== null) for cache miss check to handle both null
  and undefined returns from cache implementations
- Set HAS_DB_CONFIGS_KEY to true on successful config fetch

Minor:
- Remove dead pre('save') hook from config schema (all writes use
  findOneAndUpdate which bypasses document hooks)
- Consolidate duplicate type imports in resolution.ts
- Remove dead deepGet/deepSet/deepUnset functions (replaced by atomic ops)
- Add .sort({ priority: 1 }) to getApplicableConfigs query
- Rename _impliedBy to impliedByMap

* fix: self-referencing BROAD_CONFIG_ACCESS constant

* fix: replace type-cast sentinel with proper null parameter

Update hasConfigCapability to accept ConfigSection | null where null
means broad access check (MANAGE_CONFIGS or READ_CONFIGS only).
Removes the '' as ConfigSection type lie from admin config handlers.

* fix: remaining review findings + add tests

- listAllConfigs accepts optional { isActive } filter so admin listing
  can show inactive configs (#9)
- Standardize session application to .session(session ?? null) across
  all config DB methods (#15)
- Export isValidFieldPath and getTopLevelSection for testability
- Add 38 tests across 3 spec files:
  - config.spec.ts (api): path validation, prototype pollution rejection
  - resolution.spec.ts: deep merge, priority ordering, array replacement
  - config.spec.ts (data-schemas): full CRUD, ObjectId normalization,
    atomic $set/$unset, configVersion increment, toggle, __base__ query

* fix: address second code review findings

- Fix cross-user cache contamination: overrideCacheKey now handles
  userId-without-role case with its own cache key (#1)
- Add broad capability check before DB lookup in getConfig to prevent
  config existence enumeration (#2/#3)
- Move deleteConfigField fieldPath from request body to query parameter
  for proxy/load balancer compatibility (#5)
- Derive BaseSystemCapability from SystemCapabilities const instead of
  manual string union (#6)
- Return 201 on upsert creation, 200 on update (#11)
- Remove inline narration comments per AGENTS.md (#12)
- Type overrides as Partial<TCustomConfig> in DB methods and handler
  deps (#13)
- Replace double as-unknown-as casts in resolution.ts with generic
  deepMerge<T> (#14)
- Make override cache TTL injectable via AppConfigServiceDeps (#16)
- Add exhaustive never check in principalModel switch (#17)

* fix: remaining review findings — tests, rename, semantics

- Rename signalConfigChange → markConfigsDirty with JSDoc documenting
  the stale-window tradeoff and overrideCacheTtl knob
- Fix DEFAULT_OVERRIDE_CACHE_TTL naming convention
- Add createAppConfigService tests (14 cases): cache behavior, feature
  flag, cross-user key isolation, fallback on error, markConfigsDirty
- Add admin handler integration tests (13 cases): auth ordering,
  201/200 on create/update, fieldPath from query param, markConfigsDirty
  calls, capability checks

* fix: global flag corruption + empty overrides auth bypass

- Remove HAS_DB_CONFIGS_KEY=false optimization: a scoped query returning
  no configs does not mean no configs exist globally. Setting the flag
  false from a per-principal query short-circuited all subsequent users.
- Add broad manage capability check before section checks in
  upsertConfigOverrides: empty overrides {} no longer bypasses auth.

* test: add regression and invariant tests for config system

Regression tests:
- Bug 1: User A's empty result does not short-circuit User B's overrides
- Bug 2: Empty overrides {} returns 403 without MANAGE_CONFIGS

Invariant tests (applied across ALL handlers):
- All 5 mutation handlers call markConfigsDirty on success
- All 5 mutation handlers return 401 without auth
- All 5 mutation handlers return 403 without capability
- All 3 read handlers return 403 without capability

* fix: third review pass — all findings addressed

Service (service.ts):
- Restore HAS_DB_CONFIGS=false for base-only queries (no role/userId)
  so deployments with zero DB configs skip DB queries (#1)
- Resolve cache once at factory init instead of per-invocation (#8)
- Use BASE_CONFIG_PRINCIPAL_ID constant in overrideCacheKey (#10)
- Add JSDoc to clearAppConfigCache documenting stale-window (#4)
- Fix log message to not say "from YAML" (#14)

Admin handlers (config.ts):
- Use configVersion===1 for 201 vs 200, eliminating TOCTOU race (#2)
- Add Array.isArray guard on overrides body (#5)
- Import CapabilityUser from capabilities.ts, remove duplicate (#6)
- Replace as-unknown-as cast with targeted type assertion (#7)
- Add MAX_PATCH_ENTRIES=100 cap on entries array (#15)
- Reorder deleteConfigField to validate principalType first (#12)
- Export CapabilityUser from middleware/capabilities.ts

DB methods (config.ts):
- Remove isActive:true from patchConfigFields to prevent silent
  reactivation of disabled configs (#3)

Schema (config.ts):
- Change principalId from Schema.Types.Mixed to String (#11)

Tests:
- Add patchConfigField unsafe fieldPath rejection test (#9)
- Add base-only HAS_DB_CONFIGS=false test (#1)
- Update 201/200 tests to use configVersion instead of findConfig (#2)

* fix: add read handler 401 invariant tests + document flag behavior

- Add invariant: all 3 read handlers return 401 without auth
- Document on markConfigsDirty that HAS_DB_CONFIGS stays true after
  all configs are deleted until clearAppConfigCache or restart

* fix: remove HAS_DB_CONFIGS false optimization entirely

getApplicableConfigs([]) only queries for __base__, not all configs.
A deployment with role/group configs but no __base__ doc gets the
flag poisoned to false by a base-only query, silently ignoring all
scoped overrides. The optimization is not safe without a comprehensive
Config.exists() check, which adds its own DB cost. Removed entirely.

The flag is now write-once-true (set when configs are found or by
markConfigsDirty) and only cleared by clearAppConfigCache/restart.

* chore: reorder import statements in app.js for clarity

* refactor: remove HAS_DB_CONFIGS_KEY machinery entirely

The three-state flag (false/null/true) was the source of multiple bugs
across review rounds. Every attempt to safely set it to false was
defeated by getApplicableConfigs querying only a subset of principals.

Removed: HAS_DB_CONFIGS_KEY constant, all reads/writes of the flag,
markConfigsDirty (now a no-op concept), notifyChange wrapper, and all
tests that seeded false manually.

The per-user/role TTL cache (overrideCacheTtl, default 60s) is the
sole caching mechanism. On cache miss, getApplicableConfigs queries
the DB. This is one indexed query per user per TTL window — acceptable
for the config override use case.

* docs: rewrite admin panel remaining work with current state

* perf: cache empty override results to avoid repeated DB queries

When getApplicableConfigs returns no configs for a principal, cache
baseConfig under their override key with TTL. Without this, every
user with no per-principal overrides hits MongoDB on every request
after the 60s cache window expires.

* fix: add tenantId to cache keys + reject PUBLIC principal type

- Include tenantId in override cache keys to prevent cross-tenant
  config contamination. Single-tenant deployments (tenantId undefined)
  use '_' as placeholder — no behavior change for them.
- Reject PrincipalType.PUBLIC in admin config validation — PUBLIC has
  no PrincipalModel and is never resolved by getApplicableConfigs,
  so config docs for it would be dead data.
- Config middleware passes req.user.tenantId to getAppConfig.

* fix: fourth review pass findings

DB methods (config.ts):
- findConfigByPrincipal accepts { includeInactive } option so admin
  GET can retrieve inactive configs (#5)
- upsertConfig catches E11000 duplicate key on concurrent upserts and
  retries without upsert flag (#2)
- unsetConfigField no longer filters isActive:true, consistent with
  patchConfigFields (#11)
- Typed filter objects replace Record<string, unknown> (#12)

Admin handlers (config.ts):
- patchConfigField: serial broad capability check before Promise.all
  to pre-warm ALS principal cache, preventing N parallel DB calls (#3)
- isValidFieldPath rejects leading/trailing dots and consecutive
  dots (#7)
- Duplicate fieldPaths in patch entries return 400 (#8)
- DEFAULT_PRIORITY named constant replaces hardcoded 10 (#14)
- Admin getConfig and patchConfigField pass includeInactive to
  findConfigByPrincipal (#5)
- Route import uses barrel instead of direct file path (#13)

Resolution (resolution.ts):
- deepMerge has MAX_MERGE_DEPTH=10 guard to prevent stack overflow
  from crafted deeply nested configs (#4)

* fix: final review cleanup

- Remove ADMIN_PANEL_REMAINING.md (local dev notes with Windows paths)
- Add empty-result caching regression test
- Add tenantId to AdminConfigDeps.getAppConfig type
- Restore exhaustive never check in principalModel switch
- Standardize toggleConfigActive session handling to options pattern

* fix: validate priority in patchConfigField handler

Add the same non-negative number validation for priority that
upsertConfigOverrides already has. Without this, invalid priority
values could be stored via PATCH and corrupt merge ordering.

* chore: remove planning doc from PR

* fix: correct stale cache key strings in service tests

* fix: clean up service tests and harden tenant sentinel

- Remove no-op cache delete lines from regression tests
- Change no-tenant sentinel from '_' to '__default__' to avoid
  collision with a real tenant ID when multi-tenancy is enabled
- Remove unused CONFIG_STORE from AppConfigServiceDeps

* chore: bump @librechat/data-schemas to 0.0.46

* fix: block prototype-poisoning keys in deepMerge

Skip __proto__, constructor, and prototype keys during config merge
to prevent prototype pollution via PUT /api/admin/config overrides.
2026-03-25 19:39:29 -04:00