From 15fc27950d9a66020f8825df26eaf221f9c8069d Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 7 Apr 2026 22:38:08 -0400 Subject: [PATCH 1/3] =?UTF-8?q?=E2=9A=A1=20refactor:=20Short-Circuit=20Con?= =?UTF-8?q?fig=20Override=20Resolution=20(#12553)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- api/server/middleware/config/app.js | 2 +- packages/api/src/app/service.spec.ts | 120 ++++++++++++++++++++++++++- packages/api/src/app/service.ts | 44 +++++++--- 3 files changed, 153 insertions(+), 13 deletions(-) diff --git a/api/server/middleware/config/app.js b/api/server/middleware/config/app.js index fb5f89b229..cf02682637 100644 --- a/api/server/middleware/config/app.js +++ b/api/server/middleware/config/app.js @@ -17,7 +17,7 @@ const configMiddleware = async (req, res, next) => { }); try { - req.config = await getAppConfig(); + req.config = await getAppConfig({ tenantId: req.user?.tenantId }); next(); } catch (fallbackError) { logger.error('Fallback config middleware error:', fallbackError); diff --git a/packages/api/src/app/service.spec.ts b/packages/api/src/app/service.spec.ts index e5e076c8eb..b692aa6eb2 100644 --- a/packages/api/src/app/service.spec.ts +++ b/packages/api/src/app/service.spec.ts @@ -1,5 +1,5 @@ import type { AppConfig } from '@librechat/data-schemas'; -import { createAppConfigService } from './service'; +import { createAppConfigService, _resetOverrideStrictCache } from './service'; /** Extends AppConfig with mock fields used by merge behavior tests. */ interface TestConfig extends AppConfig { @@ -229,6 +229,124 @@ describe('createAppConfigService', () => { expect((config as TestConfig).x).toBe('admin-only'); }); + it('passes empty principals to getApplicableConfigs when buildPrincipals returns empty', async () => { + const deps = createDeps({ + getUserPrincipals: jest.fn().mockResolvedValue([]), + }); + const { getAppConfig } = createAppConfigService(deps); + + const config = await getAppConfig({ userId: 'uid1', role: 'USER' }); + + expect(deps.getUserPrincipals).toHaveBeenCalledWith({ userId: 'uid1', role: 'USER' }); + expect(deps.getApplicableConfigs).toHaveBeenCalledWith([]); + expect(config).toEqual(deps._baseConfig); + }); + + describe('strict mode (TENANT_ISOLATION_STRICT=true)', () => { + beforeEach(() => { + process.env.TENANT_ISOLATION_STRICT = 'true'; + _resetOverrideStrictCache(); + }); + afterEach(() => { + delete process.env.TENANT_ISOLATION_STRICT; + _resetOverrideStrictCache(); + }); + + it('skips DB query for empty principals without tenantId and does not cache', async () => { + const deps = createDeps(); + const { getAppConfig } = createAppConfigService(deps); + + const config = await getAppConfig(); + + expect(deps.getApplicableConfigs).not.toHaveBeenCalled(); + expect(config).toEqual(deps._baseConfig); + + const setCalls = deps._cache.set.mock.calls.filter( + ([key]: [string, unknown]) => key !== '_BASE_', + ); + expect(setCalls).toHaveLength(0); + }); + + it('queries DB when tenantId is present', async () => { + const deps = createDeps(); + const { getAppConfig } = createAppConfigService(deps); + + await getAppConfig({ tenantId: 'tenant-a' }); + + expect(deps.getApplicableConfigs).toHaveBeenCalledWith([]); + }); + + it('warns once when non-empty principals proceed without tenantId', async () => { + const { logger } = jest.requireActual('@librechat/data-schemas'); + const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {}); + const deps = createDeps(); + const { getAppConfig } = createAppConfigService(deps); + + await getAppConfig({ role: 'USER' }); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('No tenantId in strict mode')); + const warnCount = warnSpy.mock.calls.length; + + await getAppConfig({ role: 'ADMIN' }); + expect(warnSpy).toHaveBeenCalledTimes(warnCount); + + warnSpy.mockRestore(); + }); + + it('falls through to getApplicableConfigs when ALS has tenant context despite no tenantId param', async () => { + const { tenantStorage } = jest.requireActual('@librechat/data-schemas'); + const deps = createDeps({ + getApplicableConfigs: jest + .fn() + .mockResolvedValue([{ priority: 5, overrides: { restricted: true }, isActive: true }]), + }); + const { getAppConfig } = createAppConfigService(deps); + + const config = await tenantStorage.run({ tenantId: 'tenant-a' }, async () => + getAppConfig(), + ); + + expect(deps.getApplicableConfigs).toHaveBeenCalledWith([]); + expect((config as TestConfig).restricted).toBe(true); + }); + }); + + describe('non-strict mode (TENANT_ISOLATION_STRICT unset)', () => { + beforeEach(() => { + delete process.env.TENANT_ISOLATION_STRICT; + _resetOverrideStrictCache(); + }); + afterEach(() => { + _resetOverrideStrictCache(); + }); + + it('passes empty principals through to getApplicableConfigs', async () => { + const deps = createDeps(); + const { getAppConfig } = createAppConfigService(deps); + + await getAppConfig(); + + expect(deps.getApplicableConfigs).toHaveBeenCalledWith([]); + }); + }); + + it('does not cache on buildPrincipals error — retries on next request', async () => { + const deps = createDeps({ + getUserPrincipals: jest + .fn() + .mockRejectedValueOnce(new Error('transient')) + .mockResolvedValue([{ principalType: 'role', principalId: 'USER' }]), + }); + const { getAppConfig } = createAppConfigService(deps); + + const first = await getAppConfig({ userId: 'uid1', role: 'USER' }); + expect(first).toEqual(deps._baseConfig); + expect(deps.getApplicableConfigs).not.toHaveBeenCalled(); + + await getAppConfig({ userId: 'uid1', role: 'USER' }); + expect(deps.getUserPrincipals).toHaveBeenCalledTimes(2); + expect(deps.getApplicableConfigs).toHaveBeenCalledTimes(1); + }); + it('falls back to base config on getApplicableConfigs error', async () => { const deps = createDeps({ getApplicableConfigs: jest.fn().mockRejectedValue(new Error('DB down')), diff --git a/packages/api/src/app/service.ts b/packages/api/src/app/service.ts index 6c5d307709..952544f02c 100644 --- a/packages/api/src/app/service.ts +++ b/packages/api/src/app/service.ts @@ -1,11 +1,16 @@ import { PrincipalType } from 'librechat-data-provider'; -import { logger, mergeConfigOverrides, BASE_CONFIG_PRINCIPAL_ID } from '@librechat/data-schemas'; +import { + logger, + getTenantId, + mergeConfigOverrides, + BASE_CONFIG_PRINCIPAL_ID, +} from '@librechat/data-schemas'; import type { Types } from 'mongoose'; import type { AppConfig, IConfig } from '@librechat/data-schemas'; const BASE_CONFIG_KEY = '_BASE_'; -const DEFAULT_OVERRIDE_CACHE_TTL = 60_000; +export const DEFAULT_OVERRIDE_CACHE_TTL = 60_000; // ── Types ──────────────────────────────────────────────────────────── @@ -50,9 +55,9 @@ function isStrictOverrideMode(): boolean { return (_strictOverride ??= process.env.TENANT_ISOLATION_STRICT === 'true'); } -/** @internal Resets the cached strict-override flag. Exposed for test teardown only. */ let _warnedNoTenantInStrictMode = false; +/** @internal Resets the memoized strict-override flag and one-time no-tenantId warning gate. Exposed for test teardown only. */ export function _resetOverrideStrictCache(): void { _strictOverride = undefined; _warnedNoTenantInStrictMode = false; @@ -60,13 +65,6 @@ export function _resetOverrideStrictCache(): void { function overrideCacheKey(role?: string, userId?: string, tenantId?: string): string { const tenant = tenantId || '__default__'; - if (!tenantId && isStrictOverrideMode() && !_warnedNoTenantInStrictMode) { - _warnedNoTenantInStrictMode = true; - logger.warn( - '[overrideCacheKey] No tenantId in strict mode — falling back to __default__. ' + - 'This likely indicates a code path that bypasses the tenant context middleware.', - ); - } if (userId && role) { return `_OVERRIDE_:${tenant}:${role}:${userId}`; } @@ -168,8 +166,32 @@ export function createAppConfigService(deps: AppConfigServiceDeps) { } } + const principals = await buildPrincipals(role, userId).catch((error: unknown) => { + logger.error('[getAppConfig] Error building principals, falling back to base:', error); + return null; + }); + if (principals === null) { + return baseConfig; + } + + // Strict-isolation + no tenant (param or ALS) = pathological path (middleware bypass or + // unauthenticated startup). Pre-tenant calls use baseOnly:true; admin calls carry tenantId. + // If ALS has a tenant, Mongoose scopes queries to that tenant's overrides — must fall through. + // Not cached: the cache key doesn't include ALS context, so a cached __default__ entry would + // be served to later ALS-scoped calls that share the same param-derived key. + if (principals.length === 0 && !tenantId && !getTenantId() && isStrictOverrideMode()) { + return baseConfig; + } + + if (!tenantId && isStrictOverrideMode() && !_warnedNoTenantInStrictMode) { + _warnedNoTenantInStrictMode = true; + logger.warn( + '[getAppConfig] No tenantId in strict mode — falling back to __default__. ' + + 'This likely indicates a code path that bypasses the tenant context middleware.', + ); + } + try { - const principals = await buildPrincipals(role, userId); const configs = await getApplicableConfigs(principals); if (configs.length === 0) { From 223065c4116268d511562a6cdf216a2ab0f2c0a2 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 7 Apr 2026 23:08:05 -0400 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=93=A6=20chore:=20npm=20audit=20(#125?= =?UTF-8?q?70)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 📦 chore: npm audit fix - Bump `vite` from 7.3.1 to 7.3.2. - Upgrade `@chevrotain/cst-dts-gen`, `@chevrotain/gast`, `@chevrotain/regexp-to-ast`, `@chevrotain/types`, and `@chevrotain/utils` from 11.1.2 to 12.0.0. - Update `@hono/node-server` from 1.19.10 to 1.19.13. - Upgrade `chevrotain` from 11.1.2 to 12.0.0. - Bump `chevrotain-allstar` from 0.3.1 to 0.4.1. * 🔧 chore: Remove `serialize-javascript` dependency from `package.json` --- package-lock.json | 113 ++++++++++++++++++++-------------------------- package.json | 1 - 2 files changed, 48 insertions(+), 66 deletions(-) diff --git a/package-lock.json b/package-lock.json index d7ec011391..4f0a1afd0f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2586,9 +2586,9 @@ } }, "client/node_modules/vite": { - "version": "7.3.1", - "resolved": "https://registry.npmjs.org/vite/-/vite-7.3.1.tgz", - "integrity": "sha512-w+N7Hifpc3gRjZ63vYBXA56dvvRlNWRczTdmCBBa+CotUzAPf5b7YMdMR/8CQoeYE5LX3W4wj6RYTgonm1b9DA==", + "version": "7.3.2", + "resolved": "https://registry.npmjs.org/vite/-/vite-7.3.2.tgz", + "integrity": "sha512-Bby3NOsna2jsjfLVOHKes8sGwgl4TT0E6vvpYgnAYDIF/tie7MRaFthmKuHx1NSXjiTueXH3do80FMQgvEktRg==", "dev": true, "license": "MIT", "dependencies": { @@ -6885,54 +6885,40 @@ "license": "MIT" }, "node_modules/@chevrotain/cst-dts-gen": { - "version": "11.1.2", - "resolved": "https://registry.npmjs.org/@chevrotain/cst-dts-gen/-/cst-dts-gen-11.1.2.tgz", - "integrity": "sha512-XTsjvDVB5nDZBQB8o0o/0ozNelQtn2KrUVteIHSlPd2VAV2utEb6JzyCJaJ8tGxACR4RiBNWy5uYUHX2eji88Q==", + "version": "12.0.0", + "resolved": "https://registry.npmjs.org/@chevrotain/cst-dts-gen/-/cst-dts-gen-12.0.0.tgz", + "integrity": "sha512-fSL4KXjTl7cDgf0B5Rip9Q05BOrYvkJV/RrBTE/bKDN096E4hN/ySpcBK5B24T76dlQ2i32Zc3PAE27jFnFrKg==", "license": "Apache-2.0", "dependencies": { - "@chevrotain/gast": "11.1.2", - "@chevrotain/types": "11.1.2", - "lodash-es": "4.17.23" + "@chevrotain/gast": "12.0.0", + "@chevrotain/types": "12.0.0" } }, - "node_modules/@chevrotain/cst-dts-gen/node_modules/lodash-es": { - "version": "4.17.23", - "resolved": "https://registry.npmjs.org/lodash-es/-/lodash-es-4.17.23.tgz", - "integrity": "sha512-kVI48u3PZr38HdYz98UmfPnXl2DXrpdctLrFLCd3kOx1xUkOmpFPx7gCWWM5MPkL/fD8zb+Ph0QzjGFs4+hHWg==", - "license": "MIT" - }, "node_modules/@chevrotain/gast": { - "version": "11.1.2", - "resolved": "https://registry.npmjs.org/@chevrotain/gast/-/gast-11.1.2.tgz", - "integrity": "sha512-Z9zfXR5jNZb1Hlsd/p+4XWeUFugrHirq36bKzPWDSIacV+GPSVXdk+ahVWZTwjhNwofAWg/sZg58fyucKSQx5g==", + "version": "12.0.0", + "resolved": "https://registry.npmjs.org/@chevrotain/gast/-/gast-12.0.0.tgz", + "integrity": "sha512-1ne/m3XsIT8aEdrvT33so0GUC+wkctpUPK6zU9IlOyJLUbR0rg4G7ZiApiJbggpgPir9ERy3FRjT6T7lpgetnQ==", "license": "Apache-2.0", "dependencies": { - "@chevrotain/types": "11.1.2", - "lodash-es": "4.17.23" + "@chevrotain/types": "12.0.0" } }, - "node_modules/@chevrotain/gast/node_modules/lodash-es": { - "version": "4.17.23", - "resolved": "https://registry.npmjs.org/lodash-es/-/lodash-es-4.17.23.tgz", - "integrity": "sha512-kVI48u3PZr38HdYz98UmfPnXl2DXrpdctLrFLCd3kOx1xUkOmpFPx7gCWWM5MPkL/fD8zb+Ph0QzjGFs4+hHWg==", - "license": "MIT" - }, "node_modules/@chevrotain/regexp-to-ast": { - "version": "11.1.2", - "resolved": "https://registry.npmjs.org/@chevrotain/regexp-to-ast/-/regexp-to-ast-11.1.2.tgz", - "integrity": "sha512-nMU3Uj8naWer7xpZTYJdxbAs6RIv/dxYzkYU8GSwgUtcAAlzjcPfX1w+RKRcYG8POlzMeayOQ/znfwxEGo5ulw==", + "version": "12.0.0", + "resolved": "https://registry.npmjs.org/@chevrotain/regexp-to-ast/-/regexp-to-ast-12.0.0.tgz", + "integrity": "sha512-p+EW9MaJwgaHguhoqwOtx/FwuGr+DnNn857sXWOi/mClXIkPGl3rn7hGNWvo31HA3vyeQxjqe+H36yZJwYU8cA==", "license": "Apache-2.0" }, "node_modules/@chevrotain/types": { - "version": "11.1.2", - "resolved": "https://registry.npmjs.org/@chevrotain/types/-/types-11.1.2.tgz", - "integrity": "sha512-U+HFai5+zmJCkK86QsaJtoITlboZHBqrVketcO2ROv865xfCMSFpELQoz1GkX5GzME8pTa+3kbKrZHQtI0gdbw==", + "version": "12.0.0", + "resolved": "https://registry.npmjs.org/@chevrotain/types/-/types-12.0.0.tgz", + "integrity": "sha512-S+04vjFQKeuYw0/eW3U52LkAHQsB1ASxsPGsLPUyQgrZ2iNNibQrsidruDzjEX2JYfespXMG0eZmXlhA6z7nWA==", "license": "Apache-2.0" }, "node_modules/@chevrotain/utils": { - "version": "11.1.2", - "resolved": "https://registry.npmjs.org/@chevrotain/utils/-/utils-11.1.2.tgz", - "integrity": "sha512-4mudFAQ6H+MqBTfqLmU7G1ZwRzCLfJEooL/fsF6rCX5eePMbGhoy5n4g+G4vlh2muDcsCTJtL+uKbOzWxs5LHA==", + "version": "12.0.0", + "resolved": "https://registry.npmjs.org/@chevrotain/utils/-/utils-12.0.0.tgz", + "integrity": "sha512-lB59uJoaGIfOOL9knQqQRfhl9g7x8/wqFkp13zTdkRu1huG9kg6IJs1O8hqj9rs6h7orGxHJUKb+mX3rPbWGhA==", "license": "Apache-2.0" }, "node_modules/@codemirror/autocomplete": { @@ -9980,9 +9966,9 @@ } }, "node_modules/@hono/node-server": { - "version": "1.19.10", - "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-1.19.10.tgz", - "integrity": "sha512-hZ7nOssGqRgyV3FVVQdfi+U4q02uB23bpnYpdvNXkYTRRyWx84b7yf1ans+dnJ/7h41sGL3CeQTfO+ZGxuO+Iw==", + "version": "1.19.13", + "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-1.19.13.tgz", + "integrity": "sha512-TsQLe4i2gvoTtrHje625ngThGBySOgSK3Xo2XRYOdqGN1teR8+I7vchQC46uLJi8OF62YTYA3AhSpumtkhsaKQ==", "license": "MIT", "engines": { "node": ">=18.14.1" @@ -23524,37 +23510,33 @@ } }, "node_modules/chevrotain": { - "version": "11.1.2", - "resolved": "https://registry.npmjs.org/chevrotain/-/chevrotain-11.1.2.tgz", - "integrity": "sha512-opLQzEVriiH1uUQ4Kctsd49bRoFDXGGSC4GUqj7pGyxM3RehRhvTlZJc1FL/Flew2p5uwxa1tUDWKzI4wNM8pg==", + "version": "12.0.0", + "resolved": "https://registry.npmjs.org/chevrotain/-/chevrotain-12.0.0.tgz", + "integrity": "sha512-csJvb+6kEiQaqo1woTdSAuOWdN0WTLIydkKrBnS+V5gZz0oqBrp4kQ35519QgK6TpBThiG3V1vNSHlIkv4AglQ==", "license": "Apache-2.0", "dependencies": { - "@chevrotain/cst-dts-gen": "11.1.2", - "@chevrotain/gast": "11.1.2", - "@chevrotain/regexp-to-ast": "11.1.2", - "@chevrotain/types": "11.1.2", - "@chevrotain/utils": "11.1.2", - "lodash-es": "4.17.23" + "@chevrotain/cst-dts-gen": "12.0.0", + "@chevrotain/gast": "12.0.0", + "@chevrotain/regexp-to-ast": "12.0.0", + "@chevrotain/types": "12.0.0", + "@chevrotain/utils": "12.0.0" + }, + "engines": { + "node": ">=22.0.0" } }, "node_modules/chevrotain-allstar": { - "version": "0.3.1", - "resolved": "https://registry.npmjs.org/chevrotain-allstar/-/chevrotain-allstar-0.3.1.tgz", - "integrity": "sha512-b7g+y9A0v4mxCW1qUhf3BSVPg+/NvGErk/dOkrDaHA0nQIQGAtrOjlX//9OQtRlSCy+x9rfB5N8yC71lH1nvMw==", + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/chevrotain-allstar/-/chevrotain-allstar-0.4.1.tgz", + "integrity": "sha512-PvVJm3oGqrveUVW2Vt/eZGeiAIsJszYweUcYwcskg9e+IubNYKKD+rHHem7A6XVO22eDAL+inxNIGAzZ/VIWlA==", "license": "MIT", "dependencies": { "lodash-es": "^4.17.21" }, "peerDependencies": { - "chevrotain": "^11.0.0" + "chevrotain": "^12.0.0" } }, - "node_modules/chevrotain/node_modules/lodash-es": { - "version": "4.17.23", - "resolved": "https://registry.npmjs.org/lodash-es/-/lodash-es-4.17.23.tgz", - "integrity": "sha512-kVI48u3PZr38HdYz98UmfPnXl2DXrpdctLrFLCd3kOx1xUkOmpFPx7gCWWM5MPkL/fD8zb+Ph0QzjGFs4+hHWg==", - "license": "MIT" - }, "node_modules/chokidar": { "version": "3.5.3", "resolved": "https://registry.npmjs.org/chokidar/-/chokidar-3.5.3.tgz", @@ -28641,9 +28623,9 @@ "integrity": "sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ==" }, "node_modules/hono": { - "version": "4.12.7", - "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.7.tgz", - "integrity": "sha512-jq9l1DM0zVIvsm3lv9Nw9nlJnMNPOcAtsbsgiUhWcFzPE99Gvo6yRTlszSLLYacMeQ6quHD6hMfId8crVHvexw==", + "version": "4.12.12", + "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.12.tgz", + "integrity": "sha512-p1JfQMKaceuCbpJKAPKVqyqviZdS0eUxH9v82oWo1kb9xjQ5wA6iP3FNVAPDFlz5/p7d45lO+BpSk1tuSZMF4Q==", "license": "MIT", "engines": { "node": ">=16.9.0" @@ -31608,13 +31590,14 @@ "integrity": "sha512-Xq9nH7KlWZmXAtodXDDRE7vs6DU1gTU8zYDHDiWLSip45Egwq3plLHzPn27NgvzL2r1LMPC1vdqh98sQxtqj4A==" }, "node_modules/langium": { - "version": "4.2.1", - "resolved": "https://registry.npmjs.org/langium/-/langium-4.2.1.tgz", - "integrity": "sha512-zu9QWmjpzJcomzdJQAHgDVhLGq5bLosVak1KVa40NzQHXfqr4eAHupvnPOVXEoLkg6Ocefvf/93d//SB7du4YQ==", + "version": "4.2.2", + "resolved": "https://registry.npmjs.org/langium/-/langium-4.2.2.tgz", + "integrity": "sha512-JUshTRAfHI4/MF9dH2WupvjSXyn8JBuUEWazB8ZVJUtXutT0doDlAv1XKbZ1Pb5sMexa8FF4CFBc0iiul7gbUQ==", "license": "MIT", "dependencies": { - "chevrotain": "~11.1.1", - "chevrotain-allstar": "~0.3.1", + "@chevrotain/regexp-to-ast": "~12.0.0", + "chevrotain": "~12.0.0", + "chevrotain-allstar": "~0.4.1", "vscode-languageserver": "~9.0.1", "vscode-languageserver-textdocument": "~1.0.11", "vscode-uri": "~3.1.0" diff --git a/package.json b/package.json index 25e6da4ab2..58cab93522 100644 --- a/package.json +++ b/package.json @@ -174,7 +174,6 @@ "monaco-editor": { "dompurify": "3.3.2" }, - "serialize-javascript": "^7.0.3", "svgo": "^2.8.2" }, "nodemonConfig": { From d350c586335ce9f4b8501ee71c262fd85b37f20e Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 7 Apr 2026 23:51:23 -0400 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=9A=AB=20fix:=20Hide=20Delete=20Accou?= =?UTF-8?q?nt=20Button=20When=20ALLOW=5FACCOUNT=5FDELETION=20Is=20Disabled?= =?UTF-8?q?=20(#12568)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: hide Delete Account button when ALLOW_ACCOUNT_DELETION is false * fix: add admin bypass, inline env read, and tests for allowAccountDeletion - Show delete button for admin users even when ALLOW_ACCOUNT_DELETION=false, matching the canDeleteAccount middleware's ACCESS_ADMIN bypass - Move env var read inline in buildSharedPayload() for per-request evaluation - Add 4 frontend tests for Account conditional rendering - Add 3 backend tests for allowAccountDeletion config field * fix: use server-side ACCESS_ADMIN capability check instead of frontend role check - Replace frontend SystemRoles.ADMIN check with server-side hasCapability() in the authenticated config route, matching canDeleteAccount middleware exactly - Admin bypass now evaluates ACCESS_ADMIN capability per-user in GET /api/config, so users with the grant (regardless of role) see the button, and admins without the grant do not - Add 3 authenticated backend tests: without capability, with capability, and skip-when-already-enabled - Simplify frontend to pure config check (no role logic) - Remove redundant jest-dom import; add inline env var comment * test: add missing toHaveBeenCalled assertion in ACCESS_ADMIN test --- api/server/routes/__tests__/config.spec.js | 69 +++++++++++++++++++ api/server/routes/config.js | 24 ++++++- .../Nav/SettingsTabs/Account/Account.spec.tsx | 64 +++++++++++++++++ .../Nav/SettingsTabs/Account/Account.tsx | 10 ++- packages/data-provider/src/config.ts | 1 + 5 files changed, 164 insertions(+), 4 deletions(-) create mode 100644 client/src/components/Nav/SettingsTabs/Account/Account.spec.tsx diff --git a/api/server/routes/__tests__/config.spec.js b/api/server/routes/__tests__/config.spec.js index 54315a7798..6acd87ef22 100644 --- a/api/server/routes/__tests__/config.spec.js +++ b/api/server/routes/__tests__/config.spec.js @@ -9,6 +9,11 @@ jest.mock('~/server/services/Config/ldap', () => ({ getLdapConfig: jest.fn(() => null), })); +const mockHasCapability = jest.fn(); +jest.mock('~/server/middleware/roles/capabilities', () => ({ + hasCapability: (...args) => mockHasCapability(...args), +})); + const mockGetTenantId = jest.fn(() => undefined); jest.mock('@librechat/data-schemas', () => ({ ...jest.requireActual('@librechat/data-schemas'), @@ -76,6 +81,7 @@ afterEach(() => { delete process.env.SAML_ISSUER; delete process.env.SAML_CERT; delete process.env.SAML_SESSION_SECRET; + delete process.env.ALLOW_ACCOUNT_DELETION; }); describe('GET /api/config', () => { @@ -181,6 +187,35 @@ describe('GET /api/config', () => { expect(response.body).toHaveProperty('serverDomain'); }); + it('should default allowAccountDeletion to true when env var is unset', async () => { + mockGetAppConfig.mockResolvedValue(baseAppConfig); + const app = createApp(null); + + const response = await request(app).get('/api/config'); + + expect(response.body.allowAccountDeletion).toBe(true); + }); + + it('should set allowAccountDeletion to false when ALLOW_ACCOUNT_DELETION=false', async () => { + process.env.ALLOW_ACCOUNT_DELETION = 'false'; + mockGetAppConfig.mockResolvedValue(baseAppConfig); + const app = createApp(null); + + const response = await request(app).get('/api/config'); + + expect(response.body.allowAccountDeletion).toBe(false); + }); + + it('should set allowAccountDeletion to true when ALLOW_ACCOUNT_DELETION=true', async () => { + process.env.ALLOW_ACCOUNT_DELETION = 'true'; + mockGetAppConfig.mockResolvedValue(baseAppConfig); + const app = createApp(null); + + const response = await request(app).get('/api/config'); + + expect(response.body.allowAccountDeletion).toBe(true); + }); + it('should return 500 when getAppConfig throws', async () => { mockGetAppConfig.mockRejectedValue(new Error('Config service failure')); const app = createApp(null); @@ -277,6 +312,40 @@ describe('GET /api/config', () => { ); }); + it('should set allowAccountDeletion to false for authenticated users without ACCESS_ADMIN', async () => { + process.env.ALLOW_ACCOUNT_DELETION = 'false'; + mockGetAppConfig.mockResolvedValue(baseAppConfig); + mockHasCapability.mockResolvedValue(false); + const app = createApp(mockUser); + + const response = await request(app).get('/api/config'); + + expect(response.body.allowAccountDeletion).toBe(false); + expect(mockHasCapability).toHaveBeenCalled(); + }); + + it('should override allowAccountDeletion to true for users with ACCESS_ADMIN capability', async () => { + process.env.ALLOW_ACCOUNT_DELETION = 'false'; + mockGetAppConfig.mockResolvedValue(baseAppConfig); + mockHasCapability.mockResolvedValue(true); + const app = createApp(mockUser); + + const response = await request(app).get('/api/config'); + + expect(response.body.allowAccountDeletion).toBe(true); + expect(mockHasCapability).toHaveBeenCalled(); + }); + + it('should not call hasCapability when allowAccountDeletion is already true', async () => { + mockGetAppConfig.mockResolvedValue(baseAppConfig); + const app = createApp(mockUser); + + const response = await request(app).get('/api/config'); + + expect(response.body.allowAccountDeletion).toBe(true); + expect(mockHasCapability).not.toHaveBeenCalled(); + }); + it('should return 500 when getAppConfig throws', async () => { mockGetAppConfig.mockRejectedValue(new Error('Config service failure')); const app = createApp(mockUser); diff --git a/api/server/routes/config.js b/api/server/routes/config.js index a57e4bd958..aaa06a5ee0 100644 --- a/api/server/routes/config.js +++ b/api/server/routes/config.js @@ -1,7 +1,8 @@ const express = require('express'); const { isEnabled, getBalanceConfig } = require('@librechat/api'); const { defaultSocialLogins } = require('librechat-data-provider'); -const { logger, getTenantId } = require('@librechat/data-schemas'); +const { logger, getTenantId, SystemCapabilities } = require('@librechat/data-schemas'); +const { hasCapability } = require('~/server/middleware/roles/capabilities'); const { getLdapConfig } = require('~/server/services/Config/ldap'); const { getAppConfig } = require('~/server/services/Config/app'); @@ -77,6 +78,10 @@ function buildSharedPayload() { publicSharedLinksEnabled, analyticsGtmId: process.env.ANALYTICS_GTM_ID, openidReuseTokens, + /** Read inline (not module-level) for per-request evaluation and test isolation */ + allowAccountDeletion: + process.env.ALLOW_ACCOUNT_DELETION === undefined || + isEnabled(process.env.ALLOW_ACCOUNT_DELETION), }; const minPasswordLength = parseInt(process.env.MIN_PASSWORD_LENGTH, 10); @@ -172,6 +177,23 @@ router.get('/', async function (req, res) { payload.webSearch = webSearch; } + if (!payload.allowAccountDeletion) { + try { + const userId = req.user.id ?? req.user._id?.toString(); + if (userId) { + const canDelete = await hasCapability( + { id: userId, role: req.user.role ?? '', tenantId: req.user.tenantId }, + SystemCapabilities.ACCESS_ADMIN, + ); + if (canDelete) { + payload.allowAccountDeletion = true; + } + } + } catch (err) { + logger.warn(`[config] ACCESS_ADMIN capability check failed: ${err.message}`); + } + } + return res.status(200).send(payload); } catch (err) { logger.error('Error in startup config', err); diff --git a/client/src/components/Nav/SettingsTabs/Account/Account.spec.tsx b/client/src/components/Nav/SettingsTabs/Account/Account.spec.tsx new file mode 100644 index 0000000000..f87a01431b --- /dev/null +++ b/client/src/components/Nav/SettingsTabs/Account/Account.spec.tsx @@ -0,0 +1,64 @@ +import React from 'react'; +import { SystemRoles } from 'librechat-data-provider'; +import { render, screen } from '@testing-library/react'; +import type { TUser } from 'librechat-data-provider'; +import Account from './Account'; + +jest.mock('./DisplayUsernameMessages', () => () =>
); +jest.mock('./Avatar', () => () =>
); +jest.mock('./TwoFactorAuthentication', () => () =>
); +jest.mock('./BackupCodesItem', () => () =>
); +jest.mock('./DeleteAccount', () => () =>
); + +const mockUseAuthContext = jest.fn(); +const mockUseGetStartupConfig = jest.fn(); + +jest.mock('~/hooks', () => ({ + useAuthContext: () => mockUseAuthContext(), +})); + +jest.mock('~/data-provider', () => ({ + useGetStartupConfig: () => mockUseGetStartupConfig(), +})); + +const baseUser: TUser = { + id: 'user-123', + username: 'testuser', + email: 'test@example.com', + name: 'Test User', + avatar: '', + role: SystemRoles.USER, + provider: 'local', + createdAt: '2023-01-01T00:00:00.000Z', + updatedAt: '2023-01-01T00:00:00.000Z', +}; + +beforeEach(() => { + mockUseAuthContext.mockReturnValue({ user: baseUser }); + mockUseGetStartupConfig.mockReturnValue({ data: { allowAccountDeletion: true } }); +}); + +afterEach(() => { + jest.resetAllMocks(); +}); + +describe('Account', () => { + describe('DeleteAccount visibility', () => { + it('renders DeleteAccount when allowAccountDeletion is true', () => { + render(); + expect(screen.getByTestId('delete-account')).toBeInTheDocument(); + }); + + it('hides DeleteAccount when allowAccountDeletion is false', () => { + mockUseGetStartupConfig.mockReturnValue({ data: { allowAccountDeletion: false } }); + render(); + expect(screen.queryByTestId('delete-account')).not.toBeInTheDocument(); + }); + + it('shows DeleteAccount when startup config is still loading', () => { + mockUseGetStartupConfig.mockReturnValue({ data: undefined }); + render(); + expect(screen.getByTestId('delete-account')).toBeInTheDocument(); + }); + }); +}); diff --git a/client/src/components/Nav/SettingsTabs/Account/Account.tsx b/client/src/components/Nav/SettingsTabs/Account/Account.tsx index 27f442f96c..e19f42f4dc 100644 --- a/client/src/components/Nav/SettingsTabs/Account/Account.tsx +++ b/client/src/components/Nav/SettingsTabs/Account/Account.tsx @@ -4,10 +4,12 @@ import DeleteAccount from './DeleteAccount'; import Avatar from './Avatar'; import EnableTwoFactorItem from './TwoFactorAuthentication'; import BackupCodesItem from './BackupCodesItem'; +import { useGetStartupConfig } from '~/data-provider'; import { useAuthContext } from '~/hooks'; function Account() { const { user } = useAuthContext(); + const { data: startupConfig } = useGetStartupConfig(); return (
@@ -29,9 +31,11 @@ function Account() { )} )} -
- -
+ {startupConfig?.allowAccountDeletion !== false && ( +
+ +
+ )}
); } diff --git a/packages/data-provider/src/config.ts b/packages/data-provider/src/config.ts index ca40ec2c8c..cfd71ccfdd 100644 --- a/packages/data-provider/src/config.ts +++ b/packages/data-provider/src/config.ts @@ -859,6 +859,7 @@ export type TStartupConfig = { sharePointPickerGraphScope?: string; sharePointPickerSharePointScope?: string; openidReuseTokens?: boolean; + allowAccountDeletion: boolean; minPasswordLength?: number; webSearch?: { searchProvider?: SearchProviders;