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 408a47e9699..9633543edf4 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/experimental.js b/api/server/experimental.js index 102c6bbfedd..ba2a0630144 100644 --- a/api/server/experimental.js +++ b/api/server/experimental.js @@ -19,7 +19,7 @@ const mongoose = require('mongoose'); 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, @@ -470,6 +470,11 @@ if (cluster.isMaster) { logger.info(`Worker ${process.pid}: Connected to MongoDB`); startCodeEnvironmentLifecycleReconciler({ mongoose }); + /** 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); @@ -634,6 +639,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/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 abe310b1eca..ab268553cad 100644 --- a/api/server/experimental.spec.js +++ b/api/server/experimental.spec.js @@ -1,8 +1,17 @@ const fs = require('fs'); +const vm = require('vm'); 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.each([ + ['standard', standardSource], + ['experimental', source], + ])('parses the %s server without duplicate declarations', (_name, serverSource) => { + expect(() => new vm.Script(serverSource)).not.toThrow(); + }); it('configures HTTP timeouts for each cluster worker server', () => { const listenIndex = source.indexOf('const server = app.listen'); @@ -95,12 +104,53 @@ 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', routes.adminAuth);", + ); + const experimentalAdminTenantIndex = source.indexOf( + "app.use('/api/admin', preAuthTenantMiddleware);", + ); + const experimentalFirstAdminRouteIndex = source.indexOf( + "app.use('/api/admin', routes.adminAuth);", + ); + + 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);", ); expect(source).toContain("app.use('/api/share', preAuthTenantMiddleware, routes.share);"); }); + + it.each([ + ['standard', standardSource], + ['experimental', source], + ])('keeps Insights on the non-admin route in the %s server', (_name, serverSource) => { + expect(serverSource).toContain("app.use('/api/insights', routes.insights);"); + expect(serverSource).not.toContain("app.use('/api/admin/insights'"); + }); }); diff --git a/api/server/index.js b/api/server/index.js index 046004252da..7f03a241d94 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -12,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, @@ -190,6 +190,7 @@ const startServer = async () => { axios.defaults.headers.common['Accept-Encoding'] = 'gzip'; } await connectDb(); + await ensureConfigIndexes(mongoose); logger.info('Connected to MongoDB'); startCodeEnvironmentLifecycleReconciler({ mongoose }); @@ -375,11 +376,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/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 529879ce9ae..ee914831fee 100644 --- a/api/server/routes/__tests__/config.spec.js +++ b/api/server/routes/__tests__/config.spec.js @@ -324,13 +324,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 9333495280d..21ac8c1bff7 100644 --- a/api/server/routes/admin/config.js +++ b/api/server/routes/admin/config.js @@ -27,6 +27,8 @@ const handlers = createAdminConfigHandlers({ toggleConfigActive: db.toggleConfigActive, hasAnyConfigReadAccess, getReadableConfigSections, + mutateConfigWithRevision: db.mutateConfigWithRevision, + listConfigRevisions: db.listConfigRevisions, hasConfigCapability, hasCapability, getAppConfig, @@ -37,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); @@ -44,5 +47,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/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() {