From 7f245ce103ef603960bbbb6cc080c40cace3dee7 Mon Sep 17 00:00:00 2001 From: Romuald Wandji Date: Tue, 1 Sep 2026 12:27:20 +0200 Subject: [PATCH 1/3] feat: add atomic admin configuration mutations --- .devcontainer/docker-compose.yml | 10 +- .env.example | 2 + .github/workflows/docker-smoke.yml | 11 +- api/db/connect.js | 17 +- api/server/index.js | 4 +- api/server/routes/admin/config.js | 2 + deploy-compose.yml | 22 +- docker-compose.yml | 22 +- helm/librechat/readme.md | 27 + helm/librechat/templates/configmap-env.yaml | 8 +- helm/librechat/values.yaml | 15 + packages/api/src/admin/config.handler.spec.ts | 705 +++++++- packages/api/src/admin/config.spec.ts | 10 +- packages/api/src/admin/config.ts | 725 +++++++-- .../api/src/admin/secrets.integration.spec.ts | 15 +- .../src/admin/configOverrides.spec.ts | 212 +++ .../data-schemas/src/admin/configOverrides.ts | 278 ++++ packages/data-schemas/src/admin/index.ts | 8 + .../src/admin/indexedArrayPath.spec.ts | 49 + .../src/admin/indexedArrayPath.ts | 148 ++ .../data-schemas/src/app/resolution.spec.ts | 36 +- packages/data-schemas/src/app/resolution.ts | 54 +- packages/data-schemas/src/index.ts | 13 + .../src/methods/config.atomic.spec.ts | 906 ++++++++++ .../data-schemas/src/methods/config.spec.ts | 658 +++++++- .../src/methods/config.tenant.spec.ts | 82 + packages/data-schemas/src/methods/config.ts | 1450 +++++++++++++++-- packages/data-schemas/src/methods/index.ts | 41 +- packages/data-schemas/src/schema/config.ts | 1 + packages/data-schemas/src/types/config.ts | 4 +- utils/docker/test-compose.yml | 8 +- 31 files changed, 5197 insertions(+), 346 deletions(-) create mode 100644 packages/data-schemas/src/admin/configOverrides.spec.ts create mode 100644 packages/data-schemas/src/admin/configOverrides.ts create mode 100644 packages/data-schemas/src/admin/indexedArrayPath.spec.ts create mode 100644 packages/data-schemas/src/admin/indexedArrayPath.ts create mode 100644 packages/data-schemas/src/methods/config.atomic.spec.ts create mode 100644 packages/data-schemas/src/methods/config.tenant.spec.ts diff --git a/.devcontainer/docker-compose.yml b/.devcontainer/docker-compose.yml index 70ebf9b9558..61d24cfc4af 100644 --- a/.devcontainer/docker-compose.yml +++ b/.devcontainer/docker-compose.yml @@ -19,7 +19,7 @@ services: # - /var/run/docker.sock:/var/run/docker.sock environment: - HOST=0.0.0.0 - - MONGO_URI=mongodb://mongodb:27017/LibreChat + - MONGO_URI=mongodb://mongodb:27017/LibreChat?replicaSet=rs0 # - OPENAI_REVERSE_PROXY=http://host.docker.internal:8070/v1 - MEILI_HOST=http://meilisearch:7700 @@ -45,7 +45,13 @@ services: # restart: always volumes: - ./data-node:/data/db - command: mongod --noauth + command: ["mongod", "--replSet", "rs0", "--bind_ip_all"] + healthcheck: + test: echo "try { rs.status() } catch (err) { rs.initiate({_id:'rs0',members:[{_id:0,host:'mongodb:27017'}]}) }" | mongosh --quiet + interval: 5s + timeout: 30s + retries: 30 + start_period: 10s meilisearch: container_name: chat-meilisearch image: getmeili/meilisearch:v1.5 diff --git a/.env.example b/.env.example index a4b018bf505..224a194dddd 100644 --- a/.env.example +++ b/.env.example @@ -28,6 +28,8 @@ PORT=3080 # HTTP_REQUEST_TIMEOUT_MS=300000 MONGO_URI=mongodb://127.0.0.1:27017/LibreChat +# Config rollback uses Mongo transactions. Use a replica set or Atlas +# (`mongodb+srv://...`). Docker Compose initializes `rs0` automatically. #The maximum number of connections in the connection pool. */ MONGO_MAX_POOL_SIZE= #The minimum number of connections in the connection pool. */ diff --git a/.github/workflows/docker-smoke.yml b/.github/workflows/docker-smoke.yml index 5da967f90ef..d9fd52866e2 100644 --- a/.github/workflows/docker-smoke.yml +++ b/.github/workflows/docker-smoke.yml @@ -220,11 +220,18 @@ jobs: run: | set -u docker network create lc-smoke - docker run -d --name lc-mongo --network lc-smoke mongo:8.0.20 + docker run -d --name lc-mongo --network lc-smoke mongo:8.0.20 --replSet rs0 --bind_ip_all + for i in $(seq 1 30); do + docker exec lc-mongo mongosh --quiet --eval 'try { rs.status() } catch (e) { rs.initiate({_id:"rs0",members:[{_id:0,host:"lc-mongo:27017"}]}) }' >/dev/null 2>&1 || true + if docker exec lc-mongo mongosh --quiet --eval 'quit(rs.status().ok ? 0 : 1)' >/dev/null 2>&1; then + break + fi + sleep 1 + done docker run -d --name lc-api --network lc-smoke -p 3080:3080 \ -e HOST=0.0.0.0 -e PORT=3080 \ -e NODE_ENV=production \ - -e MONGO_URI=mongodb://lc-mongo:27017/LibreChat \ + -e MONGO_URI=mongodb://lc-mongo:27017/LibreChat?replicaSet=rs0 \ -e CREDS_KEY=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef \ -e CREDS_IV=0123456789abcdef0123456789abcdef \ -e JWT_SECRET=docker-smoke-jwt-secret \ diff --git a/api/db/connect.js b/api/db/connect.js index a63d3301b69..d1e5bf433b9 100644 --- a/api/db/connect.js +++ b/api/db/connect.js @@ -20,16 +20,13 @@ const maxConnecting = parseInt(process.env.MONGO_MAX_CONNECTING) || undefined; const maxIdleTimeMS = parseInt(process.env.MONGO_MAX_IDLE_TIME_MS) || undefined; /** The maximum time in milliseconds that a thread can wait for a connection to become available. */ const waitQueueTimeoutMS = parseInt(process.env.MONGO_WAIT_QUEUE_TIMEOUT_MS) || undefined; -/** Set to false to disable automatic index creation for all models associated with this connection. */ +/** Explicit `true`/`false` controls Mongoose autoIndex; absent variable preserves the Mongoose default (`true`). */ const autoIndex = - process.env.MONGO_AUTO_INDEX != undefined - ? isEnabled(process.env.MONGO_AUTO_INDEX) || false - : undefined; - -/** Set to `false` to disable Mongoose automatically calling `createCollection()` on every model created on this connection. */ + process.env.MONGO_AUTO_INDEX !== undefined ? isEnabled(process.env.MONGO_AUTO_INDEX) : undefined; +/** Explicit `true`/`false` controls Mongoose autoCreate; absent variable preserves the Mongoose default (`true`). */ const autoCreate = - process.env.MONGO_AUTO_CREATE != undefined - ? isEnabled(process.env.MONGO_AUTO_CREATE) || false + process.env.MONGO_AUTO_CREATE !== undefined + ? isEnabled(process.env.MONGO_AUTO_CREATE) : undefined; /** * Global is used here to maintain a cached connection across hot reloads @@ -60,8 +57,8 @@ async function connectDb() { ...(maxConnecting ? { maxConnecting } : {}), ...(maxIdleTimeMS ? { maxIdleTimeMS } : {}), ...(waitQueueTimeoutMS ? { waitQueueTimeoutMS } : {}), - ...(autoIndex != undefined ? { autoIndex } : {}), - ...(autoCreate != undefined ? { autoCreate } : {}), + ...(autoIndex !== undefined ? { autoIndex } : {}), + ...(autoCreate !== undefined ? { autoCreate } : {}), // useNewUrlParser: true, // useUnifiedTopology: true, // bufferMaxEntries: 0, diff --git a/api/server/index.js b/api/server/index.js index 5f50c0d04c8..7256ffadc85 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -1,6 +1,7 @@ require('../config/credentials'); const telemetry = require('./telemetry'); +const mongoose = require('mongoose'); const fs = require('fs'); const path = require('path'); require('module-alias')({ base: path.resolve(__dirname, '..') }); @@ -11,7 +12,7 @@ const passport = require('passport'); const compression = require('compression'); const cookieParser = require('cookie-parser'); const mongoSanitize = require('express-mongo-sanitize'); -const { logger, runAsSystem } = require('@librechat/data-schemas'); +const { logger, runAsSystem, ensureConfigIndexes } = require('@librechat/data-schemas'); const { isEnabled, issueCsp, @@ -175,6 +176,7 @@ const startServer = async () => { axios.defaults.headers.common['Accept-Encoding'] = 'gzip'; } await connectDb(); + await ensureConfigIndexes(mongoose); logger.info('Connected to MongoDB'); indexSync().catch((err) => { diff --git a/api/server/routes/admin/config.js b/api/server/routes/admin/config.js index 9333495280d..23b43c327cb 100644 --- a/api/server/routes/admin/config.js +++ b/api/server/routes/admin/config.js @@ -27,6 +27,7 @@ const handlers = createAdminConfigHandlers({ toggleConfigActive: db.toggleConfigActive, hasAnyConfigReadAccess, getReadableConfigSections, + mutateConfigWithRevision: db.mutateConfigWithRevision, hasConfigCapability, hasCapability, getAppConfig, @@ -44,5 +45,6 @@ router.post('/:principalType/:principalId/fields/tombstone', handlers.tombstoneC router.delete('/:principalType/:principalId/fields', handlers.deleteConfigField); router.delete('/:principalType/:principalId', handlers.deleteConfigOverrides); router.patch('/:principalType/:principalId/active', handlers.toggleConfig); +router.post('/:principalType/:principalId/atomic', handlers.mutateConfigAtomic); module.exports = router; diff --git a/deploy-compose.yml b/deploy-compose.yml index 2309050f62a..b5695d461c0 100644 --- a/deploy-compose.yml +++ b/deploy-compose.yml @@ -9,8 +9,10 @@ services: ports: - 3080:3080 depends_on: - - mongodb - - rag_api + mongodb: + condition: service_healthy + rag_api: + condition: service_started restart: always extra_hosts: - "host.docker.internal:host-gateway" @@ -19,7 +21,7 @@ services: environment: - HOST=0.0.0.0 - NODE_ENV=production - - MONGO_URI=mongodb://mongodb:27017/LibreChat + - MONGO_URI=mongodb://mongodb:27017/LibreChat?replicaSet=rs0 - MEILI_HOST=http://meilisearch:7700 - LIBRECHAT_TEMP_CREDENTIALS_PATH=/app/data/.env.temp - RAG_PORT=${RAG_PORT:-8000} @@ -46,13 +48,17 @@ services: image: registry.librechat.ai/clickhouse/librechat-admin-panel:latest container_name: admin-panel depends_on: - - api + api: + condition: service_started + mongodb: + condition: service_healthy restart: always environment: - PORT=3000 # Plain expansion (not :?) so `docker compose down`/`pull` still run when this is unset. # The panel itself refuses to start without it; set ADMIN_PANEL_SESSION_SECRET in .env. - SESSION_SECRET=${ADMIN_PANEL_SESSION_SECRET} + - MONGO_URI=mongodb://mongodb:27017/LibreChat?replicaSet=rs0 - API_SERVER_URL=http://api:3080 - VITE_API_BASE_URL=${DOMAIN_CLIENT:-http://localhost} - SESSION_COOKIE_SECURE=${ADMIN_PANEL_SESSION_COOKIE_SECURE:-false} @@ -77,7 +83,13 @@ services: restart: always volumes: - ./data-node:/data/db - command: mongod --noauth + command: ["mongod", "--replSet", "rs0", "--bind_ip_all"] + healthcheck: + test: echo "try { rs.status() } catch (err) { rs.initiate({_id:'rs0',members:[{_id:0,host:'mongodb:27017'}]}) }" | mongosh --quiet + interval: 5s + timeout: 30s + retries: 30 + start_period: 10s meilisearch: container_name: chat-meilisearch image: getmeili/meilisearch:v1.35.1 diff --git a/docker-compose.yml b/docker-compose.yml index cfbd4fc795b..13fa61b5666 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,8 +7,10 @@ services: ports: - "${PORT}:${PORT}" depends_on: - - mongodb - - rag_api + mongodb: + condition: service_healthy + rag_api: + condition: service_started image: registry.librechat.ai/danny-avila/librechat-dev:latest restart: always user: "${UID}:${GID}" @@ -16,7 +18,7 @@ services: - "host.docker.internal:host-gateway" environment: - HOST=0.0.0.0 - - MONGO_URI=mongodb://mongodb:27017/LibreChat + - MONGO_URI=mongodb://mongodb:27017/LibreChat?replicaSet=rs0 - MEILI_HOST=http://meilisearch:7700 - LIBRECHAT_TEMP_CREDENTIALS_PATH=/app/data/.env.temp - RAG_PORT=${RAG_PORT:-8000} @@ -43,13 +45,17 @@ services: ports: - "${ADMIN_PANEL_PORT:-3000}:3000" depends_on: - - api + api: + condition: service_started + mongodb: + condition: service_healthy restart: always environment: - PORT=3000 # Plain expansion (not :?) so `docker compose down`/`pull` still run when this is unset. # The panel itself refuses to start without it; set ADMIN_PANEL_SESSION_SECRET in .env. - SESSION_SECRET=${ADMIN_PANEL_SESSION_SECRET} + - MONGO_URI=mongodb://mongodb:27017/LibreChat?replicaSet=rs0 - API_SERVER_URL=http://api:${PORT:-3080} - VITE_API_BASE_URL=${DOMAIN_CLIENT:-http://localhost:3080} - SESSION_COOKIE_SECURE=${ADMIN_PANEL_SESSION_COOKIE_SECURE:-false} @@ -60,7 +66,13 @@ services: user: "${UID}:${GID}" volumes: - ./data-node:/data/db - command: mongod --noauth + command: ["mongod", "--replSet", "rs0", "--bind_ip_all"] + healthcheck: + test: echo "try { rs.status() } catch (err) { rs.initiate({_id:'rs0',members:[{_id:0,host:'mongodb:27017'}]}) }" | mongosh --quiet + interval: 5s + timeout: 30s + retries: 30 + start_period: 10s meilisearch: container_name: chat-meilisearch image: getmeili/meilisearch:v1.35.1 diff --git a/helm/librechat/readme.md b/helm/librechat/readme.md index cfccfb848f6..a05c4f3d434 100755 --- a/helm/librechat/readme.md +++ b/helm/librechat/readme.md @@ -37,6 +37,33 @@ kind: Secret 4. Fill out values.yaml and apply the Chart to the Cluster +## MongoDB upgrade note (standalone -> replicaset) + +The bundled Bitnami MongoDB dependency defaults to `standalone` to preserve +existing upgrade behavior. Switching an existing release from standalone to +replicaset changes the Mongo workload shape from a Deployment to a StatefulSet +and uses a differently named PVC (`datadir--mongodb-0`), which does +not automatically reuse the standalone PVC (`-mongodb`). + +Before enabling replica set mode on an existing release: + +1. Back up MongoDB data. +2. Plan and validate a migration to the new StatefulSet PVC layout. +3. Test restore/rollback in a non-production environment. + +For fresh installs that need transactions (for admin config rollback), use a +single data-node replica set and explicitly disable the arbiter: + +```yaml +mongodb: + enabled: true + architecture: replicaset + replicaCount: 1 + replicaSetName: rs0 + arbiter: + enabled: false +``` + ## Admin Panel SSO Set `librechat.adminPanelUrl` to the admin panel base URL used for OAuth/SSO diff --git a/helm/librechat/templates/configmap-env.yaml b/helm/librechat/templates/configmap-env.yaml index e6dc7a5855f..54f718ffca7 100755 --- a/helm/librechat/templates/configmap-env.yaml +++ b/helm/librechat/templates/configmap-env.yaml @@ -13,7 +13,13 @@ data: MEILI_HOST: http://{{ include "meilisearch.fullname" .Subcharts.meilisearch }}.{{ .Release.Namespace | lower }}.svc.cluster.local:7700 {{- end }} {{- if and (not (dig "configEnv" "MONGO_URI" "" .Values.librechat)) .Values.mongodb.enabled }} - MONGO_URI: mongodb://{{ include "mongodb.service.nameOverride" .Subcharts.mongodb }}.{{ .Release.Namespace | lower }}.svc.cluster.local:27017/LibreChat + {{- $mongoNs := .Release.Namespace | lower }} + {{- $mongoName := include "mongodb.fullname" .Subcharts.mongodb }} + {{- if eq (default "standalone" .Values.mongodb.architecture) "replicaset" }} + MONGO_URI: mongodb://{{ $mongoName }}-0.{{ $mongoName }}-headless.{{ $mongoNs }}.svc.cluster.local:27017/LibreChat?replicaSet={{ .Values.mongodb.replicaSetName | default "rs0" }} + {{- else }} + MONGO_URI: mongodb://{{ include "mongodb.service.nameOverride" .Subcharts.mongodb }}.{{ $mongoNs }}.svc.cluster.local:27017/LibreChat + {{- end }} {{- end }} {{- if and (not (dig "configEnv" "USE_REDIS" "" .Values.librechat)) .Values.redis.enabled }} USE_REDIS: "true" diff --git a/helm/librechat/values.yaml b/helm/librechat/values.yaml index c72936e1da2..5b95a3b85b0 100755 --- a/helm/librechat/values.yaml +++ b/helm/librechat/values.yaml @@ -363,8 +363,23 @@ additionalConfigMaps: {} # # ... add more ConfigMaps as needed # MongoDB Parameters +# IMPORTANT: The admin panel routes every base-config save, import, reset, +# delete, and restore through a transactional atomic endpoint. With the default +# standalone architecture all base-config editing fails at runtime with a 503 +# "requires a MongoDB replica set" error. This includes ordinary saves, not +# just rollback. To enable base-config editing with the admin panel, opt in to +# a single-node replica set: +# mongodb.architecture=replicaset +# mongodb.replicaCount=1 +# mongodb.replicaSetName=rs0 +# mongodb.arbiter.enabled=false +# Existing installations: read helm/librechat/readme.md first for migration +# and backup requirements before switching topology. +# Default remains standalone to preserve existing PVC/workload shape for +# deployments that do not use the admin panel for base-config editing. mongodb: enabled: true + architecture: standalone # Bitnami moved versioned image tags to docker.io/bitnamilegacy on 2025-08-28. # See https://github.com/bitnami/charts/issues/35164 image: diff --git a/packages/api/src/admin/config.handler.spec.ts b/packages/api/src/admin/config.handler.spec.ts index 11c16c4e93f..f1a24a86535 100644 --- a/packages/api/src/admin/config.handler.spec.ts +++ b/packages/api/src/admin/config.handler.spec.ts @@ -69,6 +69,11 @@ function createHandlers(overrides = {}) { unsetConfigField: jest.fn().mockResolvedValue({ _id: 'c1', overrides: {} }), deleteConfig: jest.fn().mockResolvedValue({ _id: 'c1' }), toggleConfigActive: jest.fn().mockResolvedValue({ _id: 'c1', isActive: false }), + mutateConfigWithRevision: jest.fn().mockResolvedValue({ + changed: true, + config: { _id: 'c1', configVersion: 6, overrides: { cache: true } }, + revision: { id: 'rev-1', status: 'final', configVersion: 5 }, + }), hasConfigCapability: jest.fn().mockResolvedValue(true), hasAnyConfigReadAccess: jest.fn().mockResolvedValue(true), hasCapability: jest.fn().mockResolvedValue(true), @@ -944,28 +949,7 @@ describe('createAdminConfigHandlers', () => { 'admin', expect.anything(), 'mcpServers.github', - 10, - ); - }); - - it('uses the existing config priority when priority is omitted', async () => { - const { handlers, deps } = createHandlers({ - findConfigByPrincipal: jest.fn().mockResolvedValue({ _id: 'c1', priority: 42 }), - }); - const req = mockReq({ - params: { principalType: 'role', principalId: 'admin' }, - body: { fieldPath: 'mcpServers.github' }, - }); - const res = mockRes(); - - await handlers.tombstoneConfigField(req, res); - - expect(deps.tombstoneConfigField).toHaveBeenCalledWith( - 'role', - 'admin', - expect.anything(), - 'mcpServers.github', - 42, + undefined, ); }); @@ -1028,6 +1012,31 @@ describe('createAdminConfigHandlers', () => { expect(deps.tombstoneConfigField).not.toHaveBeenCalled(); }); + it('blocks protected ancestor and alias tombstones', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { fieldPath: 'interface' }, + }); + const res = mockRes(); + + await handlers.tombstoneConfigField(req, res); + + expect(res.statusCode).toBe(200); + expect(res.body!.message).toBeDefined(); + expect(deps.tombstoneConfigField).not.toHaveBeenCalled(); + + const aliasReq = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { fieldPath: 'interfaceConfig.prompts' }, + }); + const aliasRes = mockRes(); + await handlers.tombstoneConfigField(aliasReq, aliasRes); + expect(aliasRes.statusCode).toBe(200); + expect(aliasRes.body!.message).toBeDefined(); + expect(deps.tombstoneConfigField).not.toHaveBeenCalled(); + }); + it('rejects unsafe field paths', async () => { const { handlers, deps } = createHandlers(); const req = mockReq({ @@ -1044,6 +1053,28 @@ describe('createAdminConfigHandlers', () => { }); describe('patchConfigField', () => { + it('rejects malformed entries before mutation', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { entries: [null] }, + }); + const res = mockRes(); + await handlers.patchConfigField(req, res); + expect(res.statusCode).toBe(400); + expect(res.body?.error).toBe('each entry must be an object with fieldPath and value'); + expect(deps.patchConfigFields).not.toHaveBeenCalled(); + + const req2 = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { entries: [{ fieldPath: 'cache' }] }, + }); + const res2 = mockRes(); + await handlers.patchConfigField(req2, res2); + expect(res2.statusCode).toBe(400); + expect(res2.body?.error).toBe('each entry must include a value property'); + }); + it('returns 403 when user lacks capability for section', async () => { const { handlers } = createHandlers({ hasConfigCapability: jest.fn().mockResolvedValue(false), @@ -1117,6 +1148,29 @@ describe('createAdminConfigHandlers', () => { expect(patchedFields['interface.schedules']).toEqual({ maxPerUser: 2 }); }); + it('strips protected ancestor and alias field entries', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { + entries: [ + { fieldPath: 'interface', value: null }, + { fieldPath: 'interfaceConfig.prompts', value: false }, + { fieldPath: 'interface.modelSelect', value: false }, + ], + }, + }); + const res = mockRes(); + + await handlers.patchConfigField(req, res); + + expect(res.statusCode).toBe(200); + const patchedFields = deps.patchConfigFields.mock.calls[0][3]; + expect(patchedFields.interface).toBeUndefined(); + expect(patchedFields['interfaceConfig.prompts']).toBeUndefined(); + expect(patchedFields['interface.modelSelect']).toBe(false); + }); + it('preserves skillSync field entries in patches', async () => { const { handlers, deps } = createHandlers(); const req = mockReq({ @@ -1396,9 +1450,6 @@ describe('createAdminConfigHandlers', () => { it('ignores request-supplied priority when caller lacks broad manage:configs', async () => { const { handlers, deps } = createHandlers({ hasConfigCapability: jest.fn(async (_user, section) => section === 'memory'), - findConfigByPrincipal: jest - .fn() - .mockResolvedValue({ _id: 'c1', priority: 7, overrides: {} }), }); const req = mockReq({ params: { principalType: 'role', principalId: 'admin' }, @@ -1413,13 +1464,13 @@ describe('createAdminConfigHandlers', () => { expect(res.statusCode).toBe(200); const [, , , , priorityArg] = deps.patchConfigFields.mock.calls[0]; - expect(priorityArg).toBe(7); + expect(priorityArg).toBeUndefined(); + expect(deps.findConfigByPrincipal).not.toHaveBeenCalled(); }); - it('falls back to default priority when no existing doc and caller lacks broad manage', async () => { + it('omits priority when no existing doc and caller lacks broad manage:configs', async () => { const { handlers, deps } = createHandlers({ hasConfigCapability: jest.fn(async (_user, section) => section === 'memory'), - findConfigByPrincipal: jest.fn().mockResolvedValue(null), }); const req = mockReq({ params: { principalType: 'role', principalId: 'admin' }, @@ -1434,7 +1485,7 @@ describe('createAdminConfigHandlers', () => { expect(res.statusCode).toBe(200); const [, , , , priorityArg] = deps.patchConfigFields.mock.calls[0]; - expect(priorityArg).toBe(10); + expect(priorityArg).toBeUndefined(); }); it('honors request-supplied priority when caller holds broad manage:configs', async () => { @@ -1478,12 +1529,9 @@ describe('createAdminConfigHandlers', () => { expect(priorityArg).toBe(0); }); - it('preserves existing priority 0 for section-scoped callers', async () => { + it('omits priority for section-scoped callers even when existing priority is 0', async () => { const { handlers, deps } = createHandlers({ hasConfigCapability: jest.fn(async (_user, section) => section === 'memory'), - findConfigByPrincipal: jest - .fn() - .mockResolvedValue({ _id: 'c1', priority: 0, overrides: {} }), }); const req = mockReq({ params: { principalType: 'role', principalId: 'admin' }, @@ -1498,7 +1546,7 @@ describe('createAdminConfigHandlers', () => { expect(res.statusCode).toBe(200); const [, , , , priorityArg] = deps.patchConfigFields.mock.calls[0]; - expect(priorityArg).toBe(0); + expect(priorityArg).toBeUndefined(); }); }); @@ -1506,9 +1554,6 @@ describe('createAdminConfigHandlers', () => { it('ignores request-supplied priority when caller lacks broad manage:configs', async () => { const { handlers, deps } = createHandlers({ hasConfigCapability: jest.fn(async (_user, section) => section === 'memory'), - findConfigByPrincipal: jest - .fn() - .mockResolvedValue({ _id: 'c1', priority: 7, overrides: {} }), }); const req = mockReq({ params: { principalType: 'role', principalId: 'admin' }, @@ -1520,13 +1565,13 @@ describe('createAdminConfigHandlers', () => { expect(res.statusCode).toBe(200); const [, , , , priorityArg] = deps.tombstoneConfigField.mock.calls[0]; - expect(priorityArg).toBe(7); + expect(priorityArg).toBeUndefined(); + expect(deps.findConfigByPrincipal).not.toHaveBeenCalled(); }); - it('falls back to default priority when no existing doc and caller lacks broad manage', async () => { + it('omits priority when no existing doc and caller lacks broad manage:configs', async () => { const { handlers, deps } = createHandlers({ hasConfigCapability: jest.fn(async (_user, section) => section === 'memory'), - findConfigByPrincipal: jest.fn().mockResolvedValue(null), }); const req = mockReq({ params: { principalType: 'role', principalId: 'admin' }, @@ -1538,7 +1583,7 @@ describe('createAdminConfigHandlers', () => { expect(res.statusCode).toBe(200); const [, , , , priorityArg] = deps.tombstoneConfigField.mock.calls[0]; - expect(priorityArg).toBe(10); + expect(priorityArg).toBeUndefined(); }); it('honors request-supplied priority when caller holds broad manage:configs', async () => { @@ -2229,6 +2274,9 @@ describe('createAdminConfigHandlers', () => { unsetConfigField: jest.fn(), deleteConfig: jest.fn().mockResolvedValue({ _id: 'c1' }), toggleConfigActive: jest.fn().mockResolvedValue({ _id: 'c1', isActive: false }), + mutateConfigWithRevision: jest + .fn() + .mockResolvedValue({ config: null, revision: { id: 'rev1' } }), hasConfigCapability: jest.fn().mockResolvedValue(false), }; const handlers = createAdminConfigHandlers(deps); @@ -2435,3 +2483,578 @@ describe('createAdminConfigHandlers', () => { }); }); }); + +describe('mutateConfigAtomic', () => { + it('requires expectedVersion', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { entries: [{ fieldPath: 'cache', value: true }], cause: 'save' }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(400); + expect(deps.mutateConfigWithRevision).not.toHaveBeenCalled(); + }); + + it('rejects a non-object request body', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: 'not-an-object', + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(400); + expect(res.body?.error).toBe('request body must be a JSON object'); + expect(deps.mutateConfigWithRevision).not.toHaveBeenCalled(); + }); + + it('rejects malformed entries before mutation', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 0, + entries: [null], + cause: 'save', + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(400); + expect(res.body?.error).toBe('each entry must be an object with fieldPath and value'); + expect(deps.mutateConfigWithRevision).not.toHaveBeenCalled(); + }); + + it('rejects malformed resetPaths before mutation', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 0, + entries: [{ fieldPath: 'cache', value: true }], + resetPaths: 'cache', + cause: 'save', + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(400); + expect(res.body?.error).toBe('resetPaths must be an array'); + expect(deps.mutateConfigWithRevision).not.toHaveBeenCalled(); + }); + + it('rejects malformed operation properties before mode detection', async () => { + const { handlers, deps } = createHandlers(); + const cases = [ + { + body: { + expectedVersion: 0, + entries: [{ fieldPath: 'cache', value: true }], + overrides: 'invalid', + }, + error: 'overrides must be an object', + }, + { + body: { + expectedVersion: 0, + entries: [{ fieldPath: 'cache', value: true }], + deleteDocument: 'true', + }, + error: 'deleteDocument must be a boolean', + }, + { + body: { + expectedVersion: 0, + entries: [{ fieldPath: 'cache', value: true }], + restoreRevisionId: 123, + }, + error: 'restoreRevisionId must be a non-empty string', + }, + ] as const; + + for (const testCase of cases) { + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: testCase.body, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(400); + expect(res.body?.error).toBe(testCase.error); + } + + expect(deps.mutateConfigWithRevision).not.toHaveBeenCalled(); + }); + + it('returns 404 when the restore revision is missing', async () => { + const { handlers, deps } = createHandlers({ + mutateConfigWithRevision: jest.fn().mockRejectedValue( + Object.assign(new Error('Revision not found'), { + name: 'ConfigRevisionNotFoundError', + revisionId: '11111111-1111-4111-8111-111111111111', + }), + ), + }); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 5, + restoreRevisionId: '11111111-1111-4111-8111-111111111111', + cause: 'restore', + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(404); + expect(res.body).toEqual({ error: 'Revision not found' }); + expect(deps.mutateConfigWithRevision).toHaveBeenCalledTimes(1); + }); + + it('rejects field entries without an explicit value property', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 0, + entries: [{ fieldPath: 'cache' }], + cause: 'save', + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(400); + expect(res.body?.error).toBe('each entry must include a value property'); + expect(deps.mutateConfigWithRevision).not.toHaveBeenCalled(); + }); + + it('accepts explicit null and falsy entry values', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 0, + entries: [ + { fieldPath: 'cache', value: null }, + { fieldPath: 'registration.enabled', value: false }, + ], + cause: 'save', + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(200); + expect(deps.mutateConfigWithRevision).toHaveBeenCalledWith( + expect.objectContaining({ + op: expect.objectContaining({ + kind: 'fields', + fields: { cache: null, 'registration.enabled': false }, + }), + }), + ); + }); + + it('rejects process-backed MCP fields before an atomic mutation', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 5, + entries: [{ fieldPath: 'mcpServers.injected.command', value: '/bin/sh' }], + }, + }); + const res = mockRes(); + + await handlers.mutateConfigAtomic(req, res); + + expect(res.statusCode).toBe(400); + expect(res.body).toEqual({ + error: 'Process-backed MCP servers can only be configured in librechat.yaml', + }); + expect(deps.mutateConfigWithRevision).not.toHaveBeenCalled(); + }); + + it('rejects Langfuse request headers before an atomic mutation', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 5, + entries: [{ fieldPath: 'langfuse.headers.X-Proxy-Token', value: 'secret' }], + }, + }); + const res = mockRes(); + + await handlers.mutateConfigAtomic(req, res); + + expect(res.statusCode).toBe(400); + expect(res.body).toEqual({ + error: 'Langfuse request headers can only be configured in librechat.yaml', + }); + expect(deps.mutateConfigWithRevision).not.toHaveBeenCalled(); + }); + + it('encrypts atomic secret fields and redacts config and revision responses', async () => { + const secret = 'sk-atomic-secret'; + const mutateConfigWithRevision = jest.fn(async ({ op }) => { + const fields = op.kind === 'fields' ? op.fields : {}; + const overrides = { + ocr: { + apiKey: fields['ocr.apiKey'], + apiKeyPreview: fields['ocr.apiKeyPreview'], + }, + }; + return { + changed: true, + config: { _id: 'c1', configVersion: 6, overrides }, + revision: { id: 'rev-1', status: 'final', configVersion: 5, overrides }, + }; + }); + const { handlers, deps } = createHandlers({ mutateConfigWithRevision }); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 5, + entries: [{ fieldPath: 'ocr.apiKey', value: secret }], + }, + }); + const res = mockRes(); + + await handlers.mutateConfigAtomic(req, res); + + expect(res.statusCode).toBe(200); + const mutation = deps.mutateConfigWithRevision.mock.calls[0][0]; + expect(mutation.op).toEqual( + expect.objectContaining({ + kind: 'fields', + fields: expect.objectContaining({ + 'ocr.apiKey': `v3:test:${secret}`, + 'ocr.apiKeyPreview': expect.any(String), + }), + }), + ); + expect(JSON.stringify(res.body)).not.toContain(secret); + expect(JSON.stringify(res.body)).not.toContain('v3:test:'); + expect(res.body?.config).toEqual( + expect.objectContaining({ + overrides: { ocr: { apiKeyPreview: expect.any(String) } }, + }), + ); + expect(res.body?.revision).toEqual( + expect.objectContaining({ + overrides: { ocr: { apiKeyPreview: expect.any(String) } }, + }), + ); + }); + + it('rejects a single oversized deeply nested reset path', async () => { + const { handlers, deps } = createHandlers(); + const deepResetPath = Array.from({ length: 33 }, (_, index) => `seg${index}`).join('.'); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 0, + resetPaths: [deepResetPath], + cause: 'save', + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(400); + expect(res.body?.error).toMatch(/maximum depth of 32 segments/); + expect(deps.mutateConfigWithRevision).not.toHaveBeenCalled(); + }); + + it('rejects oversized combined entries and resetPaths', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 0, + entries: Array.from({ length: 51 }, (_, index) => ({ + fieldPath: `field${index}`, + value: true, + })), + resetPaths: Array.from({ length: 50 }, (_, index) => `reset${index}`), + cause: 'save', + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(400); + expect(res.body?.error).toBe('combined entries and resetPaths exceed maximum of 100'); + expect(deps.mutateConfigWithRevision).not.toHaveBeenCalled(); + }); + + it('returns 409 on version conflict', async () => { + const { handlers } = createHandlers({ + mutateConfigWithRevision: jest.fn().mockRejectedValue( + Object.assign(new Error('Config version conflict'), { + name: 'ConfigVersionConflictError', + currentVersion: 7, + }), + ), + }); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 5, + entries: [{ fieldPath: 'cache', value: true }], + cause: 'save', + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(409); + expect(res.body).toEqual({ error: 'Config version conflict', currentVersion: 7 }); + }); + + it('applies fields mutation then invalidates caches', async () => { + const invalidateConfigCaches = jest.fn().mockResolvedValue(undefined); + const { handlers, deps } = createHandlers({ invalidateConfigCaches }); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 5, + resetPaths: ['registration.enabled'], + entries: [{ fieldPath: 'cache', value: true }], + cause: 'save', + priority: 0, + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(200); + expect(deps.mutateConfigWithRevision).toHaveBeenCalledWith( + expect.objectContaining({ + expectedVersion: 5, + cause: 'save', + op: expect.objectContaining({ + kind: 'fields', + resetPaths: ['registration.enabled'], + fields: { cache: true }, + }), + }), + ); + expect(invalidateConfigCaches).toHaveBeenCalled(); + expect(res.body).toEqual({ + changed: true, + config: { _id: 'c1', configVersion: 6, overrides: { cache: true } }, + revision: { id: 'rev-1', status: 'final', configVersion: 5 }, + }); + }); + + it('strips protected ancestor and alias entries/reset paths in atomic fields mode', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 5, + resetPaths: ['interface', 'interfaceConfig.prompts', 'registration.enabled'], + entries: [ + { fieldPath: 'interface', value: null }, + { fieldPath: 'interfaceConfig.prompts', value: false }, + { fieldPath: 'cache', value: true }, + ], + cause: 'save', + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(200); + expect(deps.mutateConfigWithRevision).toHaveBeenCalledWith( + expect.objectContaining({ + op: expect.objectContaining({ + kind: 'fields', + resetPaths: ['registration.enabled'], + fields: { cache: true }, + }), + }), + ); + }); + + it('allows actionable fields when blocked reset paths are stripped before section grants', async () => { + const { handlers, deps } = createHandlers({ + hasConfigCapability: jest.fn().mockImplementation((_user, section: string | null) => { + if (section == null) { + return Promise.resolve(false); + } + return Promise.resolve(section === 'registration'); + }), + }); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 5, + resetPaths: ['interface'], + entries: [{ fieldPath: 'registration.enabled', value: false }], + cause: 'save', + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(200); + expect(deps.mutateConfigWithRevision).toHaveBeenCalledWith( + expect.objectContaining({ + op: expect.objectContaining({ + kind: 'fields', + resetPaths: [], + fields: { 'registration.enabled': false }, + }), + }), + ); + expect(res.body).toEqual({ changed: true, configVersion: 6, revisionId: 'rev-1' }); + expect(res.body).not.toHaveProperty('config'); + expect(res.body).not.toHaveProperty('revision'); + }); + + it('returns 403 for protected-only no-op atomic fields requests without section grants', async () => { + const { handlers, deps } = createHandlers({ + hasConfigCapability: jest.fn().mockResolvedValue(false), + }); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 5, + resetPaths: ['interface'], + entries: [{ fieldPath: 'interfaceConfig.prompts', value: false }], + cause: 'save', + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(403); + expect(deps.mutateConfigWithRevision).not.toHaveBeenCalled(); + }); + + it('returns 409 on stale expectedVersion for protected-only no-op with broad manage', async () => { + const { handlers, deps } = createHandlers({ + hasConfigCapability: jest.fn().mockImplementation((_user, section: string | null) => { + if (section == null) { + return Promise.resolve(true); + } + return Promise.resolve(false); + }), + findConfigByPrincipal: jest.fn().mockResolvedValue({ configVersion: 3, priority: 10 }), + }); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 1, + resetPaths: ['interface'], + entries: [{ fieldPath: 'interfaceConfig.prompts', value: false }], + cause: 'save', + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(409); + expect(res.body).toEqual({ error: 'Config version conflict', currentVersion: 3 }); + expect(deps.mutateConfigWithRevision).not.toHaveBeenCalled(); + }); + + it('rejects non-base principals', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'group', principalId: 'g1' }, + body: { expectedVersion: 1, entries: [{ fieldPath: 'cache', value: true }], cause: 'save' }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(400); + expect(deps.mutateConfigWithRevision).not.toHaveBeenCalled(); + }); + + it('strips interface permission fields from atomic replace overrides', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 5, + cause: 'import', + overrides: { + cache: true, + interface: { prompts: false, modelSelect: true }, + }, + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(200); + expect(deps.mutateConfigWithRevision).toHaveBeenCalledWith( + expect.objectContaining({ + op: expect.objectContaining({ + kind: 'replace', + overrides: { cache: true, interface: { modelSelect: true } }, + }), + }), + ); + }); + + it('strips non-object interface and internal aliases from atomic replace overrides', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 5, + cause: 'import', + overrides: { + cache: true, + interface: null, + interfaceConfig: { prompts: false }, + }, + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(200); + expect(deps.mutateConfigWithRevision).toHaveBeenCalledWith( + expect.objectContaining({ + op: expect.objectContaining({ + kind: 'replace', + overrides: { cache: true }, + }), + }), + ); + }); + + it('forwards restoreRevisionId as a restore operation', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 5, + cause: 'restore', + restoreRevisionId: '11111111-1111-4111-8111-111111111111', + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(200); + expect(deps.mutateConfigWithRevision).toHaveBeenCalledWith( + expect.objectContaining({ + op: { kind: 'restore', revisionId: '11111111-1111-4111-8111-111111111111' }, + cause: 'restore', + }), + ); + }); + + it('derives cause from the mutation mode instead of the client label', async () => { + const { handlers, deps } = createHandlers(); + const req = mockReq({ + params: { principalType: 'role', principalId: '__base__' }, + body: { + expectedVersion: 5, + cause: 'save', + restoreRevisionId: '11111111-1111-4111-8111-111111111111', + }, + }); + const res = mockRes(); + await handlers.mutateConfigAtomic(req, res); + expect(res.statusCode).toBe(200); + expect(deps.mutateConfigWithRevision).toHaveBeenCalledWith( + expect.objectContaining({ cause: 'restore' }), + ); + }); +}); diff --git a/packages/api/src/admin/config.spec.ts b/packages/api/src/admin/config.spec.ts index cee1246c20d..13862e23e51 100644 --- a/packages/api/src/admin/config.spec.ts +++ b/packages/api/src/admin/config.spec.ts @@ -10,11 +10,8 @@ describe('isValidFieldPath', () => { it('rejects empty and non-string', () => { expect(isValidFieldPath('')).toBe(false); - // @ts-expect-error testing invalid input expect(isValidFieldPath(undefined)).toBe(false); - // @ts-expect-error testing invalid input expect(isValidFieldPath(null)).toBe(false); - // @ts-expect-error testing invalid input expect(isValidFieldPath(42)).toBe(false); }); @@ -50,6 +47,13 @@ describe('isValidFieldPath', () => { expect(isValidFieldPath('a.$set')).toBe(false); expect(isValidFieldPath('$')).toBe(false); }); + + it('rejects paths that exceed shared length or depth limits', () => { + expect(isValidFieldPath('a'.repeat(513))).toBe(false); + expect( + isValidFieldPath(Array.from({ length: 33 }, (_, index) => `seg${index}`).join('.')), + ).toBe(false); + }); }); describe('getTopLevelSection', () => { diff --git a/packages/api/src/admin/config.ts b/packages/api/src/admin/config.ts index ef30bbad4e8..ff05a2634cd 100644 --- a/packages/api/src/admin/config.ts +++ b/packages/api/src/admin/config.ts @@ -1,17 +1,31 @@ -import { logger, BASE_CONFIG_PRINCIPAL_ID } from '@librechat/data-schemas'; +import { + logger, + BASE_CONFIG_PRINCIPAL_ID, + canonicalizeResetPaths, + fieldPathPolicyError, + indexedArrayPathError, + isForbiddenAdminConfigPath, + isValidFieldPath, + sanitizeAdminConfigOverrides, +} from '@librechat/data-schemas'; import { BASE_PRINCIPAL_CONFIG_SECTIONS, BASE_ONLY_CONFIG_SECTIONS, PrincipalType, PrincipalModel, - INTERFACE_PERMISSION_FIELDS, RUNTIME_CONFIG_INTERFACE_FIELDS, - PERMISSION_SUB_KEYS, hasProcessMCPServerConfig, isProcessMCPServerConfig, isProcessMCPServerField, } from 'librechat-data-provider'; -import type { AppConfig, ConfigSection, IConfig, SystemCapability } from '@librechat/data-schemas'; +import type { + AppConfig, + ConfigRevisionSnapshot, + ConfigSection, + FindConfigByPrincipalOptions, + IConfig, + SystemCapability, +} from '@librechat/data-schemas'; import type { TCustomConfig } from 'librechat-data-provider'; import type { Types, ClientSession } from 'mongoose'; import type { Response } from 'express'; @@ -30,8 +44,15 @@ import { redactConfigSecrets, } from './secrets'; -const UNSAFE_SEGMENTS = /(?:^|\.)(__[\w]*|constructor|prototype)(?:\.|$)/; +type ConfigRevisionCause = 'save' | 'import' | 'reset' | 'restore'; +type ConfigMutationOp = + | { kind: 'fields'; resetPaths: string[]; fields: Record; priority: number } + | { kind: 'replace'; overrides: Record; priority: number } + | { kind: 'delete' } + | { kind: 'restore'; revisionId: string }; + const MAX_PATCH_ENTRIES = 100; +const MAX_PATCH_MUTATIONS = 100; const DEFAULT_PRIORITY = 10; const BASE_ONLY_OVERRIDE_SECTIONS = new Set(BASE_ONLY_CONFIG_SECTIONS); const BASE_PRINCIPAL_OVERRIDE_SECTIONS = new Set(BASE_PRINCIPAL_CONFIG_SECTIONS); @@ -73,15 +94,112 @@ function hasLangfuseHeadersOverride(rawOverrides: Record): bool return Object.keys(rawLangfuse).some((key) => key === 'headers' || key.startsWith('headers.')); } -export function isValidFieldPath(path: string): boolean { +type AtomicFieldEntry = { fieldPath: string; value: unknown }; + +function isPlainObject(value: unknown): value is Record { + return value != null && typeof value === 'object' && !Array.isArray(value); +} + +function parseAtomicFieldEntries( + value: unknown, +): { ok: true; entries: AtomicFieldEntry[] } | { ok: false; error: string } { + if (value === undefined) { + return { ok: true, entries: [] }; + } + if (!Array.isArray(value)) { + return { ok: false, error: 'entries must be an array' }; + } + const entries: AtomicFieldEntry[] = []; + for (const entry of value) { + if (!isPlainObject(entry)) { + return { ok: false, error: 'each entry must be an object with fieldPath and value' }; + } + if (typeof entry.fieldPath !== 'string') { + return { ok: false, error: 'each entry must include a fieldPath string' }; + } + const fieldPathError = fieldPathPolicyError(entry.fieldPath); + if (fieldPathError) { + return { ok: false, error: fieldPathError }; + } + if (!Object.prototype.hasOwnProperty.call(entry, 'value')) { + return { ok: false, error: 'each entry must include a value property' }; + } + entries.push({ fieldPath: entry.fieldPath, value: entry.value }); + } + return { ok: true, entries }; +} + +function parseAtomicResetPaths( + value: unknown, +): { ok: true; resetPaths: string[] } | { ok: false; error: string } { + if (value === undefined) { + return { ok: true, resetPaths: [] }; + } + if (!Array.isArray(value)) { + return { ok: false, error: 'resetPaths must be an array' }; + } + for (const path of value) { + if (typeof path !== 'string') { + return { ok: false, error: 'each resetPaths element must be a string' }; + } + const fieldPathError = fieldPathPolicyError(path); + if (fieldPathError) { + return { ok: false, error: fieldPathError }; + } + } + return { ok: true, resetPaths: value }; +} + +function validateAtomicMutationProperties( + body: Record, +): { ok: true } | { ok: false; error: string } { + if ('entries' in body && body.entries !== undefined && !Array.isArray(body.entries)) { + return { ok: false, error: 'entries must be an array' }; + } + if ('resetPaths' in body && body.resetPaths !== undefined && !Array.isArray(body.resetPaths)) { + return { ok: false, error: 'resetPaths must be an array' }; + } + if ('overrides' in body && body.overrides !== undefined && !isPlainObject(body.overrides)) { + return { ok: false, error: 'overrides must be an object' }; + } + if ( + 'deleteDocument' in body && + body.deleteDocument !== undefined && + typeof body.deleteDocument !== 'boolean' + ) { + return { ok: false, error: 'deleteDocument must be a boolean' }; + } + if ('restoreRevisionId' in body && body.restoreRevisionId !== undefined) { + if (typeof body.restoreRevisionId !== 'string' || body.restoreRevisionId.length === 0) { + return { ok: false, error: 'restoreRevisionId must be a non-empty string' }; + } + } + return { ok: true }; +} + +export { isValidFieldPath } from '@librechat/data-schemas'; + +function isConfigVersionConflict(error: unknown): error is { currentVersion: number | null } { + return ( + typeof error === 'object' && + error != null && + (error as { name?: string }).name === 'ConfigVersionConflictError' + ); +} + +function isConfigRevisionNotFound(error: unknown): boolean { + return ( + typeof error === 'object' && + error != null && + (error as { name?: string }).name === 'ConfigRevisionNotFoundError' + ); +} + +function isTransactionRequired(error: unknown): error is Error { return ( - typeof path === 'string' && - path.length > 0 && - !path.startsWith('.') && - !path.endsWith('.') && - !path.includes('..') && - !path.includes('$') && - !UNSAFE_SEGMENTS.test(path) + typeof error === 'object' && + error != null && + (error as { name?: string }).name === 'TransactionRequiredError' ); } @@ -106,35 +224,35 @@ function isProcessMCPServerFieldPath(fieldPath: string, value: unknown): boolean return isProcessMCPServerField(field) || (field === 'type' && value === 'stdio'); } -/** - * Returns true if `fieldPath` targets an interface permission field or permission sub-key. - * - * - `"interface.prompts"` → true (boolean permission field) - * - `"interface.agents.use"` → true (permission sub-key) - * - `"interface.mcpServers"` → true (entire composite field) - * - `"interface.mcpServers.use"` → true (permission sub-key) - * - `"interface.mcpServers.placeholder"` → false (UI-only sub-key) - * - `"interface.peoplePicker.users"` → true (all peoplePicker sub-keys are permissions) - * - `"interface.modelSelect"` → false (UI-only field) - */ -function isInterfacePermissionPath(fieldPath: string): boolean { - const parts = fieldPath.split('.'); - if (parts[0] !== 'interface' || parts.length < 2) { - return false; - } - if (!INTERFACE_PERMISSION_FIELDS.has(parts[1])) { - return false; +function isBlockedFieldPath(fieldPath: string): boolean { + return isBaseOnlyFieldPath(fieldPath) || isForbiddenAdminConfigPath(fieldPath); +} + +function sanitizeConfigOverrides(overrides: Record): Partial { + const normalized = { ...overrides }; + delete normalized.interfaceConfig; + if ( + 'interface' in normalized && + (normalized.interface == null || + typeof normalized.interface !== 'object' || + Array.isArray(normalized.interface)) + ) { + delete normalized.interface; } - // "interface." with no sub-key → permission (blocks the whole field), - // EXCEPT dual-purpose runtime fields (e.g. schedules) whose bare top-level value - // is a runtime enable toggle, not a permission — those must pass through so admin - // field patches/tombstones can set or clear them (their .use/.create permission - // sub-keys are still blocked below). - if (parts.length === 2) { - return !RUNTIME_CONFIG_INTERFACE_FIELDS.has(parts[1]); + const sanitized = sanitizeAdminConfigOverrides(normalized) as Record; + if ( + sanitized.interface != null && + typeof sanitized.interface === 'object' && + !Array.isArray(sanitized.interface) + ) { + const interfaceConfig = sanitized.interface as Record; + for (const field of RUNTIME_CONFIG_INTERFACE_FIELDS) { + if (Object.prototype.hasOwnProperty.call(interfaceConfig, field)) { + interfaceConfig[field] = normalizeRuntimeInterfaceValue(field, interfaceConfig[field]); + } + } } - // "interface.." → only block if sub-key is a permission bit - return PERMISSION_SUB_KEYS.has(parts[2]); + return sanitized as Partial; } /** @@ -171,7 +289,7 @@ export interface AdminConfigDeps { findConfigByPrincipal: ( principalType: PrincipalType, principalId: string | Types.ObjectId, - options?: { includeInactive?: boolean }, + options?: FindConfigByPrincipalOptions, session?: ClientSession, ) => Promise; upsertConfig: ( @@ -188,7 +306,7 @@ export interface AdminConfigDeps { principalId: string | Types.ObjectId, principalModel: PrincipalModel, fields: Record, - priority: number, + priority?: number, session?: ClientSession, ) => Promise; tombstoneConfigField: ( @@ -196,7 +314,7 @@ export interface AdminConfigDeps { principalId: string | Types.ObjectId, principalModel: PrincipalModel, fieldPath: string, - priority: number, + priority?: number, session?: ClientSession, ) => Promise; unsetConfigField: ( @@ -218,6 +336,19 @@ export interface AdminConfigDeps { session?: ClientSession, options?: { expectEmpty?: boolean }, ) => Promise; + mutateConfigWithRevision: (params: { + principalType: PrincipalType; + principalId: string | Types.ObjectId; + principalModel: PrincipalModel; + expectedVersion: number | null; + op: ConfigMutationOp; + cause: ConfigRevisionCause; + actor: { actorId: string; actorEmail?: string; tenantId: string }; + }) => Promise<{ + changed: boolean; + config: IConfig | null; + revision: ConfigRevisionSnapshot | null; + }>; hasConfigCapability: ( user: CapabilityUser, section: ConfigSection | null, @@ -411,6 +542,12 @@ function redactAppConfigForResponse(appConfig: AppConfig): AppConfig { return safeConfig; } +function redactRevisionForResponse(revision: ConfigRevisionSnapshot): ConfigRevisionSnapshot { + const safeRevision = JSON.parse(JSON.stringify(revision)) as ConfigRevisionSnapshot; + redactConfigSecrets(safeRevision.overrides); + return safeRevision; +} + function preservePatchedConfigSecretFields( fields: Record, existingOverrides?: unknown, @@ -436,6 +573,7 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { deleteConfigField: (req: ServerRequest, res: Response) => Promise; deleteConfigOverrides: (req: ServerRequest, res: Response) => Promise; toggleConfig: (req: ServerRequest, res: Response) => Promise; + mutateConfigAtomic: (req: ServerRequest, res: Response) => Promise; } { const { listAllConfigs, @@ -446,6 +584,7 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { unsetConfigField, deleteConfig, toggleConfigActive, + mutateConfigWithRevision, hasConfigCapability, hasAnyConfigReadAccess = async () => false, getReadableConfigSections = async (u, sections) => { @@ -623,17 +762,7 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { return res.status(403).json({ error: 'Insufficient permissions' }); } - const filteredOverrides = { - ...(overrides as Record), - } as Partial; - for (const section of BASE_ONLY_OVERRIDE_SECTIONS) { - if (section in filteredOverrides) { - delete (filteredOverrides as Record)[section]; - logger.warn( - `[adminConfig] Stripping base-only config section "${section}" - configure it in librechat.yaml instead`, - ); - } - } + const filteredOverrides = sanitizeConfigOverrides(rawOverrides); for (const key of Object.keys(filteredOverrides)) { const section = getTopLevelSection(key); if (BASE_PRINCIPAL_OVERRIDE_SECTIONS.has(section)) { @@ -643,45 +772,6 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { ); } } - const iface = (overrides as Record).interface; - if (iface != null && typeof iface === 'object' && !Array.isArray(iface)) { - const filteredIface: Record = {}; - for (const [field, rawVal] of Object.entries(iface as Record)) { - const val = normalizeRuntimeInterfaceValue(field, rawVal); - if (!INTERFACE_PERMISSION_FIELDS.has(field)) { - filteredIface[field] = val; - } else if (val != null && typeof val === 'object' && !Array.isArray(val)) { - // Composite permission field (e.g. mcpServers): strip permission - // sub-keys but preserve UI-only sub-keys like placeholder/trustCheckbox. - const uiOnly: Record = {}; - for (const [sub, subVal] of Object.entries(val as Record)) { - if (!PERMISSION_SUB_KEYS.has(sub)) { - uiOnly[sub] = subVal; - } else { - logger.warn( - `[adminConfig] Stripping interface permission sub-field "${field}.${sub}" — use role permissions instead`, - ); - } - } - if (Object.keys(uiOnly).length > 0) { - filteredIface[field] = uiOnly; - } - } else if (RUNTIME_CONFIG_INTERFACE_FIELDS.has(field)) { - // Dual-purpose field: the boolean form is a runtime disable, not a - // permission toggle, so preserve it (e.g. schedules: false). - filteredIface[field] = val; - } else { - logger.warn( - `[adminConfig] Stripping interface permission field "${field}" — use role permissions instead`, - ); - } - } - if (Object.keys(filteredIface).length > 0) { - (filteredOverrides as Record).interface = filteredIface; - } else { - delete (filteredOverrides as Record).interface; - } - } const overrideSections = Object.keys(filteredOverrides); @@ -781,16 +871,23 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { return res.status(400).json({ error: `Invalid principalType: ${principalType}` }); } - const { entries, priority } = req.body as { - entries?: Array<{ fieldPath: string; value: unknown }>; - priority?: number; - }; + const rawBody: unknown = req.body; + if (!isPlainObject(rawBody)) { + return res.status(400).json({ error: 'request body must be a JSON object' }); + } + + const { priority } = rawBody; + const parsedEntries = parseAtomicFieldEntries(rawBody.entries); + if (!parsedEntries.ok) { + return res.status(400).json({ error: parsedEntries.error }); + } + const entries = parsedEntries.entries; if (priority != null && (typeof priority !== 'number' || priority < 0)) { return res.status(400).json({ error: 'priority must be a non-negative number' }); } - if (!Array.isArray(entries) || entries.length === 0) { + if (entries.length === 0) { return res.status(400).json({ error: 'entries array is required and must not be empty' }); } @@ -826,6 +923,10 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { error: `Cannot patch protected secret ancestor as an array: ${entry.fieldPath}`, }); } + const indexedErr = indexedArrayPathError(entry.fieldPath); + if (indexedErr) { + return res.status(400).json({ error: indexedErr }); + } } const user = getCapabilityUser(req); @@ -839,9 +940,9 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { value: normalizeInterfaceFieldPatch(entry.fieldPath, entry.value), })) .filter((entry) => { - if (isBaseOnlyFieldPath(entry.fieldPath)) { + if (isBlockedFieldPath(entry.fieldPath)) { logger.warn( - `[adminConfig] Stripping base-only config field "${entry.fieldPath}" - configure it in librechat.yaml instead`, + `[adminConfig] Stripping protected config field "${entry.fieldPath}" — use canonical YAML UI fields only`, ); return false; } @@ -851,12 +952,6 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { ); return false; } - if (isInterfacePermissionPath(entry.fieldPath)) { - logger.warn( - `[adminConfig] Stripping interface permission field "${entry.fieldPath}" — use role permissions instead`, - ); - return false; - } return true; }); @@ -901,15 +996,14 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { `[adminConfig] Ignoring caller-supplied priority on section-scoped patch to ${principalType}/${principalId}: only broad manage:configs may modify document priority`, ); } - const requestedPriority = hasBroadManage ? priority : undefined; + const requestedPriority = hasBroadManage ? (priority as number | undefined) : undefined; const hasObjectValuedSecretPatch = Object.entries(fields).some(([fieldPath, value]) => isConfigSecretPreservablePatch(fieldPath, value), ); - const existing = - requestedPriority == null || hasObjectValuedSecretPatch - ? await findConfigByPrincipal(principalType, principalId, { includeInactive: true }) - : null; + const existing = hasObjectValuedSecretPatch + ? await findConfigByPrincipal(principalType, principalId, { includeInactive: true }) + : null; const encryptedFields = encryptConfigSecretFields(fields); const preservedFields = preservePatchedConfigSecretFields( encryptedFields, @@ -921,7 +1015,7 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { principalId, principalModel(principalType), preservedFields, - requestedPriority ?? existing?.priority ?? DEFAULT_PRIORITY, + requestedPriority, ); invalidateConfigCaches?.(user.tenantId)?.catch((err) => @@ -968,6 +1062,10 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { if (secretInputError) { return res.status(400).json({ error: secretInputError }); } + const tombstoneIndexedErr = indexedArrayPathError(fieldPath); + if (tombstoneIndexedErr) { + return res.status(400).json({ error: tombstoneIndexedErr }); + } const user = getCapabilityUser(req); if (!user) { @@ -991,16 +1089,9 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { }); } - if (isBaseOnlyFieldPath(fieldPath)) { + if (isBlockedFieldPath(fieldPath)) { logger.warn( - `[adminConfig] Ignoring tombstone for base-only config field "${fieldPath}" - configure it in librechat.yaml instead`, - ); - return res.status(200).json({ message: 'No actionable field path provided' }); - } - - if (isInterfacePermissionPath(fieldPath)) { - logger.warn( - `[adminConfig] Ignoring tombstone for interface permission field "${fieldPath}" — use role permissions instead`, + `[adminConfig] Ignoring tombstone for protected config field "${fieldPath}" — use canonical YAML UI fields only`, ); return res.status(200).json({ message: 'No actionable field path provided' }); } @@ -1018,11 +1109,6 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { } const requestedPriority = hasBroadManage ? priority : undefined; - const existing = - requestedPriority == null - ? await findConfigByPrincipal(principalType, principalId, { includeInactive: true }) - : null; - let config: IConfig | null = null; for (const path of getConfigSecretMutationPaths(fieldPath)) { const fieldConfig = await writeConfigTombstone( @@ -1030,7 +1116,7 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { principalId, principalModel(principalType), path, - requestedPriority ?? existing?.priority ?? DEFAULT_PRIORITY, + requestedPriority, ); if (fieldConfig) { config = fieldConfig; @@ -1073,6 +1159,10 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { if (secretInputError) { return res.status(400).json({ error: secretInputError }); } + const unsetIndexedErr = indexedArrayPathError(fieldPath); + if (unsetIndexedErr) { + return res.status(400).json({ error: unsetIndexedErr }); + } const user = getCapabilityUser(req); if (!user) { @@ -1096,9 +1186,9 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { }); } - if (isBaseOnlyFieldPath(fieldPath)) { + if (isBlockedFieldPath(fieldPath)) { logger.warn( - `[adminConfig] Ignoring delete for base-only config field "${fieldPath}" - configure it in librechat.yaml instead`, + `[adminConfig] Ignoring delete for protected config field "${fieldPath}" — use canonical YAML UI fields only`, ); return res.status(200).json({ message: 'No actionable field path provided' }); } @@ -1110,13 +1200,6 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { return res.status(200).json({ message: 'No actionable field path provided' }); } - if (isInterfacePermissionPath(fieldPath)) { - logger.warn( - `[adminConfig] Ignoring delete for interface permission field "${fieldPath}" — use role permissions instead`, - ); - return res.status(200).json({ message: 'No actionable field path provided' }); - } - let config: IConfig | null = null; for (const path of getConfigSecretMutationPaths(fieldPath)) { const fieldConfig = await unsetConfigField(principalType, principalId, path); @@ -1257,6 +1340,375 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { } } + /** + * POST /:principalType/:principalId/atomic + * Compare-and-set mutation that snapshots the predecessor and inserts a finalized + * config revision in the same Mongo transaction, then invalidates caches. + */ + async function mutateConfigAtomic(req: ServerRequest, res: Response): Promise { + try { + const { principalType, principalId } = req.params as { + principalType: string; + principalId: string; + }; + if (!validatePrincipalType(principalType)) { + return res.status(400).json({ error: `Invalid principalType: ${principalType}` }); + } + if (principalType !== PrincipalType.ROLE || principalId !== BASE_CONFIG_PRINCIPAL_ID) { + return res.status(400).json({ + error: 'Atomic config revisions are only supported for the base configuration', + }); + } + + const user = getCapabilityUser(req); + if (!user) { + return res.status(401).json({ error: 'Authentication required' }); + } + + const rawBody: unknown = req.body; + if (!isPlainObject(rawBody)) { + return res.status(400).json({ error: 'request body must be a JSON object' }); + } + const body = rawBody; + + const propertyValidation = validateAtomicMutationProperties(body); + if (!propertyValidation.ok) { + return res.status(400).json({ error: propertyValidation.error }); + } + + const { expectedVersion, priority } = body; + if ( + expectedVersion != null && + (typeof expectedVersion !== 'number' || + !Number.isInteger(expectedVersion) || + expectedVersion < 0) + ) { + return res + .status(400) + .json({ error: 'expectedVersion must be a non-negative integer or null' }); + } + if (!('expectedVersion' in body)) { + return res.status(400).json({ error: 'expectedVersion is required' }); + } + if (priority != null && (typeof priority !== 'number' || priority < 0)) { + return res.status(400).json({ error: 'priority must be a non-negative number' }); + } + + const parsedEntries = parseAtomicFieldEntries(body.entries); + if (!parsedEntries.ok) { + return res.status(400).json({ error: parsedEntries.error }); + } + const parsedResets = parseAtomicResetPaths(body.resetPaths); + if (!parsedResets.ok) { + return res.status(400).json({ error: parsedResets.error }); + } + const entries = parsedEntries.entries; + const rawResetPaths = parsedResets.resetPaths; + if (entries.length + rawResetPaths.length > MAX_PATCH_MUTATIONS) { + return res.status(400).json({ + error: `combined entries and resetPaths exceed maximum of ${MAX_PATCH_MUTATIONS}`, + }); + } + + const CAUSES = new Set(['save', 'import', 'reset', 'restore']); + if (body.cause != null && (typeof body.cause !== 'string' || !CAUSES.has(body.cause))) { + return res.status(400).json({ error: 'cause must be one of save, import, reset, restore' }); + } + + const hasEntries = entries.length > 0; + const hasResets = rawResetPaths.length > 0; + const hasOverrides = isPlainObject(body.overrides); + const deleteDocument = body.deleteDocument === true; + const restoreRevisionId = + typeof body.restoreRevisionId === 'string' && body.restoreRevisionId.length > 0 + ? body.restoreRevisionId + : undefined; + const modeCount = + Number(hasEntries || hasResets) + + Number(hasOverrides) + + Number(deleteDocument) + + Number(Boolean(restoreRevisionId)); + if ( + modeCount !== 1 && + !(hasEntries && hasResets && !hasOverrides && !deleteDocument && !restoreRevisionId) + ) { + if (!hasEntries && !hasResets && !hasOverrides && !deleteDocument && !restoreRevisionId) { + return res.status(400).json({ + error: 'Provide resetPaths, entries, overrides, deleteDocument, or restoreRevisionId', + }); + } + if (deleteDocument && (hasEntries || hasResets || hasOverrides || restoreRevisionId)) { + return res + .status(400) + .json({ error: 'deleteDocument cannot be combined with field mutations' }); + } + if (hasOverrides && (hasEntries || hasResets || restoreRevisionId)) { + return res + .status(400) + .json({ error: 'overrides cannot be combined with entries or resetPaths' }); + } + if (restoreRevisionId && (hasEntries || hasResets || hasOverrides || deleteDocument)) { + return res + .status(400) + .json({ error: 'restoreRevisionId cannot be combined with other mutations' }); + } + } + + const hasBroadManage = await hasConfigCapability(user, null, 'manage'); + if (deleteDocument || hasOverrides || restoreRevisionId) { + if (!hasBroadManage) { + return res.status(403).json({ error: 'Insufficient permissions' }); + } + } + + let op: ConfigMutationOp; + if (restoreRevisionId) { + op = { kind: 'restore', revisionId: restoreRevisionId }; + } else if (deleteDocument) { + op = { kind: 'delete' }; + } else if (hasOverrides) { + const rawOverrides = body.overrides as Record; + if ( + hasProcessMCPServerConfig(rawOverrides.mcpServers) || + hasProcessMCPServerConfig(rawOverrides.mcpConfig) + ) { + return res.status(400).json({ error: PROCESS_MCP_CONFIG_ERROR }); + } + if (hasLangfuseHeadersOverride(rawOverrides)) { + return res.status(400).json({ error: LANGFUSE_HEADERS_CONFIG_ERROR }); + } + const sanitizedOverrides = sanitizeConfigOverrides(rawOverrides) as Record; + for (const section of BASE_PRINCIPAL_OVERRIDE_SECTIONS) { + delete sanitizedOverrides[section]; + } + for (const section of getConfigSecretSections()) { + const secretInputError = getConfigSecretInputError(section, sanitizedOverrides[section]); + if (secretInputError) { + return res.status(400).json({ error: secretInputError }); + } + } + const existing = await findConfigByPrincipal(principalType, principalId, { + includeInactive: true, + ...(user.tenantId !== undefined ? { tenantId: user.tenantId } : {}), + }); + const encryptedOverrides = encryptConfigSecrets(sanitizedOverrides); + op = { + kind: 'replace', + overrides: preserveConfigSecrets(encryptedOverrides, existing?.overrides), + priority: hasBroadManage + ? ((priority as number | null | undefined) ?? existing?.priority ?? DEFAULT_PRIORITY) + : DEFAULT_PRIORITY, + }; + } else { + if (entries.length > MAX_PATCH_ENTRIES) { + return res + .status(400) + .json({ error: `entries array exceeds maximum of ${MAX_PATCH_ENTRIES}` }); + } + for (const entry of entries) { + if (!isValidFieldPath(entry.fieldPath)) { + return res + .status(400) + .json({ error: `Invalid or unsafe field path: ${entry.fieldPath}` }); + } + if (isProcessMCPServerFieldPath(entry.fieldPath, entry.value)) { + return res.status(400).json({ error: PROCESS_MCP_CONFIG_ERROR }); + } + if (isLangfuseHeadersFieldPath(entry.fieldPath)) { + return res.status(400).json({ error: LANGFUSE_HEADERS_CONFIG_ERROR }); + } + if (isConfigSecretDescendantPath(entry.fieldPath)) { + return res + .status(400) + .json({ error: `Cannot patch inside protected secret path: ${entry.fieldPath}` }); + } + const secretInputError = getConfigSecretInputError(entry.fieldPath, entry.value); + if (secretInputError) { + return res.status(400).json({ error: secretInputError }); + } + if (Array.isArray(entry.value) && isConfigSecretAncestorPath(entry.fieldPath)) { + return res.status(400).json({ + error: `Cannot patch protected secret ancestor as an array: ${entry.fieldPath}`, + }); + } + const indexedErr = indexedArrayPathError(entry.fieldPath); + if (indexedErr) { + return res.status(400).json({ error: indexedErr }); + } + } + for (const path of rawResetPaths) { + if (!isValidFieldPath(path)) { + return res.status(400).json({ error: `Invalid or unsafe field path: ${path}` }); + } + if (isProcessMCPServerFieldPath(path, undefined)) { + return res.status(400).json({ error: PROCESS_MCP_CONFIG_ERROR }); + } + if (isLangfuseHeadersFieldPath(path)) { + return res.status(400).json({ error: LANGFUSE_HEADERS_CONFIG_ERROR }); + } + const secretInputError = getConfigSecretInputError(path, undefined); + if (secretInputError) { + return res.status(400).json({ error: secretInputError }); + } + const indexedErr = indexedArrayPathError(path); + if (indexedErr) { + return res.status(400).json({ error: indexedErr }); + } + } + + const validResets = canonicalizeResetPaths( + rawResetPaths + .filter( + (path) => + !isBlockedFieldPath(path) && + !BASE_PRINCIPAL_OVERRIDE_SECTIONS.has(getTopLevelSection(path)), + ) + .flatMap(getConfigSecretMutationPaths), + ); + const seen = new Set(); + const rawFields: Record = {}; + for (const entry of entries.map((item) => ({ + ...item, + value: normalizeInterfaceFieldPatch(item.fieldPath, item.value), + }))) { + if (isBlockedFieldPath(entry.fieldPath)) { + continue; + } + if (BASE_PRINCIPAL_OVERRIDE_SECTIONS.has(getTopLevelSection(entry.fieldPath))) { + continue; + } + if (seen.has(entry.fieldPath)) { + return res.status(400).json({ error: `Duplicate fieldPath: ${entry.fieldPath}` }); + } + seen.add(entry.fieldPath); + rawFields[entry.fieldPath] = entry.value; + } + + const requestedSections = [ + ...new Set([ + ...Object.keys(rawFields).map((fieldPath) => getTopLevelSection(fieldPath)), + ...validResets.map((path) => getTopLevelSection(path)), + ]), + ]; + + if (!hasBroadManage && requestedSections.length > 0) { + const requestedAllowed = await Promise.all( + requestedSections.map((section) => + hasConfigCapability(user, section as ConfigSection, 'manage'), + ), + ); + const requestedDenied = requestedSections.find((_, i) => !requestedAllowed[i]); + if (requestedDenied) { + return res + .status(403) + .json({ error: `Insufficient permissions for config section: ${requestedDenied}` }); + } + } + + const existing = await findConfigByPrincipal(principalType, principalId, { + includeInactive: true, + ...(user.tenantId !== undefined ? { tenantId: user.tenantId } : {}), + }); + const fields = preservePatchedConfigSecretFields( + encryptConfigSecretFields(rawFields), + existing?.overrides, + ); + + if (Object.keys(fields).length === 0 && validResets.length === 0) { + if (!hasBroadManage) { + return res.status(403).json({ error: 'Insufficient permissions' }); + } + const liveVersion = existing == null ? null : (existing.configVersion ?? 0); + if (expectedVersion !== liveVersion) { + return res.status(409).json({ + error: 'Config version conflict', + currentVersion: liveVersion, + }); + } + return res.status(200).json({ message: 'No actionable field entries provided' }); + } + + op = { + kind: 'fields', + resetPaths: validResets, + fields, + priority: hasBroadManage + ? ((priority as number | null | undefined) ?? existing?.priority ?? DEFAULT_PRIORITY) + : (existing?.priority ?? DEFAULT_PRIORITY), + }; + } + + const cause: ConfigRevisionCause = (() => { + if (op.kind === 'restore') { + return 'restore'; + } + if (op.kind === 'delete') { + return 'reset'; + } + if (op.kind === 'replace') { + return 'import'; + } + return Object.keys(op.fields).length === 0 ? 'reset' : 'save'; + })(); + + const { config, revision, changed } = await mutateConfigWithRevision({ + principalType, + principalId, + principalModel: principalModel(principalType), + expectedVersion: (expectedVersion as number | null | undefined) ?? null, + op, + cause, + actor: { + actorId: user.id, + actorEmail: (req.user as { email?: string } | undefined)?.email, + tenantId: user.tenantId ?? '', + }, + }); + + try { + await invalidateConfigCaches?.(user.tenantId); + } catch (err) { + logger.error('[adminConfig] Cache invalidation failed after atomic mutate:', err); + } + + if (!changed || revision == null) { + return res.status(200).json({ + changed: false, + configVersion: null, + revisionId: null, + }); + } + + if (!hasBroadManage) { + return res.status(200).json({ + changed: true, + configVersion: config?.configVersion ?? null, + revisionId: revision.id, + }); + } + return res.status(200).json({ + changed: true, + config: config ? redactConfigForResponse(config) : config, + revision: redactRevisionForResponse(revision), + }); + } catch (error) { + if (isConfigVersionConflict(error)) { + return res.status(409).json({ + error: 'Config version conflict', + currentVersion: error.currentVersion, + }); + } + if (isConfigRevisionNotFound(error)) { + return res.status(404).json({ error: 'Revision not found' }); + } + if (isTransactionRequired(error)) { + return res.status(503).json({ error: (error as Error).message }); + } + logger.error('[adminConfig] mutateConfigAtomic error:', error); + return res.status(500).json({ error: 'Failed to mutate config' }); + } + } + return { listConfigs, getBaseConfig, @@ -1267,5 +1719,6 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { deleteConfigField, deleteConfigOverrides, toggleConfig, + mutateConfigAtomic, }; } diff --git a/packages/api/src/admin/secrets.integration.spec.ts b/packages/api/src/admin/secrets.integration.spec.ts index 7b4a1ec2c2f..fb72f957a13 100644 --- a/packages/api/src/admin/secrets.integration.spec.ts +++ b/packages/api/src/admin/secrets.integration.spec.ts @@ -161,6 +161,10 @@ const SECRET_FIELD_CASES: SecretFieldCase[] = [ }, ]; +const GENERIC_CONFIG_SECRET_FIELD_CASES = SECRET_FIELD_CASES.filter( + ({ section }) => section !== 'langfuse', +); + /** Fields whose values conventionally hold `${ENV_VAR}` placeholder references. */ const PLACEHOLDER_CASES = [ { path: 'ocr.apiKey', placeholder: '${OCR_API_KEY}' }, @@ -252,6 +256,7 @@ beforeAll(async () => { unsetConfigField: methods.unsetConfigField, deleteConfig: methods.deleteConfig, toggleConfigActive: methods.toggleConfigActive, + mutateConfigWithRevision: methods.mutateConfigWithRevision, hasConfigCapability: async () => true, hasAnyConfigReadAccess: async () => true, hasCapability: async () => true, @@ -264,7 +269,10 @@ afterAll(async () => { }); describe('config secret registry — real handlers against a real Config collection', () => { - describe.each(SECRET_FIELD_CASES)( + // Langfuse is managed by its dedicated tenant-wide API and is intentionally + // excluded from generic config mutations. It remains in SECRET_FIELD_CASES so + // the shared registry shape and read-redaction coverage stay explicit. + describe.each(GENERIC_CONFIG_SECRET_FIELD_CASES)( '$path', ({ path, previewPath, section, object, siblingPath, siblingValue }) => { it('encrypts dotted patch writes at rest, sets the masked preview companion, and redacts the secret from the response', async () => { @@ -574,6 +582,11 @@ describe('config secret registry — real handlers against a real Config collect unsetConfigField: async () => null, deleteConfig: async () => null, toggleConfigActive: async () => null, + mutateConfigWithRevision: async () => ({ + changed: false, + config: null, + revision: null, + }), hasConfigCapability: async () => true, hasAnyConfigReadAccess: async () => true, hasCapability: async () => true, diff --git a/packages/data-schemas/src/admin/configOverrides.spec.ts b/packages/data-schemas/src/admin/configOverrides.spec.ts new file mode 100644 index 00000000000..241a58f1b49 --- /dev/null +++ b/packages/data-schemas/src/admin/configOverrides.spec.ts @@ -0,0 +1,212 @@ +import { + isForbiddenAdminConfigPath, + sanitizeAdminConfigOverrides, + sanitizeAdminConfigTombstones, +} from './configOverrides'; + +describe('sanitizeAdminConfigOverrides', () => { + it('keeps UI interface fields and strips permission flags', () => { + expect( + sanitizeAdminConfigOverrides({ + cache: true, + interface: { prompts: false, modelSelect: true }, + }), + ).toEqual({ + cache: true, + interface: { modelSelect: true }, + }); + }); + + it('strips non-object interface and internal alias keys', () => { + expect( + sanitizeAdminConfigOverrides({ + cache: true, + interface: null, + interfaceConfig: { prompts: false }, + }), + ).toEqual({ + cache: true, + }); + }); + + it('strips unknown descendants of permission fields including boolean fields', () => { + expect( + sanitizeAdminConfigOverrides({ + interface: { + runCode: { foo: true }, + mcpServers: { placeholder: 'Choose MCP', use: true, foo: 'nope' }, + modelSelect: true, + }, + }), + ).toEqual({ + interface: { + mcpServers: { placeholder: 'Choose MCP' }, + modelSelect: true, + }, + }); + }); + + it('rejects UI keys that belong to a different permission field', () => { + expect( + sanitizeAdminConfigOverrides({ + interface: { + runCode: { placeholder: 'nope' }, + prompts: { snapshotFiles: true }, + mcpServers: { verification: true, placeholder: 'Choose MCP' }, + skills: { defaultActiveOnShare: true, snapshotFiles: false }, + sharedLinks: { snapshotFiles: false, placeholder: 'nope' }, + modelSelect: true, + }, + }), + ).toEqual({ + interface: { + mcpServers: { placeholder: 'Choose MCP' }, + skills: { defaultActiveOnShare: true }, + sharedLinks: { snapshotFiles: false }, + modelSelect: true, + }, + }); + }); + + it('strips nested containers at primitive leaves (placeholder, verification)', () => { + expect( + sanitizeAdminConfigOverrides({ + interface: { + mcpServers: { placeholder: { foo: 'bad' } as never }, + marketplace: { verification: ['bad'] as never }, + skills: { defaultActiveOnShare: { nested: true } as never }, + sharedLinks: { snapshotFiles: false }, + }, + }), + ).toEqual({ + interface: { + sharedLinks: { snapshotFiles: false }, + }, + }); + }); + + it('allows localized label records but strips nested objects within them', () => { + expect( + sanitizeAdminConfigOverrides({ + interface: { + mcpServers: { + trustCheckbox: { + label: { en: 'Trust', fr: 'Confiance' }, + subLabel: { en: { nested: 'bad' } as never }, + }, + }, + }, + }), + ).toEqual({ + interface: { + mcpServers: { + trustCheckbox: { + label: { en: 'Trust', fr: 'Confiance' }, + }, + }, + }, + }); + }); + + it('preserves runtime interface settings while stripping their permission bits', () => { + expect( + sanitizeAdminConfigOverrides({ + interface: { + schedules: { use: true, create: true, maxPerUser: 2 }, + }, + }), + ).toEqual({ interface: { schedules: { maxPerUser: 2 } } }); + expect( + sanitizeAdminConfigOverrides({ + interface: { + schedules: { use: false, maxPerUser: 2 }, + }, + }), + ).toEqual({ interface: { schedules: { use: false, maxPerUser: 2 } } }); + }); +}); + +describe('sanitizeAdminConfigTombstones', () => { + it('strips forbidden interface permission tombstones', () => { + expect( + sanitizeAdminConfigTombstones(['interface.prompts', 'interface.modelSelect', 'cache']), + ).toEqual(['interface.modelSelect', 'cache']); + }); + + it('strips protected ancestors and internal alias tombstones', () => { + expect( + sanitizeAdminConfigTombstones([ + 'interface', + 'interface.mcpServers', + 'interfaceConfig.prompts', + 'interface.modelSelect', + ]), + ).toEqual(['interface.modelSelect']); + }); + + it('strips unknown descendants of permission fields', () => { + expect( + sanitizeAdminConfigTombstones([ + 'interface.runCode.foo', + 'interface.mcpServers.placeholder', + 'cache', + ]), + ).toEqual(['interface.mcpServers.placeholder', 'cache']); + }); + + it('strips UI keys that belong to a different permission field', () => { + expect( + sanitizeAdminConfigTombstones([ + 'interface.runCode.placeholder', + 'interface.prompts.snapshotFiles', + 'interface.mcpServers.trustCheckbox.label', + 'interface.marketplace.verification', + 'cache', + ]), + ).toEqual([ + 'interface.mcpServers.trustCheckbox.label', + 'interface.marketplace.verification', + 'cache', + ]); + }); + + it('allows runtime interface paths but strips their permission sub-paths', () => { + expect( + sanitizeAdminConfigTombstones([ + 'interface.schedules', + 'interface.schedules.maxPerUser', + 'interface.schedules.use', + ]), + ).toEqual(['interface.schedules', 'interface.schedules.maxPerUser']); + }); +}); + +describe('isForbiddenAdminConfigPath – localized label traversal', () => { + it('allows the label leaf itself', () => { + expect(isForbiddenAdminConfigPath('interface.mcpServers.trustCheckbox.label')).toBe(false); + }); + + it('allows one language-key segment beneath a localized leaf', () => { + expect(isForbiddenAdminConfigPath('interface.mcpServers.trustCheckbox.label.en')).toBe(false); + }); + + it('allows subLabel with a language-key', () => { + expect(isForbiddenAdminConfigPath('interface.mcpServers.trustCheckbox.subLabel.fr')).toBe( + false, + ); + }); + + it('blocks two segments beneath a localized leaf (depth exceeded)', () => { + expect(isForbiddenAdminConfigPath('interface.mcpServers.trustCheckbox.label.en.foo')).toBe( + true, + ); + }); + + it('blocks a child of a primitive leaf (placeholder is not a localized leaf)', () => { + expect(isForbiddenAdminConfigPath('interface.mcpServers.placeholder.foo')).toBe(true); + }); + + it('blocks a child of a primitive leaf in another permission field', () => { + expect(isForbiddenAdminConfigPath('interface.marketplace.verification.foo')).toBe(true); + }); +}); diff --git a/packages/data-schemas/src/admin/configOverrides.ts b/packages/data-schemas/src/admin/configOverrides.ts new file mode 100644 index 00000000000..87e5f0d3da2 --- /dev/null +++ b/packages/data-schemas/src/admin/configOverrides.ts @@ -0,0 +1,278 @@ +import { + BASE_ONLY_CONFIG_SECTIONS, + INTERFACE_PERMISSION_FIELDS, + RUNTIME_CONFIG_INTERFACE_FIELDS, + PERMISSION_SUB_KEYS, +} from 'librechat-data-provider'; +import logger from '~/config/winston'; + +const BASE_ONLY_OVERRIDE_SECTIONS = new Set(BASE_ONLY_CONFIG_SECTIONS); +const INTERNAL_CONFIG_ALIASES = new Set(['interfaceConfig', 'mcpConfig', 'turnstileConfig']); + +function isPlainObject(value: unknown): value is Record { + return value != null && typeof value === 'object' && !Array.isArray(value); +} + +function canonicalizeOverridePath(path: string): string { + if (path === 'interfaceConfig') { + return 'interface'; + } + if (path.startsWith('interfaceConfig.')) { + return `interface.${path.slice('interfaceConfig.'.length)}`; + } + return path; +} + +/** Nested allowlist of UI-only keys under a composite interface permission field. */ +export type InterfacePermissionUiNode = + | true + | { readonly [key: string]: InterfacePermissionUiNode }; + +/** + * UI-only paths allowed under each interface permission field, including nesting. + * Fields omitted here (booleans and permission-only objects) reject every descendant. + */ +export const INTERFACE_PERMISSION_UI_SHAPES: Readonly> = { + mcpServers: { + placeholder: true, + trustCheckbox: { + label: true, + subLabel: true, + }, + }, + marketplace: { + verification: true, + }, + skills: { + defaultActiveOnShare: true, + }, + sharedLinks: { + snapshotFiles: true, + }, +}; + +function isUiShapeMap( + node: InterfacePermissionUiNode, +): node is { readonly [key: string]: InterfacePermissionUiNode } { + return node !== true; +} + +/** Shape leaves that accept exactly one string sub-key (a language code for localized strings). */ +const LOCALIZED_UI_LEAVES = new Set(['label', 'subLabel']); + +function isAllowedInterfacePermissionUiPath(field: string, descendant: readonly string[]): boolean { + if (descendant.length === 0) { + return false; + } + let node: InterfacePermissionUiNode | undefined = INTERFACE_PERMISSION_UI_SHAPES[field]; + if (node == null) { + return false; + } + let lastKey = ''; + let inLocalizedLeaf = false; + for (const segment of descendant) { + if (inLocalizedLeaf) { + // Already consumed one language-key segment; no further depth is valid. + return false; + } + if (!isUiShapeMap(node)) { + // Reached a primitive leaf; localized record leaves accept exactly one more key. + if (!LOCALIZED_UI_LEAVES.has(lastKey)) { + return false; + } + inLocalizedLeaf = true; + continue; + } + lastKey = segment; + node = node[segment]; + if (node == null) { + return false; + } + } + return true; +} + +function pickAllowedUiSubtree( + value: Record, + shape: { readonly [key: string]: InterfacePermissionUiNode }, + fieldPath: string, +): Record | undefined { + const uiOnly: Record = {}; + for (const [sub, subVal] of Object.entries(value)) { + const childPath = `${fieldPath}.${sub}`; + if (PERMISSION_SUB_KEYS.has(sub)) { + logger.warn( + `[adminConfig] Stripping interface permission sub-field "${childPath}" — use role permissions instead`, + ); + continue; + } + const childShape = shape[sub]; + if (childShape == null) { + logger.warn(`[adminConfig] Stripping unknown interface permission descendant "${childPath}"`); + continue; + } + if (childShape === true) { + if (LOCALIZED_UI_LEAVES.has(sub)) { + if (!isPlainObject(subVal) && !Array.isArray(subVal)) { + uiOnly[sub] = subVal; + } else if (isPlainObject(subVal)) { + const localized: Record = {}; + for (const [langKey, langVal] of Object.entries(subVal)) { + if (!isPlainObject(langVal) && !Array.isArray(langVal)) { + localized[langKey] = langVal; + } + } + if (Object.keys(localized).length > 0) { + uiOnly[sub] = localized; + } + } + } else if (!isPlainObject(subVal) && !Array.isArray(subVal)) { + uiOnly[sub] = subVal; + } + continue; + } + if (!isPlainObject(subVal)) { + logger.warn(`[adminConfig] Stripping unknown interface permission descendant "${childPath}"`); + continue; + } + const nested = pickAllowedUiSubtree(subVal, childShape, childPath); + if (nested != null) { + uiOnly[sub] = nested; + } + } + return Object.keys(uiOnly).length > 0 ? uiOnly : undefined; +} + +function isInterfacePermissionPath(fieldPath: string): boolean { + const parts = fieldPath.split('.'); + if (parts[0] !== 'interface' || parts.length < 2) { + return false; + } + if (!INTERFACE_PERMISSION_FIELDS.has(parts[1])) { + return false; + } + if (parts.length === 2) { + return !RUNTIME_CONFIG_INTERFACE_FIELDS.has(parts[1]); + } + if (PERMISSION_SUB_KEYS.has(parts[2])) { + return true; + } + if (RUNTIME_CONFIG_INTERFACE_FIELDS.has(parts[1])) { + return false; + } + return !isAllowedInterfacePermissionUiPath(parts[1], parts.slice(2)); +} + +export function isForbiddenAdminConfigPath(fieldPath: string): boolean { + const canonicalPath = canonicalizeOverridePath(fieldPath); + const topLevel = canonicalPath.split('.')[0]; + if (INTERNAL_CONFIG_ALIASES.has(fieldPath.split('.')[0])) { + return true; + } + if (BASE_ONLY_OVERRIDE_SECTIONS.has(topLevel)) { + return true; + } + if (canonicalPath === 'interface') { + return true; + } + return isInterfacePermissionPath(canonicalPath); +} + +/** + * Strips interface permission fields and base-only sections that must not be + * persisted as config overrides (PUT, atomic replace, and restore). + */ +export function sanitizeAdminConfigOverrides( + overrides: Record, +): Record { + const filteredOverrides = { ...overrides }; + for (const alias of INTERNAL_CONFIG_ALIASES) { + if (alias in filteredOverrides) { + delete filteredOverrides[alias]; + logger.warn( + `[adminConfig] Stripping internal config alias "${alias}" — use canonical YAML keys instead`, + ); + } + } + for (const section of BASE_ONLY_OVERRIDE_SECTIONS) { + if (section in filteredOverrides) { + delete filteredOverrides[section]; + logger.warn( + `[adminConfig] Stripping base-only config section "${section}" - configure it in librechat.yaml instead`, + ); + } + } + const hasInterface = Object.prototype.hasOwnProperty.call(filteredOverrides, 'interface'); + if (!hasInterface) { + return filteredOverrides; + } + const iface = filteredOverrides.interface; + if (!isPlainObject(iface)) { + delete filteredOverrides.interface; + logger.warn( + '[adminConfig] Stripping non-object "interface" override — use an object with UI-only keys', + ); + return filteredOverrides; + } + + const filteredIface: Record = {}; + for (const [field, val] of Object.entries(iface)) { + if (!INTERFACE_PERMISSION_FIELDS.has(field)) { + filteredIface[field] = val; + continue; + } + if (RUNTIME_CONFIG_INTERFACE_FIELDS.has(field)) { + if (!isPlainObject(val)) { + filteredIface[field] = val; + continue; + } + const runtimeOnly: Record = {}; + if (val.use === false) { + runtimeOnly.use = false; + } + for (const [sub, subVal] of Object.entries(val)) { + if (!PERMISSION_SUB_KEYS.has(sub)) { + runtimeOnly[sub] = subVal; + } + } + if (Object.keys(runtimeOnly).length > 0) { + filteredIface[field] = runtimeOnly; + } + continue; + } + const shape = INTERFACE_PERMISSION_UI_SHAPES[field]; + if (shape == null || !isUiShapeMap(shape) || !isPlainObject(val)) { + logger.warn( + `[adminConfig] Stripping interface permission field "${field}" — use role permissions instead`, + ); + continue; + } + const uiOnly = pickAllowedUiSubtree(val, shape, field); + if (uiOnly != null) { + filteredIface[field] = uiOnly; + } + } + if (Object.keys(filteredIface).length > 0) { + filteredOverrides.interface = filteredIface; + } else { + delete filteredOverrides.interface; + } + return filteredOverrides; +} + +export function sanitizeAdminConfigTombstones(paths: string[] | undefined): string[] { + if (!paths || paths.length === 0) { + return []; + } + const kept: string[] = []; + for (const path of paths) { + if (!isForbiddenAdminConfigPath(path)) { + kept.push(path); + continue; + } + logger.warn( + `[adminConfig] Stripping forbidden tombstone path "${path}" — use role permissions instead`, + ); + } + return kept; +} diff --git a/packages/data-schemas/src/admin/index.ts b/packages/data-schemas/src/admin/index.ts index 8d43daada65..a6b8f891730 100644 --- a/packages/data-schemas/src/admin/index.ts +++ b/packages/data-schemas/src/admin/index.ts @@ -1 +1,9 @@ export * from './capabilities'; +export type { InterfacePermissionUiNode } from './configOverrides'; +export { + INTERFACE_PERMISSION_UI_SHAPES, + isForbiddenAdminConfigPath, + sanitizeAdminConfigOverrides, + sanitizeAdminConfigTombstones, +} from './configOverrides'; +export { indexedArrayPathError } from './indexedArrayPath'; diff --git a/packages/data-schemas/src/admin/indexedArrayPath.spec.ts b/packages/data-schemas/src/admin/indexedArrayPath.spec.ts new file mode 100644 index 00000000000..21d6322d2fb --- /dev/null +++ b/packages/data-schemas/src/admin/indexedArrayPath.spec.ts @@ -0,0 +1,49 @@ +import { indexedArrayPathError } from './indexedArrayPath'; + +describe('indexedArrayPathError', () => { + describe('array-typed paths', () => { + it('rejects a bare array index (endpoints.custom.0)', () => { + expect(indexedArrayPathError('endpoints.custom.0')).toMatch(/indexed array/i); + }); + + it('rejects a deep array index (endpoints.custom.0.baseURL)', () => { + expect(indexedArrayPathError('endpoints.custom.0.baseURL')).toMatch(/indexed array/i); + }); + + it('rejects a simple string-array index (registration.socialLogins.0)', () => { + expect(indexedArrayPathError('registration.socialLogins.0')).toMatch(/indexed array/i); + }); + }); + + describe('ZodRecord paths with numeric keys', () => { + it('allows a numeric MCP server name (mcpServers.123.type)', () => { + expect(indexedArrayPathError('mcpServers.123.type')).toBeNull(); + }); + + it('allows a numeric key in an MCP server headers record (mcpServers.my-server.headers.2024)', () => { + expect(indexedArrayPathError('mcpServers.my-server.headers.2024')).toBeNull(); + }); + + it('allows a numeric key in an MCP server env record (mcpServers.my-tool.env.8080)', () => { + expect(indexedArrayPathError('mcpServers.my-tool.env.8080')).toBeNull(); + }); + }); + + describe('non-array paths', () => { + it('allows a plain top-level field (cache)', () => { + expect(indexedArrayPathError('cache')).toBeNull(); + }); + + it('allows a nested non-array path (registration.allowedDomains)', () => { + expect(indexedArrayPathError('registration.allowedDomains')).toBeNull(); + }); + + it('allows an unknown path (unknownField.sub)', () => { + expect(indexedArrayPathError('unknownField.sub')).toBeNull(); + }); + + it('allows a whole-array write without index (endpoints.custom)', () => { + expect(indexedArrayPathError('endpoints.custom')).toBeNull(); + }); + }); +}); diff --git a/packages/data-schemas/src/admin/indexedArrayPath.ts b/packages/data-schemas/src/admin/indexedArrayPath.ts new file mode 100644 index 00000000000..b073a1f40cb --- /dev/null +++ b/packages/data-schemas/src/admin/indexedArrayPath.ts @@ -0,0 +1,148 @@ +import { configSchema } from 'librechat-data-provider'; + +const WRAPPER_TYPES = new Set([ + 'ZodOptional', + 'ZodNullable', + 'ZodDefault', + 'ZodCatch', + 'ZodBranded', + 'ZodReadonly', + 'ZodEffects', + 'ZodPipeline', + 'ZodLazy', +]); + +interface ZodLike { + _def?: { + typeName?: string; + innerType?: ZodLike; + schema?: ZodLike; + getter?: () => ZodLike; + out?: ZodLike; + options?: ZodLike[]; + left?: ZodLike; + right?: ZodLike; + valueType?: ZodLike; + }; + shape?: Record; +} + +function unwrapSchema(schema: ZodLike | undefined): ZodLike | undefined { + const seen = new Set(); + let current = schema; + while (current?._def?.typeName && WRAPPER_TYPES.has(current._def.typeName)) { + if (seen.has(current)) { + break; + } + seen.add(current); + const def = current._def; + let next: ZodLike | undefined; + if (def.typeName === 'ZodLazy') { + next = def.getter?.(); + } else if (def.typeName === 'ZodPipeline') { + next = def.out; + } else if (def.typeName === 'ZodEffects') { + next = def.schema; + } else { + next = def.innerType; + } + if (!next) { + break; + } + current = next; + } + return current; +} + +function schemaContainerFlags(schema: ZodLike | undefined): { + canBeArray: boolean; + canBeRecord: boolean; +} { + const unwrapped = unwrapSchema(schema); + if (!unwrapped?._def) { + return { canBeArray: false, canBeRecord: false }; + } + const typeName = unwrapped._def.typeName; + if (typeName === 'ZodArray') { + return { canBeArray: true, canBeRecord: false }; + } + if (typeName === 'ZodRecord') { + return { canBeArray: false, canBeRecord: true }; + } + if (typeName === 'ZodUnion') { + let canBeArray = false; + let canBeRecord = false; + for (const opt of unwrapped._def.options ?? []) { + const flags = schemaContainerFlags(opt); + canBeArray = canBeArray || flags.canBeArray; + canBeRecord = canBeRecord || flags.canBeRecord; + } + return { canBeArray, canBeRecord }; + } + return { canBeArray: false, canBeRecord: false }; +} + +function descendNonArraySegment(schema: ZodLike, segment: string): ZodLike | null { + const unwrapped = unwrapSchema(schema); + if (!unwrapped?._def) { + return null; + } + const typeName = unwrapped._def.typeName; + if (unwrapped.shape && typeof unwrapped.shape === 'object') { + return unwrapped.shape[segment] ?? null; + } + if (typeName === 'ZodRecord') { + return unwrapped._def.valueType ?? null; + } + if (typeName === 'ZodUnion') { + const candidates: ZodLike[] = []; + for (const opt of unwrapped._def.options ?? []) { + const resolved = descendNonArraySegment(opt, segment); + if (resolved) { + candidates.push(resolved); + } + } + if (candidates.length === 0) { + return null; + } + if (candidates.length === 1) { + return candidates[0]; + } + return { _def: { typeName: 'ZodUnion', options: candidates } }; + } + if (typeName === 'ZodIntersection') { + const left = unwrapSchema(unwrapped._def.left); + const right = unwrapSchema(unwrapped._def.right); + const merged = { ...(left?.shape ?? {}), ...(right?.shape ?? {}) }; + return merged[segment] ?? null; + } + return null; +} + +/** + * Returns an error when `fieldPath` addresses a schema array by index + * (`endpoints.custom.0`) or crosses an array (`endpoints.custom.0.baseURL`). + * Unknown non-array paths and whole-array writes (`endpoints.custom`) are allowed. + */ +export function indexedArrayPathError(fieldPath: string): string | null { + const segments = fieldPath.split('.'); + if (segments.length === 0 || segments.some((segment) => segment.length === 0)) { + return null; + } + + let current: ZodLike = configSchema as unknown as ZodLike; + for (let i = 0; i < segments.length; i += 1) { + const flags = schemaContainerFlags(current); + if (flags.canBeArray) { + return flags.canBeRecord + ? `Unsupported array path: ${fieldPath}` + : `Indexed array paths are not supported: ${fieldPath}`; + } + const next = descendNonArraySegment(current, segments[i]); + if (!next) { + return null; + } + current = next; + } + return null; +} diff --git a/packages/data-schemas/src/app/resolution.spec.ts b/packages/data-schemas/src/app/resolution.spec.ts index 594f87b6799..fab12079b61 100644 --- a/packages/data-schemas/src/app/resolution.spec.ts +++ b/packages/data-schemas/src/app/resolution.spec.ts @@ -652,6 +652,30 @@ describe('mergeConfigOverrides', () => { expect(iface.agents).toBeUndefined(); }); + it('ignores legacy internal aliases and malformed interface overrides', () => { + const base = { + interfaceConfig: { modelSelect: true }, + cache: false, + } as unknown as AppConfig; + + const configs = [ + fakeConfig( + { + interfaceConfig: { prompts: false }, + interface: null, + cache: true, + }, + 10, + ), + ]; + + const result = mergeConfigOverrides(base, configs) as unknown as Record; + const iface = result.interfaceConfig as Record; + expect(iface.modelSelect).toBe(true); + expect(iface.prompts).toBeUndefined(); + expect(result.cache).toBe(true); + }); + it('remaps YAML-level keys to AppConfig equivalents', () => { const configs = [ fakeConfig( @@ -775,7 +799,7 @@ describe('mergeConfigOverrides', () => { const result = mergeConfigOverrides(baseConfig, configs) as unknown as Record; - expect(result.mcpConfig).toEqual({}); + expect(result.mcpConfig).toBeUndefined(); }); it('applies tombstones after remapping YAML paths to AppConfig paths', () => { @@ -878,6 +902,16 @@ describe('mergeConfigOverrides', () => { expect(mcpConfig.github).toBeUndefined(); }); + + it('ignores forbidden legacy tombstones during resolution', () => { + const result = mergeConfigOverrides(baseConfig, [ + fakeConfig({}, 10, ['interface', 'interfaceConfig.prompts']), + ]) as unknown as Record; + const iface = result.interfaceConfig as Record; + + expect(iface.modelSelect).toBe(true); + expect(iface.parameters).toBe(true); + }); }); describe('INTERFACE_PERMISSION_FIELDS', () => { diff --git a/packages/data-schemas/src/app/resolution.ts b/packages/data-schemas/src/app/resolution.ts index 26b7a5b5bb8..34b97458507 100644 --- a/packages/data-schemas/src/app/resolution.ts +++ b/packages/data-schemas/src/app/resolution.ts @@ -3,11 +3,15 @@ import { BASE_ONLY_CONFIG_SECTIONS, INTERFACE_PERMISSION_FIELDS, RUNTIME_CONFIG_INTERFACE_FIELDS, - PERMISSION_SUB_KEYS, isProcessMCPServerConfig, } from 'librechat-data-provider'; import type { TCustomConfig } from 'librechat-data-provider'; import type { AppConfig, IConfig } from '~/types'; +import { + sanitizeAdminConfigOverrides, + sanitizeAdminConfigTombstones, + INTERFACE_PERMISSION_UI_SHAPES, +} from '~/admin/configOverrides'; import { BASE_CONFIG_PRINCIPAL_ID } from '~/admin/capabilities'; type AnyObject = { [key: string]: unknown }; @@ -17,7 +21,6 @@ const UNSAFE_KEYS = new Set(['__proto__', 'constructor', 'prototype']); /** Filters are a fail-closed security boundary even during mixed-package rollouts. */ const BASE_ONLY_OVERRIDE_SECTIONS = new Set(['filters', ...BASE_ONLY_CONFIG_SECTIONS]); const BASE_PRINCIPAL_OVERRIDE_SECTIONS = new Set(BASE_PRINCIPAL_CONFIG_SECTIONS); - /** * Paths within the config tree where arrays of objects should be merged by * a key field rather than replaced wholesale. `deepMerge` matches items by @@ -291,21 +294,21 @@ export function mergeConfigOverrides(baseConfig: AppConfig, configs: IConfig[]): let merged = { ...baseConfig }; for (const config of sorted) { const isBasePrincipal = config.principalId?.toString() === BASE_CONFIG_PRINCIPAL_ID; - if (Array.isArray(config.tombstones)) { - for (const path of config.tombstones) { - if ( - typeof path === 'string' && - !isBaseOnlyOverridePath(path) && - (isBasePrincipal || !BASE_PRINCIPAL_OVERRIDE_SECTIONS.has(path.split('.')[0])) - ) { - merged = deleteConfigPath(merged, remapOverridePath(path)); - } + for (const path of sanitizeAdminConfigTombstones(config.tombstones)) { + if ( + !isBaseOnlyOverridePath(path) && + (isBasePrincipal || !BASE_PRINCIPAL_OVERRIDE_SECTIONS.has(path.split('.')[0])) + ) { + merged = deleteConfigPath(merged, remapOverridePath(path)); } } if (config.overrides && typeof config.overrides === 'object') { + const sanitizedOverrides = sanitizeAdminConfigOverrides( + config.overrides as Record, + ); const remapped: AnyObject = {}; - for (const [key, value] of Object.entries(config.overrides)) { + for (const [key, value] of Object.entries(sanitizedOverrides)) { if ( BASE_ONLY_OVERRIDE_SECTIONS.has(key) || (!isBasePrincipal && BASE_PRINCIPAL_OVERRIDE_SECTIONS.has(key)) @@ -328,26 +331,27 @@ export function mergeConfigOverrides(baseConfig: AppConfig, configs: IConfig[]): for (const [field, fieldVal] of Object.entries(value as AnyObject)) { if (!INTERFACE_PERMISSION_FIELDS.has(field)) { filtered[field] = fieldVal; + } else if (RUNTIME_CONFIG_INTERFACE_FIELDS.has(field)) { + filtered[field] = fieldVal; } else if ( fieldVal != null && typeof fieldVal === 'object' && !Array.isArray(fieldVal) ) { - // Composite permission field (e.g. mcpServers): strip permission - // sub-keys but preserve UI-only sub-keys like placeholder/trustCheckbox. - const uiOnly: AnyObject = {}; - for (const [sub, subVal] of Object.entries(fieldVal as AnyObject)) { - if (!PERMISSION_SUB_KEYS.has(sub)) { - uiOnly[sub] = subVal; + // Composite permission field (e.g. mcpServers): keep only + // field-specific UI-only sub-keys after sanitization. + const shape = INTERFACE_PERMISSION_UI_SHAPES[field]; + if (shape != null && shape !== true) { + const uiOnly: AnyObject = {}; + for (const [sub, subVal] of Object.entries(fieldVal as AnyObject)) { + if (Object.prototype.hasOwnProperty.call(shape, sub)) { + uiOnly[sub] = subVal; + } + } + if (Object.keys(uiOnly).length > 0) { + filtered[field] = uiOnly; } } - if (Object.keys(uiOnly).length > 0) { - filtered[field] = uiOnly; - } - } else if (RUNTIME_CONFIG_INTERFACE_FIELDS.has(field)) { - // Dual-purpose field: the boolean form is a runtime disable, not a - // permission toggle, so preserve it (e.g. schedules: false). - filtered[field] = fieldVal; } // other boolean permission fields (e.g. runCode: false) are fully stripped } diff --git a/packages/data-schemas/src/index.ts b/packages/data-schemas/src/index.ts index 499a8e2aa2b..1ddae6df65d 100644 --- a/packages/data-schemas/src/index.ts +++ b/packages/data-schemas/src/index.ts @@ -10,6 +10,19 @@ export { CLIENT_MESSAGE_SELECT, SUBAGENT_TRANSCRIPT_SOURCE_BYTE_LIMIT, RoleConflictError, + ConfigVersionConflictError, + ConfigRevisionNotFoundError, + canonicalizeResetPaths, + fieldPathLimitError, + fieldPathPolicyError, + isValidFieldPath, + type FindConfigByPrincipalOptions, + MAX_FIELD_PATH_LENGTH, + MAX_FIELD_PATH_SEGMENTS, + ensureConfigIndexes, + ADMIN_CONFIG_REVISIONS_COLLECTION, + ADMIN_CONFIG_VERSION_EPOCHS_COLLECTION, + MAX_CONFIG_REVISIONS, DEFAULT_REFRESH_TOKEN_EXPIRY, DEFAULT_SESSION_EXPIRY, tokenValues, diff --git a/packages/data-schemas/src/methods/config.atomic.spec.ts b/packages/data-schemas/src/methods/config.atomic.spec.ts new file mode 100644 index 00000000000..006992f37cc --- /dev/null +++ b/packages/data-schemas/src/methods/config.atomic.spec.ts @@ -0,0 +1,906 @@ +import mongoose from 'mongoose'; +import { MongoMemoryReplSet } from 'mongodb-memory-server'; +import { PrincipalType, PrincipalModel } from 'librechat-data-provider'; +import type { ConfigMutationResult, ConfigRevisionSnapshot } from './config'; +import type { IConfig } from '~/types'; +import { + ADMIN_CONFIG_REVISIONS_COLLECTION, + ADMIN_CONFIG_VERSION_EPOCHS_COLLECTION, + ConfigVersionConflictError, + ConfigRevisionNotFoundError, + canonicalizeResetPaths, + createConfigMethods, +} from './config'; +import { BASE_CONFIG_PRINCIPAL_ID } from '~/admin/capabilities'; +import configSchema from '~/schema/config'; + +function expectChanged(result: ConfigMutationResult): asserts result is { + changed: true; + config: NonNullable | null; + revision: ConfigRevisionSnapshot; +} { + expect(result.changed).toBe(true); + expect(result.revision).not.toBeNull(); +} + +describe('canonicalizeResetPaths', () => { + it('deduplicates and keeps the highest ancestor', () => { + expect( + canonicalizeResetPaths(['registration', 'registration.enabled', 'registration.enabled']), + ).toEqual(['registration']); + }); + + it('rejects a single deeply nested path before ancestor checks', () => { + const deep = Array.from({ length: 33 }, (_, index) => `seg${index}`).join('.'); + expect(() => canonicalizeResetPaths([deep])).toThrow(/maximum depth of 32 segments/); + }); + + it('rejects unsafe reset paths', () => { + expect(() => canonicalizeResetPaths(['__proto__.polluted'])).toThrow(/forbidden segment/); + }); +}); + +describe('mutateConfigWithRevision', () => { + let replSet: MongoMemoryReplSet; + let methods: ReturnType; + + beforeAll(async () => { + replSet = await MongoMemoryReplSet.create({ replSet: { count: 1 } }); + await mongoose.connect(replSet.getUri()); + const Config = mongoose.models.Config ?? mongoose.model('Config', configSchema); + await Config.syncIndexes(); + methods = createConfigMethods(mongoose); + }, 60_000); + + afterAll(async () => { + await mongoose.disconnect(); + await replSet.stop(); + }); + + beforeEach(async () => { + await mongoose.models.Config.deleteMany({}); + await mongoose.connection.collection(ADMIN_CONFIG_REVISIONS_COLLECTION).deleteMany({}); + await mongoose.connection.collection(ADMIN_CONFIG_VERSION_EPOCHS_COLLECTION).deleteMany({}); + }); + + const actor = { actorId: 'admin-1', actorEmail: 'admin@test', tenantId: '' }; + + it('rejects unsafe field paths without writing a revision', async () => { + await expect( + methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: null, + op: { + kind: 'fields', + resetPaths: [], + fields: { '__proto__.polluted': true }, + priority: 0, + }, + cause: 'save', + actor, + }), + ).rejects.toThrow(/forbidden segment/); + + expect( + await mongoose.connection.collection(ADMIN_CONFIG_REVISIONS_COLLECTION).countDocuments(), + ).toBe(0); + expect( + await mongoose.models.Config.countDocuments({ principalId: BASE_CONFIG_PRINCIPAL_ID }), + ).toBe(0); + expect(Object.prototype).not.toHaveProperty('polluted'); + }); + + it('rejects a stale expectedVersion without writing a revision', async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + BASE_CONFIG_PRINCIPAL_ID, + PrincipalModel.ROLE, + { cache: true }, + 0, + ); + + await expect( + methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 0, + op: { kind: 'fields', resetPaths: ['cache'], fields: {}, priority: 0 }, + cause: 'save', + actor, + }), + ).rejects.toBeInstanceOf(ConfigVersionConflictError); + + expect( + await mongoose.connection.collection(ADMIN_CONFIG_REVISIONS_COLLECTION).countDocuments(), + ).toBe(0); + const doc = await mongoose.models.Config.findOne({ principalId: BASE_CONFIG_PRINCIPAL_ID }); + expect(doc?.configVersion).toBe(1); + expect(doc?.overrides).toEqual({ cache: true }); + }); + + it('applies reset and set in one transaction and inserts a finalized revision', async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + BASE_CONFIG_PRINCIPAL_ID, + PrincipalModel.ROLE, + { cache: false, registration: { socialLogins: ['local'] } }, + 0, + ); + + const result = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { + kind: 'fields', + resetPaths: ['registration'], + fields: { cache: true }, + priority: 0, + }, + cause: 'save', + actor, + }); + + expectChanged(result); + expect(result.revision.status).toBe('final'); + expect(result.revision.committed).toBe(true); + expect(result.revision.configVersion).toBe(1); + expect(result.revision.overrides).toEqual({ + cache: false, + registration: { socialLogins: ['local'] }, + }); + expect(result.config?.configVersion).toBe(2); + expect(result.config?.overrides).toEqual({ cache: true }); + + const stored = await mongoose.connection.collection(ADMIN_CONFIG_REVISIONS_COLLECTION).findOne({ + id: result.revision.id, + }); + expect(stored?.status).toBe('final'); + expect(stored?.expiresAt).toBeUndefined(); + expect(stored?.principalType).toBe(PrincipalType.ROLE); + expect(stored?.principalId).toBe(BASE_CONFIG_PRINCIPAL_ID); + expect(stored?.tombstones).toEqual([]); + expect(stored?.priority).toBe(0); + expect(stored?.isActive).toBe(true); + }); + + it('rejects non-base principals without writing', async () => { + await expect( + methods.mutateConfigWithRevision({ + principalType: PrincipalType.GROUP, + principalId: 'group-1', + principalModel: PrincipalModel.GROUP, + expectedVersion: null, + op: { kind: 'fields', resetPaths: [], fields: { cache: true }, priority: 0 }, + cause: 'save', + actor, + }), + ).rejects.toThrow(/base configuration/); + expect( + await mongoose.connection.collection(ADMIN_CONFIG_REVISIONS_COLLECTION).countDocuments(), + ).toBe(0); + }); + + it('mutates a legacy document whose configVersion field is missing', async () => { + await mongoose.models.Config.collection.insertOne({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + overrides: { cache: false }, + tombstones: ['registration.enabled'], + priority: 0, + isActive: true, + }); + + const result = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 0, + op: { kind: 'fields', resetPaths: [], fields: { cache: true }, priority: 0 }, + cause: 'save', + actor, + }); + + expect(result.config?.configVersion).toBe(1); + expect(result.config?.overrides).toEqual({ cache: true }); + expectChanged(result); + expect(result.revision.configVersion).toBe(0); + expect(result.revision.tombstones).toEqual(['registration.enabled']); + }); + + it('returns the committed outcome when retention pruning fails', async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + BASE_CONFIG_PRINCIPAL_ID, + PrincipalModel.ROLE, + { cache: false }, + 0, + ); + const coll = mongoose.connection.collection(ADMIN_CONFIG_REVISIONS_COLLECTION); + const originalFind = coll.find.bind(coll); + coll.find = (() => { + throw new Error('prune failed'); + }) as typeof coll.find; + + try { + const result = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { kind: 'fields', resetPaths: [], fields: { cache: true }, priority: 0 }, + cause: 'save', + actor, + }); + expect(result.config?.overrides).toEqual({ cache: true }); + expectChanged(result); + expect(result.revision.status).toBe('final'); + } finally { + coll.find = originalFind; + } + }); + + it('restores tombstones, priority, and isActive from a snapshot', async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + BASE_CONFIG_PRINCIPAL_ID, + PrincipalModel.ROLE, + { cache: false, registration: { socialLogins: ['local'] } }, + 3, + ); + await mongoose.models.Config.updateOne( + { principalId: BASE_CONFIG_PRINCIPAL_ID }, + { $set: { tombstones: ['mcpServers.github'], isActive: true } }, + ); + + const first = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { + kind: 'fields', + resetPaths: ['registration'], + fields: { cache: true }, + priority: 0, + }, + cause: 'save', + actor, + }); + expectChanged(first); + expect(first.revision.tombstones).toEqual(['mcpServers.github']); + expect(first.revision.priority).toBe(3); + + const restored = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 2, + op: { kind: 'restore', revisionId: first.revision.id }, + cause: 'restore', + actor, + }); + + expect(restored.config?.overrides).toEqual({ + cache: false, + registration: { socialLogins: ['local'] }, + }); + expect(restored.config?.tombstones).toEqual(['mcpServers.github']); + expect(restored.config?.priority).toBe(3); + }); + + it('preserves isActive on field saves for an inactive document', async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + BASE_CONFIG_PRINCIPAL_ID, + PrincipalModel.ROLE, + { cache: false }, + 0, + ); + await methods.toggleConfigActive(PrincipalType.ROLE, BASE_CONFIG_PRINCIPAL_ID, false); + const inactive = await mongoose.models.Config.findOne({ + principalId: BASE_CONFIG_PRINCIPAL_ID, + }); + expect(inactive?.isActive).toBe(false); + expect(inactive?.configVersion).toBe(2); + + const result = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 2, + op: { kind: 'fields', resetPaths: [], fields: { cache: true }, priority: 0 }, + cause: 'save', + actor, + }); + + expect(result.config?.isActive).toBe(false); + expect(result.config?.overrides).toEqual({ cache: true }); + }); + + it('strips forbidden interface permission overrides when restoring a snapshot', async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + BASE_CONFIG_PRINCIPAL_ID, + PrincipalModel.ROLE, + { cache: false, interface: { modelSelect: true } }, + 0, + ); + + const revisionId = '11111111-1111-4111-8111-111111111111'; + await mongoose.connection.collection(ADMIN_CONFIG_REVISIONS_COLLECTION).insertOne({ + id: revisionId, + createdAt: new Date().toISOString(), + cause: 'import', + actorId: actor.actorId, + tenantId: '', + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + overrides: { + cache: true, + interface: { prompts: false, modelSelect: false }, + interfaceConfig: { prompts: false }, + }, + tombstones: ['interface', 'interfaceConfig.prompts', 'cache'], + priority: 0, + isActive: true, + absent: false, + configVersion: 1, + status: 'final', + committed: true, + }); + + const result = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { kind: 'restore', revisionId }, + cause: 'restore', + actor, + }); + + expect(result.config?.overrides).toEqual({ + cache: true, + interface: { modelSelect: false }, + }); + expect(result.config?.tombstones).toEqual(['cache']); + }); + + it('strips forbidden tombstones preserved by replace mutations', async () => { + await mongoose.models.Config.create({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + overrides: { cache: false }, + tombstones: ['interface.prompts', 'cache'], + priority: 0, + isActive: true, + configVersion: 1, + }); + + const result = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { kind: 'replace', overrides: { cache: true }, priority: 0 }, + cause: 'import', + actor, + }); + + expect(result.config?.overrides).toEqual({ cache: true }); + expect(result.config?.tombstones).toEqual(['cache']); + }); + + it('sanitizes legacy protected tombstones during field mutations', async () => { + await mongoose.models.Config.create({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + overrides: { cache: false }, + tombstones: ['interface', 'interfaceConfig.prompts', 'cache'], + priority: 0, + isActive: true, + configVersion: 1, + }); + + const result = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { kind: 'fields', resetPaths: [], fields: { cache: true }, priority: 0 }, + cause: 'save', + actor, + }); + + expect(result.config?.overrides).toEqual({ cache: true }); + expect(result.config?.tombstones).toEqual([]); + }); + + it('sanitizes legacy unsafe overrides during field mutations', async () => { + await mongoose.models.Config.create({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + overrides: { + cache: false, + interfaceConfig: { prompts: false }, + interface: null, + }, + tombstones: [], + priority: 0, + isActive: true, + configVersion: 1, + }); + + const result = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { kind: 'fields', resetPaths: [], fields: { cache: true }, priority: 0 }, + cause: 'save', + actor, + }); + + expect(result.config?.overrides).toEqual({ cache: true }); + }); + + it('does not create a config document for reset-only mutation against an absent base', async () => { + const result = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: null, + op: { kind: 'fields', resetPaths: ['cache'], fields: {}, priority: 0 }, + cause: 'reset', + actor, + }); + + expect(result).toEqual({ changed: false, config: null, revision: null }); + expect( + await mongoose.models.Config.countDocuments({ principalId: BASE_CONFIG_PRINCIPAL_ID }), + ).toBe(0); + expect( + await mongoose.connection.collection(ADMIN_CONFIG_REVISIONS_COLLECTION).countDocuments(), + ).toBe(0); + }); + + it('rejects stale expectedVersion after delete/recreate (ABA)', async () => { + const created = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: null, + op: { kind: 'fields', resetPaths: [], fields: { cache: true }, priority: 0 }, + cause: 'save', + actor, + }); + expect(created.changed).toBe(true); + expect(created.config?.configVersion).toBe(1); + + await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { kind: 'delete' }, + cause: 'reset', + actor, + }); + + const recreated = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: null, + op: { kind: 'fields', resetPaths: [], fields: { cache: false }, priority: 0 }, + cause: 'save', + actor, + }); + expect(recreated.changed).toBe(true); + expect(recreated.config?.configVersion).toBe(2); + expect(recreated.config?.overrides).toEqual({ cache: false }); + + await expect( + methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { kind: 'fields', resetPaths: [], fields: { cache: true }, priority: 0 }, + cause: 'save', + actor, + }), + ).rejects.toBeInstanceOf(ConfigVersionConflictError); + + const doc = await mongoose.models.Config.findOne({ principalId: BASE_CONFIG_PRINCIPAL_ID }); + expect(doc?.configVersion).toBe(2); + expect(doc?.overrides).toEqual({ cache: false }); + }); + + it('keeps the epoch monotonic when legacy writers update and delete the base config', async () => { + await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: null, + op: { kind: 'fields', resetPaths: [], fields: { cache: true }, priority: 0 }, + cause: 'save', + actor, + }); + + const legacyUpdated = await methods.upsertConfig( + PrincipalType.ROLE, + BASE_CONFIG_PRINCIPAL_ID, + PrincipalModel.ROLE, + { cache: false }, + 0, + ); + expect(legacyUpdated?.configVersion).toBe(2); + + await methods.deleteConfig(PrincipalType.ROLE, BASE_CONFIG_PRINCIPAL_ID); + + const legacyRecreated = await methods.upsertConfig( + PrincipalType.ROLE, + BASE_CONFIG_PRINCIPAL_ID, + PrincipalModel.ROLE, + { cache: true }, + 0, + ); + expect(legacyRecreated?.configVersion).toBe(3); + + await expect( + methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { kind: 'fields', resetPaths: [], fields: { cache: false }, priority: 0 }, + cause: 'save', + actor, + }), + ).rejects.toBeInstanceOf(ConfigVersionConflictError); + + const recreated = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 3, + op: { kind: 'fields', resetPaths: [], fields: { cache: false }, priority: 0 }, + cause: 'save', + actor, + }); + expect(recreated.changed).toBe(true); + expect(recreated.config?.configVersion).toBe(4); + + await expect( + methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 2, + op: { kind: 'fields', resetPaths: [], fields: { cache: true }, priority: 0 }, + cause: 'save', + actor, + }), + ).rejects.toBeInstanceOf(ConfigVersionConflictError); + }); + + it('treats delete of an already absent base config as a no-op', async () => { + const result = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: null, + op: { kind: 'delete' }, + cause: 'reset', + actor, + }); + + expect(result).toEqual({ changed: false, config: null, revision: null }); + expect( + await mongoose.connection.collection(ADMIN_CONFIG_REVISIONS_COLLECTION).countDocuments(), + ).toBe(0); + }); + + it('preserves dedicated base-principal sections when resetting to defaults', async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + BASE_CONFIG_PRINCIPAL_ID, + PrincipalModel.ROLE, + { cache: true, langfuse: { publicKey: 'pk-current' } }, + 0, + ); + + const result = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { kind: 'delete' }, + cause: 'reset', + actor, + }); + + expectChanged(result); + expect(result.config?.overrides).toEqual({ langfuse: { publicKey: 'pk-current' } }); + expect(result.revision.overrides).toEqual({ + cache: true, + langfuse: { publicKey: 'pk-current' }, + }); + }); + + it('preserves dedicated base-principal sections when restoring an absent snapshot', async () => { + const initial = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: null, + op: { kind: 'fields', resetPaths: [], fields: { cache: true }, priority: 0 }, + cause: 'save', + actor, + }); + expectChanged(initial); + expect(initial.revision.absent).toBe(true); + + await mongoose.models.Config.updateOne( + { principalId: BASE_CONFIG_PRINCIPAL_ID }, + { $set: { 'overrides.langfuse': { publicKey: 'pk-current' } }, $inc: { configVersion: 1 } }, + ); + + const restored = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 2, + op: { kind: 'restore', revisionId: initial.revision.id }, + cause: 'restore', + actor, + }); + + expectChanged(restored); + expect(restored.config?.overrides).toEqual({ langfuse: { publicKey: 'pk-current' } }); + }); + + it('sanitizes unauthorized descendants on container field assignments', async () => { + const result = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: null, + op: { + kind: 'fields', + resetPaths: [], + fields: { + 'interface.mcpServers.trustCheckbox': { + label: 'Allowed', + use: true, + arbitrary: 'kept', + }, + }, + priority: 0, + }, + cause: 'save', + actor, + }); + + expect(result.config?.overrides).toEqual({ + interface: { + mcpServers: { + trustCheckbox: { + label: 'Allowed', + }, + }, + }, + }); + + const existing = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { + kind: 'fields', + resetPaths: [], + fields: { + 'interface.mcpServers.trustCheckbox': { + label: 'Updated', + use: false, + arbitrary: 'again', + }, + }, + priority: 0, + }, + cause: 'save', + actor, + }); + + expect(existing.config?.overrides).toEqual({ + interface: { + mcpServers: { + trustCheckbox: { + label: 'Updated', + }, + }, + }, + }); + }); + + it('strips non-object interface and internal aliases during replace/import', async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + BASE_CONFIG_PRINCIPAL_ID, + PrincipalModel.ROLE, + { cache: false, langfuse: { publicKey: 'pk-current' } }, + 0, + ); + + const result = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { + kind: 'replace', + overrides: { + cache: true, + interface: null, + interfaceConfig: { prompts: false }, + }, + priority: 0, + }, + cause: 'import', + actor, + }); + + expect(result.config?.overrides).toEqual({ + cache: true, + langfuse: { publicKey: 'pk-current' }, + }); + }); + + it('rejects restore of provisional revisions', async () => { + const revisionId = '22222222-2222-4222-8222-222222222222'; + await mongoose.connection.collection(ADMIN_CONFIG_REVISIONS_COLLECTION).insertOne({ + id: revisionId, + createdAt: new Date().toISOString(), + cause: 'save', + actorId: actor.actorId, + tenantId: '', + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + overrides: { cache: true }, + tombstones: [], + priority: 0, + isActive: true, + absent: false, + configVersion: 1, + status: 'provisional', + committed: false, + }); + await methods.upsertConfig( + PrincipalType.ROLE, + BASE_CONFIG_PRINCIPAL_ID, + PrincipalModel.ROLE, + { cache: false }, + 0, + ); + + await expect( + methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { kind: 'restore', revisionId }, + cause: 'restore', + actor, + }), + ).rejects.toBeInstanceOf(ConfigRevisionNotFoundError); + }); + + it('treats cross-tenant revisions as not found', async () => { + const revisionId = '33333333-3333-4333-8333-333333333333'; + await mongoose.connection.collection(ADMIN_CONFIG_REVISIONS_COLLECTION).insertOne({ + id: revisionId, + createdAt: new Date().toISOString(), + cause: 'save', + actorId: actor.actorId, + tenantId: 'other-tenant', + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + overrides: { cache: true }, + tombstones: [], + priority: 0, + isActive: true, + absent: false, + configVersion: 1, + status: 'final', + committed: true, + }); + await methods.upsertConfig( + PrincipalType.ROLE, + BASE_CONFIG_PRINCIPAL_ID, + PrincipalModel.ROLE, + { cache: false }, + 0, + ); + + await expect( + methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { kind: 'restore', revisionId }, + cause: 'restore', + actor, + }), + ).rejects.toBeInstanceOf(ConfigRevisionNotFoundError); + }); + + it('returns 409 when restore races a concurrent toggle', async () => { + const Config = mongoose.models.Config; + await Config.deleteMany({}); + await Config.create({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + overrides: { cache: false }, + tombstones: [], + priority: 0, + isActive: true, + configVersion: 7, + }); + + await expect( + methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 1, + op: { kind: 'fields', resetPaths: [], fields: { cache: true }, priority: 0 }, + cause: 'save', + actor, + }), + ).rejects.toMatchObject({ + name: 'ConfigVersionConflictError', + currentVersion: 7, + }); + }); + + it('replace import preserves inactive base config', async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + BASE_CONFIG_PRINCIPAL_ID, + PrincipalModel.ROLE, + { cache: false }, + 0, + ); + await methods.toggleConfigActive(PrincipalType.ROLE, BASE_CONFIG_PRINCIPAL_ID, false); + + const result = await methods.mutateConfigWithRevision({ + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + principalModel: PrincipalModel.ROLE, + expectedVersion: 2, + op: { + kind: 'replace', + overrides: { cache: true }, + priority: 0, + }, + cause: 'import', + actor, + }); + + expect(result.config?.isActive).toBe(false); + }); +}); diff --git a/packages/data-schemas/src/methods/config.spec.ts b/packages/data-schemas/src/methods/config.spec.ts index f22684fa67d..44ef9dd0831 100644 --- a/packages/data-schemas/src/methods/config.spec.ts +++ b/packages/data-schemas/src/methods/config.spec.ts @@ -2,19 +2,62 @@ import mongoose, { Types } from 'mongoose'; import { MongoMemoryServer } from 'mongodb-memory-server'; import { PrincipalType, PrincipalModel } from 'librechat-data-provider'; import type { IConfig } from '~/types'; -import { createConfigMethods } from './config'; +import { + createConfigMethods, + ensureConfigIndexes, + fieldPathPolicyError, + isValidFieldPath, +} from './config'; import configSchema from '~/schema/config'; +async function dropNonDefaultIndexes(configModel: typeof mongoose.models.Config): Promise { + await configModel.createCollection(); + const indexes = await configModel.collection.indexes(); + for (const index of indexes) { + if (index.name && index.name !== '_id_') { + await configModel.collection.dropIndex(index.name); + } + } +} + +async function openIsolatedConfigContext(): Promise<{ + server: MongoMemoryServer; + conn: mongoose.Connection; + Config: mongoose.Model; + methods: ReturnType; + mongooseLike: typeof mongoose; +}> { + const server = await MongoMemoryServer.create(); + const conn = mongoose.createConnection(server.getUri(), { autoIndex: false }); + await conn.asPromise(); + const Config = conn.model('Config', configSchema); + await Config.createCollection(); + await dropNonDefaultIndexes(Config); + return { + server, + conn, + Config, + methods: createConfigMethods(conn as unknown as typeof mongoose), + mongooseLike: conn as unknown as typeof mongoose, + }; +} + +async function closeIsolatedConfigContext(ctx: { + conn: mongoose.Connection; + server: MongoMemoryServer; +}): Promise { + await ctx.conn.close(); + await ctx.server.stop(); +} + let mongoServer: MongoMemoryServer; let methods: ReturnType; beforeAll(async () => { mongoServer = await MongoMemoryServer.create(); await mongoose.connect(mongoServer.getUri()); - if (!mongoose.models.Config) { - mongoose.model('Config', configSchema); - } - await mongoose.models.Config.init(); + const Config = mongoose.models.Config ?? mongoose.model('Config', configSchema); + await Config.init(); methods = createConfigMethods(mongoose); }); @@ -27,6 +70,22 @@ beforeEach(async () => { await mongoose.models.Config.deleteMany({}); }); +describe('field path policy', () => { + it('accepts simple dot paths', () => { + expect(isValidFieldPath('interface.modelSelect')).toBe(true); + expect(isValidFieldPath('registration.socialLogins')).toBe(true); + }); + + it('rejects empty, non-string, and unsafe segments', () => { + expect(isValidFieldPath('')).toBe(false); + expect(isValidFieldPath(undefined)).toBe(false); + expect(isValidFieldPath('__proto__.polluted')).toBe(false); + expect(isValidFieldPath('cache.__internal-key.value')).toBe(false); + expect(isValidFieldPath('cache.__på.value')).toBe(false); + expect(fieldPathPolicyError('__proto__.polluted')).toMatch(/forbidden segment/); + }); +}); + describe('upsertConfig tombstone preservation', () => { it('creates a new config document', async () => { const result = await methods.upsertConfig( @@ -50,7 +109,7 @@ describe('upsertConfig tombstone preservation', () => { PrincipalType.ROLE, 'admin', PrincipalModel.ROLE, - { interface: { modelSelect: false } }, + { cache: true }, 10, ); @@ -272,6 +331,21 @@ describe('patchConfigFields', () => { expect(result!.principalId).toBe('newrole'); }); + it('rejects unsafe field paths before writing or polluting prototypes', async () => { + await expect( + methods.patchConfigFields( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + { '__proto__.polluted': true }, + 10, + ), + ).rejects.toThrow(/forbidden segment/); + + expect(await mongoose.models.Config.countDocuments({ principalId: 'admin' })).toBe(0); + expect(Object.prototype).not.toHaveProperty('polluted'); + }); + it('clears tombstones for patched paths and their ancestors', async () => { await methods.tombstoneConfigField( PrincipalType.ROLE, @@ -311,6 +385,144 @@ describe('patchConfigFields', () => { expect(result!.tombstones).toContain('mcpServers'); }); + + it('sanitizes legacy protected tombstones during field patches', async () => { + await mongoose.models.Config.create({ + principalType: PrincipalType.ROLE, + principalId: 'admin', + principalModel: PrincipalModel.ROLE, + overrides: {}, + tombstones: ['interface', 'interfaceConfig.prompts', 'cache'], + priority: 10, + isActive: true, + configVersion: 1, + }); + + const result = await methods.patchConfigFields( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + { 'registration.enabled': false }, + 10, + ); + + expect(result!.tombstones).toEqual(['cache']); + }); + + it('retries patch updates after a CAS miss', async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + { cache: false }, + 10, + ); + + const findOneAndUpdateSpy = jest.spyOn(mongoose.models.Config, 'findOneAndUpdate'); + findOneAndUpdateSpy.mockImplementationOnce((async () => null) as never); + + try { + const result = await methods.patchConfigFields( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + { cache: true }, + 10, + ); + + expect(result).toBeTruthy(); + expect(result!.overrides).toEqual({ cache: true }); + expect(findOneAndUpdateSpy).toHaveBeenCalledTimes(2); + } finally { + findOneAndUpdateSpy.mockRestore(); + } + }); + + it('preserves concurrent priority changes during field patch retries', async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + { cache: false }, + 10, + ); + + const findOneAndUpdateSpy = jest.spyOn(mongoose.models.Config, 'findOneAndUpdate'); + findOneAndUpdateSpy.mockImplementationOnce((async () => { + await mongoose.models.Config.updateOne( + { principalId: 'admin' }, + { $set: { priority: 20 }, $inc: { configVersion: 1 } }, + ); + return null; + }) as never); + + try { + const result = await methods.patchConfigFields( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + { cache: true }, + ); + + expect(result!.priority).toBe(20); + expect(findOneAndUpdateSpy).toHaveBeenCalledTimes(2); + } finally { + findOneAndUpdateSpy.mockRestore(); + } + }); + + it('prevents duplicate config documents on concurrent patch creates', async () => { + const [first, second] = await Promise.all([ + methods.patchConfigFields( + PrincipalType.ROLE, + 'race-admin', + PrincipalModel.ROLE, + { cache: true }, + 10, + ), + methods.patchConfigFields( + PrincipalType.ROLE, + 'race-admin', + PrincipalModel.ROLE, + { 'registration.enabled': false }, + 10, + ), + ]); + + expect(first).toBeTruthy(); + expect(second).toBeTruthy(); + expect(await mongoose.models.Config.countDocuments({ principalId: 'race-admin' })).toBe(1); + }); + + it('sanitizes legacy unsafe overrides during field patches', async () => { + await mongoose.models.Config.create({ + principalType: PrincipalType.ROLE, + principalId: 'admin', + principalModel: PrincipalModel.ROLE, + overrides: { + cache: false, + interfaceConfig: { prompts: false }, + interface: null, + }, + tombstones: [], + priority: 10, + isActive: true, + configVersion: 1, + }); + + const result = await methods.patchConfigFields( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + { 'registration.enabled': false }, + 10, + ); + const overrides = result!.overrides as Record; + expect(overrides.cache).toBe(false); + expect(overrides.interfaceConfig).toBeUndefined(); + expect(overrides.interface).toBeUndefined(); + expect(overrides.registration).toEqual({ enabled: false }); + }); }); describe('tombstoneConfigField', () => { @@ -380,6 +592,91 @@ describe('tombstoneConfigField', () => { expect(result!.isActive).toBe(false); expect(result!.tombstones).toContain('mcpServers.github'); }); + + it('sanitizes legacy protected tombstones when adding a new tombstone', async () => { + await mongoose.models.Config.create({ + principalType: PrincipalType.ROLE, + principalId: 'admin', + principalModel: PrincipalModel.ROLE, + overrides: {}, + tombstones: ['interface', 'interfaceConfig.prompts', 'cache'], + priority: 10, + isActive: true, + configVersion: 1, + }); + + const result = await methods.tombstoneConfigField( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + 'mcpServers.github', + 10, + ); + + expect(result!.tombstones).toEqual(['cache', 'mcpServers.github']); + }); + + it('retries tombstone updates after a CAS miss', async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + { cache: false }, + 10, + ); + + const findOneAndUpdateSpy = jest.spyOn(mongoose.models.Config, 'findOneAndUpdate'); + findOneAndUpdateSpy.mockImplementationOnce((async () => null) as never); + + try { + const result = await methods.tombstoneConfigField( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + 'cache', + 10, + ); + + expect(result).toBeTruthy(); + expect(result!.tombstones).toContain('cache'); + expect(findOneAndUpdateSpy).toHaveBeenCalledTimes(2); + } finally { + findOneAndUpdateSpy.mockRestore(); + } + }); + + it('preserves concurrent priority changes during tombstone retries', async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + { cache: false }, + 10, + ); + + const findOneAndUpdateSpy = jest.spyOn(mongoose.models.Config, 'findOneAndUpdate'); + findOneAndUpdateSpy.mockImplementationOnce((async () => { + await mongoose.models.Config.updateOne( + { principalId: 'admin' }, + { $set: { priority: 20 }, $inc: { configVersion: 1 } }, + ); + return null; + }) as never); + + try { + const result = await methods.tombstoneConfigField( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + 'cache', + ); + + expect(result!.priority).toBe(20); + expect(findOneAndUpdateSpy).toHaveBeenCalledTimes(2); + } finally { + findOneAndUpdateSpy.mockRestore(); + } + }); }); describe('upsertConfig', () => { @@ -469,6 +766,15 @@ describe('toggleConfigActive', () => { expect(result!.isActive).toBe(false); }); + it('increments configVersion when toggling active state', async () => { + await methods.upsertConfig(PrincipalType.ROLE, 'admin', PrincipalModel.ROLE, {}, 10); + const before = await mongoose.models.Config.findOne({ principalId: 'admin' }); + expect(before?.configVersion).toBe(1); + + const result = await methods.toggleConfigActive(PrincipalType.ROLE, 'admin', false); + expect(result!.configVersion).toBe(2); + }); + it('reactivates an inactive config', async () => { await methods.upsertConfig(PrincipalType.ROLE, 'admin', PrincipalModel.ROLE, {}, 10); await methods.toggleConfigActive(PrincipalType.ROLE, 'admin', false); @@ -646,3 +952,343 @@ describe('expectEmpty atomic guard', () => { expect(Object.keys(doc!.overrides ?? {}).length).toBeGreaterThan(0); }); }); + +describe('ensureConfigIndexes', () => { + it('builds indexes independently for each Config model', async () => { + const first = await openIsolatedConfigContext(); + const secondServer = await MongoMemoryServer.create(); + const secondConn = mongoose.createConnection(secondServer.getUri(), { autoIndex: false }); + await secondConn.asPromise(); + const secondConfig = secondConn.model('Config', configSchema); + await secondConfig.createCollection(); + + const firstSpy = jest.spyOn(first.Config, 'createIndexes'); + const secondSpy = jest.spyOn(secondConfig, 'createIndexes'); + + try { + await ensureConfigIndexes(first.mongooseLike); + await ensureConfigIndexes(secondConn as unknown as typeof mongoose); + + expect(firstSpy).toHaveBeenCalledTimes(1); + expect(secondSpy).toHaveBeenCalledTimes(1); + } finally { + firstSpy.mockRestore(); + secondSpy.mockRestore(); + await closeIsolatedConfigContext(first); + await secondConn.close(); + await secondServer.stop(); + } + }); + + it('creates the unique index before concurrent patch creates when autoIndex is disabled', async () => { + const ctx = await openIsolatedConfigContext(); + const createIndexesSpy = jest.spyOn(ctx.Config, 'createIndexes'); + + try { + const [first, second] = await Promise.all([ + ctx.methods.patchConfigFields( + PrincipalType.ROLE, + 'race-admin', + PrincipalModel.ROLE, + { cache: true }, + 10, + ), + ctx.methods.patchConfigFields( + PrincipalType.ROLE, + 'race-admin', + PrincipalModel.ROLE, + { 'registration.enabled': false }, + 10, + ), + ]); + + expect(createIndexesSpy).toHaveBeenCalled(); + expect(first).toBeTruthy(); + expect(second).toBeTruthy(); + expect(await ctx.Config.countDocuments({ principalId: 'race-admin' })).toBe(1); + } finally { + createIndexesSpy.mockRestore(); + await closeIsolatedConfigContext(ctx); + } + }); + + it('retries ensureConfigIndexes after a rejected build', async () => { + const ctx = await openIsolatedConfigContext(); + const originalCreateIndexes = ctx.Config.createIndexes.bind(ctx.Config); + let createIndexCalls = 0; + const createIndexesSpy = jest.spyOn(ctx.Config, 'createIndexes').mockImplementation(function ( + this: typeof ctx.Config, + ...args: Parameters + ) { + createIndexCalls += 1; + if (createIndexCalls === 1) { + return Promise.reject(new Error('index build denied')); + } + return originalCreateIndexes(...args); + }); + + try { + await expect(ensureConfigIndexes(ctx.mongooseLike)).rejects.toThrow('index build denied'); + + await ensureConfigIndexes(ctx.mongooseLike); + + expect(createIndexCalls).toBe(2); + + const indexes = await ctx.Config.collection.indexes(); + const uniquePrincipalIndex = indexes.find( + (index) => + index.unique === true && + index.key?.principalType === 1 && + index.key?.principalId === 1 && + index.key?.tenantId === 1, + ); + expect(uniquePrincipalIndex).toBeDefined(); + } finally { + createIndexesSpy.mockRestore(); + await closeIsolatedConfigContext(ctx); + } + }); + + it('does not build indexes when patchConfigFields rejects oversized paths', async () => { + const ctx = await openIsolatedConfigContext(); + const createIndexesSpy = jest.spyOn(ctx.Config, 'createIndexes'); + const oversizedKey = Array.from({ length: 33 }, (_, i) => `s${i}`).join('.'); + + try { + await expect( + ctx.methods.patchConfigFields( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + { [oversizedKey]: true }, + 10, + ), + ).rejects.toThrow(/maximum depth/); + expect(createIndexesSpy).not.toHaveBeenCalled(); + } finally { + createIndexesSpy.mockRestore(); + await closeIsolatedConfigContext(ctx); + } + }); + + it('does not build indexes when tombstoneConfigField rejects oversized paths', async () => { + const ctx = await openIsolatedConfigContext(); + const createIndexesSpy = jest.spyOn(ctx.Config, 'createIndexes'); + const oversizedPath = Array.from({ length: 33 }, (_, i) => `s${i}`).join('.'); + + try { + await expect( + ctx.methods.tombstoneConfigField( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + oversizedPath, + 10, + ), + ).rejects.toThrow(/maximum depth/); + expect(createIndexesSpy).not.toHaveBeenCalled(); + } finally { + createIndexesSpy.mockRestore(); + await closeIsolatedConfigContext(ctx); + } + }); + + it('deduplicates principals with the same key and keeps the highest configVersion', async () => { + const ctx = await openIsolatedConfigContext(); + try { + await ctx.Config.collection.insertMany([ + { + principalType: PrincipalType.ROLE, + principalId: 'dup-role', + principalModel: PrincipalModel.ROLE, + priority: 10, + overrides: { cache: false }, + tombstones: [], + isActive: true, + configVersion: 1, + tenantId: '', + }, + { + principalType: PrincipalType.ROLE, + principalId: 'dup-role', + principalModel: PrincipalModel.ROLE, + priority: 10, + overrides: { cache: true }, + tombstones: [], + isActive: true, + configVersion: 2, + tenantId: '', + }, + ]); + + await ensureConfigIndexes(ctx.mongooseLike); + + expect(await ctx.Config.countDocuments({ principalId: 'dup-role' })).toBe(1); + const survivor = await ctx.Config.findOne({ principalId: 'dup-role' }); + expect((survivor?.overrides as { cache?: boolean })?.cache).toBe(true); + } finally { + await closeIsolatedConfigContext(ctx); + } + }); + + it('normalizes missing tenantId to null before building the index', async () => { + const ctx = await openIsolatedConfigContext(); + try { + // Insert a doc without tenantId (simulates a write from an old pod without the default) + await ctx.Config.collection.insertOne({ + principalType: PrincipalType.ROLE, + principalId: 'missing-tenant', + principalModel: PrincipalModel.ROLE, + priority: 10, + overrides: { cache: false }, + tombstones: [], + isActive: true, + configVersion: 1, + } as never); + + await ensureConfigIndexes(ctx.mongooseLike); + + const doc = await ctx.Config.collection.findOne({ principalId: 'missing-tenant' }); + expect(doc?.tenantId).toBeNull(); + } finally { + await closeIsolatedConfigContext(ctx); + } + }); + + it('deduplicates an alias pair (null and empty-string tenantId) even when the unique index already exists', async () => { + const ctx = await openIsolatedConfigContext(); + try { + // Pre-build the unique index directly (simulates a prior deployment) + await ctx.Config.collection.createIndex( + { principalType: 1, principalId: 1, tenantId: 1 }, + { unique: true }, + ); + + // Two logical-duplicate docs — '' and null are different index keys so both inserts succeed + await ctx.Config.collection.insertOne({ + principalType: PrincipalType.ROLE, + principalId: 'alias-pair', + principalModel: PrincipalModel.ROLE, + priority: 10, + overrides: { cache: false }, + tombstones: [], + isActive: true, + configVersion: 1, + tenantId: '', + }); + await ctx.Config.collection.insertOne({ + principalType: PrincipalType.ROLE, + principalId: 'alias-pair', + principalModel: PrincipalModel.ROLE, + priority: 10, + overrides: { cache: true }, + tombstones: [], + isActive: true, + configVersion: 2, + tenantId: null, + }); + + // Must not throw E11000 — dedup runs before any canonicalization + await ensureConfigIndexes(ctx.mongooseLike); + + expect(await ctx.Config.countDocuments({ principalId: 'alias-pair' })).toBe(1); + const survivor = await ctx.Config.findOne({ principalId: 'alias-pair' }); + expect((survivor?.overrides as { cache?: boolean })?.cache).toBe(true); + // Winner (null, v2) should stay null — no regression from canonicalization + expect(survivor?.tenantId).toBeNull(); + } finally { + await closeIsolatedConfigContext(ctx); + } + }); + + it('canonicalizes an empty-string winner to null and then rejects a missing-tenantId insert', async () => { + const ctx = await openIsolatedConfigContext(); + try { + // Empty-string doc has the higher version — it wins the dedup sort + await ctx.Config.collection.insertMany([ + { + principalType: PrincipalType.ROLE, + principalId: 'empty-winner', + principalModel: PrincipalModel.ROLE, + priority: 10, + overrides: { cache: true }, + tombstones: [], + isActive: true, + configVersion: 2, + tenantId: '', + }, + { + principalType: PrincipalType.ROLE, + principalId: 'empty-winner', + principalModel: PrincipalModel.ROLE, + priority: 10, + overrides: { cache: false }, + tombstones: [], + isActive: true, + configVersion: 1, + tenantId: null, + }, + ]); + + await ensureConfigIndexes(ctx.mongooseLike); + + expect(await ctx.Config.countDocuments({ principalId: 'empty-winner' })).toBe(1); + const survivor = await ctx.Config.collection.findOne({ principalId: 'empty-winner' }); + // '' winner must be canonicalized to null so it occupies the same index slot + expect(survivor?.tenantId).toBeNull(); + + // A subsequent missing-tenantId write for the same principal must be rejected + await expect( + ctx.Config.collection.insertOne({ + principalType: PrincipalType.ROLE, + principalId: 'empty-winner', + principalModel: PrincipalModel.ROLE, + priority: 10, + overrides: {}, + tombstones: [], + isActive: true, + configVersion: 3, + } as never), + ).rejects.toMatchObject({ code: 11000 }); + } finally { + await closeIsolatedConfigContext(ctx); + } + }); + + it('retries on E11000 thrown by the canonicalization step inside deduplicateConfigPrincipals', async () => { + const ctx = await openIsolatedConfigContext(); + const originalUpdateMany = ctx.Config.collection.updateMany.bind(ctx.Config.collection); + let updateManyCalls = 0; + ctx.Config.collection.updateMany = ((...args: Parameters) => { + updateManyCalls += 1; + if (updateManyCalls === 1) { + return Promise.reject( + Object.assign(new Error('E11000 simulated concurrent alias'), { code: 11000 }), + ); + } + return originalUpdateMany(...args); + }) as typeof ctx.Config.collection.updateMany; + + try { + await ctx.Config.collection.insertOne({ + principalType: PrincipalType.ROLE, + principalId: 'retry-canon', + principalModel: PrincipalModel.ROLE, + priority: 10, + overrides: {}, + tombstones: [], + isActive: true, + configVersion: 1, + tenantId: null, + }); + + // With deduplicateConfigPrincipals inside the retry boundary, the E11000 from + // the canonicalization step is caught and the next attempt succeeds. + await expect(ensureConfigIndexes(ctx.mongooseLike)).resolves.not.toThrow(); + expect(updateManyCalls).toBeGreaterThanOrEqual(2); + } finally { + ctx.Config.collection.updateMany = originalUpdateMany; + await closeIsolatedConfigContext(ctx); + } + }); +}); diff --git a/packages/data-schemas/src/methods/config.tenant.spec.ts b/packages/data-schemas/src/methods/config.tenant.spec.ts new file mode 100644 index 00000000000..5f1d81b8e2e --- /dev/null +++ b/packages/data-schemas/src/methods/config.tenant.spec.ts @@ -0,0 +1,82 @@ +import mongoose from 'mongoose'; +import { MongoMemoryServer } from 'mongodb-memory-server'; +import { PrincipalType, PrincipalModel } from 'librechat-data-provider'; +import { tenantStorage } from '~/config/tenantContext'; +import { createConfigModel } from '~/models/config'; +import { createConfigMethods } from './config'; + +const TENANT_A = 'tenant-aaaaaaaaaaaaaaaaaaaa'; +const TENANT_B = 'tenant-bbbbbbbbbbbbbbbbbbbb'; + +let mongoServer: MongoMemoryServer; +let methods: ReturnType; + +function runAs(tenantId: string, fn: () => Promise): Promise { + return tenantStorage.run({ tenantId }, fn); +} + +beforeAll(async () => { + mongoServer = await MongoMemoryServer.create(); + await mongoose.connect(mongoServer.getUri()); + createConfigModel(mongoose); + methods = createConfigMethods(mongoose); +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongoServer.stop(); +}); + +beforeEach(async () => { + await mongoose.models.Config.deleteMany({}); +}); + +describe('findConfigByPrincipal tenant isolation', () => { + it('returns only configs for the active tenant context when tenantId is omitted', async () => { + await runAs(TENANT_A, async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + { cache: true }, + 10, + ); + }); + await runAs(TENANT_B, async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + { cache: false }, + 20, + ); + }); + + const tenantAConfig = await runAs(TENANT_A, () => + methods.findConfigByPrincipal(PrincipalType.ROLE, 'admin'), + ); + expect(tenantAConfig?.overrides).toEqual({ cache: true }); + + const tenantBConfig = await runAs(TENANT_B, () => + methods.findConfigByPrincipal(PrincipalType.ROLE, 'admin'), + ); + expect(tenantBConfig?.overrides).toEqual({ cache: false }); + }); + + it('does not cross-match when explicit legacy tenantId filter contradicts plugin context', async () => { + await runAs(TENANT_A, async () => { + await methods.upsertConfig( + PrincipalType.ROLE, + 'admin', + PrincipalModel.ROLE, + { cache: true }, + 10, + ); + }); + + const withoutContext = await methods.findConfigByPrincipal(PrincipalType.ROLE, 'admin', { + tenantId: '', + }); + expect(withoutContext).toBeNull(); + }); +}); diff --git a/packages/data-schemas/src/methods/config.ts b/packages/data-schemas/src/methods/config.ts index 16f94efd9ea..c44b6e5d5ec 100644 --- a/packages/data-schemas/src/methods/config.ts +++ b/packages/data-schemas/src/methods/config.ts @@ -1,29 +1,710 @@ import { Types } from 'mongoose'; -import { PrincipalType, PrincipalModel } from 'librechat-data-provider'; +import { randomUUID } from 'crypto'; +import { + BASE_PRINCIPAL_CONFIG_SECTIONS, + PrincipalType, + PrincipalModel, +} from 'librechat-data-provider'; import type { FilterQuery, Model, ClientSession } from 'mongoose'; import type { TCustomConfig } from 'librechat-data-provider'; import type { IConfig } from '~/types'; +import { + sanitizeAdminConfigOverrides, + sanitizeAdminConfigTombstones, +} from '~/admin/configOverrides'; +import { getTenantId, SYSTEM_TENANT_ID } from '~/config/tenantContext'; +import { indexedArrayPathError } from '~/admin/indexedArrayPath'; import { BASE_CONFIG_PRINCIPAL_ID } from '~/admin/capabilities'; import { escapeRegExp } from '~/utils/string'; +export const ADMIN_CONFIG_REVISIONS_COLLECTION = 'admin_config_revisions'; +/** High-water CAS versions that survive document deletion (prevents ABA reuse). */ +export const ADMIN_CONFIG_VERSION_EPOCHS_COLLECTION = 'admin_config_version_epochs'; +export const MAX_CONFIG_REVISIONS = 50; +const MAX_CONFIG_CAS_RETRIES = 5; +const DEFAULT_CONFIG_PRIORITY = 10; +const BASE_PRINCIPAL_OVERRIDE_SECTIONS = new Set(BASE_PRINCIPAL_CONFIG_SECTIONS); + +const configIndexPromises = new WeakMap, Promise>(); + +const TENANT_ALIAS_FILTER = { $or: [{ tenantId: { $exists: false } }, { tenantId: '' }] }; + +function epochTenantKey(tenantId?: string | null): string | null { + if (tenantId == null || tenantId === '' || tenantId === SYSTEM_TENANT_ID) { + return null; + } + return tenantId; +} + +function isBaseConfigPrincipal(principalType: PrincipalType, principalId: string): boolean { + return principalType === PrincipalType.ROLE && principalId === BASE_CONFIG_PRINCIPAL_ID; +} + +function baseConfigEpochFilter(tenantId?: string | null): { + principalType: PrincipalType; + principalId: string; + tenantId: string | null; +} { + return { + principalType: PrincipalType.ROLE, + principalId: BASE_CONFIG_PRINCIPAL_ID, + tenantId: epochTenantKey(tenantId ?? getTenantId()), + }; +} + +function epochCollection(Config: Model) { + return Config.db.collection(ADMIN_CONFIG_VERSION_EPOCHS_COLLECTION); +} + +function readAllocatedEpochVersion(result: unknown): number { + if (result == null || typeof result !== 'object') { + return 1; + } + const direct = result as { version?: number; value?: { version?: number } | null }; + if (typeof direct.version === 'number') { + return direct.version; + } + if (direct.value != null && typeof direct.value.version === 'number') { + return direct.value.version; + } + return 1; +} + +/** + * Raises the durable CAS high-water mark for the base config. Uses `Config.db` + * so connection-local models (not only the default mongoose connection) work. + */ +async function raiseBaseConfigVersionEpoch( + Config: Model, + version: number, + session?: ClientSession, + tenantId?: string | null, +): Promise { + await epochCollection(Config).updateOne( + baseConfigEpochFilter(tenantId), + { $max: { version } }, + { upsert: true, ...(session ? { session } : {}) }, + ); +} + +/** + * Atomically allocates the next base-config CAS version from the epoch. + * Prefer calling inside the same session/transaction as the Config create that + * persists the returned value; session may be omitted on standalone MongoDB. + */ +async function allocateBaseConfigVersion( + Config: Model, + session?: ClientSession, + tenantId?: string | null, +): Promise { + const result = await epochCollection(Config).findOneAndUpdate( + baseConfigEpochFilter(tenantId), + { $inc: { version: 1 } }, + { upsert: true, returnDocument: 'after', ...(session ? { session } : {}) }, + ); + return readAllocatedEpochVersion(result); +} + +function isTransactionUnsupported(err: unknown): boolean { + const message = err instanceof Error ? err.message : String(err); + return ( + message.includes('Transaction numbers are only allowed') || + message.includes('transaction numbers are only allowed') + ); +} + +/** + * Runs `fn` in a transaction on `Config.db` when possible. Falls back to + * session-less execution on standalone MongoDB (e.g. unit-test memory servers) + * while still allocating/raising the epoch around the Config write. + */ +async function withOwnedSession( + Config: Model, + session: ClientSession | undefined, + fn: (session: ClientSession | undefined) => Promise, +): Promise { + if (session) { + return fn(session); + } + const owned = await Config.db.startSession(); + try { + let outcome!: T; + try { + await owned.withTransaction(async () => { + outcome = await fn(owned); + }); + return outcome; + } catch (err) { + if (!isTransactionUnsupported(err)) { + throw err; + } + } + } finally { + await owned.endSession(); + } + return fn(undefined); +} + +/** + * Removes duplicate Config docs for the same principal+tenant before the unique + * index is built. Uses a fast path when the unique index already exists and all + * legacy empty-string aliases have been canonicalized, so repeated pod startups + * after migration complete do not scan the whole collection. + * + * Uses raw collection operations to bypass tenant-isolation middleware. + * Groups by $ifNull so null, missing, and '' are all treated as the same logical + * tenant. Deletes only explicit loser IDs to avoid racing with concurrent inserts. + * + * Canonical value for "no tenant" is null. MongoDB indexes null and absent fields + * identically, so old-pod writes (missing tenantId) and new-pod writes (null) + * collide correctly in the unique index during rolling deployments. + */ +async function deduplicateConfigPrincipals(Config: Model): Promise { + let indexes: Array<{ + unique?: boolean; + key?: Record; + partialFilterExpression?: unknown; + sparse?: boolean; + }> = []; + try { + // eslint-disable-next-line no-restricted-syntax -- cross-tenant migration; must bypass tenant middleware + indexes = await Config.collection.listIndexes().toArray(); + } catch (err) { + if ((err as { code?: number }).code === 26) { + return; + } + throw err; + } + + const REQUIRED_KEYS = new Set(['principalType', 'principalId', 'tenantId']); + const hasUnique = indexes.some((idx) => { + if (!idx.unique || !idx.key) return false; + const keys = Object.keys(idx.key); + if (keys.length !== REQUIRED_KEYS.size) return false; + if (keys.some((k) => !REQUIRED_KEYS.has(k))) return false; + if (idx.partialFilterExpression != null || idx.sparse === true) return false; + return true; + }); + + if (hasUnique) { + // Migration has already run. A cheap indexed probe (single-field tenantId index) + // checks for any surviving legacy empty-string aliases. If none remain, skip the + // collection-wide aggregation — it would scan every null/missing entry anyway + // because MongoDB's non-sparse index cannot distinguish them. + // eslint-disable-next-line no-restricted-syntax -- cross-tenant migration; must bypass tenant middleware + const legacyCount = await Config.collection.countDocuments({ tenantId: '' }, { limit: 1 }); + if (legacyCount === 0) { + return; + } + } + + // Full dedup: index not yet built, or legacy '' aliases still present. + // Dedup before canonicalization so the updateMany below cannot produce E11000 + // when the existing index already contains both '' and null for the same principal. + // eslint-disable-next-line no-restricted-syntax -- cross-tenant migration; must bypass tenant middleware + const loserDocs = await Config.collection + .aggregate<{ loserId: Types.ObjectId }>([ + { $sort: { configVersion: -1, createdAt: -1 } }, + { + $group: { + _id: { + principalType: '$principalType', + principalId: '$principalId', + tenantId: { $ifNull: ['$tenantId', ''] }, + }, + keepId: { $first: '$_id' }, + allIds: { $push: '$_id' }, + }, + }, + { $unwind: '$allIds' }, + { $match: { $expr: { $ne: ['$allIds', '$keepId'] } } }, + { $project: { loserId: '$allIds' } }, + ]) + .toArray(); + + const loserIds = loserDocs.map((d) => d.loserId); + if (loserIds.length > 0) { + // eslint-disable-next-line no-restricted-syntax -- cross-tenant migration; must bypass tenant middleware + await Config.collection.deleteMany({ _id: { $in: loserIds } }); + } + + // Canonicalize both missing and empty-string tenantId to null after dedup. + // After deletion there is at most one doc per logical principal, so this update + // cannot collide even when the unique index already exists. + // eslint-disable-next-line no-restricted-syntax -- cross-tenant migration; must bypass tenant middleware + await Config.collection.updateMany(TENANT_ALIAS_FILTER, { $set: { tenantId: null } }); +} + +/** + * The concurrent-create retry in patch/tombstone flows depends on duplicate-key + * error 11000 from `{ principalType, principalId, tenantId }` unique index. + * With `MONGO_AUTO_INDEX=false` or blank `MONGO_AUTO_INDEX`, a fresh deployment + * may never build that index and both creates can succeed. Build it once before + * the first write so duplicate prevention never depends on a background build. + * + * A dedup migration runs first when the unique index is absent so that existing + * deployments with logical duplicates do not fail startup with a build error. + */ +export function ensureConfigIndexes(mongoose: typeof import('mongoose')): Promise { + const Config = mongoose.models.Config as Model | undefined; + if (!Config) { + return Promise.resolve(); + } + const existing = configIndexPromises.get(Config); + if (existing) { + return existing; + } + const MAX_INDEX_BUILD_RETRIES = 3; + const promise = (async () => { + let lastErr: unknown; + for (let attempt = 0; attempt < MAX_INDEX_BUILD_RETRIES; attempt += 1) { + try { + await deduplicateConfigPrincipals(Config); + await Promise.all([ + Config.createIndexes(), + ensureRevisionCollectionIndexes(Config), + ensureVersionEpochIndexes(Config), + ]); + // Best-effort startup warmup: raise epoch to match all existing base + // config versions so pre-epoch writes cannot be reused as CAS targets. + // This is NOT a rolling-upgrade fence — an old pod that writes and + // deletes after this scan can still leave the epoch behind. Deployments + // with concurrent pre-epoch writers must drain old pods before new ones + // begin serving requests. + // Use Config.collection (raw) to bypass tenant-isolation middleware so + // this scan works under TENANT_ISOLATION_STRICT=true at startup. + // eslint-disable-next-line no-restricted-syntax -- intentional cross-tenant startup migration + const baseDocs = await Config.collection + .find( + { principalType: PrincipalType.ROLE, principalId: BASE_CONFIG_PRINCIPAL_ID }, + { projection: { configVersion: 1, tenantId: 1 } }, + ) + .toArray(); + await Promise.all( + baseDocs.map(async (doc) => { + const version = interpretedConfigVersion(doc as { configVersion?: number | null }); + if (version != null) { + await raiseBaseConfigVersionEpoch( + Config, + version, + undefined, + (doc.tenantId as string | null | undefined) ?? null, + ); + } + }), + ); + return; + } catch (err) { + if ((err as { code?: number }).code === 11000 && attempt < MAX_INDEX_BUILD_RETRIES - 1) { + lastErr = err; + continue; + } + throw err; + } + } + throw lastErr; + })().catch((err) => { + configIndexPromises.delete(Config); + throw err; + }); + configIndexPromises.set(Config, promise); + return promise; +} + +async function ensureRevisionCollectionIndexes(Config: Model): Promise { + const revisions = Config.db.collection(ADMIN_CONFIG_REVISIONS_COLLECTION); + await Promise.all([ + revisions.createIndex( + { id: 1 }, + { name: 'revision_id_lookup', unique: true, background: true }, + ), + revisions.createIndex( + { tenantId: 1, principalType: 1, principalId: 1, status: 1, createdAt: -1 }, + { name: 'scope_status_created', background: true }, + ), + ]); +} + +async function ensureVersionEpochIndexes(Config: Model): Promise { + const epochs = Config.db.collection(ADMIN_CONFIG_VERSION_EPOCHS_COLLECTION); + await epochs.createIndex( + { tenantId: 1, principalType: 1, principalId: 1 }, + { name: 'epoch_scope_unique', unique: true, background: true }, + ); +} + +export type ConfigRevisionCause = 'save' | 'import' | 'reset' | 'restore'; + +export type ConfigMutationResult = + | { changed: true; config: IConfig | null; revision: ConfigRevisionSnapshot } + | { changed: false; config: null; revision: null }; + +export class ConfigVersionConflictError extends Error { + readonly currentVersion: number | null; + constructor(currentVersion: number | null) { + super('Config version conflict'); + this.name = 'ConfigVersionConflictError'; + this.currentVersion = currentVersion; + } +} + +export class ConfigRevisionNotFoundError extends Error { + constructor(revisionId: string) { + super('Revision not found'); + this.name = 'ConfigRevisionNotFoundError'; + this.revisionId = revisionId; + } + readonly revisionId: string; +} + +export class TransactionRequiredError extends Error { + constructor() { + super( + 'Base config saves require a MongoDB replica set. ' + + 'Set mongodb.architecture to "replicaset" in your Helm values.', + ); + this.name = 'TransactionRequiredError'; + } +} + +export interface ConfigRevisionActor { + actorId: string; + actorEmail?: string; + tenantId: string; +} + +export type ConfigMutationOp = + | { kind: 'fields'; resetPaths: string[]; fields: Record; priority: number } + | { kind: 'replace'; overrides: Record; priority: number } + | { kind: 'delete' } + | { kind: 'restore'; revisionId: string }; + +export interface ConfigRevisionSnapshot { + id: string; + createdAt: string; + cause: ConfigRevisionCause; + actorId: string; + actorEmail?: string; + tenantId: string; + principalType: PrincipalType; + principalId: string; + overrides: Record; + tombstones: string[]; + priority: number | null; + isActive: boolean | null; + absent: boolean; + configVersion: number | null; + status: 'final'; + committed: true; +} + +export const MAX_FIELD_PATH_LENGTH = 512; +export const MAX_FIELD_PATH_SEGMENTS = 32; + +const UNSAFE_FIELD_PATH_SEGMENTS = /(?:^|\.)(\$[^.]*|__[^.]*|constructor|prototype)(?:\.|$)/; + +export function fieldPathLimitError(path: string): string | null { + if (path.length > MAX_FIELD_PATH_LENGTH) { + return `field path exceeds maximum length of ${MAX_FIELD_PATH_LENGTH}`; + } + let segmentCount = 1; + for (let i = 0; i < path.length; i += 1) { + if (path[i] === '.') { + segmentCount += 1; + if (segmentCount > MAX_FIELD_PATH_SEGMENTS) { + return `field path exceeds maximum depth of ${MAX_FIELD_PATH_SEGMENTS} segments`; + } + } + } + return null; +} + +export function fieldPathPolicyError(path: unknown): string | null { + if (typeof path !== 'string') { + return 'field path must be a string'; + } + const limitError = fieldPathLimitError(path); + if (limitError) { + return limitError; + } + if (path.length === 0) { + return 'field path must not be empty'; + } + if (path.startsWith('.') || path.endsWith('.') || path.includes('..')) { + return 'field path has invalid structure'; + } + if (UNSAFE_FIELD_PATH_SEGMENTS.test(path)) { + return 'field path contains forbidden segment'; + } + return null; +} + +export function isValidFieldPath(path: unknown): path is string { + return fieldPathPolicyError(path) === null; +} + +export type FindConfigByPrincipalOptions = { + includeInactive?: boolean; + /** + * When set, applies an explicit tenant predicate (empty string matches legacy untagged docs). + * When omitted, tenant isolation on the Config model applies tenant from async context. + */ + tenantId?: string; +}; + +function assertValidFieldPath(fieldPath: string): void { + const policyError = fieldPathPolicyError(fieldPath); + if (policyError) { + throw new Error(policyError); + } + const indexedError = indexedArrayPathError(fieldPath); + if (indexedError) { + throw new Error(indexedError); + } +} + +/** Deduplicate reset paths and drop descendants when an ancestor is already reset. */ +export function canonicalizeResetPaths(paths: string[]): string[] { + const unique = new Set(); + for (const path of paths) { + const policyError = fieldPathPolicyError(path); + if (policyError) { + throw new Error(policyError); + } + unique.add(path); + } + const kept: string[] = []; + for (const path of unique) { + let dominated = false; + const parts = path.split('.'); + if (parts.length === 0) { + continue; + } + let ancestor = parts[0]; + for (let i = 1; i < parts.length; i += 1) { + if (unique.has(ancestor)) { + dominated = true; + break; + } + ancestor = `${ancestor}.${parts[i]}`; + } + if (!dominated) { + kept.push(path); + } + } + kept.sort((a, b) => a.localeCompare(b)); + return kept; +} + +function cloneOverrides(source: unknown): Record { + if (!source || typeof source !== 'object' || Array.isArray(source)) { + return {}; + } + return structuredClone(source) as Record; +} + +function isBasePrincipalSectionPath(path: string): boolean { + return BASE_PRINCIPAL_OVERRIDE_SECTIONS.has(path.split('.')[0]); +} + +function preserveBasePrincipalOverrides( + next: Record, + current: unknown, +): Record { + const preserved = cloneOverrides(next); + const currentOverrides = cloneOverrides(current); + for (const section of BASE_PRINCIPAL_OVERRIDE_SECTIONS) { + if (Object.prototype.hasOwnProperty.call(currentOverrides, section)) { + preserved[section] = structuredClone(currentOverrides[section]); + } else { + delete preserved[section]; + } + } + return preserved; +} + +function preserveBasePrincipalTombstones(next: string[], current?: string[]): string[] { + return [ + ...next.filter((path) => !isBasePrincipalSectionPath(path)), + ...(current ?? []).filter(isBasePrincipalSectionPath), + ]; +} + +function hasBasePrincipalState(config: IConfig | null): boolean { + const overrides = cloneOverrides(config?.overrides); + return ( + [...BASE_PRINCIPAL_OVERRIDE_SECTIONS].some((section) => + Object.prototype.hasOwnProperty.call(overrides, section), + ) || (config?.tombstones ?? []).some(isBasePrincipalSectionPath) + ); +} + +function unsetPath(obj: Record, fieldPath: string): void { + const parts = fieldPath.split('.'); + let current: unknown = obj; + for (let i = 0; i < parts.length - 1; i += 1) { + if (current == null || typeof current !== 'object' || Array.isArray(current)) { + return; + } + current = (current as Record)[parts[i]]; + } + if (current && typeof current === 'object' && !Array.isArray(current)) { + delete (current as Record)[parts[parts.length - 1]]; + } +} + +function setPath(obj: Record, fieldPath: string, value: unknown): void { + const parts = fieldPath.split('.'); + let current = obj; + for (let i = 0; i < parts.length - 1; i += 1) { + const key = parts[i]; + const next = current[key]; + if (next == null || typeof next !== 'object' || Array.isArray(next)) { + current[key] = {}; + } + current = current[key] as Record; + } + current[parts[parts.length - 1]] = value; +} + +function applyFieldsMutation( + overrides: Record, + resetPaths: string[], + fields: Record, +): Record { + for (const path of resetPaths) { + assertValidFieldPath(path); + } + for (const path of Object.keys(fields)) { + assertValidFieldPath(path); + } + const next = cloneOverrides(overrides); + for (const path of canonicalizeResetPaths(resetPaths)) { + unsetPath(next, path); + } + for (const [path, value] of Object.entries(fields)) { + setPath(next, path, value); + } + return next; +} + function getTombstonePathsToClear(fieldPath: string): string[] { + assertValidFieldPath(fieldPath); const parts = fieldPath.split('.'); if (parts.length <= 1) { return [fieldPath]; } - return parts.slice(1).map((_, index) => parts.slice(0, index + 2).join('.')); + const paths: string[] = []; + let prefix = parts[0]; + for (let i = 1; i < parts.length; i += 1) { + prefix = `${prefix}.${parts[i]}`; + paths.push(prefix); + } + return paths; } function getPathAndDescendantsRegex(fieldPath: string): RegExp { return new RegExp(`^${escapeRegExp(fieldPath)}(?:\\.|$)`); } +function nextTombstones( + current: string[] | undefined, + resetPaths: string[], + fieldPaths: string[], +): string[] { + const existing = current ?? []; + const resetMatchers = canonicalizeResetPaths(resetPaths).map(getPathAndDescendantsRegex); + const cleared = new Set(fieldPaths.flatMap(getTombstonePathsToClear)); + return existing.filter((tombstone) => { + if (cleared.has(tombstone)) { + return false; + } + return !resetMatchers.some((matcher) => matcher.test(tombstone)); + }); +} + +function tenantRevisionFilter(tenantId: string): Record { + if (tenantId.length > 0) { + return { tenantId }; + } + return { $or: [{ tenantId: { $exists: false } }, { tenantId: null }, { tenantId: '' }] }; +} + +function tenantPrincipalFilter( + tenantId: string, + principalType: PrincipalType, + principalId: string, +): FilterQuery { + return { + principalType, + principalId, + ...tenantRevisionFilter(tenantId), + }; +} + +function revisionScopeFilter( + tenantId: string, + principalType: PrincipalType, + principalId: string, +): Record { + return { + ...tenantRevisionFilter(tenantId), + principalType, + principalId, + }; +} + +function interpretedConfigVersion(doc: { configVersion?: number | null } | null): number | null { + if (doc == null) return null; + return doc.configVersion ?? 0; +} + +/** CAS filter: a missing/null configVersion is treated as 0 for legacy documents. */ +function versionCasFilter(id: unknown, currentVersion: number | null): Record { + if (currentVersion === 0) { + return { + _id: id, + $or: [{ configVersion: 0 }, { configVersion: null }, { configVersion: { $exists: false } }], + }; + } + return { _id: id, configVersion: currentVersion }; +} + +function snapshotFromConfig( + current: IConfig | null, + params: { + cause: ConfigRevisionCause; + actor: ConfigRevisionActor; + principalType: PrincipalType; + principalId: string; + }, +): ConfigRevisionSnapshot { + return { + id: randomUUID(), + createdAt: new Date().toISOString(), + cause: params.cause, + actorId: params.actor.actorId, + actorEmail: params.actor.actorEmail, + tenantId: params.actor.tenantId, + principalType: params.principalType, + principalId: params.principalId, + overrides: cloneOverrides(current?.overrides), + tombstones: [...(current?.tombstones ?? [])], + priority: current?.priority ?? null, + isActive: current == null ? null : current.isActive, + absent: current == null, + configVersion: interpretedConfigVersion(current), + status: 'final', + committed: true, + }; +} + export function createConfigMethods(mongoose: typeof import('mongoose')): { listAllConfigs: (filter?: { isActive?: boolean }, session?: ClientSession) => Promise; findConfigByPrincipal: ( principalType: PrincipalType, principalId: string | Types.ObjectId, - options?: { includeInactive?: boolean }, + options?: FindConfigByPrincipalOptions, session?: ClientSession, ) => Promise; getApplicableConfigs: ( @@ -44,7 +725,7 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { principalId: string | Types.ObjectId, principalModel: PrincipalModel, fields: Record, - priority: number, + priority?: number, session?: ClientSession, ) => Promise; tombstoneConfigField: ( @@ -52,7 +733,7 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { principalId: string | Types.ObjectId, principalModel: PrincipalModel, fieldPath: string, - priority: number, + priority?: number, session?: ClientSession, ) => Promise; unsetConfigField: ( @@ -74,18 +755,30 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { session?: ClientSession, options?: { expectEmpty?: boolean }, ) => Promise; + mutateConfigWithRevision: (params: { + principalType: PrincipalType; + principalId: string | Types.ObjectId; + principalModel: PrincipalModel; + expectedVersion: number | null; + op: ConfigMutationOp; + cause: ConfigRevisionCause; + actor: ConfigRevisionActor; + }) => Promise; } { async function findConfigByPrincipal( principalType: PrincipalType, principalId: string | Types.ObjectId, - options?: { includeInactive?: boolean }, + options?: FindConfigByPrincipalOptions, session?: ClientSession, ): Promise { const Config = mongoose.models.Config as Model; - const filter: { principalType: PrincipalType; principalId: string; isActive?: boolean } = { + const filter: FilterQuery = { principalType, principalId: principalId.toString(), }; + if (options?.tenantId !== undefined) { + Object.assign(filter, tenantRevisionFilter(options.tenantId)); + } if (!options?.includeInactive) { filter.isActive = true; } @@ -152,10 +845,12 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { options?: { expectEmpty?: boolean; preservePriority?: boolean }, ): Promise { const Config = mongoose.models.Config as Model; + const principalIdString = principalId.toString(); + const isBase = isBaseConfigPrincipal(principalType, principalIdString); const query: FilterQuery = { principalType, - principalId: principalId.toString(), + principalId: principalIdString, }; if (options?.expectEmpty) { query.$and = [ @@ -164,39 +859,91 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { ]; } - const update = { - $set: { - principalModel, - overrides, - ...(options?.preservePriority ? {} : { priority }), - isActive: true, - }, - ...(options?.preservePriority ? { $setOnInsert: { priority } } : {}), - $inc: { configVersion: 1 }, - }; - - const mongoOptions = { - upsert: true, - new: true, - setDefaultsOnInsert: true, - ...(session ? { session } : {}), - }; - - try { - return await Config.findOneAndUpdate(query, update, mongoOptions); - } catch (err: unknown) { - if ((err as { code?: number }).code === 11000) { - if (options?.expectEmpty) { - return null; + if (!isBase) { + const update = { + $set: { + principalModel, + overrides, + ...(options?.preservePriority ? {} : { priority }), + isActive: true, + }, + ...(options?.preservePriority ? { $setOnInsert: { priority } } : {}), + $inc: { configVersion: 1 }, + }; + const mongoOptions = { + upsert: true, + new: true, + setDefaultsOnInsert: true, + ...(session ? { session } : {}), + }; + try { + return await Config.findOneAndUpdate(query, update, mongoOptions); + } catch (err: unknown) { + if ((err as { code?: number }).code === 11000) { + if (options?.expectEmpty) { + return null; + } + return await Config.findOneAndUpdate( + { principalType, principalId: principalIdString }, + { $set: update.$set, $inc: update.$inc }, + { new: true, ...(session ? { session } : {}) }, + ); } - return await Config.findOneAndUpdate( - { principalType, principalId: principalId.toString() }, - { $set: update.$set, $inc: update.$inc }, - { new: true, ...(session ? { session } : {}) }, - ); + throw err; } - throw err; } + + return withOwnedSession(Config, session, async (txn) => { + const current = await Config.findOne(query, null, { session: txn }); + if (!current) { + const configVersion = await allocateBaseConfigVersion(Config, txn); + try { + const created = await Config.create( + [ + { + principalType, + principalId: principalIdString, + principalModel, + overrides, + priority, + isActive: true, + configVersion, + tombstones: [], + }, + ], + { ...(txn ? { session: txn } : {}) }, + ); + return created[0] ?? null; + } catch (err: unknown) { + if ((err as { code?: number }).code === 11000 && options?.expectEmpty) { + return null; + } + throw err; + } + } + + const currentVersion = interpretedConfigVersion(current); + const nextVersion = (currentVersion ?? 0) + 1; + if (!txn) await raiseBaseConfigVersionEpoch(Config, nextVersion, undefined); + const updated = await Config.findOneAndUpdate( + versionCasFilter(current._id, currentVersion), + { + $set: { + principalModel, + overrides, + ...(options?.preservePriority ? {} : { priority }), + isActive: true, + }, + $inc: { configVersion: 1 }, + }, + { new: true, ...(txn ? { session: txn } : {}) }, + ); + if (!updated) { + throw new Error('Failed to upsert base config after concurrent update'); + } + if (txn) await raiseBaseConfigVersionEpoch(Config, nextVersion, txn); + return updated; + }); } async function patchConfigFields( @@ -204,43 +951,105 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { principalId: string | Types.ObjectId, principalModel: PrincipalModel, fields: Record, - priority: number, + priority?: number, session?: ClientSession, ): Promise { + for (const fieldPath of Object.keys(fields)) { + assertValidFieldPath(fieldPath); + } + await ensureConfigIndexes(mongoose); const Config = mongoose.models.Config as Model; + const principalIdString = principalId.toString(); + const isBase = isBaseConfigPrincipal(principalType, principalIdString); - const setPayload: { principalModel: PrincipalModel; priority: number; [key: string]: unknown } = - { - principalModel, - priority, - }; + const applyOnce = async (txn?: ClientSession): Promise => { + const current = await Config.findOne( + { principalType, principalId: principalIdString }, + null, + { session: txn }, + ); + const resolvedPriority = priority ?? current?.priority ?? DEFAULT_CONFIG_PRIORITY; + const sanitizedCurrentOverrides = sanitizeAdminConfigOverrides( + cloneOverrides(current?.overrides), + ); + const sanitizedCurrentTombstones = sanitizeAdminConfigTombstones(current?.tombstones); + const nextOverrides = sanitizeAdminConfigOverrides( + applyFieldsMutation(sanitizedCurrentOverrides, [], fields), + ); + const nextTombstonesValue = nextTombstones( + sanitizedCurrentTombstones, + [], + Object.keys(fields), + ); - for (const [path, value] of Object.entries(fields)) { - setPayload[`overrides.${path}`] = value; - } - - const tombstonesToClear = [...new Set(Object.keys(fields).flatMap(getTombstonePathsToClear))]; + if (!current) { + const configVersion = isBase ? await allocateBaseConfigVersion(Config, txn) : 1; + try { + const created = await Config.create( + [ + { + principalType, + principalId: principalIdString, + principalModel, + overrides: nextOverrides, + tombstones: nextTombstonesValue, + priority: resolvedPriority, + isActive: true, + configVersion, + }, + ], + { ...(txn ? { session: txn } : {}) }, + ); + return created[0] ?? null; + } catch (error: unknown) { + if ((error as { code?: number }).code === 11000) { + return 'retry'; + } + throw error; + } + } - const options = { - upsert: true, - new: true, - setDefaultsOnInsert: true, - ...(session ? { session } : {}), + const currentVersion = interpretedConfigVersion(current); + const nextVersion = (currentVersion ?? 0) + 1; + if (isBase && !txn) await raiseBaseConfigVersionEpoch(Config, nextVersion, undefined); + const updated = await Config.findOneAndUpdate( + versionCasFilter(current._id, currentVersion), + { + $set: { + principalModel, + priority: resolvedPriority, + overrides: nextOverrides, + tombstones: nextTombstonesValue, + isActive: current.isActive ?? true, + }, + $inc: { configVersion: 1 }, + }, + { new: true, ...(txn ? { session: txn } : {}) }, + ); + if (!updated) { + return 'retry'; + } + if (isBase && txn) await raiseBaseConfigVersionEpoch(Config, nextVersion, txn); + return updated; }; - const update: Record = { - $set: setPayload, - $inc: { configVersion: 1 }, - }; - if (tombstonesToClear.length > 0) { - update.$pull = { tombstones: { $in: tombstonesToClear } }; + if (isBase) { + for (let attempt = 0; attempt < MAX_CONFIG_CAS_RETRIES; attempt += 1) { + const result = await withOwnedSession(Config, session, (txn) => applyOnce(txn)); + if (result !== 'retry') { + return result; + } + } + throw new Error('Failed to patch config fields after concurrent update retries'); } - return await Config.findOneAndUpdate( - { principalType, principalId: principalId.toString() }, - update, - options, - ); + for (let attempt = 0; attempt < MAX_CONFIG_CAS_RETRIES; attempt += 1) { + const result = await applyOnce(session); + if (result !== 'retry') { + return result; + } + } + throw new Error('Failed to patch config fields after concurrent update retries'); } async function tombstoneConfigField( @@ -248,31 +1057,98 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { principalId: string | Types.ObjectId, principalModel: PrincipalModel, fieldPath: string, - priority: number, + priority?: number, session?: ClientSession, ): Promise { + assertValidFieldPath(fieldPath); + await ensureConfigIndexes(mongoose); const Config = mongoose.models.Config as Model; + const principalIdString = principalId.toString(); + const isBase = isBaseConfigPrincipal(principalType, principalIdString); - const options = { - upsert: true, - new: true, - setDefaultsOnInsert: true, - ...(session ? { session } : {}), - }; + const applyOnce = async (txn?: ClientSession): Promise => { + const current = await Config.findOne( + { principalType, principalId: principalIdString }, + null, + { session: txn }, + ); + const resolvedPriority = priority ?? current?.priority ?? DEFAULT_CONFIG_PRIORITY; + const sanitizedCurrentOverrides = sanitizeAdminConfigOverrides( + cloneOverrides(current?.overrides), + ); + const nextOverrides = applyFieldsMutation(sanitizedCurrentOverrides, [fieldPath], {}); + const nextTombstoneSet = new Set(sanitizeAdminConfigTombstones(current?.tombstones)); + nextTombstoneSet.add(fieldPath); + const nextTombstonesValue = [...nextTombstoneSet]; - return await Config.findOneAndUpdate( - { principalType, principalId: principalId.toString() }, - { - $set: { - principalModel, - priority, + if (!current) { + const configVersion = isBase ? await allocateBaseConfigVersion(Config, txn) : 1; + try { + const created = await Config.create( + [ + { + principalType, + principalId: principalIdString, + principalModel, + overrides: nextOverrides, + tombstones: nextTombstonesValue, + priority: resolvedPriority, + isActive: true, + configVersion, + }, + ], + { ...(txn ? { session: txn } : {}) }, + ); + return created[0] ?? null; + } catch (error: unknown) { + if ((error as { code?: number }).code === 11000) { + return 'retry'; + } + throw error; + } + } + + const currentVersion = interpretedConfigVersion(current); + const nextVersion = (currentVersion ?? 0) + 1; + if (isBase && !txn) await raiseBaseConfigVersionEpoch(Config, nextVersion, undefined); + const updated = await Config.findOneAndUpdate( + versionCasFilter(current._id, currentVersion), + { + $set: { + principalModel, + priority: resolvedPriority, + overrides: nextOverrides, + tombstones: nextTombstonesValue, + isActive: current.isActive ?? true, + }, + $inc: { configVersion: 1 }, }, - $unset: { [`overrides.${fieldPath}`]: '' }, - $addToSet: { tombstones: fieldPath }, - $inc: { configVersion: 1 }, - }, - options, - ); + { new: true, ...(txn ? { session: txn } : {}) }, + ); + if (!updated) { + return 'retry'; + } + if (isBase && txn) await raiseBaseConfigVersionEpoch(Config, nextVersion, txn); + return updated; + }; + + if (isBase) { + for (let attempt = 0; attempt < MAX_CONFIG_CAS_RETRIES; attempt += 1) { + const result = await withOwnedSession(Config, session, (txn) => applyOnce(txn)); + if (result !== 'retry') { + return result; + } + } + throw new Error('Failed to tombstone config field after concurrent update retries'); + } + + for (let attempt = 0; attempt < MAX_CONFIG_CAS_RETRIES; attempt += 1) { + const result = await applyOnce(session); + if (result !== 'retry') { + return result; + } + } + throw new Error('Failed to tombstone config field after concurrent update retries'); } async function unsetConfigField( @@ -281,22 +1157,53 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { fieldPath: string, session?: ClientSession, ): Promise { + assertValidFieldPath(fieldPath); const Config = mongoose.models.Config as Model; + const principalIdString = principalId.toString(); + const isBase = isBaseConfigPrincipal(principalType, principalIdString); - const options = { - new: true, - ...(session ? { session } : {}), + const apply = async (txn?: ClientSession): Promise => { + if (!txn && isBase) { + const current = await Config.findOne({ principalType, principalId: principalIdString }); + if (!current) return null; + const currentVersion = interpretedConfigVersion(current); + const nextVersion = (currentVersion ?? 0) + 1; + await raiseBaseConfigVersionEpoch(Config, nextVersion, undefined); + const updated = await Config.findOneAndUpdate( + versionCasFilter(current._id, currentVersion), + { + $unset: { [`overrides.${fieldPath}`]: '' }, + $pull: { tombstones: { $regex: getPathAndDescendantsRegex(fieldPath) } }, + $inc: { configVersion: 1 }, + }, + { new: true }, + ); + if (!updated) return 'retry'; + return updated; + } + const updated = await Config.findOneAndUpdate( + { principalType, principalId: principalIdString }, + { + $unset: { [`overrides.${fieldPath}`]: '' }, + $pull: { tombstones: { $regex: getPathAndDescendantsRegex(fieldPath) } }, + $inc: { configVersion: 1 }, + }, + { new: true, ...(txn ? { session: txn } : {}) }, + ); + if (updated && isBase && txn) { + await raiseBaseConfigVersionEpoch(Config, updated.configVersion ?? 0, txn); + } + return updated; }; - return await Config.findOneAndUpdate( - { principalType, principalId: principalId.toString() }, - { - $unset: { [`overrides.${fieldPath}`]: '' }, - $pull: { tombstones: { $regex: getPathAndDescendantsRegex(fieldPath) } }, - $inc: { configVersion: 1 }, - }, - options, - ); + if (isBase) { + for (let attempt = 0; attempt < MAX_CONFIG_CAS_RETRIES; attempt += 1) { + const result = await withOwnedSession(Config, session, (txn) => apply(txn)); + if (result !== 'retry') return result; + } + throw new Error('Failed to unset config field after concurrent update retries'); + } + return apply(session) as Promise; } async function deleteConfig( @@ -306,9 +1213,11 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { options?: { expectEmpty?: boolean }, ): Promise { const Config = mongoose.models.Config as Model; + const principalIdString = principalId.toString(); + const isBase = isBaseConfigPrincipal(principalType, principalIdString); const filter: FilterQuery = { principalType, - principalId: principalId.toString(), + principalId: principalIdString, }; if (options?.expectEmpty) { filter.$and = [ @@ -316,7 +1225,43 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { { $or: [{ tombstones: { $size: 0 } }, { tombstones: { $exists: false } }] }, ]; } - return await Config.findOneAndDelete(filter).session(session ?? null); + + const apply = async (txn?: ClientSession): Promise => { + if (!txn && isBase) { + const current = await Config.findOne({ principalType, principalId: principalIdString }); + if (!current) return null; + if (options?.expectEmpty) { + const hasOverrides = + current.overrides != null && Object.keys(current.overrides).length > 0; + const hasTombstones = current.tombstones != null && current.tombstones.length > 0; + if (hasOverrides || hasTombstones) return null; + } + const version = interpretedConfigVersion(current); + if (version != null) { + await raiseBaseConfigVersionEpoch(Config, version, undefined); + } + const deleted = await Config.findOneAndDelete(versionCasFilter(current._id, version)); + if (!deleted) return 'retry'; + return deleted; + } + const deleted = await Config.findOneAndDelete(filter, txn ? { session: txn } : {}); + if (deleted && isBase && txn) { + const deletedVersion = interpretedConfigVersion(deleted); + if (deletedVersion != null) { + await raiseBaseConfigVersionEpoch(Config, deletedVersion, txn); + } + } + return deleted; + }; + + if (isBase) { + for (let attempt = 0; attempt < MAX_CONFIG_CAS_RETRIES; attempt += 1) { + const result = await withOwnedSession(Config, session, (txn) => apply(txn)); + if (result !== 'retry') return result; + } + throw new Error('Failed to delete base config after concurrent update retries'); + } + return apply(session) as Promise; } async function toggleConfigActive( @@ -327,9 +1272,11 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { options?: { expectEmpty?: boolean }, ): Promise { const Config = mongoose.models.Config as Model; + const principalIdString = principalId.toString(); + const isBase = isBaseConfigPrincipal(principalType, principalIdString); const filter: FilterQuery = { principalType, - principalId: principalId.toString(), + principalId: principalIdString, }; if (options?.expectEmpty) { filter.$and = [ @@ -337,11 +1284,307 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { { $or: [{ tombstones: { $size: 0 } }, { tombstones: { $exists: false } }] }, ]; } - return await Config.findOneAndUpdate( - filter, - { $set: { isActive } }, - { new: true, ...(session ? { session } : {}) }, + + const apply = async (txn?: ClientSession): Promise => { + if (!txn && isBase) { + const current = await Config.findOne({ principalType, principalId: principalIdString }); + if (!current) return null; + if (options?.expectEmpty) { + const hasOverrides = + current.overrides != null && Object.keys(current.overrides).length > 0; + const hasTombstones = current.tombstones != null && current.tombstones.length > 0; + if (hasOverrides || hasTombstones) return null; + } + const currentVersion = interpretedConfigVersion(current); + const nextVersion = (currentVersion ?? 0) + 1; + await raiseBaseConfigVersionEpoch(Config, nextVersion, undefined); + const updated = await Config.findOneAndUpdate( + versionCasFilter(current._id, currentVersion), + { $set: { isActive }, $inc: { configVersion: 1 } }, + { new: true }, + ); + if (!updated) return 'retry'; + return updated; + } + const updated = await Config.findOneAndUpdate( + filter, + { $set: { isActive }, $inc: { configVersion: 1 } }, + { new: true, ...(txn ? { session: txn } : {}) }, + ); + if (updated && isBase && txn) { + await raiseBaseConfigVersionEpoch(Config, updated.configVersion ?? 0, txn); + } + return updated; + }; + + if (isBase) { + for (let attempt = 0; attempt < MAX_CONFIG_CAS_RETRIES; attempt += 1) { + const result = await withOwnedSession(Config, session, (txn) => apply(txn)); + if (result !== 'retry') return result; + } + throw new Error('Failed to toggle base config after concurrent update retries'); + } + return apply(session) as Promise; + } + + async function mutateConfigWithRevision(params: { + principalType: PrincipalType; + principalId: string | Types.ObjectId; + principalModel: PrincipalModel; + expectedVersion: number | null; + op: ConfigMutationOp; + cause: ConfigRevisionCause; + actor: ConfigRevisionActor; + }): Promise { + const Config = mongoose.models.Config as Model; + const principalId = params.principalId.toString(); + if (params.principalType !== PrincipalType.ROLE || principalId !== BASE_CONFIG_PRINCIPAL_ID) { + throw new Error('Atomic config revisions are only supported for the base configuration'); + } + + const revisions = Config.db.collection(ADMIN_CONFIG_REVISIONS_COLLECTION); + const session = await Config.db.startSession(); + const scope = revisionScopeFilter(params.actor.tenantId, params.principalType, principalId); + const principalFilter = tenantPrincipalFilter( + params.actor.tenantId, + params.principalType, + principalId, ); + + const raiseVersionEpoch = async (version: number) => { + await raiseBaseConfigVersionEpoch(Config, version, session, params.actor.tenantId); + }; + + const allocateCreateVersion = async (): Promise => + allocateBaseConfigVersion(Config, session, params.actor.tenantId); + + try { + // Return the outcome from withTransaction so each retry attempt gets a + // fresh local result — a shared outer `outcome` would skip revision + // insertOne after TransientTransactionError retries. + const outcome = await session.withTransaction(async (): Promise => { + const current = await Config.findOne(principalFilter, null, { session }); + const currentVersion = interpretedConfigVersion(current); + const versionMatches = + current == null + ? params.expectedVersion == null + : params.expectedVersion === currentVersion; + if (!versionMatches) { + throw new ConfigVersionConflictError(currentVersion); + } + + const { op } = params; + const revision = snapshotFromConfig(current, { + cause: params.cause, + actor: params.actor, + principalType: params.principalType, + principalId, + }); + + let config: IConfig | null = current; + + const applyReplace = async (state: { + overrides: Record; + tombstones: string[]; + priority: number; + isActive: boolean; + }) => { + const nextOverrides = preserveBasePrincipalOverrides(state.overrides, current?.overrides); + const nextTombstones = preserveBasePrincipalTombstones( + state.tombstones, + current?.tombstones, + ); + if (current == null) { + const configVersion = await allocateCreateVersion(); + const created = await Config.create( + [ + { + principalType: params.principalType, + principalId, + principalModel: params.principalModel, + overrides: nextOverrides, + tombstones: nextTombstones, + priority: state.priority, + isActive: state.isActive, + configVersion, + ...(params.actor.tenantId.length > 0 ? { tenantId: params.actor.tenantId } : {}), + }, + ], + { session }, + ); + config = created[0] ?? null; + return; + } + const nextVersion = (currentVersion ?? 0) + 1; + const updated = await Config.findOneAndUpdate( + versionCasFilter(current._id, currentVersion), + { + $set: { + principalModel: params.principalModel, + overrides: nextOverrides, + tombstones: nextTombstones, + priority: state.priority, + isActive: state.isActive, + }, + $inc: { configVersion: 1 }, + }, + { session, new: true }, + ); + if (!updated) { + throw new ConfigVersionConflictError(currentVersion); + } + await raiseVersionEpoch(nextVersion); + config = updated; + }; + + const applyDelete = async (): Promise => { + if (!current) { + config = null; + return false; + } + const deleted = await Config.deleteOne(versionCasFilter(current._id, currentVersion), { + session, + }); + if (deleted.deletedCount !== 1) { + throw new ConfigVersionConflictError(currentVersion); + } + if (currentVersion != null) { + await raiseVersionEpoch(currentVersion); + } + config = null; + return true; + }; + + if (op.kind === 'restore') { + const stored = (await revisions.findOne( + { + id: op.revisionId, + status: { $ne: 'provisional' }, + $and: [ + tenantRevisionFilter(params.actor.tenantId), + { + $or: [ + { principalType: params.principalType, principalId }, + { + principalType: { $exists: false }, + principalId: { $exists: false }, + }, + ], + }, + ], + }, + { session }, + )) as ConfigRevisionSnapshot | null; + if (!stored) { + throw new ConfigRevisionNotFoundError(op.revisionId); + } + if (stored.absent) { + if (hasBasePrincipalState(current)) { + await applyReplace({ + overrides: {}, + tombstones: [], + priority: current?.priority ?? 0, + isActive: current?.isActive ?? true, + }); + } else if (!(await applyDelete())) { + return { changed: false, config: null, revision: null }; + } + } else { + await applyReplace({ + overrides: sanitizeAdminConfigOverrides(cloneOverrides(stored.overrides)), + tombstones: sanitizeAdminConfigTombstones(stored.tombstones), + priority: stored.priority ?? 0, + isActive: stored.isActive ?? true, + }); + } + } else if (op.kind === 'delete') { + if (hasBasePrincipalState(current)) { + await applyReplace({ + overrides: {}, + tombstones: [], + priority: current?.priority ?? 0, + isActive: current?.isActive ?? true, + }); + } else if (!(await applyDelete())) { + return { changed: false, config: null, revision: null }; + } + } else if (op.kind === 'replace') { + await applyReplace({ + overrides: sanitizeAdminConfigOverrides(cloneOverrides(op.overrides)), + tombstones: sanitizeAdminConfigTombstones(current?.tombstones), + priority: op.priority, + isActive: current?.isActive ?? true, + }); + } else if (current == null) { + // Reset-only against an absent base config is a no-op: do not create an + // empty document / revision that would change persistent CAS state. + if (Object.keys(op.fields).length === 0) { + return { changed: false, config: null, revision: null }; + } + await applyReplace({ + overrides: sanitizeAdminConfigOverrides( + applyFieldsMutation({}, op.resetPaths, op.fields), + ), + tombstones: nextTombstones([], op.resetPaths, Object.keys(op.fields)), + priority: op.priority, + isActive: true, + }); + } else { + await applyReplace({ + overrides: sanitizeAdminConfigOverrides( + applyFieldsMutation( + sanitizeAdminConfigOverrides(cloneOverrides(current.overrides)), + op.resetPaths, + op.fields, + ), + ), + tombstones: nextTombstones( + sanitizeAdminConfigTombstones(current.tombstones), + op.resetPaths, + Object.keys(op.fields), + ), + priority: op.priority, + isActive: current.isActive ?? true, + }); + } + + await revisions.insertOne(revision, { session }); + return { changed: true, config, revision }; + }); + + if (outcome.changed) { + try { + const stale = await revisions + .find( + { ...scope, status: { $ne: 'provisional' } }, + { projection: { id: 1 }, sort: { createdAt: -1 }, skip: MAX_CONFIG_REVISIONS }, + ) + .toArray(); + if (stale.length > 0) { + await revisions.deleteMany({ + ...scope, + id: { $in: stale.map((doc) => doc.id) }, + }); + } + } catch { + /* retention is best-effort after a committed mutation */ + } + } + + return outcome; + } catch (error) { + if ((error as { code?: number }).code === 11000) { + const Config = mongoose.models.Config as Model; + const existing = await Config.findOne(principalFilter); + throw new ConfigVersionConflictError(interpretedConfigVersion(existing)); + } + if (isTransactionUnsupported(error)) { + throw new TransactionRequiredError(); + } + throw error; + } finally { + await session.endSession(); + } } return { @@ -354,6 +1597,7 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { unsetConfigField, deleteConfig, toggleConfigActive, + mutateConfigWithRevision, }; } diff --git a/packages/data-schemas/src/methods/index.ts b/packages/data-schemas/src/methods/index.ts index b6cc67b3123..49833fbfd2e 100644 --- a/packages/data-schemas/src/methods/index.ts +++ b/packages/data-schemas/src/methods/index.ts @@ -149,7 +149,28 @@ import type { /* Tier 5 — Agent */ import { createAgentMethods, type AgentMethods, type AgentDeps } from './agent'; /* Config */ -import { createConfigMethods, type ConfigMethods } from './config'; +import { + createConfigMethods, + ConfigVersionConflictError, + ConfigRevisionNotFoundError, + canonicalizeResetPaths, + fieldPathLimitError, + fieldPathPolicyError, + isValidFieldPath, + type FindConfigByPrincipalOptions, + MAX_FIELD_PATH_LENGTH, + MAX_FIELD_PATH_SEGMENTS, + ensureConfigIndexes, + ADMIN_CONFIG_REVISIONS_COLLECTION, + ADMIN_CONFIG_VERSION_EPOCHS_COLLECTION, + MAX_CONFIG_REVISIONS, + type ConfigMethods, + type ConfigMutationOp, + type ConfigMutationResult, + type ConfigRevisionCause, + type ConfigRevisionSnapshot, + type ConfigRevisionActor, +} from './config'; import { createMCPAuthorityMethods, MCPAuthorityProofError, @@ -172,6 +193,19 @@ export { RoleConflictError, MCPAuthorityProofError, MAX_MCP_AUTHORITY_TARGETS, + ConfigVersionConflictError, + ConfigRevisionNotFoundError, + canonicalizeResetPaths, + fieldPathLimitError, + fieldPathPolicyError, + isValidFieldPath, + type FindConfigByPrincipalOptions, + MAX_FIELD_PATH_LENGTH, + MAX_FIELD_PATH_SEGMENTS, + ensureConfigIndexes, + ADMIN_CONFIG_REVISIONS_COLLECTION, + ADMIN_CONFIG_VERSION_EPOCHS_COLLECTION, + MAX_CONFIG_REVISIONS, DEFAULT_REFRESH_TOKEN_EXPIRY, DEFAULT_SESSION_EXPIRY, createMCPAuthorityBootRevision, @@ -548,6 +582,11 @@ export type { MCPAuthorityConfigSourceDocument, MCPAuthorityCredentialSourceDocument, InsightsMethods, + ConfigMutationOp, + ConfigMutationResult, + ConfigRevisionCause, + ConfigRevisionSnapshot, + ConfigRevisionActor, }; export { recordAgentEventActorReceiptMetric, setAgentEventActorReceiptMetricObserver }; diff --git a/packages/data-schemas/src/schema/config.ts b/packages/data-schemas/src/schema/config.ts index bc8654e9dfe..ae4eb6b34ee 100644 --- a/packages/data-schemas/src/schema/config.ts +++ b/packages/data-schemas/src/schema/config.ts @@ -46,6 +46,7 @@ const configSchema: Schema = new Schema( tenantId: { type: String, index: true, + default: null, }, }, { timestamps: true }, diff --git a/packages/data-schemas/src/types/config.ts b/packages/data-schemas/src/types/config.ts index b980a709939..592f555f34a 100644 --- a/packages/data-schemas/src/types/config.ts +++ b/packages/data-schemas/src/types/config.ts @@ -24,8 +24,8 @@ export type Config = { isActive: boolean; /** Version number for cache invalidation, auto-increments on overrides change */ configVersion: number; - /** Tenant identifier for multi-tenancy isolation */ - tenantId?: string; + /** Tenant identifier for multi-tenancy isolation; null means default/no tenant */ + tenantId?: string | null; /** When this config was created */ createdAt?: Date; /** When this config was last updated */ diff --git a/utils/docker/test-compose.yml b/utils/docker/test-compose.yml index 31a7f63418c..5bc747e99b2 100644 --- a/utils/docker/test-compose.yml +++ b/utils/docker/test-compose.yml @@ -27,7 +27,13 @@ services: restart: always volumes: - ./data-node:/data/db - command: mongod --noauth + command: ["mongod", "--replSet", "rs0", "--bind_ip_all"] + healthcheck: + test: echo "try { rs.status() } catch (err) { rs.initiate({_id:'rs0',members:[{_id:0,host:'mongodb:27017'}]}) }" | mongosh --quiet + interval: 5s + timeout: 30s + retries: 30 + start_period: 10s meilisearch: container_name: chat-meilisearch image: getmeili/meilisearch:v1.7.3 From 0f1ca2b29de72221d7728223395e510e9a7ca680 Mon Sep 17 00:00:00 2001 From: Romuald Wandji Date: Wed, 2 Sep 2026 07:27:17 +0200 Subject: [PATCH 2/3] fix(admin): address atomic config review findings --- packages/api/src/admin/config.spec.ts | 4 + .../data-schemas/src/methods/config.spec.ts | 103 ++++++++++++++++++ packages/data-schemas/src/methods/config.ts | 24 +++- 3 files changed, 126 insertions(+), 5 deletions(-) diff --git a/packages/api/src/admin/config.spec.ts b/packages/api/src/admin/config.spec.ts index 13862e23e51..a53c3a6b84a 100644 --- a/packages/api/src/admin/config.spec.ts +++ b/packages/api/src/admin/config.spec.ts @@ -15,6 +15,10 @@ describe('isValidFieldPath', () => { expect(isValidFieldPath(42)).toBe(false); }); + it('rejects NUL bytes before BSON persistence', () => { + expect(isValidFieldPath('cache.\0value')).toBe(false); + }); + it('rejects __proto__ and dunder-prefixed segments', () => { expect(isValidFieldPath('__proto__')).toBe(false); expect(isValidFieldPath('a.__proto__')).toBe(false); diff --git a/packages/data-schemas/src/methods/config.spec.ts b/packages/data-schemas/src/methods/config.spec.ts index 44ef9dd0831..24a8a8c7774 100644 --- a/packages/data-schemas/src/methods/config.spec.ts +++ b/packages/data-schemas/src/methods/config.spec.ts @@ -79,9 +79,11 @@ describe('field path policy', () => { it('rejects empty, non-string, and unsafe segments', () => { expect(isValidFieldPath('')).toBe(false); expect(isValidFieldPath(undefined)).toBe(false); + expect(isValidFieldPath('cache.\0value')).toBe(false); expect(isValidFieldPath('__proto__.polluted')).toBe(false); expect(isValidFieldPath('cache.__internal-key.value')).toBe(false); expect(isValidFieldPath('cache.__på.value')).toBe(false); + expect(fieldPathPolicyError('cache.\0value')).toMatch(/NUL byte/); expect(fieldPathPolicyError('__proto__.polluted')).toMatch(/forbidden segment/); }); }); @@ -680,6 +682,107 @@ describe('tombstoneConfigField', () => { }); describe('upsertConfig', () => { + it('retries base upserts after a CAS miss', async () => { + const Config = mongoose.models.Config; + await Config.collection.insertOne({ + principalType: PrincipalType.ROLE, + principalId: '__base__', + principalModel: PrincipalModel.ROLE, + overrides: { cache: false }, + tombstones: [], + priority: 10, + isActive: true, + configVersion: 1, + tenantId: null, + }); + + const findOneAndUpdateSpy = jest.spyOn(Config, 'findOneAndUpdate'); + const startSessionSpy = jest.spyOn(Config.db, 'startSession'); + startSessionSpy.mockImplementation( + (async () => + ({ + withTransaction: async () => { + throw new Error('Transaction numbers are only allowed on a replica set member'); + }, + endSession: async () => undefined, + }) as never) as never, + ); + findOneAndUpdateSpy.mockImplementationOnce((async () => { + await Config.updateOne( + { principalId: '__base__' }, + { $set: { priority: 20 }, $inc: { configVersion: 1 } }, + ); + return null; + }) as never); + + try { + const result = await methods.upsertConfig( + PrincipalType.ROLE, + '__base__', + PrincipalModel.ROLE, + { cache: true }, + 10, + undefined, + { preservePriority: true }, + ); + + expect(result!.overrides).toEqual({ cache: true }); + expect(result!.priority).toBe(20); + expect(result!.configVersion).toBe(3); + expect(findOneAndUpdateSpy).toHaveBeenCalledTimes(2); + } finally { + findOneAndUpdateSpy.mockRestore(); + startSessionSpy.mockRestore(); + } + }); + + it('retries base upserts after a concurrent create', async () => { + const Config = mongoose.models.Config; + const createSpy = jest.spyOn(Config, 'create'); + const startSessionSpy = jest.spyOn(Config.db, 'startSession'); + startSessionSpy.mockImplementation( + (async () => + ({ + withTransaction: async () => { + throw new Error('Transaction numbers are only allowed on a replica set member'); + }, + endSession: async () => undefined, + }) as never) as never, + ); + createSpy.mockImplementationOnce((async () => { + await Config.collection.insertOne({ + principalType: PrincipalType.ROLE, + principalId: '__base__', + principalModel: PrincipalModel.ROLE, + overrides: { cache: false }, + tombstones: [], + priority: 10, + isActive: true, + configVersion: 1, + tenantId: null, + }); + throw Object.assign(new Error('duplicate key'), { code: 11000 }); + }) as never); + + try { + const result = await methods.upsertConfig( + PrincipalType.ROLE, + '__base__', + PrincipalModel.ROLE, + { cache: true }, + 10, + ); + + expect(result!.overrides).toEqual({ cache: true }); + expect(result!.configVersion).toBe(2); + expect(await Config.countDocuments({ principalId: '__base__' })).toBe(1); + expect(createSpy).toHaveBeenCalledTimes(1); + } finally { + createSpy.mockRestore(); + startSessionSpy.mockRestore(); + } + }); + it('preserves tombstones when replacing overrides', async () => { await methods.tombstoneConfigField( PrincipalType.ROLE, diff --git a/packages/data-schemas/src/methods/config.ts b/packages/data-schemas/src/methods/config.ts index c44b6e5d5ec..57fcd33103e 100644 --- a/packages/data-schemas/src/methods/config.ts +++ b/packages/data-schemas/src/methods/config.ts @@ -430,6 +430,9 @@ export function fieldPathPolicyError(path: unknown): string | null { if (path.length === 0) { return 'field path must not be empty'; } + if (path.includes('\0')) { + return 'field path contains NUL byte'; + } if (path.startsWith('.') || path.endsWith('.') || path.includes('..')) { return 'field path has invalid structure'; } @@ -893,7 +896,7 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { } } - return withOwnedSession(Config, session, async (txn) => { + const applyOnce = async (txn?: ClientSession): Promise => { const current = await Config.findOne(query, null, { session: txn }); if (!current) { const configVersion = await allocateBaseConfigVersion(Config, txn); @@ -915,8 +918,11 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { ); return created[0] ?? null; } catch (err: unknown) { - if ((err as { code?: number }).code === 11000 && options?.expectEmpty) { - return null; + if ((err as { code?: number }).code === 11000) { + if (options?.expectEmpty) { + return null; + } + return 'retry'; } throw err; } @@ -939,11 +945,19 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): { { new: true, ...(txn ? { session: txn } : {}) }, ); if (!updated) { - throw new Error('Failed to upsert base config after concurrent update'); + return 'retry'; } if (txn) await raiseBaseConfigVersionEpoch(Config, nextVersion, txn); return updated; - }); + }; + + for (let attempt = 0; attempt < MAX_CONFIG_CAS_RETRIES; attempt += 1) { + const result = await withOwnedSession(Config, session, (txn) => applyOnce(txn)); + if (result !== 'retry') { + return result; + } + } + throw new Error('Failed to upsert base config after concurrent update retries'); } async function patchConfigFields( From a643dd414bd580984fa619e5a6637c28a5b954ce Mon Sep 17 00:00:00 2001 From: Romuald Wandji Date: Sat, 5 Sep 2026 08:07:57 +0200 Subject: [PATCH 3/3] fix(admin): harden atomic config isolation and secret lifecycle --- api/server/experimental.js | 9 +- api/server/experimental.spec.js | 30 + api/server/index.js | 3 +- .../__tests__/requireJwtAuth.spec.js | 15 +- api/server/middleware/config/app.js | 8 +- api/server/middleware/config/app.spec.js | 60 + api/server/routes/__tests__/config.spec.js | 17 +- api/server/routes/admin/config.js | 2 + api/server/routes/admin/langfuse.js | 11 +- api/server/routes/admin/langfuse.test.js | 1 + api/server/services/Config/app.js | 3 +- .../initialize.encryptedAzureKey.spec.js | 132 + .../Endpoints/azureAssistants/initialize.js | 13 +- .../Integrations/LangfuseConnection.tsx | 354 ++- .../__tests__/LangfuseConnection.spec.tsx | 961 +++++- client/src/locales/en/translation.json | 5 + deploy-compose.yml | 3 - docker-compose.yml | 3 - helm/librechat/readme.md | 69 +- helm/librechat/templates/NOTES.txt | 14 + helm/librechat/templates/configmap-env.yaml | 26 +- .../tests/mongo_architecture_ack_test.sh | 85 + .../tests/mongo_uri_replicaset_test.sh | 65 + helm/librechat/values.yaml | 53 +- .../admin/config.atomic.integration.spec.ts | 465 +++ packages/api/src/admin/config.handler.spec.ts | 2628 ++++++++++++++++- packages/api/src/admin/config.ts | 806 ++++- packages/api/src/admin/grants.spec.ts | 25 +- packages/api/src/admin/grants.ts | 10 +- .../api/src/admin/langfuse.handler.spec.ts | 564 +++- packages/api/src/admin/langfuse.ts | 199 +- .../api/src/admin/secrets.integration.spec.ts | 2 + packages/api/src/admin/secrets.spec.ts | 1595 ++++++++++ packages/api/src/admin/secrets.ts | 1508 +++++++++- packages/api/src/app/service.spec.ts | 13 +- packages/api/src/app/service.ts | 19 +- .../initialize.encryptedAzureKey.spec.ts | 109 + .../api/src/endpoints/openai/initialize.ts | 7 +- .../src/middleware/__tests__/tenant.spec.ts | 65 + packages/api/src/middleware/capabilities.ts | 3 +- packages/api/src/middleware/index.ts | 1 + packages/api/src/middleware/preAuthTenant.ts | 1 + packages/api/src/middleware/tenant.ts | 58 +- .../src/utils/env.encryptedSecrets.spec.ts | 297 ++ packages/api/src/utils/env.ts | 32 +- .../api/src/web/web.encryptedSecrets.spec.ts | 160 + packages/api/src/web/web.ts | 173 +- packages/data-provider/src/config.ts | 4 + packages/data-provider/src/types.ts | 5 + .../src/admin/configOverrides.spec.ts | 11 + .../data-schemas/src/admin/configOverrides.ts | 8 +- packages/data-schemas/src/admin/index.ts | 2 +- .../src/admin/indexedArrayPath.spec.ts | 18 +- .../src/admin/indexedArrayPath.ts | 30 +- .../data-schemas/src/app/resolution.spec.ts | 16 + packages/data-schemas/src/app/resolution.ts | 5 +- packages/data-schemas/src/index.ts | 2 + .../src/methods/config.atomic.spec.ts | 783 ++++- .../data-schemas/src/methods/config.spec.ts | 374 ++- .../src/methods/config.tenant.spec.ts | 31 + packages/data-schemas/src/methods/config.ts | 813 ++--- packages/data-schemas/src/methods/index.ts | 6 + 62 files changed, 11638 insertions(+), 1152 deletions(-) create mode 100644 api/server/middleware/config/app.spec.js create mode 100644 api/server/services/Endpoints/azureAssistants/initialize.encryptedAzureKey.spec.js create mode 100755 helm/librechat/tests/mongo_architecture_ack_test.sh create mode 100755 helm/librechat/tests/mongo_uri_replicaset_test.sh create mode 100644 packages/api/src/admin/config.atomic.integration.spec.ts create mode 100644 packages/api/src/endpoints/openai/initialize.encryptedAzureKey.spec.ts create mode 100644 packages/api/src/utils/env.encryptedSecrets.spec.ts create mode 100644 packages/api/src/web/web.encryptedSecrets.spec.ts diff --git a/api/server/experimental.js b/api/server/experimental.js index ce21fae0ddc..913c5e5992f 100644 --- a/api/server/experimental.js +++ b/api/server/experimental.js @@ -2,6 +2,7 @@ require('../config/credentials'); const fs = require('fs'); const path = require('path'); require('module-alias')({ base: path.resolve(__dirname, '..') }); +const mongoose = require('mongoose'); const cluster = require('cluster'); const Redis = require('ioredis'); const cors = require('cors'); @@ -10,7 +11,7 @@ const express = require('express'); const passport = require('passport'); const compression = require('compression'); const cookieParser = require('cookie-parser'); -const { logger, runAsSystem } = require('@librechat/data-schemas'); +const { logger, runAsSystem, ensureConfigIndexes } = require('@librechat/data-schemas'); const mongoSanitize = require('express-mongo-sanitize'); const { isEnabled, @@ -353,6 +354,11 @@ if (cluster.isMaster) { await connectDb(); logger.info(`Worker ${process.pid}: Connected to MongoDB`); + /** Mirrors `server/index.js`; must run before workers accept traffic so the + * epoch collection's unique index and config uniqueness index exist before + * concurrent upserts from multiple workers can race. */ + await ensureConfigIndexes(mongoose); + /** Background index sync (non-blocking) */ indexSync().catch((err) => { logger.error(`[Worker ${process.pid}][indexSync] Background sync failed:`, err); @@ -517,6 +523,7 @@ if (cluster.isMaster) { /** Routes */ app.use('/oauth', preAuthTenantMiddleware, routes.oauth); app.use('/api/auth', preAuthTenantMiddleware, routes.auth); + app.use('/api/admin', preAuthTenantMiddleware); app.use('/api/admin/insights', routes.insights); app.use('/api/admin', routes.adminAuth); app.use('/api/admin/skills', routes.adminSkills); diff --git a/api/server/experimental.spec.js b/api/server/experimental.spec.js index 365171c409b..e7e164151f4 100644 --- a/api/server/experimental.spec.js +++ b/api/server/experimental.spec.js @@ -3,6 +3,7 @@ const path = require('path'); describe('Experimental server configuration', () => { const source = fs.readFileSync(path.join(__dirname, 'experimental.js'), 'utf8'); + const standardSource = fs.readFileSync(path.join(__dirname, 'index.js'), 'utf8'); it('configures HTTP timeouts for each cluster worker server', () => { const listenIndex = source.indexOf('const server = app.listen'); @@ -82,9 +83,38 @@ describe('Experimental server configuration', () => { expect(listenIndex).toBeGreaterThan(eventRuntimeIndex); }); + it('initializes config indexes right after connecting to Mongo, before workers accept traffic', () => { + const connectIndex = source.indexOf('await connectDb();'); + const ensureIndexesIndex = source.indexOf('await ensureConfigIndexes(mongoose);'); + const listenIndex = source.indexOf('const server = app.listen'); + + expect(connectIndex).toBeGreaterThan(-1); + expect(ensureIndexesIndex).toBeGreaterThan(-1); + expect(listenIndex).toBeGreaterThan(-1); + // Without the epoch collection's unique index and the config uniqueness + // index in place first, concurrent upserts from multiple cluster workers + // can create duplicate epochs and weaken CAS/ABA protection. + expect(ensureIndexesIndex).toBeGreaterThan(connectIndex); + expect(listenIndex).toBeGreaterThan(ensureIndexesIndex); + }); + it('matches the standard server pre-authentication tenant routes', () => { + const standardAdminTenantIndex = standardSource.indexOf( + "app.use('/api/admin', preAuthTenantMiddleware);", + ); + const standardFirstAdminRouteIndex = standardSource.indexOf("app.use('/api/admin/insights'"); + const experimentalAdminTenantIndex = source.indexOf( + "app.use('/api/admin', preAuthTenantMiddleware);", + ); + const experimentalFirstAdminRouteIndex = source.indexOf("app.use('/api/admin/insights'"); + + expect(standardAdminTenantIndex).toBeGreaterThan(-1); + expect(standardFirstAdminRouteIndex).toBeGreaterThan(standardAdminTenantIndex); + expect(experimentalAdminTenantIndex).toBeGreaterThan(-1); + expect(experimentalFirstAdminRouteIndex).toBeGreaterThan(experimentalAdminTenantIndex); expect(source).toContain("app.use('/oauth', preAuthTenantMiddleware, routes.oauth);"); expect(source).toContain("app.use('/api/auth', preAuthTenantMiddleware, routes.auth);"); + expect(source).toContain("app.use('/api/admin', preAuthTenantMiddleware);"); expect(source).toContain( "app.use('/api/config', preAuthTenantMiddleware, optionalJwtAuth, routes.config);", ); diff --git a/api/server/index.js b/api/server/index.js index 7256ffadc85..9a4cfb2f479 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -361,11 +361,12 @@ const startServer = async () => { /* Per-request capability cache — must be registered before any route that calls hasCapability */ app.use(capabilityContextMiddleware); - /* Pre-auth tenant context for unauthenticated routes that need tenant scoping. + /* Pre-auth tenant context for routes that need request-selected tenant scoping. * The reverse proxy / auth gateway sets `X-Tenant-Id` header for multi-tenant deployments. */ app.use('/oauth', preAuthTenantMiddleware, routes.oauth); /* API Endpoints */ app.use('/api/auth', preAuthTenantMiddleware, routes.auth); + app.use('/api/admin', preAuthTenantMiddleware); app.use('/api/admin/insights', routes.insights); app.use('/api/admin', routes.adminAuth); app.use('/api/admin/config', routes.adminConfig); diff --git a/api/server/middleware/__tests__/requireJwtAuth.spec.js b/api/server/middleware/__tests__/requireJwtAuth.spec.js index bbadd5fb042..6ad418ae9cb 100644 --- a/api/server/middleware/__tests__/requireJwtAuth.spec.js +++ b/api/server/middleware/__tests__/requireJwtAuth.spec.js @@ -86,8 +86,12 @@ jest.mock('@librechat/api', () => { }, maybeRefreshCloudFrontAuthCookiesMiddleware: jest.fn((req, res, next) => next()), tenantContextMiddleware: (req, res, next) => { + const trustedTenantId = tenantStorage.getStore()?.tenantId; const context = { - tenantId: normalizeContextValue(req.user?.tenantId), + tenantId: + normalizeContextValue(req.tenantId) ?? + normalizeContextValue(trustedTenantId) ?? + normalizeContextValue(req.user?.tenantId), userId: getUserId(req.user), requestId: getRequestId(req), }; @@ -185,6 +189,15 @@ describe('requireJwtAuth tenant context chaining', () => { expect(tenantId).toBe('tenant-abc'); }); + it('preserves a trusted pre-auth tenant when the JWT user belongs to another tenant', async () => { + const { tenantStorage } = require('@librechat/data-schemas'); + const tenantId = await tenantStorage.run({ tenantId: 'tenant-from-header' }, () => + runAuth({ tenantId: 'tenant-from-jwt', role: 'user' }), + ); + + expect(tenantId).toBe('tenant-from-header'); + }); + it('refreshes CloudFront auth cookies after passport auth succeeds', () => { const req = mockReq({ tenantId: 'tenant-abc', role: 'user' }); const res = mockRes(); diff --git a/api/server/middleware/config/app.js b/api/server/middleware/config/app.js index 3768089b249..d22b5827d46 100644 --- a/api/server/middleware/config/app.js +++ b/api/server/middleware/config/app.js @@ -1,10 +1,12 @@ const { logger } = require('@librechat/data-schemas'); -const { getAppConfigOptionsFromUser } = require('@librechat/api'); +const { getAppConfigOptionsFromUser, getEffectiveTenantId } = require('@librechat/api'); const { getAppConfig } = require('~/server/services/Config'); const configMiddleware = async (req, res, next) => { try { - req.config = await getAppConfig(getAppConfigOptionsFromUser(req.user)); + req.config = await getAppConfig( + getAppConfigOptionsFromUser(req.user, getEffectiveTenantId(req)), + ); next(); } catch (error) { @@ -15,7 +17,7 @@ const configMiddleware = async (req, res, next) => { }); try { - req.config = await getAppConfig({ tenantId: req.user?.tenantId }); + req.config = await getAppConfig({ tenantId: getEffectiveTenantId(req) }); next(); } catch (fallbackError) { logger.error('Fallback config middleware error:', fallbackError); diff --git a/api/server/middleware/config/app.spec.js b/api/server/middleware/config/app.spec.js new file mode 100644 index 00000000000..740b5d0f62b --- /dev/null +++ b/api/server/middleware/config/app.spec.js @@ -0,0 +1,60 @@ +const { tenantStorage } = require('@librechat/data-schemas'); + +const mockGetAppConfig = jest.fn().mockResolvedValue({ ok: true }); +jest.mock('~/server/services/Config', () => ({ + getAppConfig: (...args) => mockGetAppConfig(...args), +})); + +const configMiddleware = require('./app'); + +describe('configMiddleware — tenant resolution', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + function runInTenantContext(tenantId, fn) { + return tenantStorage.run({ tenantId }, fn); + } + + /** + * Reproduces the exact mismatch the review flagged: a deployment that + * resolves the authoritative tenant server-side (ALS, seeded from + * `req.tenantId` by `tenantContextMiddleware`) must not have `req.config` + * loaded — or cached — under a *different*, stale `req.user.tenantId` JWT + * claim. Every other admin capability check and write already trusts ALS + * via `getEffectiveTenantId`; `configMiddleware` must match. + */ + it('loads config for the ALS-resolved tenant, not a mismatched user.tenantId claim', async () => { + const req = { user: { tenantId: 'tenant-A-from-jwt', role: 'admin' }, path: '/api/config' }; + const next = jest.fn(); + + await runInTenantContext('tenant-B-from-als', () => configMiddleware(req, {}, next)); + + expect(mockGetAppConfig).toHaveBeenCalledWith( + expect.objectContaining({ tenantId: 'tenant-B-from-als' }), + ); + expect(next).toHaveBeenCalledWith(); + }); + + it('falls back to the ALS-resolved tenant (not the user claim) on the error-recovery path too', async () => { + mockGetAppConfig.mockRejectedValueOnce(new Error('boom')).mockResolvedValueOnce({ ok: true }); + const req = { user: { tenantId: 'tenant-A-from-jwt', role: 'admin' }, path: '/api/config' }; + const next = jest.fn(); + + await runInTenantContext('tenant-B-from-als', () => configMiddleware(req, {}, next)); + + expect(mockGetAppConfig).toHaveBeenLastCalledWith({ tenantId: 'tenant-B-from-als' }); + expect(next).toHaveBeenCalledWith(); + }); + + it('falls back to the user claim when no ALS tenant is active (no divergence to resolve)', async () => { + const req = { user: { tenantId: 'tenant-A-from-jwt', role: 'admin' }, path: '/api/config' }; + const next = jest.fn(); + + await configMiddleware(req, {}, next); + + expect(mockGetAppConfig).toHaveBeenCalledWith( + expect.objectContaining({ tenantId: 'tenant-A-from-jwt' }), + ); + }); +}); diff --git a/api/server/routes/__tests__/config.spec.js b/api/server/routes/__tests__/config.spec.js index 7b81f5a1458..f76506d75e1 100644 --- a/api/server/routes/__tests__/config.spec.js +++ b/api/server/routes/__tests__/config.spec.js @@ -310,13 +310,28 @@ describe('GET /api/config', () => { }); }); - it('should prefer user tenantId over getTenantId fallback', async () => { + it('should prefer the effective request tenant over the user tenantId', async () => { mockGetAppConfig.mockResolvedValue(baseAppConfig); mockGetTenantId.mockReturnValue('fallback-tenant'); const app = createApp({ ...mockUser, tenantId: 'user-tenant' }); await request(app).get('/api/config'); + expect(mockGetAppConfig).toHaveBeenCalledWith({ + role: 'USER', + userId: 'user123', + idOnTheSource: undefined, + tenantId: 'fallback-tenant', + }); + }); + + it('should use the user tenantId when no effective request tenant exists', async () => { + mockGetAppConfig.mockResolvedValue(baseAppConfig); + mockGetTenantId.mockReturnValue(undefined); + const app = createApp({ ...mockUser, tenantId: 'user-tenant' }); + + await request(app).get('/api/config'); + expect(mockGetAppConfig).toHaveBeenCalledWith({ role: 'USER', userId: 'user123', diff --git a/api/server/routes/admin/config.js b/api/server/routes/admin/config.js index 23b43c327cb..21ac8c1bff7 100644 --- a/api/server/routes/admin/config.js +++ b/api/server/routes/admin/config.js @@ -28,6 +28,7 @@ const handlers = createAdminConfigHandlers({ hasAnyConfigReadAccess, getReadableConfigSections, mutateConfigWithRevision: db.mutateConfigWithRevision, + listConfigRevisions: db.listConfigRevisions, hasConfigCapability, hasCapability, getAppConfig, @@ -38,6 +39,7 @@ router.use(requireJwtAuth, requireAdminAccess); router.get('/', handlers.listConfigs); router.get('/base', handlers.getBaseConfig); +router.get('/:principalType/:principalId/revisions', handlers.listConfigRevisions); router.get('/:principalType/:principalId', handlers.getConfig); router.put('/:principalType/:principalId', handlers.upsertConfigOverrides); router.patch('/:principalType/:principalId/fields', handlers.patchConfigField); diff --git a/api/server/routes/admin/langfuse.js b/api/server/routes/admin/langfuse.js index 515a5893b76..54b425a65be 100644 --- a/api/server/routes/admin/langfuse.js +++ b/api/server/routes/admin/langfuse.js @@ -1,5 +1,5 @@ const express = require('express'); -const { createAdminLangfuseHandlers } = require('@librechat/api'); +const { createAdminLangfuseHandlers, getEffectiveTenantId } = require('@librechat/api'); const { SystemCapabilities } = require('@librechat/data-schemas'); const { hasConfigCapability, @@ -20,10 +20,14 @@ async function requireLangfuseManage(req, res, next) { if (!id) { return res.status(401).json({ message: 'Authentication required' }); } + // The effective request tenant, not the raw user claim — `updateConnection` + // writes the config, revision, and epoch under the same value, so checking + // grants against `req.user.tenantId` could authorize a different tenant + // than the one written. const user = { id, role: req.user.role ?? '', - tenantId: req.user.tenantId, + tenantId: getEffectiveTenantId(req), idOnTheSource: req.user.idOnTheSource ?? null, }; if (await hasConfigCapability(user, 'langfuse')) { @@ -37,8 +41,7 @@ async function requireLangfuseManage(req, res, next) { const handlers = createAdminLangfuseHandlers({ findConfigByPrincipal: db.findConfigByPrincipal, - patchConfigFields: db.patchConfigFields, - toggleConfigActive: db.toggleConfigActive, + mutateConfigWithRevision: db.mutateConfigWithRevision, getMessages: db.getMessages, invalidateConfigCaches, }); diff --git a/api/server/routes/admin/langfuse.test.js b/api/server/routes/admin/langfuse.test.js index 52a13386916..ff9307aad53 100644 --- a/api/server/routes/admin/langfuse.test.js +++ b/api/server/routes/admin/langfuse.test.js @@ -30,6 +30,7 @@ jest.mock('@librechat/data-schemas', () => ({ jest.mock('@librechat/api', () => ({ createAdminLangfuseHandlers: jest.fn(() => mockHandlers), + getEffectiveTenantId: jest.fn((req) => req.tenantId ?? req.user?.tenantId), })); jest.mock('~/server/middleware/roles/capabilities', () => ({ diff --git a/api/server/services/Config/app.js b/api/server/services/Config/app.js index ce157640fc5..4115c5d3e7c 100644 --- a/api/server/services/Config/app.js +++ b/api/server/services/Config/app.js @@ -50,7 +50,8 @@ const { getAppConfig, clearAppConfigCache, clearOverrideCache } = createAppConfi setCachedTools, getCache: getLogStores, cacheKeys: CacheKeys, - getApplicableConfigs: db.getApplicableConfigs, + getApplicableConfigs: (principals, options) => + db.getApplicableConfigs(principals, undefined, options), getUserPrincipals: db.getUserPrincipals, augmentConfig: ({ appConfig, baseConfig, principals, options }) => { if (!options.userId) return appConfig; diff --git a/api/server/services/Endpoints/azureAssistants/initialize.encryptedAzureKey.spec.js b/api/server/services/Endpoints/azureAssistants/initialize.encryptedAzureKey.spec.js new file mode 100644 index 00000000000..ad7e16c4999 --- /dev/null +++ b/api/server/services/Endpoints/azureAssistants/initialize.encryptedAzureKey.spec.js @@ -0,0 +1,132 @@ +// jestSetup.js (this workspace's global setup) already sets CREDS_KEY to the +// literal 'test' for the many suites that never touch real encryption — too +// short for encryptV3's 32-byte key requirement, so it must be overridden +// unconditionally here, not with `??`, and restored after so the override +// doesn't leak into other test files sharing this worker. +const originalCredsKey = process.env.CREDS_KEY; +process.env.CREDS_KEY = '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef'; + +const { EModelEndpoint } = require('librechat-data-provider'); + +const mockCheckUserKeyExpiry = jest.fn(); +const mockGetProxyDispatcher = jest.fn(() => null); +jest.mock('@librechat/api', () => ({ + ...jest.requireActual('@librechat/api'), + isUserProvided: (val) => val === 'user_provided', + checkUserKeyExpiry: (...args) => mockCheckUserKeyExpiry(...args), + getProxyDispatcher: (...args) => mockGetProxyDispatcher(...args), +})); + +const mockGetUserKeyValues = jest.fn(); +const mockGetUserKeyExpiry = jest.fn(); +jest.mock('~/models', () => ({ + getUserKeyValues: (...args) => mockGetUserKeyValues(...args), + getUserKeyExpiry: (...args) => mockGetUserKeyExpiry(...args), +})); + +let capturedOpenAIOptions; +jest.mock('openai', () => { + return jest.fn().mockImplementation((options) => { + capturedOpenAIOptions = options; + return { beta: { assistants: {} } }; + }); +}); + +// Loaded via dynamic import in beforeAll so encryption initializes after +// CREDS_KEY is set above (encryptV3 reads the key at module load) — matches +// the pattern in admin/secrets.spec.ts and the sibling azureOpenAI +// initializer's own encrypted-key spec, required here because this +// initializer transitively imports the admin secrets module via +// `@librechat/api`'s `resolveConfigSecret`. +let initializeClient; +let encryptV3; + +beforeAll(async () => { + initializeClient = require('./initialize'); + ({ encryptV3 } = await import('@librechat/data-schemas')); +}); + +/** + * `endpoints.azureOpenAI.groups[].apiKey` is encrypted at rest by the admin + * config write path. Unlike the regular Azure OpenAI initializer, this + * Assistants-specific initializer assigned the stored ciphertext straight to + * `apiKey`/`azureOptions` without ever decrypting it — the OpenAI client's + * own `apiKey` (used to build its default `Authorization` header) and + * anything reading `openai.locals.azureOptions` afterward would receive the + * ciphertext, not the real secret. + */ +describe('azureAssistants initializeClient decrypts an encrypted Azure group apiKey', () => { + afterAll(() => { + process.env.CREDS_KEY = originalCredsKey; + }); + + afterEach(() => { + jest.clearAllMocks(); + capturedOpenAIOptions = undefined; + }); + + function createReq(groupOverrides) { + return { + user: { id: 'user-1' }, + body: {}, + query: {}, + config: { + endpoints: { + [EModelEndpoint.azureOpenAI]: { + assistants: true, + modelGroupMap: { 'gpt-4': { group: 'prod' } }, + groupMap: { prod: groupOverrides }, + assistantModels: ['gpt-4'], + }, + }, + }, + }; + } + + it('decrypts an encrypted serverless group apiKey for both the api-key header and the OpenAI client', async () => { + const encrypted = encryptV3('sk-azure-serverless-secret'); + const req = createReq({ + serverless: true, + baseURL: 'https://prod.example.com', + apiKey: encrypted, + instanceName: 'prod-instance', + deploymentName: 'gpt-4-deployment', + version: '2024-02-01', + models: { 'gpt-4': true }, + }); + + const { openAIApiKey } = await initializeClient({ + req, + res: {}, + version: 'v2', + endpointOption: {}, + initAppClient: true, + }); + + expect(openAIApiKey).toBe('sk-azure-serverless-secret'); + expect(capturedOpenAIOptions.apiKey).toBe('sk-azure-serverless-secret'); + }); + + it('decrypts an encrypted non-serverless group apiKey for the OpenAI client and azureOptions locals', async () => { + const encrypted = encryptV3('sk-azure-managed-secret'); + const req = createReq({ + instanceName: 'prod-instance', + deploymentName: 'gpt-4-deployment', + version: '2024-02-01', + models: { 'gpt-4': true }, + apiKey: encrypted, + }); + + const { openai, openAIApiKey } = await initializeClient({ + req, + res: {}, + version: 'v2', + endpointOption: {}, + initAppClient: true, + }); + + expect(openAIApiKey).toBe('sk-azure-managed-secret'); + expect(capturedOpenAIOptions.apiKey).toBe('sk-azure-managed-secret'); + expect(openai.locals.azureOptions.azureOpenAIApiKey).toBe('sk-azure-managed-secret'); + }); +}); diff --git a/api/server/services/Endpoints/azureAssistants/initialize.js b/api/server/services/Endpoints/azureAssistants/initialize.js index c170de29683..74480cb2f4e 100644 --- a/api/server/services/Endpoints/azureAssistants/initialize.js +++ b/api/server/services/Endpoints/azureAssistants/initialize.js @@ -5,6 +5,7 @@ const { constructAzureURL, checkUserKeyExpiry, getProxyDispatcher, + resolveConfigSecret, } = require('@librechat/api'); const { ErrorTypes, EModelEndpoint, mapModelToAzureConfig } = require('librechat-data-provider'); const { getUserKeyValues, getUserKeyExpiry } = require('~/models'); @@ -101,14 +102,20 @@ const initializeClient = async ({ req, res, version, endpointOption, initAppClie groupMap, }); - azureOptions = currentOptions; + // groupMap's apiKey is encrypted at rest (see ARRAY_SECRET_FIELDS in + // packages/api/src/admin/secrets.ts) — resolve it once here, the same + // way the regular Azure OpenAI initializer does, so neither the OpenAI + // client's own `apiKey` (which it uses to build its default + // `Authorization` header) nor anything else reading `azureOptions` later + // (e.g. `openai.locals.azureOptions`) ends up holding ciphertext instead + // of the real key. + apiKey = resolveConfigSecret(currentOptions.azureOpenAIApiKey) ?? ''; + azureOptions = { ...currentOptions, azureOpenAIApiKey: apiKey }; baseURL = constructAzureURL({ baseURL: azureBaseURL ?? 'https://${INSTANCE_NAME}.openai.azure.com/openai', azureOptions, }); - - apiKey = azureOptions.azureOpenAIApiKey; opts.defaultQuery = { 'api-version': azureOptions.azureOpenAIApiVersion }; opts.defaultHeaders = resolveHeaders({ headers: { diff --git a/client/src/components/Nav/SettingsTabs/Integrations/LangfuseConnection.tsx b/client/src/components/Nav/SettingsTabs/Integrations/LangfuseConnection.tsx index fa6b728a02b..515bfb2b5d9 100644 --- a/client/src/components/Nav/SettingsTabs/Integrations/LangfuseConnection.tsx +++ b/client/src/components/Nav/SettingsTabs/Integrations/LangfuseConnection.tsx @@ -27,7 +27,9 @@ import { } from '~/data-provider'; import { useLocalize } from '~/hooks'; -type ConnectionTestState = 'idle' | 'unverified' | 'checking' | 'connected' | 'failed'; +type ConnectionTestState = 'idle' | 'inactive' | 'unverified' | 'checking' | 'connected' | 'failed'; + +type VersionedTenant = Pick; function getStoredConnectionTestKey(status?: TLangfuseConnectionStatus): string | undefined { if (status?.configured !== true || !status.destination || !status.publicKey) { @@ -45,6 +47,8 @@ function getConnectionStatusLabelKey(state: ConnectionTestState): TranslationKey return 'com_ui_langfuse_status_connected'; case 'failed': return 'com_ui_langfuse_status_failed'; + case 'inactive': + return 'com_ui_langfuse_status_inactive'; case 'unverified': return 'com_ui_langfuse_status_not_verified'; case 'idle': @@ -93,6 +97,23 @@ function getConnectionStatusDotClass(state: ConnectionTestState): string { } } +/** + * A 409 body carries the server's current version, but resending it as the + * next `expectedVersion` without also refreshing the fields it belongs to is + * exactly the unsafe retry this component must avoid — so the version out of + * the error body is never used; only whether the status is 409 matters. + */ +function isVersionConflict(error: unknown): boolean { + return (error as { response?: { status?: number } } | undefined)?.response?.status === 409; +} + +function isTenantConflict(error: unknown): boolean { + return ( + (error as { response?: { data?: { error?: string } } } | undefined)?.response?.data?.error === + 'Tenant context changed' + ); +} + function getDisplayPublicKey(publicKey: string): string { const trimmedPublicKey = publicKey.trim(); if (trimmedPublicKey.length <= 12) { @@ -123,10 +144,76 @@ export default function LangfuseConnection() { const [isEditingSecretKey, setIsEditingSecretKey] = useState(false); const [connectionTestState, setConnectionTestState] = useState('idle'); const [connectionTestMessage, setConnectionTestMessage] = useState(''); + /** CAS tokens for the next write. Tracked together and separately from + * `connectionStatus` so a conflict can advance them without resetting the + * form fields, and a tenant refresh cannot pair one tenant's ID with + * another tenant's version. */ + const [writeBaseline, setWriteBaseline] = useState<{ + expectedVersion: number | null; + expectedTenantId: string; + }>({ expectedVersion: null, expectedTenantId: '' }); const autoTestedConnectionRef = useRef(); const connectionTestRequestRef = useRef(0); const publicKeyInputRef = useRef(null); const secretKeyInputRef = useRef(null); + /** + * Whether this field's current value differs from `connectionStatus` right + * now — re-derived on every change against that moment's baseline, not + * "was this ever edited," so typing away and back to the original value + * clears it again. A background sync must not clobber a real divergence + * with the refetched value; only fields that still match get the fresh + * baseline. Reset (to false, since the local value then *is* the new + * baseline) once a save succeeds. + */ + const destinationTouchedRef = useRef(false); + const publicKeyTouchedRef = useRef(false); + /** + * Mirrors of `destination`/`publicKey` state. `applyFreshRecord` must read + * these instead of the state variables directly: it's called from + * `rebaseOnConflict`'s `refetchConnection().then(...)` callback, a closure + * captured at the moment the conflict was handled — if the admin edits + * destination/publicKey while that refetch is still pending (inputs stay + * editable; `busy` doesn't cover the refetch), the callback's own + * closed-over `destination`/`publicKey` would still be the PRE-edit + * values. Comparing against those stale values instead of the truly-current + * ones can wrongly conclude the draft "already matches" the refetched + * baseline and clear the touched ref, letting the very next passive sync + * overwrite the admin's in-progress edit with the stale refetched value. + * + * Kept in sync in TWO ways, deliberately: the change handlers below write + * `.current` synchronously in the same tick as `setDestination`/ + * `setPublicKey`, and the effects further down mirror the same state as a + * backstop for any OTHER path that changes this state (e.g. a fresh-record + * adoption resetting the field). The synchronous write is the one that + * actually matters here — a passive `useEffect` only runs after React + * commits the render, which leaves a window, within the same tick, where a + * pending `refetchConnection()` promise can resolve and read a still-stale + * ref: the admin's keystroke handler has already fired (and `setState` + * has been called) but the effect hasn't flushed yet. That is the exact + * one-tick-later version of the bug this ref was added to fix in the + * first place. + */ + const destinationRef = useRef(destination); + const publicKeyRef = useRef(publicKey); + useEffect(() => { + destinationRef.current = destination; + }, [destination]); + useEffect(() => { + publicKeyRef.current = publicKey; + }, [publicKey]); + /** Same "current dirtiness" role as the refs above, but for the secret key + * draft — tracked via a ref instead of reading `secretKey` state directly + * inside the sync effect below, so that effect stays free of a dependency + * that would otherwise fire it on every keystroke. There's no baseline to + * compare against (the server never sends back a real secret), so any + * non-empty draft counts as dirty. */ + const secretKeyDraftRef = useRef(false); + /** + * The highest `configVersion` this component has adopted for the current + * tenant. Versions belong to tenant-specific epochs and cannot be ordered + * across different effective tenants. + */ + const latestVersionRef = useRef(null); useEffect(() => { if (isEditingPublicKey) { @@ -140,22 +227,164 @@ export default function LangfuseConnection() { } }, [isEditingSecretKey]); + /** + * Whether `candidate` is older than the highest version this component + * has already adopted — i.e. it must be discarded outright rather than + * partially applied. A background query that started before a successful + * save or conflict rebase can still resolve afterward with the pre-save + * content: `useGetLangfuseConnectionQuery` never cancels an in-flight + * fetch on mutation success, so without this guard that stale response + * would pass through the effect below and silently revert + * destination/publicKey (for whichever fields the admin hasn't touched) + * back to the pre-save values, while `expectedVersion` stays correctly + * frozen at the new version if a secret-key draft is in progress — the + * next Save would then pass CAS on that new version while resubmitting + * the reverted, stale destination/publicKey. + * + * Within one tenant, a numeric latest version always outranks a `null` + * candidate version. A different tenant starts a separate version epoch and + * must be adopted even when its version is lower or absent. + */ + const isStale = (candidate: TLangfuseConnectionStatus): boolean => { + const latest = latestVersionRef.current; + return ( + latest != null && + (candidate.effectiveTenantId ?? '') === latest.effectiveTenantId && + latest.configVersion != null && + (candidate.configVersion == null || candidate.configVersion < latest.configVersion) + ); + }; + + const rememberVersion = (candidate: TLangfuseConnectionStatus) => { + latestVersionRef.current = { + configVersion: candidate.configVersion ?? null, + effectiveTenantId: candidate.effectiveTenantId ?? '', + }; + }; + useEffect(() => { - if (!status) { + if (!status || isStale(status)) { return; } + rememberVersion(status); setConnectionStatus(status); }, [status]); + /** + * Handles *passive* syncs only — `connectionStatus` changing because + * `status` (the query's own data) changed, e.g. a reconnect-triggered + * background refetch, not because this component explicitly adopted a + * fresh record. Explicit actions (save success, conflict rebase) call + * `applyFreshRecord` directly instead of relying on this effect, precisely + * because they can't depend on it firing: React bails out of an identical + * state update (`Object.is`), and React Query's structural sharing can + * return the very same object reference `connectionStatus` already holds + * when a refetch's result is unchanged — silently skipping this effect and + * leaving `expectedVersion` stuck at whatever it was. + * + * For a passive sync specifically, that same bail-out is harmless: if the + * object didn't change, there's nothing to react to. What must not happen + * is advancing `expectedVersion` here while a draft survives — that would + * let a later Save pass CAS on a version never actually paired with this + * draft's content. + */ useEffect(() => { if (!connectionStatus) { return; } - setDestination(connectionStatus.destination ?? ''); - setPublicKey(connectionStatus.publicKey ?? ''); + const effectiveTenantId = connectionStatus.effectiveTenantId ?? ''; + const tenantChanged = effectiveTenantId !== writeBaseline.expectedTenantId; + if (tenantChanged) { + destinationTouchedRef.current = false; + publicKeyTouchedRef.current = false; + secretKeyDraftRef.current = false; + autoTestedConnectionRef.current = undefined; + connectionTestRequestRef.current += 1; + setSecretKey(''); + setIsEditingPublicKey(false); + setIsEditingSecretKey(false); + } + if (tenantChanged || !destinationTouchedRef.current) { + setDestination(connectionStatus.destination ?? ''); + } else if (destination === (connectionStatus.destination ?? '')) { + destinationTouchedRef.current = false; + } + if (tenantChanged || !publicKeyTouchedRef.current) { + setPublicKey(connectionStatus.publicKey ?? ''); + } else if (publicKey.trim() === (connectionStatus.publicKey ?? '')) { + publicKeyTouchedRef.current = false; + } + const hasLocalDraft = + destinationTouchedRef.current || publicKeyTouchedRef.current || secretKeyDraftRef.current; + if (tenantChanged || !hasLocalDraft) { + setWriteBaseline({ + expectedVersion: connectionStatus.configVersion ?? null, + expectedTenantId: effectiveTenantId, + }); + } + // destination/publicKey are read only for the reconvergence check above, + // which must run when `connectionStatus` changes, not on every keystroke — + // adding them here would fire this effect on every edit instead of only + // on a genuine sync, same reasoning as secretKeyDraftRef's docstring above. + // eslint-disable-next-line react-hooks/exhaustive-deps }, [connectionStatus]); + /** + * The one place an *explicit* action (save success, conflict rebase) + * adopts a fresh record. Sets `expectedVersion` directly and synchronously + * instead of going through `setConnectionStatus` + the effect above, since + * that effect isn't guaranteed to fire (see its docstring) and this call + * site's whole point is to always pair the fresh version with whatever + * same-tenant draft remains. A tenant change discards the old tenant's + * draft before adopting the new record. Guarded by the same `isStale` check + * as the passive sync effect above: an explicit action's own read can itself + * be superseded by a different action that already landed a higher version + * while this one was in flight (e.g. two rapid Save clicks), and adopting + * the loser here would be exactly the same regression, just triggered by a + * different path. + */ + const applyFreshRecord = (fresh: TLangfuseConnectionStatus) => { + if (isStale(fresh)) { + return; + } + const effectiveTenantId = fresh.effectiveTenantId ?? ''; + const tenantChanged = effectiveTenantId !== writeBaseline.expectedTenantId; + rememberVersion(fresh); + if (tenantChanged) { + destinationTouchedRef.current = false; + publicKeyTouchedRef.current = false; + secretKeyDraftRef.current = false; + autoTestedConnectionRef.current = undefined; + connectionTestRequestRef.current += 1; + setSecretKey(''); + setIsEditingPublicKey(false); + setIsEditingSecretKey(false); + } + if (tenantChanged || !destinationTouchedRef.current) { + setDestination(fresh.destination ?? ''); + } else if (destinationRef.current === (fresh.destination ?? '')) { + // The surviving draft happens to already match the fresh baseline + // (e.g. a rebase reveals another admin's change that coincides with + // this one) — recompute rather than leave it stuck "touched", or + // passive syncs would keep freezing expectedVersion for a divergence + // that no longer exists, causing unnecessary 409s. Reads the ref, not + // the closed-over `destination` — see the ref's doc comment. + destinationTouchedRef.current = false; + } + if (tenantChanged || !publicKeyTouchedRef.current) { + setPublicKey(fresh.publicKey ?? ''); + } else if (publicKeyRef.current.trim() === (fresh.publicKey ?? '')) { + publicKeyTouchedRef.current = false; + } + setWriteBaseline({ + expectedVersion: fresh.configVersion ?? null, + expectedTenantId: effectiveTenantId, + }); + setConnectionStatus(fresh); + }; + const secretConfigured = connectionStatus?.configured === true; + const configActive = connectionStatus?.configActive !== false; const destinations = connectionStatus?.destinations ?? []; const connectionDestinationAvailable = destinations.some( ({ key }) => key === connectionStatus?.destination, @@ -190,6 +419,7 @@ export default function LangfuseConnection() { const isEditing = !secretConfigured || isEditingPublicKey || isEditingSecretKey || hasUnsavedChanges; const canSubmit = + configActive && destination !== '' && trimmedPublicKey !== '' && ((!connectionCredentialsChanged && secretConfigured) || trimmedSecretKey !== ''); @@ -200,6 +430,13 @@ export default function LangfuseConnection() { if (!connectionStatus) { return; } + if (!configActive) { + autoTestedConnectionRef.current = undefined; + connectionTestRequestRef.current += 1; + setConnectionTestState('inactive'); + setConnectionTestMessage(''); + return; + } if (!storedConnectionTestKey) { return; } @@ -242,7 +479,7 @@ export default function LangfuseConnection() { }, }, ); - }, [connectionStatus, localize, testMutation]); + }, [configActive, connectionStatus, localize, testMutation]); const connectionStatusLabel = connectionTestState === 'failed' && connectionTestMessage !== '' @@ -254,27 +491,78 @@ export default function LangfuseConnection() { const connectionStatusTitle = connectionTestState === 'failed' ? localize('com_ui_langfuse_status_failed_hover') : undefined; + /** + * A 409 means another admin's write landed since this form's baseline was + * read. Resending the local draft under the server's bumped version would + * silently reapply this form's stale `destination`/`publicKey` (and + * `enabled`, for the toggle path) over that concurrent change. Refetching + * and re-basing via `applyFreshRecord` avoids that — fields the admin + * hasn't touched pick up the latest server value, `expectedVersion` comes + * from that same read, not the error body, and it's set directly rather + * than left to the passive sync effect. Same-tenant drafts survive; a tenant + * change clears the previous tenant's draft before adopting the new record. + * + * A failed or stale refetch must not advance `expectedVersion` on its own: + * React Query resolves a failed refetch with `isError: true` while still + * holding the previous `data`, so `result.data` alone doesn't prove the + * read is fresh. Leaving the token at its pre-conflict (now known-stale) + * value means the next Save attempt safely 409s again instead of risking a + * pass built on fields that were never actually refreshed. + */ + const rebaseOnConflict = (error: unknown) => { + showToast({ + message: localize( + isTenantConflict(error) + ? 'com_ui_langfuse_tenant_changed' + : 'com_ui_langfuse_version_conflict', + ), + status: 'warning', + }); + setConnectionTestState('unverified'); + setConnectionTestMessage(''); + refetchConnection().then((result) => { + if (result.isError || !result.data) { + setConnectionTestState('failed'); + setConnectionTestMessage(localize('com_ui_langfuse_conflict_refresh_error')); + return; + } + applyFreshRecord(result.data); + }); + }; + const handleSave = () => { + if (!configActive) { + return; + } const payload = { enabled: !secretConfigured || connectionStatus?.enabled === true, destination, publicKey: trimmedPublicKey, ...(trimmedSecretKey ? { secretKey: trimmedSecretKey } : {}), + ...writeBaseline, }; connectionTestRequestRef.current += 1; updateMutation.mutate(payload, { onSuccess: (nextStatus) => { - autoTestedConnectionRef.current = getStoredConnectionTestKey(nextStatus); - setConnectionStatus(nextStatus); - setConnectionTestState('connected'); + autoTestedConnectionRef.current = + nextStatus.configActive === false ? undefined : getStoredConnectionTestKey(nextStatus); + destinationTouchedRef.current = false; + publicKeyTouchedRef.current = false; + secretKeyDraftRef.current = false; + applyFreshRecord(nextStatus); + setConnectionTestState(nextStatus.configActive === false ? 'inactive' : 'connected'); setConnectionTestMessage(''); setSecretKey(''); setIsEditingPublicKey(false); setIsEditingSecretKey(false); showToast({ message: localize('com_ui_langfuse_saved'), status: 'success' }); }, - onError: () => { + onError: (error) => { + if (isVersionConflict(error)) { + rebaseOnConflict(error); + return; + } setConnectionTestState('failed'); setConnectionTestMessage(localize('com_ui_langfuse_save_error')); showToast({ message: localize('com_ui_langfuse_save_error'), status: 'error' }); @@ -283,6 +571,8 @@ export default function LangfuseConnection() { }; const handleDestinationChange = (nextDestination: string) => { + destinationTouchedRef.current = nextDestination !== (connectionStatus?.destination ?? ''); + destinationRef.current = nextDestination; setDestination(nextDestination); const requestId = ++connectionTestRequestRef.current; const credentialsChanged = @@ -333,7 +623,12 @@ export default function LangfuseConnection() { }; const handleEnabledChange = () => { - if (!secretConfigured || !connectionStatus?.destination || !connectionStatus.publicKey) { + if ( + !configActive || + !secretConfigured || + !connectionStatus?.destination || + !connectionStatus.publicKey + ) { return; } @@ -345,6 +640,7 @@ export default function LangfuseConnection() { enabled: nextEnabled, destination: connectionStatus.destination ?? '', publicKey: connectionStatus.publicKey ?? '', + ...writeBaseline, }, { onSuccess: (nextStatus) => { @@ -352,13 +648,17 @@ export default function LangfuseConnection() { return; } autoTestedConnectionRef.current = getStoredConnectionTestKey(nextStatus); - setConnectionStatus(nextStatus); + applyFreshRecord(nextStatus); showToast({ message: localize('com_ui_langfuse_saved'), status: 'success' }); }, - onError: () => { + onError: (error) => { if (requestId !== connectionTestRequestRef.current) { return; } + if (isVersionConflict(error)) { + rebaseOnConflict(error); + return; + } showToast({ message: localize('com_ui_langfuse_save_error'), status: 'error' }); }, }, @@ -441,6 +741,15 @@ export default function LangfuseConnection() { + {!configActive && ( +
+ {localize('com_ui_langfuse_config_inactive')} +
+ )} +
setIsEditingPublicKey(true)} > @@ -482,11 +791,14 @@ export default function LangfuseConnection() { data-bwignore="true" data-form-type="other" value={publicKey} - disabled={busy} + disabled={!configActive || busy} placeholder="pk-lf-..." onChange={(e) => { connectionTestRequestRef.current += 1; const nextPublicKey = e.target.value; + publicKeyTouchedRef.current = + nextPublicKey.trim() !== (connectionStatus?.publicKey ?? ''); + publicKeyRef.current = nextPublicKey; setPublicKey(nextPublicKey); if ( secretConfigured && @@ -508,7 +820,7 @@ export default function LangfuseConnection() { type="button" className="w-full rounded-lg border border-border-light px-3 py-2 text-left hover:border-border-medium focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring-primary" aria-label={`${localize('com_ui_edit')} ${localize('com_ui_langfuse_secret_key')}`} - disabled={busy} + disabled={!configActive || busy} onClick={() => setIsEditingSecretKey(true)} > @@ -526,11 +838,13 @@ export default function LangfuseConnection() { data-bwignore="true" data-form-type="other" value={secretKey} - disabled={busy} + disabled={!configActive || busy} placeholder="sk-lf-..." onChange={(e) => { connectionTestRequestRef.current += 1; - setSecretKey(e.target.value); + const nextSecretKey = e.target.value; + secretKeyDraftRef.current = nextSecretKey.trim() !== ''; + setSecretKey(nextSecretKey); setConnectionTestState('unverified'); setConnectionTestMessage(''); }} @@ -554,7 +868,9 @@ export default function LangfuseConnection() {