diff --git a/.env.example b/.env.example index b78fe8363cb..408a47e9699 100644 --- a/.env.example +++ b/.env.example @@ -326,19 +326,28 @@ NODE_MAX_OLD_SPACE_SIZE=6144 # LANGFUSE_FANOUT_BATCH_SEND_SIZE=128 # LANGFUSE_FANOUT_METADATA_CARDINALITY_LIMIT=1000 -#=======================# -# OpenTelemetry Tracing # -#=======================# +#===============# +# OpenTelemetry # +#===============# # Enables backend OpenTelemetry tracing. General backend visibility only; # use Langfuse for GenAI-specific prompt/model observability. # OTEL_TRACING_ENABLED=false +# Exports application logs as OpenTelemetry log records, correlated with the +# active trace. Logs below OTEL_LOGS_LEVEL are not exported. +# OTEL_LOGS_ENABLED=false +# OTEL_LOGS_LEVEL=info # OTEL_SERVICE_NAME=librechat # OTEL_SERVICE_VERSION= +# Exporter protocol for every signal: http/protobuf (default, port 4318) or grpc (port 4317). +# Per-signal overrides: OTEL_EXPORTER_OTLP_TRACES_PROTOCOL, OTEL_EXPORTER_OTLP_LOGS_PROTOCOL. +# OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf # OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318 # OTEL_EXPORTER_OTLP_TRACES_ENDPOINT= +# OTEL_EXPORTER_OTLP_LOGS_ENDPOINT= # OTEL_EXPORTER_OTLP_HEADERS= # OTEL_TRACES_EXPORTER=otlp +# OTEL_LOGS_EXPORTER=otlp # OTEL_TRACES_SAMPLER=parentbased_always_on # OTEL_LOG_LEVEL=INFO # OTEL_SDK_DISABLED=false @@ -578,7 +587,7 @@ GOOGLE_KEY=user_provided #============# OPENAI_API_KEY=user_provided -# OPENAI_MODELS=gpt-5.6,gpt-5.6-terra,gpt-5.6-luna,gpt-5.5,gpt-5.5-pro,chat-latest,gpt-5.4,gpt-5.4-pro,gpt-5.4-mini,gpt-5.4-nano,gpt-5.3-codex,gpt-5.2,gpt-5,gpt-5-codex,gpt-5-mini,gpt-5-nano,o3-pro,o3,o4-mini,gpt-4.1,gpt-4.1-mini,gpt-4.1-nano,o3-mini,o1-pro,o1,gpt-4o,gpt-4o-mini +# OPENAI_MODELS=gpt-6-astra,gpt-5.6,gpt-5.6-terra,gpt-5.6-luna,gpt-5.5,gpt-5.5-pro,chat-latest,gpt-5.4,gpt-5.4-pro,gpt-5.4-mini,gpt-5.4-nano,gpt-5.3-codex,gpt-5.2,gpt-5,gpt-5-codex,gpt-5-mini,gpt-5-nano,o3-pro,o3,o4-mini,gpt-4.1,gpt-4.1-mini,gpt-4.1-nano,o3-mini,o1-pro,o1,gpt-4o,gpt-4o-mini DEBUG_OPENAI=false @@ -805,7 +814,15 @@ TTS_VIOLATION_SCORE=0 STT_VIOLATION_SCORE=0 FORK_VIOLATION_SCORE=0 IMPORT_VIOLATION_SCORE=0 +SHARE_VIOLATION_SCORE=0 FILE_UPLOAD_VIOLATION_SCORE=0 +# Shared link retrieval re-inspects the whole shared snapshot on every request +# and is reachable without authentication (defaults: 100 per IP and 60 per user, +# both per minute). +# SHARE_IP_MAX=100 +# SHARE_IP_WINDOW=1 +# SHARE_USER_MAX=60 +# SHARE_USER_WINDOW=1 # Per-user limiter for metadata-only /files/usage requests that renew the TTL # of attachments waiting in queued Agent messages (default: 120 per 15 minutes). # FILE_USAGE_USER_MAX=120 @@ -1258,6 +1275,18 @@ HELP_AND_FAQ_URL=https://librechat.ai # When unset or 0, no pinging is performed (recommended for most use cases) # REDIS_PING_INTERVAL=300 +# Milliseconds a heartbeat PING may go unanswered before the socket is presumed dead and +# reconnected (default: 5000). Applies to REDIS_PING_INTERVAL and the subscriber heartbeat. +# REDIS_PING_TIMEOUT=5000 + +# Heartbeat interval in seconds for dedicated pub/sub subscriber connections (default: 15, 0 = disabled) +# Subscribers carry no traffic between generations, so a peer that vanished without closing the +# socket would otherwise go unnoticed until the kernel gives up (about 15 minutes at Linux defaults) +# REDIS_SUBSCRIBER_PING_INTERVAL=15 + +# TCP keepalive idle delay in milliseconds for ioredis sockets (default: 10000, 0 = kernel default) +# REDIS_KEEP_ALIVE=10000 + # Force specific cache namespaces to use in-memory storage even when Redis is enabled # Comma-separated list of CacheKeys # Defaults to CONFIG_STORE,APP_CONFIG so YAML-derived config stays per-container (safe for blue/green deployments) diff --git a/CONTEXT.md b/CONTEXT.md index db384f8b3e9..fbc2f1085c8 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -1,5 +1,6 @@ # Domain language +- **Attached code environment**: A principal- or deployment-authorized stateful workspace owned by an outbound `librechat-code` worker on a user-chosen machine or VM. LibreChat selects it and enforces approval policy, Code API authenticates and dispatches to it, and the worker's local sandbox and capability flags remain the final execution ceiling. The environment interface is runtime-neutral: native SRT, WSL2, Docker/NsJail, and future adapters expose the same workspace operations without leaking host paths or runtime configuration into agent tools. - **Agent run envelope**: the versioned, JSON-safe request contract created after ingress authentication and protocol validation but before agent, provider, tool, or MCP initialization. It carries only the validated protocol payload and the minimum trusted principal identifiers. The execution host rehydrates all runtime state from those identifiers. - **Agent execution context**: runtime-only, transport-free state rehydrated beside an Agent run envelope. It contains the authenticated user, application configuration, normalized request metadata, and resolved conversation facts needed by initialization, but never Express request/response objects or serialized credentials. - **Agent execution host**: the protocol-neutral module that owns run admission, disconnect cancellation, provider-start fencing, and terminal settlement. Protocol implementations execute behind its callback interface; HTTP adapters retain validation and final stream rendering. diff --git a/api/cache/getLogStores.js b/api/cache/getLogStores.js index 487f67b421d..72e36bc09e9 100644 --- a/api/cache/getLogStores.js +++ b/api/cache/getLogStores.js @@ -30,6 +30,7 @@ const namespaces = { [ViolationTypes.TTS_LIMIT]: violationCache(ViolationTypes.TTS_LIMIT), [ViolationTypes.STT_LIMIT]: violationCache(ViolationTypes.STT_LIMIT), [ViolationTypes.CONVO_ACCESS]: violationCache(ViolationTypes.CONVO_ACCESS), + [ViolationTypes.SHARE_LIMIT]: violationCache(ViolationTypes.SHARE_LIMIT), [ViolationTypes.TOOL_CALL_LIMIT]: violationCache(ViolationTypes.TOOL_CALL_LIMIT), [ViolationTypes.FILE_UPLOAD_LIMIT]: violationCache(ViolationTypes.FILE_UPLOAD_LIMIT), [ViolationTypes.VERIFY_EMAIL_LIMIT]: violationCache(ViolationTypes.VERIFY_EMAIL_LIMIT), diff --git a/api/jest.config.js b/api/jest.config.js index daa12004d67..f9f03da125a 100644 --- a/api/jest.config.js +++ b/api/jest.config.js @@ -7,6 +7,13 @@ const esModules = [ '@langchain/langgraph-sdk', '@mistralai/mistralai', 'uuid', + 'sanitize-html', + 'htmlparser2', + 'domhandler', + 'domelementtype', + 'domutils', + 'dom-serializer', + 'entities', ].join('|'); module.exports = { diff --git a/api/package.json b/api/package.json index 61bbc73bcd0..a8eba00ab72 100644 --- a/api/package.json +++ b/api/package.json @@ -46,7 +46,7 @@ "@azure/storage-blob": "^12.30.0", "@google/genai": "^2.8.0", "@keyv/redis": "5.1.6", - "@librechat/agents": "^3.7.17", + "@librechat/agents": "^3.7.21", "@librechat/api": "*", "@librechat/data-schemas": "*", "@microsoft/microsoft-graph-client": "^3.0.7", @@ -62,6 +62,7 @@ "@opentelemetry/resources": "^2.6.1", "@opentelemetry/sdk-node": "^0.221.0", "@opentelemetry/semantic-conventions": "^1.39.0", + "@opentelemetry/winston-transport": "^0.28.0", "@redis/client": "5.10.0", "@smithy/node-http-handler": "^4.4.5", "ai-tokenizer": "^1.0.6", @@ -73,6 +74,7 @@ "cookie-parser": "^1.4.7", "cors": "^2.8.5", "dedent": "^1.5.3", + "dompurify": "^3.4.12", "dotenv": "^16.0.3", "eventsource": "^3.0.2", "express": "^5.2.1", @@ -88,6 +90,7 @@ "https-proxy-agent": "^7.0.6", "ioredis": "^5.3.2", "js-yaml": "^4.2.0", + "jsdom": "^26.1.0", "jsonwebtoken": "^9.0.0", "jszip": "^3.10.1", "jwks-rsa": "^3.2.0", diff --git a/api/server/controllers/__tests__/tools.callTool.contentFilter.spec.js b/api/server/controllers/__tests__/tools.callTool.contentFilter.spec.js index 0565c265955..6eb4cac8f50 100644 --- a/api/server/controllers/__tests__/tools.callTool.contentFilter.spec.js +++ b/api/server/controllers/__tests__/tools.callTool.contentFilter.spec.js @@ -523,8 +523,12 @@ describe('callTool generated-content protection', () => { expect(mockProcessCodeOutput).toHaveBeenCalledWith( expect.objectContaining({ downloadFallback: true, preparedBuffer: undefined }), ); + /* The cause rides the warning so the failure is diagnosable at all, but + * only through `getSafeErrorMetadata` — the upstream message here is + * `PRIVATE-DOWNLOAD-ERROR`, and that is precisely what must not appear. */ expect(mockLogger.warn).toHaveBeenCalledWith( '[preflightCodeOutputBatch] Generated artifact 1 could not be inspected', + { type: 'Error' }, ); expect(JSON.stringify(mockLogger.warn.mock.calls)).not.toContain('PRIVATE-'); expect(res.status).toHaveBeenCalledWith(200); diff --git a/api/server/controllers/agents/__tests__/resume.spec.js b/api/server/controllers/agents/__tests__/resume.spec.js index fcf040f763c..79f31815c80 100644 --- a/api/server/controllers/agents/__tests__/resume.spec.js +++ b/api/server/controllers/agents/__tests__/resume.spec.js @@ -1530,7 +1530,7 @@ describe('ResumeAgentController (POST /agents/chat/resume)', () => { const res = await post(approveBody()); expect(res.status).toBe(400); - expect(mockGetActions).toHaveBeenCalledWith({ agent_id: { $in: [AGENT_ID] } }, false); + expect(mockGetActions).toHaveBeenCalledWith({ agentId: [AGENT_ID] }, false); expect(mockDecryptMetadata).not.toHaveBeenCalled(); expect(mockGenerationJobManager.approvals.resolve).not.toHaveBeenCalled(); expect(mockInitializeClient).not.toHaveBeenCalled(); diff --git a/api/server/controllers/agents/__tests__/v1.spec.js b/api/server/controllers/agents/__tests__/v1.spec.js index be2e4553f73..0b2b7722ae6 100644 --- a/api/server/controllers/agents/__tests__/v1.spec.js +++ b/api/server/controllers/agents/__tests__/v1.spec.js @@ -68,7 +68,7 @@ describe('duplicateAgent', () => { await duplicateAgent(req, res); expect(getAgent).toHaveBeenCalledWith({ id: 'agent_123' }); - expect(getActions).toHaveBeenCalledWith({ agent_id: 'agent_123' }, true); + expect(getActions).toHaveBeenCalledWith({ agentId: 'agent_123' }, true); expect(createAgent).toHaveBeenCalledWith( expect.objectContaining({ id: 'agent_new_123', diff --git a/api/server/controllers/agents/v1.js b/api/server/controllers/agents/v1.js index 20c1478697d..cd455dc67b4 100644 --- a/api/server/controllers/agents/v1.js +++ b/api/server/controllers/agents/v1.js @@ -1465,7 +1465,7 @@ const duplicateAgentHandler = async (req, res) => { return res.status(subagentReferenceError.status).json(subagentReferenceError.body); } - const originalActions = (await db.getActions({ agent_id: id }, true)) ?? []; + const originalActions = (await db.getActions({ agentId: id }, true)) ?? []; const sanitizedActions = originalActions.map((action) => { const metadata = { ...(action.metadata || {}) }; for (const field of sensitiveFields) { @@ -1550,7 +1550,7 @@ const duplicateAgentHandler = async (req, res) => { const fullActionId = `${domain}${actionDelimiter}${newActionId}`; const newAction = await db.updateAction( - { action_id: newActionId, agent_id: newAgentId }, + { actionId: newActionId, agentId: newAgentId }, { metadata: action.metadata, agent_id: newAgentId, @@ -1575,7 +1575,7 @@ const duplicateAgentHandler = async (req, res) => { try { newAgent = await db.createAgent(newAgentData); } catch (error) { - await db.deleteActions({ agent_id: newAgentId, user: userId }).catch((cleanupError) => { + await db.deleteActions({ agentId: newAgentId, user: userId }).catch((cleanupError) => { logger.error( '[/agents/:id/duplicate] Failed to clean up cloned Actions after Agent creation failed:', cleanupError, @@ -2065,7 +2065,7 @@ const revertAgentVersionHandler = async (req, res) => { .filter(Boolean); const actions = actionIds.length > 0 - ? ((await db.getActions({ agent_id: id, action_id: { $in: actionIds } }, true)) ?? []) + ? ((await db.getActions({ agentId: id, actionId: actionIds }, true)) ?? []) : []; if ( diff --git a/api/server/controllers/agents/v1.spec.js b/api/server/controllers/agents/v1.spec.js index f3e3131d47c..7fc941f83ca 100644 --- a/api/server/controllers/agents/v1.spec.js +++ b/api/server/controllers/agents/v1.spec.js @@ -1,12 +1,16 @@ const mongoose = require('mongoose'); +const express = require('express'); +const request = require('supertest'); const { nanoid } = require('nanoid'); const { v4: uuidv4 } = require('uuid'); -const { agentSchema, aclEntrySchema, fileSchema, userSchema } = require('@librechat/data-schemas'); +const { createModels, tenantStorage } = require('@librechat/data-schemas'); const { Tools, SkillsScope, FileSources, + Permissions, PermissionBits, + PermissionTypes, PrincipalModel, PrincipalType, ResourceType, @@ -98,9 +102,16 @@ const { const { CONTENT_TRAVERSAL_MAX_DEPTH, + createAgentManagementAuth, + createAgentManagementCreateHandler, + createAgentManagementDeleteHandler, + createAgentManagementReadHandlers, + createAgentManagementUpdateHandler, mergeDeploymentSkillIds, refreshS3Url, } = require('@librechat/api'); +const { grantPermission } = require('~/server/services/PermissionService'); +const db = require('~/models'); /** * @type {import('mongoose').Model} @@ -153,12 +164,10 @@ describe('Agent Controllers - Mass Assignment Protection', () => { mongoServer = await MongoMemoryServer.create(); const mongoUri = mongoServer.getUri(); await mongoose.connect(mongoUri); - Agent = mongoose.models.Agent || mongoose.model('Agent', agentSchema); - AclEntry = mongoose.models.AclEntry || mongoose.model('AclEntry', aclEntrySchema); - User = mongoose.models.User || mongoose.model('User', userSchema); - // Register File so orphan-pruning tests (and the tool_resources validation - // test, which now needs real File docs for its ids) have a working model. - mongoose.models.File || mongoose.model('File', fileSchema); + createModels(mongoose); + Agent = mongoose.models.Agent; + AclEntry = mongoose.models.AclEntry; + User = mongoose.models.User; }, 20000); afterAll(async () => { @@ -373,6 +382,328 @@ describe('Agent Controllers - Mass Assignment Protection', () => { expect(agentInDb.author.toString()).toBe(mockReq.user.id); }); + test('management creation can be updated and read through the authenticated management API', async () => { + const tenantId = `tenant-${nanoid(8)}`; + const clientId = `client-${nanoid(8)}`; + const principal = await tenantStorage.run({ tenantId }, () => createOwner()); + const userId = principal._id.toString(); + const getRoleByName = jest.fn().mockResolvedValue({ + permissions: { + [PermissionTypes.AGENTS]: { + [Permissions.USE]: true, + [Permissions.CREATE]: true, + }, + }, + }); + grantPermission.mockImplementation( + async ({ principalType, principalId, resourceType, resourceId, grantedBy }) => + AclEntry.create({ + principalType, + principalModel: PrincipalModel.USER, + principalId, + resourceType, + resourceId, + permBits: OWNER_PERMISSION_BITS, + grantedBy, + grantedAt: new Date(), + }), + ); + const auth = createAgentManagementAuth({ + findUser: db.findUser, + isPrincipalActive: jest.fn().mockResolvedValue(true), + getAppConfig: jest.fn().mockResolvedValue({ + endpoints: { + agents: { + managementApi: { + auth: { + oidc: { + enabled: true, + audience: 'agent-management', + issuer: 'https://issuer.example.com/', + }, + clients: [{ clientId, tenantId, userId }], + }, + }, + }, + }, + }), + verifyAccessToken: jest.fn().mockResolvedValue({ + azp: clientId, + sub: `${clientId}@clients`, + exp: Math.floor(Date.now() / 1000) + 300, + }), + }); + const create = createAgentManagementCreateHandler({ + getRoleByName, + createAgent: createAgentHandler, + }); + const checkAgentPermission = async ({ + userId: accessibleUserId, + resourceType, + resourceId, + requiredPermission, + }) => + (await AclEntry.exists({ + principalId: accessibleUserId, + resourceType, + resourceId, + permBits: { $bitsAllSet: requiredPermission }, + })) != null; + const hasCapability = jest.fn().mockResolvedValue(false); + const reads = createAgentManagementReadHandlers({ + getRoleByName, + getAgentWithVersionCount: db.getAgentWithVersionCount, + getAgentManagementListByAccess: db.getAgentManagementListByAccess, + findAccessibleResources: async ({ userId: accessibleUserId, resourceType }) => + AclEntry.distinct('resourceId', { + principalId: accessibleUserId, + resourceType, + permBits: { $bitsAllSet: PermissionBits.EDIT }, + }), + checkPermission: checkAgentPermission, + hasCapability, + }); + const update = createAgentManagementUpdateHandler({ + getRoleByName, + getAgentWithVersionCount: db.getAgentWithVersionCount, + checkPermission: checkAgentPermission, + hasCapability, + updateAgent: updateAgentHandler, + }); + const remove = createAgentManagementDeleteHandler({ + getRoleByName, + getAgentWithVersionCount: db.getAgentWithVersionCount, + checkPermission: checkAgentPermission, + hasCapability, + deleteAgent: db.deleteAgent, + }); + const app = express(); + app.use(express.json()); + app.use('/api/agents/v1/agents', auth); + app.post('/api/agents/v1/agents', (req, res) => { + req.config = {}; + return create(req, res); + }); + app.get('/api/agents/v1/agents', reads.list); + app.get('/api/agents/v1/agents/:id', reads.get); + app.patch('/api/agents/v1/agents/:id', (req, res) => { + req.config = {}; + return update(req, res); + }); + app.delete('/api/agents/v1/agents/:id', remove); + + const createdResponse = await request(app) + .post('/api/agents/v1/agents') + .set('Authorization', 'Bearer valid-token') + .send({ + name: 'Managed Agent', + description: 'Description that remains unchanged', + provider: 'openai', + model: 'gpt-4', + avatar: { filepath: 'avatars/managed.png', source: 'local' }, + conversation_starters: ['Help me get started'], + }); + + expect(createdResponse.status).toBe(201); + expect(createdResponse.body).toEqual( + expect.objectContaining({ + id: expect.stringMatching(/^agent_/), + name: 'Managed Agent', + provider: 'openai', + model: 'gpt-4', + version: 1, + }), + ); + expect(createdResponse.body).not.toHaveProperty('_id'); + expect(createdResponse.body).not.toHaveProperty('author'); + expect(createdResponse.body).not.toHaveProperty('tenantId'); + + const [retrievedResponse, listedResponse] = await Promise.all([ + request(app) + .get(`/api/agents/v1/agents/${createdResponse.body.id}`) + .set('Authorization', 'Bearer valid-token'), + request(app).get('/api/agents/v1/agents').set('Authorization', 'Bearer valid-token'), + ]); + + expect(retrievedResponse.status).toBe(200); + expect(retrievedResponse.body).toEqual(createdResponse.body); + expect(listedResponse.status).toBe(200); + expect(listedResponse.body.data).toEqual([createdResponse.body]); + + const otherTenantId = `tenant-${nanoid(8)}`; + const currentTenantGraphId = `agent_${nanoid()}`; + const otherTenantGraphId = `agent_${nanoid()}`; + await tenantStorage.run({ tenantId }, async () => { + await Agent.create({ + id: currentTenantGraphId, + name: 'Current tenant graph', + provider: 'openai', + model: 'gpt-4', + author: principal._id, + edges: [{ from: '', to: createdResponse.body.id, edgeType: 'handoff' }], + }); + await User.updateOne( + { _id: principal._id }, + { $set: { favorites: [{ agentId: createdResponse.body.id }] } }, + ); + }); + const otherTenantPrincipal = await tenantStorage.run( + { tenantId: otherTenantId }, + async () => { + const owner = await createOwner(); + await Agent.create({ + id: createdResponse.body.id, + name: 'Other tenant Agent', + provider: 'openai', + model: 'gpt-4', + author: owner._id, + }); + await Agent.create({ + id: otherTenantGraphId, + name: 'Other tenant graph', + provider: 'openai', + model: 'gpt-4', + author: owner._id, + edges: [{ from: '', to: createdResponse.body.id, edgeType: 'handoff' }], + }); + await User.updateOne( + { _id: owner._id }, + { $set: { favorites: [{ agentId: createdResponse.body.id }] } }, + ); + return owner; + }, + ); + + const updatedResponse = await request(app) + .patch(`/api/agents/v1/agents/${createdResponse.body.id}`) + .set('Authorization', 'Bearer valid-token') + .send({ + name: 'Updated Managed Agent', + avatar: null, + conversation_starters: [], + }); + + expect(updatedResponse.status).toBe(200); + expect(updatedResponse.body).toEqual( + expect.objectContaining({ + id: createdResponse.body.id, + name: 'Updated Managed Agent', + description: 'Description that remains unchanged', + avatar: null, + conversation_starters: [], + version: 2, + }), + ); + expect(updatedResponse.body).not.toHaveProperty('_id'); + expect(updatedResponse.body).not.toHaveProperty('author'); + expect(updatedResponse.body).not.toHaveProperty('tenantId'); + + const retrievedAfterUpdate = await request(app) + .get(`/api/agents/v1/agents/${createdResponse.body.id}`) + .set('Authorization', 'Bearer valid-token'); + expect(retrievedAfterUpdate.status).toBe(200); + expect(retrievedAfterUpdate.body).toEqual(updatedResponse.body); + + const created = await Agent.collection.findOne({ + id: createdResponse.body.id, + tenantId, + }); + const otherTenantAgent = await Agent.collection.findOne({ + id: createdResponse.body.id, + tenantId: otherTenantId, + }); + expect(created.tenantId).toBe(tenantId); + expect(created.author.toString()).toBe(userId); + expect(created.name).toBe('Updated Managed Agent'); + expect(created.description).toBe('Description that remains unchanged'); + expect(created.avatar).toBeNull(); + expect(created.conversation_starters).toEqual([]); + expect(created.versions).toHaveLength(2); + expect(otherTenantAgent.name).toBe('Other tenant Agent'); + expect(grantPermission).toHaveBeenCalledWith( + expect.objectContaining({ + principalType: PrincipalType.USER, + principalId: userId, + resourceType: ResourceType.AGENT, + }), + ); + + const deletedResponse = await request(app) + .delete(`/api/agents/v1/agents/${createdResponse.body.id}`) + .set('Authorization', 'Bearer valid-token'); + expect(deletedResponse.status).toBe(200); + expect(deletedResponse.body).toEqual({ id: createdResponse.body.id, deleted: true }); + + const [retrievedAfterDelete, repeatedDelete] = await Promise.all([ + request(app) + .get(`/api/agents/v1/agents/${createdResponse.body.id}`) + .set('Authorization', 'Bearer valid-token'), + request(app) + .delete(`/api/agents/v1/agents/${createdResponse.body.id}`) + .set('Authorization', 'Bearer valid-token'), + ]); + expect(retrievedAfterDelete.status).toBe(404); + expect(repeatedDelete.status).toBe(404); + await expect(Agent.exists({ id: createdResponse.body.id, tenantId })).resolves.toBeNull(); + await expect( + Agent.exists({ id: createdResponse.body.id, tenantId: otherTenantId }), + ).resolves.not.toBeNull(); + await expect( + Agent.exists({ + id: currentTenantGraphId, + tenantId, + 'edges.to': createdResponse.body.id, + }), + ).resolves.toBeNull(); + await expect( + Agent.exists({ + id: otherTenantGraphId, + tenantId: otherTenantId, + 'edges.to': createdResponse.body.id, + }), + ).resolves.not.toBeNull(); + await expect( + User.exists({ + _id: principal._id, + tenantId, + 'favorites.agentId': createdResponse.body.id, + }), + ).resolves.toBeNull(); + await expect( + User.exists({ + _id: otherTenantPrincipal._id, + tenantId: otherTenantId, + 'favorites.agentId': createdResponse.body.id, + }), + ).resolves.not.toBeNull(); + }); + + test('management creation rejects caller-controlled ownership', async () => { + const handler = createAgentManagementCreateHandler({ + getRoleByName: jest.fn().mockResolvedValue({ + permissions: { + [PermissionTypes.AGENTS]: { + [Permissions.USE]: true, + [Permissions.CREATE]: true, + }, + }, + }), + createAgent: createAgentHandler, + }); + mockReq.user.tenantId = 'tenant-a'; + mockReq.body = { + name: 'Managed Agent', + provider: 'openai', + model: 'gpt-4', + author: new mongoose.Types.ObjectId().toString(), + }; + + await handler(mockReq, mockRes); + + expect(mockRes.status).toHaveBeenCalledWith(400); + expect(await Agent.countDocuments()).toBe(0); + }); + test('should reject creation with unauthorized fields (mass assignment protection)', async () => { const maliciousData = { // Required fields diff --git a/api/server/controllers/assistants/v1.js b/api/server/controllers/assistants/v1.js index 2f53c076324..425fb98de88 100644 --- a/api/server/controllers/assistants/v1.js +++ b/api/server/controllers/assistants/v1.js @@ -104,7 +104,7 @@ const createAssistant = async (req, res) => { createData.append_current_datetime = append_current_datetime; } - const document = await updateAssistantDoc({ assistant_id: assistant.id }, createData); + const document = await updateAssistantDoc({ assistantId: assistant.id }, createData); if (azureModelIdentifier) { assistant.model = azureModelIdentifier; @@ -220,14 +220,14 @@ const patchAssistant = async (req, res) => { if (conversation_starters !== undefined) { const conversationStartersUpdate = await updateAssistantDoc( - { assistant_id }, + { assistantId: assistant_id }, { conversation_starters }, ); updatedAssistant.conversation_starters = conversationStartersUpdate.conversation_starters; } if (append_current_datetime !== undefined) { - await updateAssistantDoc({ assistant_id }, { append_current_datetime }); + await updateAssistantDoc({ assistantId: assistant_id }, { append_current_datetime }); updatedAssistant.append_current_datetime = append_current_datetime; } @@ -418,7 +418,7 @@ const uploadAssistantAvatar = async (req, res) => { const promises = []; promises.push( updateAssistantDoc( - { assistant_id }, + { assistantId: assistant_id }, { avatar: { filepath: image.filepath, diff --git a/api/server/controllers/assistants/v2.js b/api/server/controllers/assistants/v2.js index e6e9a5d0eff..063354d946b 100644 --- a/api/server/controllers/assistants/v2.js +++ b/api/server/controllers/assistants/v2.js @@ -94,7 +94,7 @@ const createAssistant = async (req, res) => { createData.append_current_datetime = append_current_datetime; } - const document = await updateAssistantDoc({ assistant_id: assistant.id }, createData); + const document = await updateAssistantDoc({ assistantId: assistant.id }, createData); if (azureModelIdentifier) { assistant.model = azureModelIdentifier; @@ -135,7 +135,7 @@ const updateAssistant = async ({ req, openai, assistant_id, updateData }) => { if (updateData?.conversation_starters) { const conversationStartersUpdate = await updateAssistantDoc( - { assistant_id: assistant_id }, + { assistantId: assistant_id }, { conversation_starters: updateData.conversation_starters }, ); conversation_starters = conversationStartersUpdate.conversation_starters; @@ -145,7 +145,7 @@ const updateAssistant = async ({ req, openai, assistant_id, updateData }) => { if (updateData?.append_current_datetime !== undefined) { await updateAssistantDoc( - { assistant_id: assistant_id }, + { assistantId: assistant_id }, { append_current_datetime: updateData.append_current_datetime }, ); delete updateData.append_current_datetime; diff --git a/api/server/controllers/mcp.js b/api/server/controllers/mcp.js index 447c3f1abb8..dd871e5731a 100644 --- a/api/server/controllers/mcp.js +++ b/api/server/controllers/mcp.js @@ -18,6 +18,7 @@ const { normalizeServerName, findShadowedServerNames, redactServerSecrets, + sanitizeMcpIconPath, redactAllServerSecrets, isMCPDomainNotAllowedError, isMCPInspectionFailedError, @@ -460,6 +461,9 @@ const createMCPServerController = async (req, res) => { errors: validation.error.errors, }); } + if (validation.data.iconPath) { + validation.data.iconPath = sanitizeMcpIconPath(validation.data.iconPath); + } if (configHasObo(validation.data) && !(await callerCanConfigureObo(req))) { logger.warn( `[createMCPServer] User ${userId} attempted to configure OBO without ${Permissions.CONFIGURE_OBO} permission`, @@ -549,6 +553,9 @@ const updateMCPServerController = async (req, res) => { errors: validation.error.errors, }); } + if (validation.data.iconPath) { + validation.data.iconPath = sanitizeMcpIconPath(validation.data.iconPath); + } /** * On an existing OBO server, lock down every user-input field except the diff --git a/api/server/experimental.js b/api/server/experimental.js index ce21fae0ddc..fbaa62d9040 100644 --- a/api/server/experimental.js +++ b/api/server/experimental.js @@ -1,4 +1,12 @@ require('../config/credentials'); +/** + * The primary kills every worker and then force-exits the whole cluster this long after its own + * shutdown signal, regardless of what the workers are still doing. + */ +const CLUSTER_FORCE_EXIT_MS = 10_000; +/** Absolute time the primary will force-exit the cluster, propagated to this worker over IPC. */ +let clusterShutdownDeadlineAt = null; + const fs = require('fs'); const path = require('path'); require('module-alias')({ base: path.resolve(__dirname, '..') }); @@ -32,6 +40,9 @@ const { requestContextMiddleware, configureServerTimeouts, setupGracefulShutdown, + registerShutdownTask, + getRemainingShutdownMs, + getShutdownElapsedMs, configureMessageFilterRegexValidator, configureFileConfigRegexEngine, configureAgentEventRuntime, @@ -261,6 +272,55 @@ if (cluster.isMaster) { cluster.fork(); }); + /** + * Deliver the absolute deadline, then SIGTERM only once the worker has acknowledged it. + * IPC is asynchronous and worker.kill() is immediate, so without the acknowledgement a + * busy worker can enter its shutdown handler before the deadline arrives and fall back to + * a worker-local estimate the primary will not honor. Bounded so a stalled worker cannot + * hold the others. + */ + const signalWorkerAfterDeadlineAck = (worker, deadlineAt) => { + let signaled = false; + const onMessage = (msg) => { + if (msg != null && msg.type === 'cluster-shutdown-ack') { + signal(); + } + }; + /** A closed IPC channel is reported asynchronously, not thrown from send(); without a + * listener it reaches the global uncaughtException handler and exits the primary, + * killing every other worker before it can record its drain. */ + const onError = (err) => { + logger.warn('Worker IPC error during the shutdown handoff; treating it as gone:', err); + signal(); + }; + const signal = () => { + if (signaled) { + return; + } + signaled = true; + worker.off('message', onMessage); + worker.off('error', onError); + try { + worker.kill(); + } catch (err) { + logger.debug('Worker already gone before SIGTERM:', err); + } + }; + worker.on('message', onMessage); + worker.on('error', onError); + /** Deliberately no separate timeout. SIGTERM is sent only after the worker has recorded + * the deadline; a worker that never acknowledges is ended by the primary's own + * force-exit, the one deadline it can honor. Signaling sooner would let a stalled + * worker's SIGTERM handler run before the queued deadline message and fall back to a + * local budget the primary will not honor, the exact case this handoff exists for. + * Handshakes are per worker, so a stalled one holds no other. */ + worker.send({ type: 'cluster-shutdown', deadlineAt }, (err) => { + if (err) { + onError(err); + } + }); + }; + /** Graceful shutdown on SIGTERM/SIGINT */ const shutdown = () => { if (shuttingDown) { @@ -274,13 +334,16 @@ if (cluster.isMaster) { process.exit(0); return; } + /** Workers derive their settlement budget from THIS deadline — not from their own + * coordinator, and not from whenever their signal handler happened to run. */ + const deadlineAt = Date.now() + CLUSTER_FORCE_EXIT_MS; for (const worker of liveWorkers) { - worker.kill(); + signalWorkerAfterDeadlineAck(worker, deadlineAt); } setTimeout(() => { logger.info('Forcing shutdown after timeout'); process.exit(0); - }, 10000); + }, CLUSTER_FORCE_EXIT_MS); }; process.on('SIGTERM', shutdown); @@ -301,6 +364,43 @@ if (cluster.isMaster) { }), ); GenerationJobManager.initialize(); + // Stop active generations and close their SSE streams while the HTTP server drains. + registerShutdownTask( + 'generation job manager prepare', + () => GenerationJobManager.prepareForShutdown(), + { + phase: 'pre-drain', + priority: 100, + }, + ); + /** Spend the shutdown budget that is actually left waiting for open provider executions to + * record their own drains — but the budget this worker actually has is the primary's, not + * its own 60s coordinator: the primary force-exits the whole cluster CLUSTER_FORCE_EXIT_MS + * after signalling. Measure against that, and hold back a reserve for the tasks after this + * one. Abandoning an unrecorded drain fences the next generation permanently. */ + const CLUSTER_TEARDOWN_RESERVE_MS = 3_000; + const destroyGenerationJobManager = () => { + const remaining = getRemainingShutdownMs(); + const elapsed = getShutdownElapsedMs(); + if (remaining == null || elapsed == null) { + return GenerationJobManager.destroy(); + } + /** Prefer the deadline the primary actually set. The elapsed-based estimate starts + * counting only when this worker's signal handler ran, which lags the primary's timer + * by however long the event loop was blocked. */ + const primaryRemaining = + clusterShutdownDeadlineAt != null + ? clusterShutdownDeadlineAt - Date.now() + : CLUSTER_FORCE_EXIT_MS - elapsed; + return GenerationJobManager.destroy({ + settlementBudgetMs: Math.max( + 0, + Math.min(remaining, primaryRemaining) - CLUSTER_TEARDOWN_RESERVE_MS, + ), + }); + }; + // Tear down stream resources before shared caches and telemetry exporters shut down. + registerShutdownTask('generation job manager', destroyGenerationJobManager, { priority: 100 }); /** * The master may assign the sweep worker before or after this worker has * loaded app config. These flags join the IPC assignment with config @@ -331,6 +431,20 @@ if (cluster.isMaster) { }; /** Handle inter-process messages from master */ + process.on('message', (msg) => { + if (msg != null && msg.type === 'cluster-shutdown' && Number.isFinite(msg.deadlineAt)) { + clusterShutdownDeadlineAt = msg.deadlineAt; + /** The primary holds SIGTERM until this arrives, so the deadline is in place before + * the shutdown handler can run. */ + if (typeof process.send === 'function') { + try { + process.send({ type: 'cluster-shutdown-ack' }); + } catch (err) { + logger.debug('Could not acknowledge the shutdown deadline to the primary:', err); + } + } + } + }); process.on('message', (msg) => { if (msg.type === 'file-retention-sweep-worker') { shouldStartExpiredFileSweep = true; diff --git a/api/server/index.js b/api/server/index.js index 5f50c0d04c8..70e0024fcbf 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -45,6 +45,7 @@ const { preAuthTenantMiddleware, requestContextMiddleware, registerShutdownTask, + getRemainingShutdownMs, configureServerTimeouts, setupGracefulShutdown, updateInterfacePermissions, @@ -143,12 +144,24 @@ const configureGenerationStreams = () => { priority: 100, }, ); + /** Spend the shutdown budget that is actually left waiting for detached generations to + * record their own provider drains, holding back a reserve for the tasks after this one. + * Abandoning an unrecorded drain fences the next generation permanently. */ + const destroyGenerationJobManager = () => { + const remaining = getRemainingShutdownMs(); + return GenerationJobManager.destroy( + remaining == null + ? undefined + : { settlementBudgetMs: Math.max(0, remaining - SHUTDOWN_TEARDOWN_RESERVE_MS) }, + ); + }; // Tear down stream resources before shared caches and telemetry exporters shut down. - registerShutdownTask('generation job manager', () => GenerationJobManager.destroy(), { - priority: 100, - }); + registerShutdownTask('generation job manager', destroyGenerationJobManager, { priority: 100 }); }; +/** Reserved for the shutdown tasks that run after the generation job manager. */ +const SHUTDOWN_TEARDOWN_RESERVE_MS = 10_000; + const startServer = async () => { await waitForKeyvRedisClient(); await configureSubagentTaskRouting(); diff --git a/api/server/middleware/assistants/validateAuthor.js b/api/server/middleware/assistants/validateAuthor.js index 024d6abbe35..6ffc8382b49 100644 --- a/api/server/middleware/assistants/validateAuthor.js +++ b/api/server/middleware/assistants/validateAuthor.js @@ -40,7 +40,7 @@ const validateAuthor = async ({ req, openai, overrideEndpoint, overrideAssistant return; } - const assistantDoc = await getAssistant({ assistant_id, user: req.user.id }); + const assistantDoc = await getAssistant({ assistantId: assistant_id, user: req.user.id }); if (assistantDoc) { return; } diff --git a/api/server/middleware/limiters/index.js b/api/server/middleware/limiters/index.js index 76afc3ce37d..2ad2b145643 100644 --- a/api/server/middleware/limiters/index.js +++ b/api/server/middleware/limiters/index.js @@ -5,6 +5,7 @@ const loginLimiter = require('./loginLimiter'); const importLimiters = require('./importLimiters'); const uploadLimiters = require('./uploadLimiters'); const forkLimiters = require('./forkLimiters'); +const shareLimiters = require('./shareLimiters'); const registerLimiter = require('./registerLimiter'); const toolCallLimiter = require('./toolCallLimiter'); const messageLimiters = require('./messageLimiters'); @@ -20,6 +21,7 @@ module.exports = { ...importLimiters, ...messageLimiters, ...forkLimiters, + ...shareLimiters, ...promptUsageLimiter, loginLimiter, registerLimiter, diff --git a/api/server/middleware/limiters/shareLimiters.js b/api/server/middleware/limiters/shareLimiters.js new file mode 100644 index 00000000000..24dcf41e293 --- /dev/null +++ b/api/server/middleware/limiters/shareLimiters.js @@ -0,0 +1,98 @@ +const rateLimit = require('express-rate-limit'); +const { ipKeyGenerator } = rateLimit; +const { ViolationTypes } = require('librechat-data-provider'); +const { limiterCache, removePorts } = require('@librechat/api'); +const logViolation = require('~/cache/logViolation'); + +const getEnvironmentVariables = () => { + const SHARE_IP_MAX = parseInt(process.env.SHARE_IP_MAX) || 100; + const SHARE_IP_WINDOW = parseInt(process.env.SHARE_IP_WINDOW) || 1; + const SHARE_USER_MAX = parseInt(process.env.SHARE_USER_MAX) || 60; + const SHARE_USER_WINDOW = parseInt(process.env.SHARE_USER_WINDOW) || 1; + const SHARE_VIOLATION_SCORE = process.env.SHARE_VIOLATION_SCORE ?? 0; + + const shareIpWindowMs = SHARE_IP_WINDOW * 60 * 1000; + const shareIpMax = SHARE_IP_MAX; + const shareIpWindowInMinutes = shareIpWindowMs / 60000; + + const shareUserWindowMs = SHARE_USER_WINDOW * 60 * 1000; + const shareUserMax = SHARE_USER_MAX; + const shareUserWindowInMinutes = shareUserWindowMs / 60000; + + return { + shareIpWindowMs, + shareIpMax, + shareIpWindowInMinutes, + shareUserWindowMs, + shareUserMax, + shareUserWindowInMinutes, + shareViolationScore: SHARE_VIOLATION_SCORE, + }; +}; + +/** + * Groups IPv6 clients by subnet so an address rotation cannot mint a fresh + * bucket; this limiter is the only bound on anonymous shared-link retrieval. + */ +const shareIpKey = (req) => { + const address = removePorts(req); + return address == null ? address : ipKeyGenerator(address); +}; + +const createShareHandler = (ip = true) => { + const { + shareIpMax, + shareUserMax, + shareViolationScore, + shareIpWindowInMinutes, + shareUserWindowInMinutes, + } = getEnvironmentVariables(); + + return async (req, res) => { + const type = ViolationTypes.SHARE_LIMIT; + const errorMessage = { + type, + max: ip ? shareIpMax : shareUserMax, + limiter: ip ? 'ip' : 'user', + windowInMinutes: ip ? shareIpWindowInMinutes : shareUserWindowInMinutes, + }; + + await logViolation(req, res, type, errorMessage, shareViolationScore); + res.status(429).json({ message: 'Too many shared link requests. Try again later' }); + }; +}; + +/** + * Bounds shared-link retrieval, which re-inspects the whole shared snapshot + * on every request and is reachable without authentication. + */ +const createShareLimiters = () => { + const { shareIpWindowMs, shareIpMax, shareUserWindowMs, shareUserMax } = + getEnvironmentVariables(); + + const ipLimiterOptions = { + windowMs: shareIpWindowMs, + max: shareIpMax, + handler: createShareHandler(), + keyGenerator: shareIpKey, + store: limiterCache('share_ip_limiter'), + }; + const userLimiterOptions = { + windowMs: shareUserWindowMs, + max: shareUserMax, + handler: createShareHandler(false), + skip: function (req) { + return req.user?.id == null; + }, + keyGenerator: function (req) { + return req.user.id; + }, + store: limiterCache('share_user_limiter'), + }; + + const shareIpLimiter = rateLimit(ipLimiterOptions); + const shareUserLimiter = rateLimit(userLimiterOptions); + return { shareIpLimiter, shareUserLimiter }; +}; + +module.exports = { createShareLimiters }; diff --git a/api/server/middleware/limiters/shareLimiters.spec.js b/api/server/middleware/limiters/shareLimiters.spec.js new file mode 100644 index 00000000000..86127c313cf --- /dev/null +++ b/api/server/middleware/limiters/shareLimiters.spec.js @@ -0,0 +1,127 @@ +const express = require('express'); +const request = require('supertest'); + +const mockLogViolation = jest.fn(); + +jest.mock('@librechat/api', () => ({ + limiterCache: jest.fn(() => undefined), + removePorts: (req) => req.ip, +})); +jest.mock( + '~/cache/logViolation', + () => + (...args) => + mockLogViolation(...args), +); + +describe('shared link limiters', () => { + let originalEnv; + + beforeEach(() => { + originalEnv = { ...process.env }; + jest.resetModules(); + jest.clearAllMocks(); + }); + + afterEach(() => { + process.env = originalEnv; + }); + + const buildApp = (user, { trustProxy = false } = {}) => { + const { createShareLimiters } = require('./shareLimiters'); + const { shareIpLimiter, shareUserLimiter } = createShareLimiters(); + const app = express(); + if (trustProxy) { + app.set('trust proxy', true); + } + app.get( + '/api/share/:shareId', + (req, _res, next) => { + req.user = user; + next(); + }, + shareIpLimiter, + shareUserLimiter, + (_req, res) => res.status(200).json({ ok: true }), + ); + return app; + }; + + it('rejects an anonymous retrieval flood by IP', async () => { + process.env.SHARE_IP_MAX = '2'; + process.env.SHARE_IP_WINDOW = '1'; + const app = buildApp(undefined); + + await request(app).get('/api/share/share-1').expect(200); + await request(app).get('/api/share/share-1').expect(200); + const response = await request(app).get('/api/share/share-1').expect(429); + + expect(response.body).toEqual({ message: 'Too many shared link requests. Try again later' }); + expect(mockLogViolation).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + 'share_limit', + expect.objectContaining({ limiter: 'ip', max: 2, windowInMinutes: 1 }), + 0, + ); + }); + + it('keeps an unset violation score from banning shared link viewers', async () => { + process.env.SHARE_IP_MAX = '1'; + delete process.env.SHARE_VIOLATION_SCORE; + const app = buildApp({ id: 'user-1' }); + + await request(app).get('/api/share/share-1').expect(200); + await request(app).get('/api/share/share-1').expect(429); + + expect(mockLogViolation).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + 'share_limit', + expect.anything(), + 0, + ); + }); + + it('buckets IPv6 viewers by subnet so address rotation cannot mint a bucket', async () => { + process.env.SHARE_IP_MAX = '1'; + const app = buildApp(undefined, { trustProxy: true }); + + const retrieve = (address) => + request(app).get('/api/share/share-1').set('X-Forwarded-For', address); + + expect((await retrieve('2001:db8:1234:5600::1')).status).toBe(200); + expect((await retrieve('2001:db8:1234:5622::9')).status).toBe(429); + expect((await retrieve('2001:db8:1234:5700::1')).status).toBe(200); + }); + + it('rejects an authenticated retrieval flood by user', async () => { + process.env.SHARE_IP_MAX = '100'; + process.env.SHARE_USER_MAX = '1'; + process.env.SHARE_USER_WINDOW = '1'; + process.env.SHARE_VIOLATION_SCORE = '3'; + const app = buildApp({ id: 'user-1' }); + + await request(app).get('/api/share/share-1').expect(200); + await request(app).get('/api/share/share-1').expect(429); + + expect(mockLogViolation).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + 'share_limit', + expect.objectContaining({ limiter: 'user', max: 1 }), + '3', + ); + }); + + it('leaves the per-user bucket untouched for anonymous viewers', async () => { + process.env.SHARE_IP_MAX = '100'; + process.env.SHARE_USER_MAX = '1'; + const app = buildApp(undefined); + + await request(app).get('/api/share/share-1').expect(200); + await request(app).get('/api/share/share-1').expect(200); + + expect(mockLogViolation).not.toHaveBeenCalled(); + }); +}); diff --git a/api/server/middleware/spec/validateImages.spec.js b/api/server/middleware/spec/validateImages.spec.js index ba04a273108..7eab98fa694 100644 --- a/api/server/middleware/spec/validateImages.spec.js +++ b/api/server/middleware/spec/validateImages.spec.js @@ -311,13 +311,11 @@ describe('validateImageRequest middleware', () => { expect(next).toHaveBeenCalled(); expect(getAssistant).toHaveBeenCalledWith( { - 'avatar.filepath': { - $in: [ - '/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png', - '/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png?manual=false', - '/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png?manual=true', - ], - }, + avatarFilepath: [ + '/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png', + '/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png?manual=false', + '/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png?manual=true', + ], }, { _id: 1, assistant_id: 1, endpoint: 1 }, ); diff --git a/api/server/routes/__test-utils__/convos-route-mocks.js b/api/server/routes/__test-utils__/convos-route-mocks.js index 9240fe27591..9c6ddc2aefa 100644 --- a/api/server/routes/__test-utils__/convos-route-mocks.js +++ b/api/server/routes/__test-utils__/convos-route-mocks.js @@ -36,6 +36,15 @@ module.exports = { } return false; }), + /** Mirrors the real helper so the route's page-size clamping is exercised. */ + normalizeLimit: jest.fn((value, { fallback = 25, max = 100 } = {}) => { + const raw = Array.isArray(value) ? value[0] : value; + const limit = parseInt(typeof raw === 'string' ? raw : '', 10); + if (!Number.isFinite(limit)) { + return fallback; + } + return Math.min(Math.max(limit, 1), max); + }), resolveImportMaxFileSize: jest.fn(() => 262144000), createAxiosInstance: jest.fn(() => ({ get: jest.fn(), diff --git a/api/server/routes/__tests__/convos.spec.js b/api/server/routes/__tests__/convos.spec.js index 65bd45141d8..620d9c1f542 100644 --- a/api/server/routes/__tests__/convos.spec.js +++ b/api/server/routes/__tests__/convos.spec.js @@ -1417,6 +1417,73 @@ describe('Convos Routes', () => { }); }); + describe('GET / limit clamping', () => { + const { getConvosByCursor } = require('~/models'); + + beforeEach(() => { + getConvosByCursor.mockResolvedValue({ conversations: [], nextCursor: null }); + }); + + /** `parseInt('-1') || 25` kept the -1, and `.limit(-1 + 1)` is `.limit(0)`, which + * MongoDB reads as "no limit" — one request drained the caller's whole list. */ + it('clamps a negative limit instead of forwarding it', async () => { + const response = await request(app).get('/api/convos').query({ limit: '-1' }); + + expect(response.status).toBe(200); + expect(getConvosByCursor).toHaveBeenCalledWith( + 'test-user-123', + expect.objectContaining({ limit: 1 }), + ); + }); + + it('caps an oversized limit at the page ceiling', async () => { + const response = await request(app).get('/api/convos').query({ limit: '999999999' }); + + expect(response.status).toBe(200); + expect(getConvosByCursor).toHaveBeenCalledWith( + 'test-user-123', + expect.objectContaining({ limit: 100 }), + ); + }); + + it('raises a zero limit to a single row', async () => { + const response = await request(app).get('/api/convos').query({ limit: '0' }); + + expect(response.status).toBe(200); + expect(getConvosByCursor).toHaveBeenCalledWith( + 'test-user-123', + expect.objectContaining({ limit: 1 }), + ); + }); + + it.each([ + ['absent', undefined], + ['non-numeric', 'all'], + ])('falls back to the default page size when the limit is %s', async (_label, limit) => { + const response = await request(app) + .get('/api/convos') + .query(limit === undefined ? {} : { limit }); + + expect(response.status).toBe(200); + expect(getConvosByCursor).toHaveBeenCalledWith( + 'test-user-123', + expect.objectContaining({ limit: 25 }), + ); + }); + + /** Express parses a repeated key into an array, which `parseInt` silently reads as + * its first element's digits rather than rejecting. */ + it('clamps a repeated limit parameter', async () => { + const response = await request(app).get('/api/convos?limit=999999&limit=5'); + + expect(response.status).toBe(200); + expect(getConvosByCursor).toHaveBeenCalledWith( + 'test-user-123', + expect.objectContaining({ limit: 100 }), + ); + }); + }); + describe('POST /archive', () => { it('should archive a conversation successfully', async () => { const mockConversationId = 'conv-123'; diff --git a/api/server/routes/__tests__/share.spec.js b/api/server/routes/__tests__/share.spec.js index 8e30f41507a..2998ab8750b 100644 --- a/api/server/routes/__tests__/share.spec.js +++ b/api/server/routes/__tests__/share.spec.js @@ -21,6 +21,8 @@ const mockCanAccessSharedLink = jest.fn((req, _res, next) => { next(); }); const mockGetAppConfig = jest.fn(); +const mockShareIpLimiter = jest.fn((_req, _res, next) => next()); +const mockShareUserLimiter = jest.fn((_req, _res, next) => next()); const mockParseSharedLinksPageSize = jest.fn(() => 10); const mockIsValidSharedLinksCursor = jest.fn(() => true); const mockAssertConversationContentAllowed = jest.fn(); @@ -205,6 +207,10 @@ jest.mock('~/server/middleware/limiters', () => ({ forkIpLimiter: (_req, _res, next) => next(), forkUserLimiter: (_req, _res, next) => next(), }), + createShareLimiters: () => ({ + shareIpLimiter: (req, res, next) => mockShareIpLimiter(req, res, next), + shareUserLimiter: (req, res, next) => mockShareUserLimiter(req, res, next), + }), })); jest.mock('~/server/utils/import/fork', () => ({ @@ -370,6 +376,15 @@ describe('share routes', () => { expect(mockGetAppConfig).toHaveBeenCalledWith({ baseOnly: true }); }); + it('rate limits shared message retrieval by IP and by user', async () => { + mockSharedMessagesResult({ shareId: 'share-123', messages: [] }); + + await request(buildApp()).get('/api/share/share-123').expect(200); + + expect(mockShareIpLimiter).toHaveBeenCalledTimes(1); + expect(mockShareUserLimiter).toHaveBeenCalledTimes(1); + }); + it('prevents successful shared message responses from being cached', async () => { mockSharedMessagesResult({ shareId: 'share-123', messages: [] }); diff --git a/api/server/routes/agents/__tests__/abort.spec.js b/api/server/routes/agents/__tests__/abort.spec.js index 3d3c748b067..82a42dd0983 100644 --- a/api/server/routes/agents/__tests__/abort.spec.js +++ b/api/server/routes/agents/__tests__/abort.spec.js @@ -77,6 +77,7 @@ jest.mock('~/server/routes/agents/chat', () => require('express').Router()); jest.mock('~/server/routes/agents/v1', () => ({ v1: require('express').Router(), })); +jest.mock('~/server/routes/agents/management', () => require('express').Router()); // Import after mocks const agentRoutes = require('~/server/routes/agents/index'); diff --git a/api/server/routes/agents/__tests__/idempotencyLimiter.spec.js b/api/server/routes/agents/__tests__/idempotencyLimiter.spec.js index 2624a654eeb..51c422c8df6 100644 --- a/api/server/routes/agents/__tests__/idempotencyLimiter.spec.js +++ b/api/server/routes/agents/__tests__/idempotencyLimiter.spec.js @@ -60,6 +60,11 @@ jest.mock('~/server/routes/agents/v1', () => ({ })); jest.mock('~/server/routes/agents/openai', () => require('express').Router()); jest.mock('~/server/routes/agents/responses', () => require('express').Router()); +jest.mock('~/server/routes/agents/management', () => { + const router = require('express').Router(); + router.use((_req, res) => res.status(200).json({ surface: 'management' })); + return router; +}); jest.mock('~/server/controllers/agents/steer', () => { const controller = (_req, _res, next) => next(); controller.SteerDeliveryController = (_req, _res, next) => next(); @@ -80,6 +85,15 @@ const app = express(); app.use(express.json()); app.use('/agents', agentsRouter); +describe('Agent Management route precedence', () => { + it('reaches management before the catch-all execution router', async () => { + const response = await request(app).get('/agents/v1/agents'); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ surface: 'management' }); + }); +}); + describe('start-generation idempotency before message limiters', () => { beforeEach(() => { jest.clearAllMocks(); diff --git a/api/server/routes/agents/__tests__/management.spec.js b/api/server/routes/agents/__tests__/management.spec.js new file mode 100644 index 00000000000..3a022079b50 --- /dev/null +++ b/api/server/routes/agents/__tests__/management.spec.js @@ -0,0 +1,177 @@ +const express = require('express'); +const request = require('supertest'); + +const mockMapAgentManagementError = jest.fn(() => ({ + status: 404, + body: { error: { code: 'not_found', message: 'Agent not found' } }, +})); +const mockList = jest.fn((_req, res) => res.status(200).json({ object: 'list', data: [] })); +const mockGet = jest.fn((_req, res) => res.status(200).json({ id: 'agent-one' })); +const mockCreateAgentManagementReadHandlers = jest.fn(() => ({ + list: mockList, + get: mockGet, +})); +const mockCreate = jest.fn((_req, res) => res.status(201).json({ id: 'agent-created' })); +let mockCreateDeps; +const mockCreateAgentManagementCreateHandler = jest.fn((deps) => { + mockCreateDeps = deps; + return mockCreate; +}); +const mockUpdate = jest.fn((_req, res) => res.status(200).json({ id: 'agent-updated' })); +let mockUpdateDeps; +const mockCreateAgentManagementUpdateHandler = jest.fn((deps) => { + mockUpdateDeps = deps; + return mockUpdate; +}); +const mockDelete = jest.fn((_req, res) => + res.status(200).json({ id: 'agent-deleted', deleted: true }), +); +let mockDeleteDeps; +const mockCreateAgentManagementDeleteHandler = jest.fn((deps) => { + mockDeleteDeps = deps; + return mockDelete; +}); +const mockBrowserCreate = jest.fn(); +const mockBrowserUpdate = jest.fn(); +const mockCheckBan = jest.fn((_req, _res, next) => next()); +const mockConfigMiddleware = jest.fn((_req, _res, next) => next()); +const mockUaParser = jest.fn((_req, _res, next) => next()); + +const mockRequireAgentManagementAuth = jest.fn((req, res, next) => { + if (req.headers.authorization !== 'Bearer valid-token') { + return res.status(401).json({ error: 'Unauthorized' }); + } + req.user = { id: 'integration-user', tenantId: 'tenant-a', role: 'USER' }; + next(); +}); + +jest.mock('../middleware', () => ({ + requireAgentManagementAuth: mockRequireAgentManagementAuth, +})); +jest.mock('@librechat/api', () => ({ + mapAgentManagementError: mockMapAgentManagementError, + createAgentManagementCreateHandler: mockCreateAgentManagementCreateHandler, + createAgentManagementDeleteHandler: mockCreateAgentManagementDeleteHandler, + createAgentManagementReadHandlers: mockCreateAgentManagementReadHandlers, + createAgentManagementUpdateHandler: mockCreateAgentManagementUpdateHandler, +})); +jest.mock('~/server/middleware', () => ({ + checkBan: mockCheckBan, + configMiddleware: mockConfigMiddleware, + uaParser: mockUaParser, +})); +jest.mock('~/server/controllers/agents/v1', () => ({ + createAgent: mockBrowserCreate, + updateAgent: mockBrowserUpdate, +})); +jest.mock('~/server/middleware/roles/capabilities', () => ({ hasCapability: jest.fn() })); +jest.mock('~/server/services/PermissionService', () => ({ + checkPermission: jest.fn(), + findAccessibleResources: jest.fn(), +})); +jest.mock('~/models', () => ({ + getRoleByName: jest.fn(), + getAgentWithVersionCount: jest.fn(), + getAgentManagementListByAccess: jest.fn(), + deleteAgent: jest.fn(), +})); + +const router = require('../management'); + +describe('Agent Management route boundary', () => { + const app = express(); + app.use('/api/agents/v1/agents', router); + + beforeEach(() => { + mockRequireAgentManagementAuth.mockClear(); + mockCheckBan.mockClear(); + mockConfigMiddleware.mockClear(); + mockUaParser.mockClear(); + mockMapAgentManagementError.mockClear(); + }); + + it('rejects a request before reaching management routes without machine authentication', async () => { + const response = await request(app) + .post('/api/agents/v1/agents') + .send({ name: 'Managed Agent', provider: 'openAI', model: 'gpt-5' }); + + expect(response.status).toBe(401); + expect(mockRequireAgentManagementAuth).toHaveBeenCalledTimes(1); + expect(mockCheckBan).not.toHaveBeenCalled(); + expect(mockUaParser).not.toHaveBeenCalled(); + }); + + it('allows an authenticated non-browser client into the management router', async () => { + const response = await request(app) + .post('/api/agents/v1/agents/not-a-management-operation') + .set('Authorization', 'Bearer valid-token') + .set('User-Agent', 'curl/8.0.0'); + + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: { code: 'not_found', message: 'Agent not found' } }); + expect(mockCheckBan).toHaveBeenCalledTimes(1); + expect(mockUaParser).not.toHaveBeenCalled(); + expect(mockMapAgentManagementError).toHaveBeenCalledWith('not_found'); + expect(mockRequireAgentManagementAuth).toHaveBeenCalledTimes(1); + }); + + it('dispatches authenticated list and retrieve requests to the management read handlers', async () => { + const listResponse = await request(app) + .get('/api/agents/v1/agents') + .set('Authorization', 'Bearer valid-token'); + const getResponse = await request(app) + .get('/api/agents/v1/agents/agent-one') + .set('Authorization', 'Bearer valid-token'); + + expect(listResponse.status).toBe(200); + expect(getResponse.status).toBe(200); + expect(mockList).toHaveBeenCalledTimes(1); + expect(mockGet).toHaveBeenCalledTimes(1); + }); + + it('loads Agent configuration and dispatches authenticated creation requests', async () => { + const response = await request(app) + .post('/api/agents/v1/agents') + .set('Authorization', 'Bearer valid-token') + .send({ name: 'Managed Agent', provider: 'openAI', model: 'gpt-5' }); + + expect(response.status).toBe(201); + expect(response.body).toEqual({ id: 'agent-created' }); + expect(mockConfigMiddleware).toHaveBeenCalledTimes(1); + expect(mockCreate).toHaveBeenCalledTimes(1); + expect(mockCreateDeps.getRoleByName).toEqual(expect.any(Function)); + expect(mockCreateDeps.createAgent).toBe(mockBrowserCreate); + }); + + it('loads Agent configuration and dispatches authenticated update requests', async () => { + const response = await request(app) + .patch('/api/agents/v1/agents/agent-one') + .set('Authorization', 'Bearer valid-token') + .send({ name: 'Updated Agent' }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ id: 'agent-updated' }); + expect(mockConfigMiddleware).toHaveBeenCalledTimes(1); + expect(mockUpdate).toHaveBeenCalledTimes(1); + expect(mockUpdateDeps.getRoleByName).toEqual(expect.any(Function)); + expect(mockUpdateDeps.getAgentWithVersionCount).toEqual(expect.any(Function)); + expect(mockUpdateDeps.checkPermission).toEqual(expect.any(Function)); + expect(mockUpdateDeps.hasCapability).toEqual(expect.any(Function)); + expect(mockUpdateDeps.updateAgent).toBe(mockBrowserUpdate); + }); + + it('dispatches authenticated deletion requests with the shared Agent dependencies', async () => { + const response = await request(app) + .delete('/api/agents/v1/agents/agent-deleted') + .set('Authorization', 'Bearer valid-token'); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ id: 'agent-deleted', deleted: true }); + expect(mockDelete).toHaveBeenCalledTimes(1); + expect(mockDeleteDeps.getRoleByName).toEqual(expect.any(Function)); + expect(mockDeleteDeps.getAgentWithVersionCount).toEqual(expect.any(Function)); + expect(mockDeleteDeps.checkPermission).toEqual(expect.any(Function)); + expect(mockDeleteDeps.hasCapability).toEqual(expect.any(Function)); + expect(mockDeleteDeps.deleteAgent).toEqual(expect.any(Function)); + }); +}); diff --git a/api/server/routes/agents/__tests__/streamTenant.spec.js b/api/server/routes/agents/__tests__/streamTenant.spec.js index 194caa12fdb..e23338c937f 100644 --- a/api/server/routes/agents/__tests__/streamTenant.spec.js +++ b/api/server/routes/agents/__tests__/streamTenant.spec.js @@ -64,6 +64,7 @@ jest.mock('~/server/routes/agents/v1', () => { }); jest.mock('~/server/routes/agents/openai', () => require('express').Router()); jest.mock('~/server/routes/agents/responses', () => require('express').Router()); +jest.mock('~/server/routes/agents/management', () => require('express').Router()); const agentsRouter = require('../index'); const app = express(); diff --git a/api/server/routes/agents/actions.js b/api/server/routes/agents/actions.js index 124fe7209ab..9846a2b8994 100644 --- a/api/server/routes/agents/actions.js +++ b/api/server/routes/agents/actions.js @@ -68,9 +68,7 @@ router.get('/', async (req, res) => { const editableAgentIds = agentsResponse.data.map((agent) => agent.id); const actions = - editableAgentIds.length > 0 - ? await db.getActions({ agent_id: { $in: editableAgentIds } }) - : []; + editableAgentIds.length > 0 ? await db.getActions({ agentId: editableAgentIds }) : []; for (const action of actions) { if (blockFilteredActionProjection(req.config?.filters, res, action)) { @@ -171,7 +169,7 @@ router.post( // Permissions already validated by middleware - load agent directly initialPromises.push(db.getAgent({ id: agent_id })); if (requestedActionId) { - initialPromises.push(db.getActions({ action_id: requestedActionId }, true)); + initialPromises.push(db.getActions({ actionId: requestedActionId }, true)); } /** @type {[Agent, [Action|undefined]]} */ @@ -254,7 +252,7 @@ router.post( /** @type {Action} */ const updatedAction = await db.updateAction( - { action_id: requestedActionId ?? action_id, agent_id }, + { actionId: requestedActionId ?? action_id, agentId: agent_id }, actionUpdateData, ); @@ -323,7 +321,7 @@ router.delete( { tools: updatedTools, actions: updatedActions }, { updatingUserId: req.user.id, forceVersion: true }, ); - const deleted = await db.deleteAction({ action_id, agent_id }); + const deleted = await db.deleteAction({ actionId: action_id, agentId: agent_id }); if (!deleted) { logger.warn('[Agent Action Delete] No matching action document found', { action_id, diff --git a/api/server/routes/agents/index.js b/api/server/routes/agents/index.js index 46c50cd88f8..f3bdd73c75e 100644 --- a/api/server/routes/agents/index.js +++ b/api/server/routes/agents/index.js @@ -54,6 +54,7 @@ const { acknowledgeScheduledStopPersistence, } = require('~/server/services/Schedules'); const responses = require('./responses'); +const management = require('./management'); const openai = require('./openai'); const { v1 } = require('./v1'); const chat = require('./chat'); @@ -120,6 +121,13 @@ const router = express.Router(); */ router.use('/v1/responses', responses); +/** + * Machine-authenticated Agent Management routes. + * Mounted before the catch-all execution router so management requests cannot + * inherit execution authentication or API-key fallback behavior. + */ +router.use('/v1/agents', management); + /** * OpenAI-compatible API routes (API key authentication handled in route file) * Mounted at /agents/v1 (full path: /api/agents/v1/chat/completions) diff --git a/api/server/routes/agents/management.js b/api/server/routes/agents/management.js new file mode 100644 index 00000000000..f44bd8721e0 --- /dev/null +++ b/api/server/routes/agents/management.js @@ -0,0 +1,58 @@ +const express = require('express'); +const { + createAgentManagementCreateHandler, + createAgentManagementDeleteHandler, + createAgentManagementReadHandlers, + createAgentManagementUpdateHandler, + mapAgentManagementError, +} = require('@librechat/api'); +const { checkBan, configMiddleware } = require('~/server/middleware'); +const { hasCapability } = require('~/server/middleware/roles/capabilities'); +const { checkPermission, findAccessibleResources } = require('~/server/services/PermissionService'); +const v1 = require('~/server/controllers/agents/v1'); +const db = require('~/models'); +const { requireAgentManagementAuth } = require('./middleware'); + +const router = express.Router(); +const readHandlers = createAgentManagementReadHandlers({ + getRoleByName: db.getRoleByName, + getAgentWithVersionCount: db.getAgentWithVersionCount, + getAgentManagementListByAccess: db.getAgentManagementListByAccess, + findAccessibleResources, + checkPermission, + hasCapability, +}); +const createHandler = createAgentManagementCreateHandler({ + getRoleByName: db.getRoleByName, + createAgent: v1.createAgent, +}); +const updateHandler = createAgentManagementUpdateHandler({ + getRoleByName: db.getRoleByName, + getAgentWithVersionCount: db.getAgentWithVersionCount, + checkPermission, + hasCapability, + updateAgent: v1.updateAgent, +}); +const deleteHandler = createAgentManagementDeleteHandler({ + getRoleByName: db.getRoleByName, + getAgentWithVersionCount: db.getAgentWithVersionCount, + checkPermission, + hasCapability, + deleteAgent: db.deleteAgent, +}); + +router.use(requireAgentManagementAuth); +router.use(checkBan); + +router.post('/', configMiddleware, createHandler); +router.get('/', readHandlers.list); +router.get('/:id', readHandlers.get); +router.patch('/:id', configMiddleware, updateHandler); +router.delete('/:id', deleteHandler); + +router.use((_req, res) => { + const { status, body } = mapAgentManagementError('not_found'); + return res.status(status).json(body); +}); + +module.exports = router; diff --git a/api/server/routes/agents/middleware.js b/api/server/routes/agents/middleware.js index 34add78631e..44f9befb1d2 100644 --- a/api/server/routes/agents/middleware.js +++ b/api/server/routes/agents/middleware.js @@ -4,6 +4,7 @@ const { preAuthTenantMiddleware, createRequireApiKeyAuth, createRemoteAgentAuth, + createAgentManagementAuth, createCheckAgentTriggerAccess, createCheckRemoteAgentAccess, } = require('@librechat/api'); @@ -26,6 +27,12 @@ const requireRemoteAgentAuth = createRemoteAgentAuth({ getAppConfig, }); +const requireAgentManagementAuth = createAgentManagementAuth({ + findUser: db.findUser, + isPrincipalActive: db.isAgentTriggerPrincipalActive, + getAppConfig, +}); + const checkRemoteAgentsFeature = generateCheckAccess({ permissionType: PermissionTypes.REMOTE_AGENTS, permissions: [Permissions.USE], @@ -45,5 +52,6 @@ module.exports = { checkAgentTriggerPermission, preAuthTenantMiddleware, requireRemoteAgentAuth, + requireAgentManagementAuth, checkRemoteAgentsFeature, }; diff --git a/api/server/routes/assistants/actions.js b/api/server/routes/assistants/actions.js index 42e37f3b472..e3e952a6039 100644 --- a/api/server/routes/assistants/actions.js +++ b/api/server/routes/assistants/actions.js @@ -70,9 +70,9 @@ router.post('/:assistant_id', async (req, res) => { const { openai } = await getOpenAIClient({ req, res }); - initialPromises.push(db.getAssistant({ assistant_id })); + initialPromises.push(db.getAssistant({ assistantId: assistant_id })); initialPromises.push(openai.beta.assistants.retrieve(assistant_id)); - !!_action_id && initialPromises.push(db.getActions({ action_id }, true)); + !!_action_id && initialPromises.push(db.getActions({ actionId: action_id }, true)); /** @type {[AssistantDocument, Assistant, [Action|undefined]]} */ const [assistant_data, assistant, actions_result] = await Promise.all(initialPromises); @@ -150,7 +150,7 @@ router.post('/:assistant_id', async (req, res) => { if (!assistant_data) { assistantUpdateData.user = req.user.id; } - promises.push(db.updateAssistantDoc({ assistant_id }, assistantUpdateData)); + promises.push(db.updateAssistantDoc({ assistantId: assistant_id }, assistantUpdateData)); // Only update user field for new actions const actionUpdateData = { metadata, assistant_id }; @@ -158,7 +158,9 @@ router.post('/:assistant_id', async (req, res) => { // For new actions, use the assistant owner's user ID actionUpdateData.user = assistant_user || req.user.id; } - promises.push(db.updateAction({ action_id, assistant_id }, actionUpdateData)); + promises.push( + db.updateAction({ actionId: action_id, assistantId: assistant_id }, actionUpdateData), + ); /** @type {[AssistantDocument, Action]} */ let [assistantDocument, updatedAction] = await Promise.all(promises); @@ -200,7 +202,7 @@ router.delete('/:assistant_id/:action_id/:model', async (req, res) => { const { openai } = await getOpenAIClient({ req, res }); const initialPromises = []; - initialPromises.push(db.getAssistant({ assistant_id })); + initialPromises.push(db.getAssistant({ assistantId: assistant_id })); initialPromises.push(openai.beta.assistants.retrieve(assistant_id)); /** @type {[AssistantDocument, Assistant]} */ @@ -238,8 +240,8 @@ router.delete('/:assistant_id/:action_id/:model', async (req, res) => { if (!assistant_data) { assistantUpdateData.user = req.user.id; } - promises.push(db.updateAssistantDoc({ assistant_id }, assistantUpdateData)); - promises.push(db.deleteAction({ action_id, assistant_id })); + promises.push(db.updateAssistantDoc({ assistantId: assistant_id }, assistantUpdateData)); + promises.push(db.deleteAction({ actionId: action_id, assistantId: assistant_id })); const [, deletedAction] = await Promise.all(promises); if (!deletedAction) { diff --git a/api/server/routes/convos.js b/api/server/routes/convos.js index 06250c8f3de..ef2de837d26 100644 --- a/api/server/routes/convos.js +++ b/api/server/routes/convos.js @@ -3,6 +3,7 @@ const express = require('express'); const { sleep } = require('@librechat/agents'); const { isEnabled, + normalizeLimit, deleteAgentCheckpoints, createArchiveAllHandler, createSubagentActivityStreamHandler, @@ -141,7 +142,7 @@ const isValidProjectFilter = (projectId) => !projectId || projectId === 'unassigned' || /^[a-f\d]{24}$/i.test(projectId); router.get('/', async (req, res) => { - const limit = parseInt(req.query.limit, 10) || 25; + const limit = normalizeLimit(req.query.limit); const cursor = req.query.cursor; const isArchived = isEnabled(req.query.isArchived); const pinned = isEnabled(req.query.pinned); diff --git a/api/server/routes/files/files.js b/api/server/routes/files/files.js index 7b2c3935c8e..daa13a8a9a8 100644 --- a/api/server/routes/files/files.js +++ b/api/server/routes/files/files.js @@ -289,10 +289,10 @@ router.delete('/', async (req, res) => { /* Handle assistant unlinking even if no valid files to delete */ if (req.body.assistant_id && req.body.tool_resource && dbFiles.length === 0) { const assistant = await db.getAssistant({ - id: req.body.assistant_id, + assistantId: req.body.assistant_id, }); - const toolResourceFiles = assistant.tool_resources?.[req.body.tool_resource]?.file_ids ?? []; + const toolResourceFiles = assistant?.tool_resources?.[req.body.tool_resource]?.file_ids ?? []; const assistantFiles = files.filter((f) => toolResourceFiles.includes(f.file_id)); const result = await processDeleteRequest({ req, files: assistantFiles }); diff --git a/api/server/routes/files/files.test.js b/api/server/routes/files/files.test.js index 8d80a9bbf06..2ad58d350dd 100644 --- a/api/server/routes/files/files.test.js +++ b/api/server/routes/files/files.test.js @@ -694,6 +694,83 @@ describe('File Routes - Delete with Agent Access', () => { }); }); + /* Mirrors api/db/connect.js, which sets strictQuery for the running server. Under it + Mongoose DROPS filter keys absent from the schema, so a lookup on a misspelled path + degrades to findOne({}) — the first document in the collection, whoever owns it. */ + describe('DELETE /files - assistant tool resource unlinking', () => { + let previousStrictQuery; + + beforeAll(() => { + previousStrictQuery = mongoose.get('strictQuery'); + mongoose.set('strictQuery', true); + }); + + afterAll(async () => { + mongoose.set('strictQuery', previousStrictQuery); + await mongoose.connection.collection('assistants').deleteMany({}); + }); + + beforeEach(async () => { + await mongoose.connection.collection('assistants').deleteMany({}); + }); + + it("does not unlink another user's assistant files when the requested assistant is missing", async () => { + const strangerFileId = uuidv4(); + /* Written straight to the collection: `tool_resources` predates the current schema. */ + await mongoose.connection.collection('assistants').insertOne({ + user: new mongoose.Types.ObjectId(), + assistant_id: 'asst_belonging_to_someone_else', + tool_resources: { file_search: { file_ids: [strangerFileId] } }, + }); + + const response = await request(app) + .delete('/files') + .send({ + assistant_id: 'asst_that_does_not_exist', + tool_resource: 'file_search', + files: [{ file_id: strangerFileId, filepath: '/uploads/stranger.txt' }], + }); + + expect(response.status).toBe(200); + expect(processDeleteRequest).toHaveBeenCalledWith(expect.objectContaining({ files: [] })); + }); + + it('unlinks the requested assistant own files', async () => { + const ownFileId = uuidv4(); + await mongoose.connection.collection('assistants').insertOne({ + user: new mongoose.Types.ObjectId(), + assistant_id: 'asst_requested', + tool_resources: { file_search: { file_ids: [ownFileId] } }, + }); + + const response = await request(app) + .delete('/files') + .send({ + assistant_id: 'asst_requested', + tool_resource: 'file_search', + files: [{ file_id: ownFileId, filepath: '/uploads/own.txt' }], + }); + + expect(response.status).toBe(200); + expect(processDeleteRequest).toHaveBeenCalledWith( + expect.objectContaining({ files: [expect.objectContaining({ file_id: ownFileId })] }), + ); + }); + + it('answers instead of throwing when no assistants exist at all', async () => { + const response = await request(app) + .delete('/files') + .send({ + assistant_id: 'asst_that_does_not_exist', + tool_resource: 'file_search', + files: [{ file_id: uuidv4(), filepath: '/uploads/ghost.txt' }], + }); + + expect(response.status).toBe(200); + expect(processDeleteRequest).toHaveBeenCalledWith(expect.objectContaining({ files: [] })); + }); + }); + describe('GET /files/download-url/:userId/:file_id', () => { it('returns a direct signed download URL when the strategy supports it', async () => { const userFileId = uuidv4(); diff --git a/api/server/routes/share.js b/api/server/routes/share.js index bbfb3407adb..95298114a0c 100644 --- a/api/server/routes/share.js +++ b/api/server/routes/share.js @@ -48,7 +48,7 @@ const { getStrategyFunctions } = require('~/server/services/Files/strategies'); const { cleanFileName, getContentDisposition } = require('~/server/utils/files'); const canAccessSharedLink = require('~/server/middleware/canAccessSharedLink'); const { forkSharedConversation } = require('~/server/utils/import/fork'); -const { createForkLimiters } = require('~/server/middleware/limiters'); +const { createForkLimiters, createShareLimiters } = require('~/server/middleware/limiters'); const optionalShareFileAuth = require('~/server/middleware/optionalShareFileAuth'); const optionalJwtAuth = require('~/server/middleware/optionalJwtAuth'); const requireJwtAuth = require('~/server/middleware/requireJwtAuth'); @@ -336,6 +336,7 @@ const streamSharedFile = async (req, res, file, requestedDisposition) => { if (allowSharedLinks) { const { forkIpLimiter, forkUserLimiter } = createForkLimiters(); + const { shareIpLimiter, shareUserLimiter } = createShareLimiters(); router.get( '/:shareId/config', @@ -357,6 +358,8 @@ if (allowSharedLinks) { router.get( '/:shareId', optionalJwtAuth, + shareIpLimiter, + shareUserLimiter, canAccessSharedLink, sharedLinkConfigMiddleware, async (req, res) => { diff --git a/api/server/services/ActionService.js b/api/server/services/ActionService.js index 7caa89064e8..9f5e7f9942b 100644 --- a/api/server/services/ActionService.js +++ b/api/server/services/ActionService.js @@ -57,7 +57,7 @@ const validateAndUpdateTool = async ({ req, tool, assistant_id }) => { } if (!toolNameRegex.test(tool.function.name)) { const [functionName, domain] = tool.function.name.split(actionDelimiter); - actions = await getActions({ assistant_id, user: req.user.id }, true); + actions = await getActions({ assistantId: assistant_id, user: req.user.id }, true); const matchingActions = actions.filter((action) => { const metadata = action.metadata; if (!metadata) { @@ -157,14 +157,11 @@ async function domainParser(domain, inverse = false) { /** * Loads action sets based on the user and assistant ID. * - * @param {Object} searchParams - The parameters for loading action sets. - * @param {string} searchParams.user - The user identifier. - * @param {string} [searchParams.agent_id]- The agent identifier. - * @param {string} [searchParams.assistant_id]- The assistant identifier. + * @param {import('@librechat/data-schemas').ActionQuery} query - The criteria for loading action sets. * @returns {Promise} A promise that resolves to an array of actions or `null` if no match. */ -async function loadActionSets(searchParams) { - return await getActions(searchParams, true); +async function loadActionSets(query) { + return await getActions(query, true); } /** @@ -513,8 +510,8 @@ async function decryptMetadata(metadata) { */ const deleteAssistantActions = async ({ req, assistant_id }) => { try { - await deleteActions({ assistant_id, user: req.user.id }); - await deleteAssistant({ assistant_id, user: req.user.id }); + await deleteActions({ assistantId: assistant_id, user: req.user.id }); + await deleteAssistant({ assistantId: assistant_id, user: req.user.id }); } catch (error) { const message = 'Trouble deleting Assistant Actions for Assistant ID: ' + assistant_id; logger.error(message, error); diff --git a/api/server/services/Endpoints/agents/initialize.spec.js b/api/server/services/Endpoints/agents/initialize.spec.js index 98cd035d0de..e6ca1f5e92f 100644 --- a/api/server/services/Endpoints/agents/initialize.spec.js +++ b/api/server/services/Endpoints/agents/initialize.spec.js @@ -1052,6 +1052,8 @@ describe('initializeClient — subagent loading', () => { 'read_file', 'create_file', 'edit_file', + 'search_workspace', + 'list_workspace_files', ]); await expect( diff --git a/api/server/services/Endpoints/agents/skillDeps.js b/api/server/services/Endpoints/agents/skillDeps.js index b58983da9af..8fe49aedc36 100644 --- a/api/server/services/Endpoints/agents/skillDeps.js +++ b/api/server/services/Endpoints/agents/skillDeps.js @@ -4,6 +4,12 @@ const { batchUploadCodeEnvFiles } = require('~/server/services/Files/Code/crud') const { getSessionInfo, checkIfActive, + readWorkspaceFile, + searchWorkspace, + listWorkspaceFiles, + writeWorkspaceFile, + previewWorkspaceEdit, + editWorkspaceFile, readSandboxFile, readSandboxImage, writeSandboxFile, @@ -380,6 +386,12 @@ const skillToolDeps = { updateSkillFileCodeEnvIds: deploymentSkillMethods.updateSkillFileCodeEnvIds, getSkillFileByPath: deploymentSkillMethods.getSkillFileByPath, updateSkillFileContent: deploymentSkillMethods.updateSkillFileContent, + readWorkspaceFile, + searchWorkspace, + listWorkspaceFiles, + writeWorkspaceFile, + previewWorkspaceEdit, + editWorkspaceFile, /** * `read_file` falls back to a sandbox `cat` for `/mnt/data/...` paths * and for `{firstSegment}/...` paths whose first segment isn't a known diff --git a/api/server/services/Endpoints/agents/skillDeps.spec.js b/api/server/services/Endpoints/agents/skillDeps.spec.js index 3782a4664a0..bf848b7045e 100644 --- a/api/server/services/Endpoints/agents/skillDeps.spec.js +++ b/api/server/services/Endpoints/agents/skillDeps.spec.js @@ -5,6 +5,12 @@ const mockGetFileStrategy = jest.fn(); const mockGetStorageMetadata = jest.fn(); const mockResolveRequestTenantId = jest.fn(); const mockCreateDeploymentSkillMethods = jest.fn((methods) => methods); +const mockReadWorkspaceFile = jest.fn(); +const mockSearchWorkspace = jest.fn(); +const mockListWorkspaceFiles = jest.fn(); +const mockWriteWorkspaceFile = jest.fn(); +const mockPreviewWorkspaceEdit = jest.fn(); +const mockEditWorkspaceFile = jest.fn(); jest.mock('~/server/services/Files/strategies', () => ({ getStrategyFunctions: (...args) => mockGetStrategyFunctions(...args), @@ -18,6 +24,12 @@ jest.mock('~/server/services/Files/Code/process', () => ({ getSessionInfo: jest.fn(), checkIfActive: jest.fn(), readSandboxFile: jest.fn(), + readWorkspaceFile: (...args) => mockReadWorkspaceFile(...args), + searchWorkspace: (...args) => mockSearchWorkspace(...args), + listWorkspaceFiles: (...args) => mockListWorkspaceFiles(...args), + writeWorkspaceFile: (...args) => mockWriteWorkspaceFile(...args), + previewWorkspaceEdit: (...args) => mockPreviewWorkspaceEdit(...args), + editWorkspaceFile: (...args) => mockEditWorkspaceFile(...args), writeSandboxFile: jest.fn(), })); @@ -79,6 +91,33 @@ describe('skillDeps saveSkillFileContent', () => { mockDb.getSkillFileByPath.mockResolvedValue(null); }); + it('exposes the stable attached-workspace reader to agent handlers', () => { + expect(getSkillToolDeps().readWorkspaceFile).toBeDefined(); + getSkillToolDeps().readWorkspaceFile({ file_path: 'src/app.ts' }); + expect(mockReadWorkspaceFile).toHaveBeenCalledWith({ file_path: 'src/app.ts' }); + }); + + it('exposes the stable attached-workspace searcher to agent handlers', () => { + expect(getSkillToolDeps().searchWorkspace).toBeDefined(); + getSkillToolDeps().searchWorkspace({ query: 'needle' }); + expect(mockSearchWorkspace).toHaveBeenCalledWith({ query: 'needle' }); + }); + + it('exposes the stable attached-workspace file lister to agent handlers', () => { + expect(getSkillToolDeps().listWorkspaceFiles).toBeDefined(); + getSkillToolDeps().listWorkspaceFiles({ path: 'src' }); + expect(mockListWorkspaceFiles).toHaveBeenCalledWith({ path: 'src' }); + }); + + it('exposes attached-workspace mutations to agent handlers', () => { + getSkillToolDeps().writeWorkspaceFile({ path: 'src/new.ts' }); + getSkillToolDeps().previewWorkspaceEdit({ path: 'src/app.ts' }); + getSkillToolDeps().editWorkspaceFile({ path: 'src/app.ts' }); + expect(mockWriteWorkspaceFile).toHaveBeenCalledWith({ path: 'src/new.ts' }); + expect(mockPreviewWorkspaceEdit).toHaveBeenCalledWith({ path: 'src/app.ts' }); + expect(mockEditWorkspaceFile).toHaveBeenCalledWith({ path: 'src/app.ts' }); + }); + it('cleans up the uploaded object when metadata upsert returns no row', async () => { mockDb.upsertSkillFile.mockResolvedValue(null); diff --git a/api/server/services/Endpoints/agents/subagentThreadStore.js b/api/server/services/Endpoints/agents/subagentThreadStore.js index 27620d3b2c2..b33b7df5415 100644 --- a/api/server/services/Endpoints/agents/subagentThreadStore.js +++ b/api/server/services/Endpoints/agents/subagentThreadStore.js @@ -3,6 +3,7 @@ const { ioredisClient, registerShutdownTask, duplicateIoRedisClient, + createIoRedisSubscriber, createSubagentThreadTaskStore, createSubagentCompletionWakeupHandler, GenerationJobManager, @@ -110,13 +111,19 @@ async function configureSubagentTaskRouting() { if (ioredisClient == null || typeof ioredisClient.duplicate !== 'function') { throw new Error('Redis subagent task routing requires a dedicated subscriber connection.'); } - const subscriber = ioredisClient.duplicate(); + const subscriber = createIoRedisSubscriber( + ioredisClient, + '[SubagentTaskRouting] task subscriber', + ); /** A dedicated publisher without the offline queue: the shared client would hold a * command issued during a disconnect and deliver it after the caller gave up, so a * steer the caller was told had failed could still reach the child. Failing fast * turns that into the honest `unavailable` the caller already handles. */ const publisher = duplicateIoRedisClient(ioredisClient, { enableOfflineQueue: false }); - const activitySubscriber = ioredisClient.duplicate(); + const activitySubscriber = createIoRedisSubscriber( + ioredisClient, + '[SubagentTaskRouting] activity subscriber', + ); const activityPublisher = duplicateIoRedisClient(ioredisClient, { enableOfflineQueue: false }); const transport = new RedisSubagentTaskControlTransport(publisher, subscriber, { namespace: cacheConfig.REDIS_KEY_PREFIX, diff --git a/api/server/services/Endpoints/agents/subagentThreadStore.spec.js b/api/server/services/Endpoints/agents/subagentThreadStore.spec.js index 3883f2ada45..bae8a2cdebd 100644 --- a/api/server/services/Endpoints/agents/subagentThreadStore.spec.js +++ b/api/server/services/Endpoints/agents/subagentThreadStore.spec.js @@ -12,6 +12,7 @@ jest.mock('@librechat/api', () => ({ ioredisClient: { duplicate: jest.fn() }, registerShutdownTask: jest.fn(), duplicateIoRedisClient: jest.fn(), + createIoRedisSubscriber: jest.fn(), createSubagentThreadTaskStore: jest.fn(() => mockTaskStore), createSubagentCompletionWakeupHandler: jest.fn(() => mockCompletionWakeupHandler), RedisSubagentTaskControlTransport: jest.fn(), @@ -50,6 +51,7 @@ const { ioredisClient, registerShutdownTask, duplicateIoRedisClient, + createIoRedisSubscriber, createSubagentThreadTaskStore, } = require('@librechat/api'); const subagentThreadTaskStore = require('./subagentThreadStore'); @@ -91,7 +93,7 @@ describe('subagent thread Redis lifecycle', () => { const activitySubscriber = { disconnect: jest.fn() }; const taskPublisher = { disconnect: jest.fn() }; const activityPublisher = { disconnect: jest.fn() }; - ioredisClient.duplicate + createIoRedisSubscriber .mockReturnValueOnce(taskSubscriber) .mockReturnValueOnce(activitySubscriber); duplicateIoRedisClient @@ -100,6 +102,10 @@ describe('subagent thread Redis lifecycle', () => { await configureSubagentTaskRouting(); + expect(createIoRedisSubscriber.mock.calls).toEqual([ + [ioredisClient, '[SubagentTaskRouting] task subscriber'], + [ioredisClient, '[SubagentTaskRouting] activity subscriber'], + ]); expect(activityPrepareRegistration).toEqual([ 'subagent activity streams prepare', expect.any(Function), diff --git a/api/server/services/Endpoints/assistants/build.js b/api/server/services/Endpoints/assistants/build.js index 85f70902118..c05fa9f2492 100644 --- a/api/server/services/Endpoints/assistants/build.js +++ b/api/server/services/Endpoints/assistants/build.js @@ -16,7 +16,7 @@ const buildOptions = async (endpoint, parsedBody) => { }); if (assistant_id) { - const assistantDoc = await getAssistant({ assistant_id }); + const assistantDoc = await getAssistant({ assistantId: assistant_id }); if (assistantDoc) { // Create a clean assistant object with only the needed properties diff --git a/api/server/services/Endpoints/azureAssistants/build.js b/api/server/services/Endpoints/azureAssistants/build.js index 315447ed7f0..bc04ae73d2d 100644 --- a/api/server/services/Endpoints/azureAssistants/build.js +++ b/api/server/services/Endpoints/azureAssistants/build.js @@ -16,7 +16,7 @@ const buildOptions = async (endpoint, parsedBody) => { }); if (assistant_id) { - const assistantDoc = await getAssistant({ assistant_id }); + const assistantDoc = await getAssistant({ assistantId: assistant_id }); if (assistantDoc) { endpointOption.assistant = { append_current_datetime: assistantDoc.append_current_datetime, diff --git a/api/server/services/Files/Code/preflight.js b/api/server/services/Files/Code/preflight.js index 156e364bab6..31bdc31f94f 100644 --- a/api/server/services/Files/Code/preflight.js +++ b/api/server/services/Files/Code/preflight.js @@ -1,5 +1,8 @@ const { logger } = require('@librechat/data-schemas'); -const { preflightCodeOutputBatch: runCodeOutputBatchPreflight } = require('@librechat/api'); +const { + getSafeErrorMetadata, + preflightCodeOutputBatch: runCodeOutputBatchPreflight, +} = require('@librechat/api'); const { EModelEndpoint, mergeFileConfig, @@ -37,9 +40,17 @@ const preflightCodeOutputBatch = async ({ req, artifact, codeExecutionContext }) bridgeWorkerId: codeExecutionContext?.bridgeWorkerId, executionRouteKey: codeExecutionContext?.executionRouteKey, }), - onInspectionUnavailable: (index) => { + /** The batch degrades this artifact to the download fallback and the turn + * still succeeds, so this warning is the only trace the failure leaves. + * Carry the cause: without it a Code API that refused the download, one + * routed to the wrong execution profile, and a file too large to inspect + * all read as the same sentence. `getSafeErrorMetadata` is what keeps + * that from reaching for the artifact's name or the upstream body — + * both can echo submitted content into the log. */ + onInspectionUnavailable: (index, cause) => { logger.warn( `[preflightCodeOutputBatch] Generated artifact ${index + 1} could not be inspected`, + getSafeErrorMetadata(cause), ); }, }); diff --git a/api/server/services/Files/Code/preflight.spec.js b/api/server/services/Files/Code/preflight.spec.js index 317618a579b..f9baa15b9eb 100644 --- a/api/server/services/Files/Code/preflight.spec.js +++ b/api/server/services/Files/Code/preflight.spec.js @@ -1,5 +1,6 @@ const mockRunCodeOutputBatchPreflight = jest.fn(); const mockPrepareCodeOutputForInspection = jest.fn(); +const mockGetSafeErrorMetadata = jest.fn(() => ({ type: 'Error', status: 404 })); jest.mock('@librechat/data-schemas', () => ({ logger: { warn: jest.fn() }, @@ -7,6 +8,7 @@ jest.mock('@librechat/data-schemas', () => ({ jest.mock('@librechat/api', () => ({ preflightCodeOutputBatch: (...args) => mockRunCodeOutputBatchPreflight(...args), + getSafeErrorMetadata: (...args) => mockGetSafeErrorMetadata(...args), })); jest.mock('librechat-data-provider', () => ({ @@ -23,6 +25,7 @@ jest.mock('./process', () => ({ prepareCodeOutputForInspection: (...args) => mockPrepareCodeOutputForInspection(...args), })); +const { logger } = require('@librechat/data-schemas'); const { preflightCodeOutputBatch } = require('./preflight'); describe('preflightCodeOutputBatch Code API routing', () => { @@ -68,4 +71,31 @@ describe('preflightCodeOutputBatch Code API routing', () => { executionRouteKey: 'stateful:route', }); }); + + /* The batch degrades an uninspectable artifact to the download fallback and + * the turn still succeeds, so this warning is the only trace it leaves. + * Without the cause, a refused download, a misrouted execution profile and + * an oversized file are one indistinguishable sentence. */ + it('reports why an artifact could not be inspected, through the safe-metadata filter', async () => { + const cause = Object.assign(new Error('Request failed with status code 404'), { + response: { status: 404 }, + }); + mockRunCodeOutputBatchPreflight.mockImplementation(async ({ onInspectionUnavailable }) => { + onInspectionUnavailable(0, cause); + return []; + }); + + await preflightCodeOutputBatch({ + req: { config: {}, user: { id: 'user-1' } }, + artifact: { files: [{ id: 'file-1', name: 'PRIVATE-name.txt' }] }, + }); + + expect(mockGetSafeErrorMetadata).toHaveBeenCalledWith(cause); + expect(logger.warn).toHaveBeenCalledWith( + '[preflightCodeOutputBatch] Generated artifact 1 could not be inspected', + { type: 'Error', status: 404 }, + ); + /* The artifact's name is submitted content and must never ride the log. */ + expect(JSON.stringify(logger.warn.mock.calls)).not.toContain('PRIVATE-'); + }); }); diff --git a/api/server/services/Files/Code/process.js b/api/server/services/Files/Code/process.js index 479386d4590..2b5968c117e 100644 --- a/api/server/services/Files/Code/process.js +++ b/api/server/services/Files/Code/process.js @@ -13,6 +13,7 @@ const { getCodeApiAuthHeaders, withCodeApiRateLimit, classifyCodeArtifact, + isMissingSandboxPathError, parseSandboxImageChunk, readWindowedSandboxImage, createCodeApiRateLimitBudget, @@ -27,6 +28,7 @@ const { getCodeExecutionBaseUrl, buildCodeEnvDownloadQuery, codeExecutionHeaders, + executeWorkspaceTool, claimCodeDestination, createCodeDestinationSet, CODE_OUTPUT_PREFLIGHT_MAX_BYTES, @@ -1617,9 +1619,10 @@ async function readSandboxFile({ postData.files = files; } + let response; try { const authHeaders = await getCodeApiAuthHeaders(req, bridgeWorkerId); - const response = await axios({ + response = await axios({ method: 'post', url: `${baseURL}/exec`, data: postData, @@ -1633,14 +1636,6 @@ async function readSandboxFile({ httpsAgent: codeServerHttpsAgent, timeout: 15000, }); - const result = response?.data ?? {}; - if (result.stderr && (result.stdout == null || result.stdout === '')) { - throw new Error(String(result.stderr).trim()); - } - if (result.stdout == null) { - return null; - } - return { content: String(result.stdout) }; } catch (error) { logAxiosError({ message: `Error reading sandbox file "${file_path}"`, @@ -1648,6 +1643,250 @@ async function readSandboxFile({ }); throw error; } + + const result = response?.data ?? {}; + if (result.stderr && (result.stdout == null || result.stdout === '')) { + const reason = String(result.stderr).trim(); + /** An absent path is the ordinary outcome, not a fault: `create_file` + * reads its target before writing so it can tell a create from an + * overwrite. Logging that at error level with a stack made every + * file creation look like a file that had gone missing. */ + if (isMissingSandboxPathError(reason)) { + logger.debug(`[readSandboxFile] "${file_path}" is not present in the sandbox: ${reason}`); + } else { + logger.error(`[readSandboxFile] Error reading sandbox file "${file_path}": ${reason}`); + } + throw new Error(reason); + } + if (result.stdout == null) { + return null; + } + return { content: String(result.stdout) }; +} + +/** + * Reads a bounded range from the workspace directory registered by an attached worker. + * The authenticated worker route is derived from the selected environment and + * the host path remains private to the worker. + * + * @param {Object} params + * @param {string} params.file_path + * @param {string} params.workspace_id + * @param {number} params.start_line + * @param {number} params.max_lines + * @param {string} params.codeApiBaseUrl + * @param {'default' | 'stateful'} params.executionProfile + * @param {string} [params.bridgeWorkerId] + * @param {ServerRequest} [params.req] + * @param {AbortSignal} [params.signal] + */ +async function readWorkspaceFile({ + file_path, + workspace_id, + start_line, + max_lines, + codeApiBaseUrl, + executionProfile, + bridgeWorkerId, + req, + signal, +}) { + const authHeaders = await getCodeApiAuthHeaders(req, bridgeWorkerId); + return executeWorkspaceTool({ + baseURL: codeApiBaseUrl, + authHeaders: { + ...authHeaders, + ...codeExecutionHeaders({ executionProfile, bridgeWorkerId }), + }, + request: { + protocolVersion: 1, + operation: 'read_file', + workspaceId: workspace_id, + path: file_path, + startLine: start_line, + maxLines: max_lines, + }, + ...(signal ? { signal } : {}), + }); +} + +/** + * Searches literal text within the workspace directory registered by an attached worker. + * + * @param {Object} params + * @param {string} params.query + * @param {string} params.workspace_id + * @param {string} [params.path] + * @param {number} params.max_results + * @param {string} params.codeApiBaseUrl + * @param {'default' | 'stateful'} params.executionProfile + * @param {string} [params.bridgeWorkerId] + * @param {ServerRequest} [params.req] + * @param {AbortSignal} [params.signal] + */ +async function searchWorkspace({ + query, + workspace_id, + path, + max_results, + codeApiBaseUrl, + executionProfile, + bridgeWorkerId, + req, + signal, +}) { + const authHeaders = await getCodeApiAuthHeaders(req, bridgeWorkerId); + return executeWorkspaceTool({ + baseURL: codeApiBaseUrl, + authHeaders: { + ...authHeaders, + ...codeExecutionHeaders({ executionProfile, bridgeWorkerId }), + }, + request: { + protocolVersion: 1, + operation: 'search_text', + workspaceId: workspace_id, + query, + ...(path ? { path } : {}), + maxResults: max_results, + }, + ...(signal ? { signal } : {}), + }); +} + +/** + * Lists relative files within the workspace directory registered by an attached worker. + * + * @param {Object} params + * @param {string} params.workspace_id + * @param {string} [params.path] + * @param {string} [params.after_path] + * @param {number} params.max_results + * @param {string} params.codeApiBaseUrl + * @param {'default' | 'stateful'} params.executionProfile + * @param {string} [params.bridgeWorkerId] + * @param {ServerRequest} [params.req] + * @param {AbortSignal} [params.signal] + */ +async function listWorkspaceFiles({ + workspace_id, + path, + after_path, + max_results, + codeApiBaseUrl, + executionProfile, + bridgeWorkerId, + req, + signal, +}) { + const authHeaders = await getCodeApiAuthHeaders(req, bridgeWorkerId); + return executeWorkspaceTool({ + baseURL: codeApiBaseUrl, + authHeaders: { + ...authHeaders, + ...codeExecutionHeaders({ executionProfile, bridgeWorkerId }), + }, + request: { + protocolVersion: 1, + operation: 'list_files', + workspaceId: workspace_id, + ...(path ? { path } : {}), + ...(after_path ? { afterPath: after_path } : {}), + maxResults: max_results, + }, + ...(signal ? { signal } : {}), + }); +} + +/** Writes a UTF-8 file in the workspace registered by an attached worker. */ +async function writeWorkspaceFile({ + file_path, + content, + overwrite, + workspace_id, + codeApiBaseUrl, + executionProfile, + bridgeWorkerId, + req, + signal, +}) { + const authHeaders = await getCodeApiAuthHeaders(req, bridgeWorkerId); + return executeWorkspaceTool({ + baseURL: codeApiBaseUrl, + authHeaders: { + ...authHeaders, + ...codeExecutionHeaders({ executionProfile, bridgeWorkerId }), + }, + request: { + protocolVersion: 1, + operation: 'write_file', + workspaceId: workspace_id, + path: file_path, + content, + overwrite, + }, + ...(signal ? { signal } : {}), + }); +} + +/** Applies an ordered exact-edit batch in one attached-worker mutation. */ +async function editWorkspaceFile({ + file_path, + edits, + expected_base_sha256, + workspace_id, + codeApiBaseUrl, + executionProfile, + bridgeWorkerId, + req, + signal, +}) { + const authHeaders = await getCodeApiAuthHeaders(req, bridgeWorkerId); + return executeWorkspaceTool({ + baseURL: codeApiBaseUrl, + authHeaders: { + ...authHeaders, + ...codeExecutionHeaders({ executionProfile, bridgeWorkerId }), + }, + request: { + protocolVersion: 1, + operation: 'edit_file', + workspaceId: workspace_id, + path: file_path, + edits, + ...(expected_base_sha256 ? { expectedBaseSha256: expected_base_sha256 } : {}), + }, + ...(signal ? { signal } : {}), + }); +} + +/** Previews an ordered exact-edit batch without mutating the attached workspace. */ +async function previewWorkspaceEdit({ + file_path, + edits, + workspace_id, + codeApiBaseUrl, + executionProfile, + bridgeWorkerId, + req, + signal, +}) { + const authHeaders = await getCodeApiAuthHeaders(req, bridgeWorkerId); + return executeWorkspaceTool({ + baseURL: codeApiBaseUrl, + authHeaders: { + ...authHeaders, + ...codeExecutionHeaders({ executionProfile, bridgeWorkerId }), + }, + request: { + protocolVersion: 1, + operation: 'preview_edit', + workspaceId: workspace_id, + path: file_path, + edits, + }, + ...(signal ? { signal } : {}), + }); } /** @@ -1916,6 +2155,12 @@ module.exports = { getSessionInfo, processCodeOutput, prepareCodeOutputForInspection, + readWorkspaceFile, + searchWorkspace, + listWorkspaceFiles, + writeWorkspaceFile, + previewWorkspaceEdit, + editWorkspaceFile, readSandboxFile, readSandboxImage, writeSandboxFile, diff --git a/api/server/services/Files/Code/process.spec.js b/api/server/services/Files/Code/process.spec.js index 15b7fa9d3b7..aef4233c37c 100644 --- a/api/server/services/Files/Code/process.spec.js +++ b/api/server/services/Files/Code/process.spec.js @@ -48,6 +48,7 @@ const mockExtractCodeArtifactInspectionText = jest.fn(async () => ({ complete: false, })); const mockExtractCodeArtifactText = jest.fn(async () => null); +const mockExecuteWorkspaceTool = jest.fn(); const mockGetExtractedTextFormat = jest.fn((_name, _mime, text) => (text == null ? null : 'text')); /* `hasOfficeHtmlPath` gates the persist-then-render split: when true, processCodeOutput * returns `{ file, finalize }` with the file persisted at `status: 'pending'` @@ -66,10 +67,17 @@ jest.mock('@librechat/api', () => { const https = require('https'); return { logAxiosError: jest.fn(), + /* Behaviourally identical to the real predicate in + * `packages/api/src/files/code/errors.ts`, which owns the contract and + * its own spec. Inlined because this factory replaces the whole module + * and `requireActual` here would load the entire package. */ + isMissingSandboxPathError: (reason) => + /no such file or directory|cannot access|cannot find the path|enoent/i.test(reason), getBasePath: jest.fn(() => ''), sanitizeArtifactPath: jest.fn((name) => name), flattenArtifactPath: jest.fn((name) => name.replace(/\//g, '__')), createAxiosInstance: jest.fn(() => mockAxios), + executeWorkspaceTool: (...args) => mockExecuteWorkspaceTool(...args), getCodeApiAuthHeaders: jest.fn(async () => ({})), /* Windowing, sizing and rate-limit policy are real code in * `packages/api` with their own tests (`files/code/image.spec.ts`, @@ -240,6 +248,12 @@ const { prepareCodeOutputForInspection, getSessionInfo, readSandboxFile, + readWorkspaceFile, + searchWorkspace, + listWorkspaceFiles, + writeWorkspaceFile, + previewWorkspaceEdit, + editWorkspaceFile, readSandboxImage, writeSandboxFile, primeFiles, @@ -1832,6 +1846,282 @@ describe('Code Process', () => { }); }); + describe('readWorkspaceFile', () => { + it('forwards authenticated reads to the selected attached worker', async () => { + const controller = new AbortController(); + const result = { + protocolVersion: 1, + operation: 'read_file', + workspaceId: 'primary', + path: 'src/app.ts', + content: 'const ready = true;', + startLine: 1, + endLine: 1, + truncated: false, + }; + getCodeApiAuthHeaders.mockResolvedValueOnce({ Authorization: 'Bearer workspace-token' }); + mockExecuteWorkspaceTool.mockResolvedValueOnce(result); + + await expect( + readWorkspaceFile({ + file_path: 'src/app.ts', + workspace_id: 'primary', + start_line: 1, + max_lines: 200, + codeApiBaseUrl: 'https://attached-code.example.com/v1', + executionProfile: 'stateful', + bridgeWorkerId: 'worker-user-1', + req: mockReq, + signal: controller.signal, + }), + ).resolves.toBe(result); + + expect(getCodeApiAuthHeaders).toHaveBeenCalledWith(mockReq, 'worker-user-1'); + expect(mockExecuteWorkspaceTool).toHaveBeenCalledWith({ + baseURL: 'https://attached-code.example.com/v1', + authHeaders: { + Authorization: 'Bearer workspace-token', + 'X-CodeAPI-Expected-Profile': 'stateful', + 'X-LibreChat-Code-Worker-ID': 'worker-user-1', + }, + request: { + protocolVersion: 1, + operation: 'read_file', + workspaceId: 'primary', + path: 'src/app.ts', + startLine: 1, + maxLines: 200, + }, + signal: controller.signal, + }); + }); + }); + + describe('searchWorkspace', () => { + it('forwards authenticated literal search to the selected attached worker', async () => { + const controller = new AbortController(); + const result = { + protocolVersion: 1, + operation: 'search_text', + workspaceId: 'primary', + matches: [], + truncated: false, + }; + getCodeApiAuthHeaders.mockResolvedValueOnce({ Authorization: 'Bearer workspace-token' }); + mockExecuteWorkspaceTool.mockResolvedValueOnce(result); + + await expect( + searchWorkspace({ + query: 'needle', + workspace_id: 'primary', + path: 'src', + max_results: 20, + codeApiBaseUrl: 'https://attached-code.example.com/v1', + executionProfile: 'stateful', + bridgeWorkerId: 'worker-user-1', + req: mockReq, + signal: controller.signal, + }), + ).resolves.toBe(result); + + expect(mockExecuteWorkspaceTool).toHaveBeenCalledWith({ + baseURL: 'https://attached-code.example.com/v1', + authHeaders: { + Authorization: 'Bearer workspace-token', + 'X-CodeAPI-Expected-Profile': 'stateful', + 'X-LibreChat-Code-Worker-ID': 'worker-user-1', + }, + request: { + protocolVersion: 1, + operation: 'search_text', + workspaceId: 'primary', + query: 'needle', + path: 'src', + maxResults: 20, + }, + signal: controller.signal, + }); + }); + }); + + describe('listWorkspaceFiles', () => { + it('forwards authenticated listing to the selected attached worker', async () => { + const controller = new AbortController(); + const result = { + protocolVersion: 1, + operation: 'list_files', + workspaceId: 'primary', + paths: ['src/app.ts'], + truncated: false, + }; + getCodeApiAuthHeaders.mockResolvedValueOnce({ Authorization: 'Bearer workspace-token' }); + mockExecuteWorkspaceTool.mockResolvedValueOnce(result); + + await expect( + listWorkspaceFiles({ + workspace_id: 'primary', + path: 'src', + after_path: 'src/app.ts', + max_results: 20, + codeApiBaseUrl: 'https://attached-code.example.com/v1', + executionProfile: 'stateful', + bridgeWorkerId: 'worker-user-1', + req: mockReq, + signal: controller.signal, + }), + ).resolves.toBe(result); + + expect(mockExecuteWorkspaceTool).toHaveBeenCalledWith({ + baseURL: 'https://attached-code.example.com/v1', + authHeaders: { + Authorization: 'Bearer workspace-token', + 'X-CodeAPI-Expected-Profile': 'stateful', + 'X-LibreChat-Code-Worker-ID': 'worker-user-1', + }, + request: { + protocolVersion: 1, + operation: 'list_files', + workspaceId: 'primary', + path: 'src', + afterPath: 'src/app.ts', + maxResults: 20, + }, + signal: controller.signal, + }); + }); + }); + + describe('workspace mutations', () => { + it('forwards create-only writes to the selected attached worker', async () => { + const controller = new AbortController(); + const result = { + protocolVersion: 1, + operation: 'write_file', + workspaceId: 'primary', + path: 'src/new.ts', + created: true, + bytesWritten: 5, + }; + getCodeApiAuthHeaders.mockResolvedValueOnce({ Authorization: 'Bearer workspace-token' }); + mockExecuteWorkspaceTool.mockResolvedValueOnce(result); + + await expect( + writeWorkspaceFile({ + file_path: 'src/new.ts', + content: 'ready', + overwrite: false, + workspace_id: 'primary', + codeApiBaseUrl: 'https://attached-code.example.com/v1', + executionProfile: 'stateful', + bridgeWorkerId: 'worker-user-1', + req: mockReq, + signal: controller.signal, + }), + ).resolves.toBe(result); + + expect(mockExecuteWorkspaceTool).toHaveBeenCalledWith({ + baseURL: 'https://attached-code.example.com/v1', + authHeaders: { + Authorization: 'Bearer workspace-token', + 'X-CodeAPI-Expected-Profile': 'stateful', + 'X-LibreChat-Code-Worker-ID': 'worker-user-1', + }, + request: { + protocolVersion: 1, + operation: 'write_file', + workspaceId: 'primary', + path: 'src/new.ts', + content: 'ready', + overwrite: false, + }, + signal: controller.signal, + }); + }); + + it('forwards an edit batch as one attached-worker request', async () => { + const result = { + protocolVersion: 1, + operation: 'edit_file', + workspaceId: 'primary', + path: 'src/app.ts', + replacements: 2, + bytesWritten: 20, + }; + getCodeApiAuthHeaders.mockResolvedValueOnce({ Authorization: 'Bearer workspace-token' }); + mockExecuteWorkspaceTool.mockResolvedValueOnce(result); + + await expect( + editWorkspaceFile({ + file_path: 'src/app.ts', + edits: [ + { oldText: 'draft', newText: 'ready' }, + { oldText: 'false', newText: 'true' }, + ], + workspace_id: 'primary', + codeApiBaseUrl: 'https://attached-code.example.com/v1', + executionProfile: 'stateful', + bridgeWorkerId: 'worker-user-1', + req: mockReq, + expected_base_sha256: 'a'.repeat(64), + }), + ).resolves.toBe(result); + + expect(mockExecuteWorkspaceTool).toHaveBeenCalledWith( + expect.objectContaining({ + request: { + protocolVersion: 1, + operation: 'edit_file', + workspaceId: 'primary', + path: 'src/app.ts', + edits: [ + { oldText: 'draft', newText: 'ready' }, + { oldText: 'false', newText: 'true' }, + ], + expectedBaseSha256: 'a'.repeat(64), + }, + }), + ); + }); + + it('forwards a non-mutating attached-workspace edit preview', async () => { + const result = { + protocolVersion: 1, + operation: 'preview_edit', + workspaceId: 'primary', + path: 'src/app.ts', + content: 'prefix SECRET', + hasUtf8Bom: false, + baseSha256: 'b'.repeat(64), + replacements: 1, + bytesWritten: 13, + }; + mockExecuteWorkspaceTool.mockResolvedValueOnce(result); + + await expect( + previewWorkspaceEdit({ + file_path: 'src/app.ts', + edits: [{ oldText: ' SEC', newText: ' SECRET' }], + workspace_id: 'primary', + codeApiBaseUrl: 'https://attached-code.example.com/v1', + executionProfile: 'stateful', + req: mockReq, + }), + ).resolves.toBe(result); + + expect(mockExecuteWorkspaceTool).toHaveBeenCalledWith( + expect.objectContaining({ + request: { + protocolVersion: 1, + operation: 'preview_edit', + workspaceId: 'primary', + path: 'src/app.ts', + edits: [{ oldText: ' SEC', newText: ' SECRET' }], + }, + }), + ); + }); + }); + describe('readSandboxFile', () => { /** * `readSandboxFile` shells `cat ` through the sandbox @@ -2071,6 +2361,43 @@ describe('Code Process', () => { }), ); }); + + /* `create_file` reads its target before writing so it can tell a create + * from an overwrite, so an absent path is the ordinary case. Reported at + * error level it made every file creation look like a lost file — and + * `logAxiosError` rendered the sandbox's own stderr as "An error occurred + * while setting up the request", which reads as a transport fault. */ + it('logs an absent path at debug, not through the error-level axios logger', async () => { + const { logAxiosError } = require('@librechat/api'); + mockAxios.mockResolvedValueOnce({ + data: { stdout: '', stderr: 'cat: /mnt/data/SKILL.md: No such file or directory\n' }, + }); + + await expect(readSandboxFile({ file_path: '/mnt/data/SKILL.md' })).rejects.toThrow( + 'No such file or directory', + ); + + expect(logAxiosError).not.toHaveBeenCalled(); + expect(logger.error).not.toHaveBeenCalled(); + expect(logger.debug).toHaveBeenCalledWith(expect.stringContaining('/mnt/data/SKILL.md')); + }); + + it('still reports a non-missing sandbox read failure at error level', async () => { + const { logAxiosError } = require('@librechat/api'); + mockAxios.mockResolvedValueOnce({ + data: { stdout: '', stderr: 'cat: /mnt/data/x.txt: Permission denied\n' }, + }); + + await expect(readSandboxFile({ file_path: '/mnt/data/x.txt' })).rejects.toThrow( + 'Permission denied', + ); + + expect(logger.debug).not.toHaveBeenCalled(); + expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('Permission denied')); + /* The sandbox answered; nothing about the request failed, so the + * axios-shaped logger would misattribute this to the transport. */ + expect(logAxiosError).not.toHaveBeenCalled(); + }); }); describe('timeout', () => { diff --git a/api/server/services/Files/images/resize.js b/api/server/services/Files/images/resize.js index c2cdaacb63a..6cbbb16c53c 100644 --- a/api/server/services/Files/images/resize.js +++ b/api/server/services/Files/images/resize.js @@ -1,4 +1,5 @@ const sharp = require('sharp'); +const { resolveImageMimeType } = require('@librechat/api'); const { EModelEndpoint } = require('librechat-data-provider'); /** @@ -10,7 +11,9 @@ const { EModelEndpoint } = require('librechat-data-provider'); * 'high' for a maximum of 768x2000 resolution, * or a custom object with percentage or px values. * @param {EModelEndpoint} endpoint - Identifier for specific endpoint handling - * @returns {Promise<{buffer: Buffer, width: number, height: number}>} An object containing the resized image buffer and its dimensions. + * @returns {Promise<{buffer: Buffer, bytes: number, width: number, height: number, type: string | undefined}>} + * The resized image buffer, its dimensions, and the media type of the re-encoded bytes. + * `type` is `undefined` when sharp reports a format with no media type of its own. * @throws Will throw an error if the resolution parameter is invalid. */ async function resizeImageBuffer(inputBuffer, resolution, endpoint) { @@ -86,6 +89,7 @@ async function resizeImageBuffer(inputBuffer, resolution, endpoint) { bytes: resizedMetadata.size, width: resizedMetadata.width, height: resizedMetadata.height, + type: resolveImageMimeType(resizedMetadata), }; } diff --git a/api/server/services/Files/process.js b/api/server/services/Files/process.js index 325645a28bc..c4e7d307b36 100644 --- a/api/server/services/Files/process.js +++ b/api/server/services/Files/process.js @@ -1314,7 +1314,14 @@ async function saveBase64Image( const effectiveResolution = resolution ?? appConfig.fileConfig?.imageGeneration ?? 'high'; const file_id = _file_id ?? v4(); let filename = `${file_id}-${_filename}`; - const { buffer: inputBuffer, type } = base64ToBuffer(url); + const { buffer: inputBuffer, type: declaredType } = base64ToBuffer(url); + + const image = await resizeImageBuffer(inputBuffer, effectiveResolution, endpoint); + /** Sharp re-encodes what it resizes, so the bytes being saved are not necessarily in the + * format the data URL declared — an SVG arrives here and is rasterized to PNG. The record has + * to describe the bytes, because `file.type` is handed to providers verbatim as `media_type` + * (Anthropic), `inlineData.mimeType` (Google), and the `data:` prefix (OpenAI). */ + const type = image.type ?? declaredType; if (!path.extname(_filename)) { const extension = mime.getExtension(type); if (extension) { @@ -1323,8 +1330,6 @@ async function saveBase64Image( throw new Error(`Could not determine file extension from MIME type: ${type}`); } } - - const image = await resizeImageBuffer(inputBuffer, effectiveResolution, endpoint); const source = getFileStrategy(appConfig, { isImage: true }); const { saveBuffer } = getStrategyFunctions(source); const filepath = await saveBuffer({ diff --git a/api/server/services/Files/saveBase64Image.spec.js b/api/server/services/Files/saveBase64Image.spec.js new file mode 100644 index 00000000000..58c6e6b4abe --- /dev/null +++ b/api/server/services/Files/saveBase64Image.spec.js @@ -0,0 +1,103 @@ +const sharp = require('sharp'); +const { resolveImageMimeType } = require('@librechat/api'); + +jest.mock('~/models', () => ({ createFile: jest.fn(async (doc) => doc) })); +jest.mock('~/server/utils/getFileStrategy', () => ({ + getFileStrategy: jest.fn(() => 'local'), +})); +jest.mock('~/server/services/Files/strategies', () => ({ + getStrategyFunctions: jest.fn(() => ({ + saveBuffer: jest.fn(async ({ fileName }) => `/images/user-1/${fileName}`), + })), +})); +jest.mock('./retention', () => ({ + getRetentionExpiry: jest.fn(async () => ({})), + getAgentFileRetentionExpiry: jest.fn(async () => ({})), +})); +jest.mock('./Audio/STTService', () => ({ STTService: class {} })); +jest.mock('~/server/controllers/assistants/v2', () => ({ + addResourceFileId: jest.fn(), + deleteResourceFileId: jest.fn(), +})); +jest.mock('~/server/controllers/assistants/helpers', () => ({ getOpenAIClient: jest.fn() })); +jest.mock('~/server/services/Tools/credentials', () => ({ loadAuthValues: jest.fn() })); +jest.mock('~/server/services/Config', () => ({ checkCapability: jest.fn() })); +jest.mock('~/server/utils/queue', () => ({ LB_QueueAsyncCall: jest.fn() })); + +const db = require('~/models'); +const { saveBase64Image } = require('./process'); + +const req = { + user: { id: 'user-1', tenantId: null }, + config: { fileConfig: { imageGeneration: 'high' }, paths: {} }, +}; + +const dataUrl = (mimeType, buffer) => `data:${mimeType};base64,${buffer.toString('base64')}`; + +const save = (url) => + saveBase64Image(url, { + req, + file_id: 'file-1', + filename: 'tool_img_abc', + endpoint: 'openAI', + context: 'image_generation', + }); + +describe('saveBase64Image records the media type of the bytes it saves', () => { + it('records PNG for an SVG that sharp rasterizes', async () => { + const svg = Buffer.from( + '' + + '', + ); + + const file = await save(dataUrl('image/svg+xml', svg)); + + expect(file.type).toBe('image/png'); + expect(file.filename).toBe('file-1-tool_img_abc.png'); + expect(db.createFile).toHaveBeenCalledWith( + expect.objectContaining({ type: 'image/png' }), + true, + ); + }); + + it('leaves a genuine PNG labeled as PNG', async () => { + const png = await sharp({ + create: { width: 40, height: 20, channels: 4, background: { r: 1, g: 2, b: 3, alpha: 1 } }, + }) + .png() + .toBuffer(); + + const file = await save(dataUrl('image/png', png)); + + expect(file.type).toBe('image/png'); + expect(file.filename).toBe('file-1-tool_img_abc.png'); + }); + + it('corrects a WebP mislabeled by the producer as PNG', async () => { + const webp = await sharp({ + create: { width: 40, height: 20, channels: 3, background: { r: 1, g: 2, b: 3 } }, + }) + .webp() + .toBuffer(); + + const file = await save(dataUrl('image/png', webp)); + + expect(file.type).toBe('image/webp'); + expect(file.filename).toBe('file-1-tool_img_abc.webp'); + }); + + it('records a type that matches the bytes actually written to storage', async () => { + const svg = Buffer.from( + '' + + '', + ); + + const file = await save(dataUrl('image/svg+xml', svg)); + + const { getStrategyFunctions } = require('~/server/services/Files/strategies'); + const { saveBuffer } = getStrategyFunctions.mock.results.at(-1).value; + const savedBuffer = saveBuffer.mock.calls.at(-1)[0].buffer; + + expect(file.type).toBe(resolveImageMimeType(await sharp(savedBuffer).metadata())); + }); +}); diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index c5d4ac0950e..61fb486acb0 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -45,8 +45,11 @@ const { AGENT_EXPECTED_MCP_TOOLS_UNAVAILABLE, isFatalAgentInitializationError, codeExecutionAuthHeaders, + createAttachedWorkspaceBashTool, resolveCodeExecutionContext, resolveCallerCapabilityProjectionSnapshot, + LIST_WORKSPACE_FILES_TOOL_NAME, + SEARCH_WORKSPACE_TOOL_NAME, getTransactionsConfig, } = require('@librechat/api'); const { @@ -196,7 +199,7 @@ const prepareActionSnapshotForTools = async ({ agentId, toolNames, filters, decr if (!toolNames.some((toolName) => isActionTool(toolName))) { return null; } - const storedActions = (await loadActionSets({ agent_id: agentId })) ?? []; + const storedActions = (await loadActionSets({ agentId })) ?? []; if (storedActions.length === 0) { return { storedActions, actionSets: [] }; } @@ -515,7 +518,7 @@ async function processRequiredActions(client, requiredActions) { /** @type {Action[]} */ const storedActions = (await loadActionSets({ - assistant_id: client.req.body.assistant_id, + assistantId: client.req.body.assistant_id, })) ?? []; const actionSets = await prepareStoredActionsForUse({ actions: storedActions, @@ -2000,7 +2003,13 @@ async function loadToolsForExecution({ const isPTCRequested = ptcToolNames.length > 0; const isBashToolRequested = toolNames.includes(AgentConstants.BASH_TOOL); const isLegacyExecuteCodeRequested = toolNames.includes(Tools.execute_code); - const isCodeExecutionToolRequested = isBashToolRequested || isLegacyExecuteCodeRequested; + const isWorkspaceSearchRequested = toolNames.includes(SEARCH_WORKSPACE_TOOL_NAME); + const isWorkspaceListRequested = toolNames.includes(LIST_WORKSPACE_FILES_TOOL_NAME); + const isCodeExecutionToolRequested = + isBashToolRequested || + isLegacyExecuteCodeRequested || + isWorkspaceSearchRequested || + isWorkspaceListRequested; const isSkillToolRequested = toolNames.includes(AgentConstants.SKILL_TOOL); const isSandboxFileToolRequested = toolNames.some((name) => [AgentConstants.READ_FILE, AgentConstants.CREATE_FILE, AgentConstants.EDIT_FILE].includes(name), @@ -2114,14 +2123,21 @@ async function loadToolsForExecution({ } if (isBashTool) { try { - const bashTool = createBashExecutionTool({ - authHeaders: () => - codeExecutionAuthHeaders( - (bridgeWorkerId) => getCodeApiAuthHeaders(req, bridgeWorkerId), - codeExecutionContext, - ), - ...codeExecutionContext, - }); + const authHeaders = () => + codeExecutionAuthHeaders( + (bridgeWorkerId) => getCodeApiAuthHeaders(req, bridgeWorkerId), + codeExecutionContext, + ); + const bashTool = + codeExecutionContext.environmentType === 'attached' + ? createAttachedWorkspaceBashTool({ + authHeaders, + baseUrl: codeExecutionContext.baseUrl, + }) + : createBashExecutionTool({ + authHeaders, + ...codeExecutionContext, + }); allLoadedTools.push(bashTool); } catch (error) { logger.error( diff --git a/api/server/services/__tests__/ToolService.spec.js b/api/server/services/__tests__/ToolService.spec.js index fe7506252a7..825ed48356f 100644 --- a/api/server/services/__tests__/ToolService.spec.js +++ b/api/server/services/__tests__/ToolService.spec.js @@ -18,6 +18,7 @@ const mockGetMCPServerTools = jest.fn(); const mockGetCachedTools = jest.fn(); const mockSendEvent = jest.fn(); const mockEmitChunk = jest.fn(); +const mockCreateAttachedWorkspaceBashTool = jest.fn(() => ({ name: AgentConstants.BASH_TOOL })); const mockResolveCodeExecutionContext = jest.fn( ({ statefulSessions, environment, userId, agentId, conversationId }) => { if (!statefulSessions) { @@ -80,6 +81,7 @@ jest.mock('@librechat/api', () => ({ emitChunk: (...args) => mockEmitChunk(...args), }, resolveCodeExecutionContext: (...args) => mockResolveCodeExecutionContext(...args), + createAttachedWorkspaceBashTool: (...args) => mockCreateAttachedWorkspaceBashTool(...args), })); const mockLoadToolsUtil = jest.fn(); @@ -2328,7 +2330,7 @@ describe('ToolService - Action Capability Gating', () => { definitionsOnly: false, }); - expect(mockLoadActionSets).toHaveBeenCalledWith({ agent_id: 'agent_123' }); + expect(mockLoadActionSets).toHaveBeenCalledWith({ agentId: 'agent_123' }); }); it('blocks persisted action metadata before generic tool initialization', async () => { @@ -2565,6 +2567,113 @@ describe('ToolService - Action Capability Gating', () => { } }); + it('preserves attached routing when workspace search is the only requested tool', async () => { + const capabilities = [ + AgentCapabilities.tools, + AgentCapabilities.execute_code, + AgentCapabilities.stateful_code_sessions, + ]; + const req = createMockReq(capabilities); + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + process.env.LIBRECHAT_CODE_BASEURL_STATEFUL = 'http://code-stateful.test/v1'; + + try { + const result = await loadToolsForExecution({ + req, + res: {}, + agent: { + id: 'stateful-agent', + tools: [Tools.execute_code], + stateful_code_sessions: true, + stateful_code_environment: 'agent-user', + }, + toolNames: ['search_workspace'], + actionsEnabled: false, + }); + + expect(result.configurable.codeExecutionContext.executionProfile).toBe('stateful'); + expect(mockResolveCodeExecutionContext).toHaveBeenLastCalledWith( + expect.objectContaining({ statefulSessions: true, environment: 'agent-user' }), + ); + } finally { + delete process.env.LIBRECHAT_CODE_BASEURL_STATEFUL; + } + }); + + it('preserves attached routing when workspace listing is the only requested tool', async () => { + const capabilities = [ + AgentCapabilities.tools, + AgentCapabilities.execute_code, + AgentCapabilities.stateful_code_sessions, + ]; + const req = createMockReq(capabilities); + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + process.env.LIBRECHAT_CODE_BASEURL_STATEFUL = 'http://code-stateful.test/v1'; + + try { + const result = await loadToolsForExecution({ + req, + res: {}, + agent: { + id: 'stateful-agent', + tools: [Tools.execute_code], + stateful_code_sessions: true, + stateful_code_environment: 'agent-user', + }, + toolNames: ['list_workspace_files'], + actionsEnabled: false, + }); + + expect(result.configurable.codeExecutionContext.executionProfile).toBe('stateful'); + expect(mockResolveCodeExecutionContext).toHaveBeenLastCalledWith( + expect.objectContaining({ statefulSessions: true, environment: 'agent-user' }), + ); + } finally { + delete process.env.LIBRECHAT_CODE_BASEURL_STATEFUL; + } + }); + + it('loads bash execution through the attached worker transport', async () => { + const capabilities = [ + AgentCapabilities.tools, + AgentCapabilities.execute_code, + AgentCapabilities.stateful_code_sessions, + ]; + const req = createMockReq(capabilities); + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + mockResolveCodeExecutionContext.mockReturnValueOnce({ + baseUrl: 'http://attached-code.test/v1', + codeSessionKey: 'execute_code:stateful:attached', + executionProfile: 'stateful', + statefulSessions: true, + environmentType: 'attached', + bridgeWorkerId: 'worker-abc', + }); + const toolRegistry = new Map([ + [AgentConstants.BASH_TOOL, { name: AgentConstants.BASH_TOOL }], + ]); + + const result = await loadToolsForExecution({ + req, + res: {}, + agent: { + id: 'attached-agent', + tools: [Tools.execute_code], + stateful_code_sessions: true, + stateful_code_environment: 'agent-user', + }, + toolNames: [AgentConstants.BASH_TOOL], + toolRegistry, + actionsEnabled: false, + }); + + expect(mockCreateAttachedWorkspaceBashTool).toHaveBeenCalledWith({ + authHeaders: expect.any(Function), + baseUrl: 'http://attached-code.test/v1', + }); + expect(result.loadedTools).toContainEqual({ name: AgentConstants.BASH_TOOL }); + }); + it('resolves stateful routing when handle_skill is the only requested tool', async () => { const capabilities = [ AgentCapabilities.tools, @@ -2904,7 +3013,7 @@ describe('ToolService - Action Capability Gating', () => { actionsEnabled: true, }); - expect(mockLoadActionSets).toHaveBeenCalledWith({ agent_id: 'agent_123' }); + expect(mockLoadActionSets).toHaveBeenCalledWith({ agentId: 'agent_123' }); }); it('blocks decrypted persisted action secrets before execution-time domain work', async () => { diff --git a/api/server/telemetry.js b/api/server/telemetry.js index cb0058355b0..0611c9c7b57 100644 --- a/api/server/telemetry.js +++ b/api/server/telemetry.js @@ -5,7 +5,9 @@ function isTruthy(value) { } function isTelemetryEnabled() { - return isTruthy(process.env.OTEL_TRACING_ENABLED) && !isTruthy(process.env.OTEL_SDK_DISABLED); + const signalEnabled = + isTruthy(process.env.OTEL_TRACING_ENABLED) || isTruthy(process.env.OTEL_LOGS_ENABLED); + return signalEnabled && !isTruthy(process.env.OTEL_SDK_DISABLED); } if (isTelemetryEnabled()) { diff --git a/api/server/telemetry.spec.js b/api/server/telemetry.spec.js index 8f848da8e76..701ca8d226e 100644 --- a/api/server/telemetry.spec.js +++ b/api/server/telemetry.spec.js @@ -4,6 +4,7 @@ describe('telemetry bootstrap', () => { beforeEach(() => { jest.resetModules(); process.env = { ...originalEnv }; + delete process.env.OTEL_LOGS_ENABLED; delete process.env.OTEL_SDK_DISABLED; delete process.env.OTEL_TRACING_ENABLED; jest.doMock('dotenv', () => ({ @@ -34,6 +35,7 @@ describe('telemetry bootstrap', () => { }); it('does not load OpenTelemetry packages when the SDK is disabled', () => { + process.env.OTEL_LOGS_ENABLED = 'true'; process.env.OTEL_SDK_DISABLED = 'true'; process.env.OTEL_TRACING_ENABLED = 'true'; jest.doMock( @@ -88,4 +90,28 @@ describe('telemetry bootstrap', () => { expect(telemetry.enabled).toBe(false); expect(telemetry.status).toBe('failed'); }); + + it('initializes telemetry when only logs are enabled', () => { + process.env.OTEL_LOGS_ENABLED = 'true'; + const initializeTelemetry = jest.fn(() => ({ + enabled: true, + status: 'started', + shutdown: jest.fn(), + })); + jest.doMock( + '@librechat/api/telemetry', + () => ({ + initializeTelemetry, + telemetryMiddleware: jest.fn(), + telemetryErrorMiddleware: jest.fn(), + }), + { virtual: true }, + ); + + const telemetry = require('./telemetry'); + + expect(initializeTelemetry).toHaveBeenCalledTimes(1); + expect(telemetry.enabled).toBe(true); + expect(telemetry.status).toBe('started'); + }); }); diff --git a/client/jest.config.cjs b/client/jest.config.cjs index ed7ce852e87..7a32adf945e 100644 --- a/client/jest.config.cjs +++ b/client/jest.config.cjs @@ -49,7 +49,7 @@ module.exports = { 'jest-file-loader', }, transformIgnorePatterns: [ - '/node_modules/(?!(@ariakit/react-components|@ariakit/react-utils|@ariakit/react-store|@ariakit/components|@ariakit/store|@ariakit/utils|@zattoo/use-double-click|@dicebear|@react-dnd|react-dnd.*|dnd-core|filenamify|filename-reserved-regex|heic-to|lowlight|highlight\\.js|fault|react-markdown|unified|bail|trough|devlop|is-.*|parse-entities|stringify-entities|character-.*|trim-lines|style-to-object|inline-style-parser|html-url-attributes|escape-string-regexp|longest-streak|zwitch|ccount|markdown-table|comma-separated-tokens|space-separated-tokens|web-namespaces|property-information|remark-.*|rehype-.*|recma-.*|hast.*|mdast-.*|unist-.*|vfile.*|micromark.*|estree-util-.*|decode-named-character-reference)/)/', + '/node_modules/(?!(@ariakit/react-components|@ariakit/react-utils|@ariakit/react-store|@ariakit/components|@ariakit/store|@ariakit/utils|@zattoo/use-double-click|@dicebear|@react-dnd|react-dnd.*|dnd-core|filenamify|filename-reserved-regex|heic-to|lowlight|highlight\\.js|fault|lucide-react|lucide|morphicons|react-markdown|unified|bail|trough|devlop|is-.*|parse-entities|stringify-entities|character-.*|trim-lines|style-to-object|inline-style-parser|html-url-attributes|escape-string-regexp|longest-streak|zwitch|ccount|markdown-table|comma-separated-tokens|space-separated-tokens|web-namespaces|property-information|remark-.*|rehype-.*|recma-.*|hast.*|mdast-.*|unist-.*|vfile.*|micromark.*|estree-util-.*|decode-named-character-reference)/)/', ], setupFiles: ['/test/polyfills.js'], setupFilesAfterEnv: ['@testing-library/jest-dom/extend-expect', '/test/setupTests.js'], diff --git a/client/package.json b/client/package.json index 3787321f3e7..0e73dbb04c1 100644 --- a/client/package.json +++ b/client/package.json @@ -80,7 +80,8 @@ "js-cookie": "^3.0.5", "librechat-data-provider": "*", "lodash": "^4.17.23", - "lucide-react": "^0.394.0", + "lucide": "^0.525.0", + "lucide-react": "^0.525.0", "match-sorter": "^8.1.0", "mdast-util-directive": "^3.0.0", "mdast-util-from-markdown": "^2.0.1", @@ -94,6 +95,7 @@ "micromark-util-character": "^2.1.0", "micromark-util-symbol": "^2.0.0", "monaco-editor": "^0.56.0", + "morphicons": "^1.4.1", "qrcode.react": "^4.2.0", "rc-input-number": "^7.4.2", "react": "^18.2.0", diff --git a/client/src/components/Agents/AgentDetail.tsx b/client/src/components/Agents/AgentDetail.tsx index 9b26aa83391..b32a0c879b7 100644 --- a/client/src/components/Agents/AgentDetail.tsx +++ b/client/src/components/Agents/AgentDetail.tsx @@ -1,10 +1,12 @@ import React, { useRef } from 'react'; -import { Link, Pin, PinOff } from 'lucide-react'; +import { Link } from 'lucide-react'; +import { Pin, PinOff } from 'lucide'; import { useQueryClient } from '@tanstack/react-query'; import { OGDialog, OGDialogContent, Button, + MorphIcon, TooltipAnchor, useToastContext, } from '@librechat/client'; @@ -17,8 +19,8 @@ import { AgentListResponse, } from 'librechat-data-provider'; import type t from 'librechat-data-provider'; +import { renderAgentAvatar, clearMessagesCache, specDisplayFieldReset } from '~/utils'; import { useLocalize, useDefaultConvo, useFavorites } from '~/hooks'; -import { renderAgentAvatar, clearMessagesCache } from '~/utils'; import { useChatContext } from '~/Providers'; interface SupportContact { @@ -79,6 +81,7 @@ const AgentDetail: React.FC = ({ agent, isOpen, onClose }) => endpoint: EModelEndpoint.agents, agent_id: agent.id, title: localize('com_agents_chat_with', { name: agent.name || localize('com_ui_agent') }), + ...specDisplayFieldReset, }; const currentConvo = getDefaultConversation({ @@ -180,7 +183,7 @@ const AgentDetail: React.FC = ({ agent, isOpen, onClose }) => onClick={handleFavoriteClick} aria-label={isFavorite ? localize('com_ui_unpin') : localize('com_ui_pin')} > - {isFavorite ? : } + } /> diff --git a/client/src/components/Agents/AgentDetailContent.tsx b/client/src/components/Agents/AgentDetailContent.tsx index 8aabe114124..0ea51d4504f 100644 --- a/client/src/components/Agents/AgentDetailContent.tsx +++ b/client/src/components/Agents/AgentDetailContent.tsx @@ -1,7 +1,14 @@ import React from 'react'; -import { Link, Pin, PinOff } from 'lucide-react'; +import { Link } from 'lucide-react'; +import { Pin, PinOff } from 'lucide'; import { useQueryClient } from '@tanstack/react-query'; -import { OGDialogContent, Button, TooltipAnchor, useToastContext } from '@librechat/client'; +import { + Button, + MorphIcon, + TooltipAnchor, + OGDialogContent, + useToastContext, +} from '@librechat/client'; import { QueryKeys, Constants, @@ -11,8 +18,8 @@ import { AgentListResponse, } from 'librechat-data-provider'; import type t from 'librechat-data-provider'; +import { renderAgentAvatar, clearMessagesCache, specDisplayFieldReset } from '~/utils'; import { useLocalize, useDefaultConvo, useFavorites } from '~/hooks'; -import { renderAgentAvatar, clearMessagesCache } from '~/utils'; import { useChatContext } from '~/Providers'; import AgentContact from './AgentContact'; @@ -64,6 +71,7 @@ const AgentDetailContent: React.FC = ({ agent }) => { endpoint: EModelEndpoint.agents, agent_id: agent.id, title: localize('com_agents_chat_with', { name: agent.name || localize('com_ui_agent') }), + ...specDisplayFieldReset, }; const currentConvo = getDefaultConversation({ @@ -128,7 +136,7 @@ const AgentDetailContent: React.FC = ({ agent }) => { onClick={handleFavoriteClick} aria-label={isFavorite ? localize('com_ui_unpin') : localize('com_ui_pin')} > - {isFavorite ? : } + } /> diff --git a/client/src/components/Agents/tests/AgentDetail.spec.tsx b/client/src/components/Agents/tests/AgentDetail.spec.tsx index 3b9919b37f7..b2d29237070 100644 --- a/client/src/components/Agents/tests/AgentDetail.spec.tsx +++ b/client/src/components/Agents/tests/AgentDetail.spec.tsx @@ -49,6 +49,15 @@ jest.mock('~/utils/agents', () => ({ )), })); +jest.mock('~/utils/endpoints', () => ({ + specDisplayFieldReset: { + spec: null, + iconURL: null, + modelLabel: null, + greeting: undefined, + }, +})); + jest.mock('~/Providers', () => ({ useChatContext: jest.fn(), })); @@ -260,10 +269,52 @@ describe('AgentDetail', () => { endpoint: EModelEndpoint.agents, agent_id: 'test-agent-id', title: 'Chat with Test Agent', + spec: null, + iconURL: null, + modelLabel: null, + greeting: undefined, }, }); }); + it('should clear model spec display fields when starting an agent chat', async () => { + const user = userEvent.setup(); + const mockNewConversation = jest.fn(); + const { useChatContext } = require('~/Providers'); + (useChatContext as jest.Mock).mockReturnValue({ + conversation: { + conversationId: 'test-convo-id', + spec: 'ClickHouse Agent', + iconURL: '/images/clickhouse.svg', + modelLabel: 'ClickHouse Agent', + }, + newConversation: mockNewConversation, + }); + const { useDefaultConvo } = require('~/hooks'); + (useDefaultConvo as jest.Mock).mockReturnValue(({ conversation, preset }) => ({ + ...conversation, + ...preset, + })); + + renderWithProviders(); + await user.click(screen.getByRole('button', { name: 'com_agents_start_chat' })); + + expect(mockNewConversation).toHaveBeenCalledWith({ + template: expect.objectContaining({ + endpoint: EModelEndpoint.agents, + agent_id: 'test-agent-id', + spec: null, + iconURL: null, + modelLabel: null, + }), + preset: expect.objectContaining({ + spec: null, + iconURL: null, + modelLabel: null, + }), + }); + }); + it('should not navigate when agent is null', async () => { const user = userEvent.setup(); renderWithProviders(); diff --git a/client/src/components/Agents/tests/AgentDetailContent.spec.tsx b/client/src/components/Agents/tests/AgentDetailContent.spec.tsx index 75d4d490b37..f6e638bec84 100644 --- a/client/src/components/Agents/tests/AgentDetailContent.spec.tsx +++ b/client/src/components/Agents/tests/AgentDetailContent.spec.tsx @@ -1,9 +1,16 @@ import React from 'react'; +import userEvent from '@testing-library/user-event'; import { render, screen } from '@testing-library/react'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import type { TConversation } from 'librechat-data-provider'; import '@testing-library/jest-dom'; +import type t from 'librechat-data-provider'; import AgentDetailContent from '../AgentDetailContent'; +const mockNewConversation = jest.fn(); +let mockConversation: Partial | undefined; +const mockIsFavoriteAgent = jest.fn(() => false); + jest.mock('librechat-data-provider', () => ({ QueryKeys: { agents: 'agents', @@ -23,23 +30,27 @@ jest.mock('librechat-data-provider', () => ({ }, })); -jest.mock('@librechat/client', () => ({ - OGDialogContent: ({ children }: { children: React.ReactNode }) => ( -
{children}
- ), - Button: ({ children, ...props }: React.ButtonHTMLAttributes) => ( - - ), - TooltipAnchor: ({ render }: { render: React.ReactNode }) => render, - useToastContext: () => ({ - showToast: jest.fn(), - }), -})); +jest.mock('@librechat/client', () => { + const { createPinMorphIconMock } = jest.requireActual('~/../test/mockMorphIcon'); + return { + OGDialogContent: ({ children }: { children: React.ReactNode }) => ( +
{children}
+ ), + Button: ({ children, ...props }: React.ButtonHTMLAttributes) => ( + + ), + TooltipAnchor: ({ render }: { render: React.ReactNode }) => render, + MorphIcon: createPinMorphIconMock(), + useToastContext: () => ({ + showToast: jest.fn(), + }), + }; +}); jest.mock('~/hooks', () => ({ useDefaultConvo: () => jest.fn((value) => value.conversation), useFavorites: () => ({ - isFavoriteAgent: jest.fn(() => false), + isFavoriteAgent: mockIsFavoriteAgent, toggleFavoriteAgent: jest.fn(), }), useLocalize: () => (key: string, values?: Record) => { @@ -62,8 +73,8 @@ jest.mock('~/hooks', () => ({ jest.mock('~/Providers', () => ({ useChatContext: () => ({ - conversation: undefined, - newConversation: jest.fn(), + conversation: mockConversation, + newConversation: mockNewConversation, }), })); @@ -71,6 +82,12 @@ jest.mock('~/utils', () => ({ cn: (...classes: string[]) => classes.filter(Boolean).join(' '), clearMessagesCache: jest.fn(), renderAgentAvatar: () =>
, + specDisplayFieldReset: { + spec: null, + iconURL: null, + modelLabel: null, + greeting: undefined, + }, })); const renderWithClient = (children: React.ReactNode) => { @@ -81,26 +98,40 @@ const renderWithClient = (children: React.ReactNode) => { return render({children}); }; -const baseAgent = { +const baseAgent: t.Agent = { id: 'agent-1', name: 'Agent One', description: 'Agent description', + created_at: 0, + avatar: null, provider: 'openai', model: 'gpt-4', - model_parameters: {}, + model_parameters: { + temperature: null, + maxContextTokens: null, + max_context_tokens: null, + max_output_tokens: null, + top_p: null, + frequency_penalty: null, + presence_penalty: null, + }, }; describe('AgentDetailContent', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockConversation = undefined; + mockIsFavoriteAgent.mockReturnValue(false); + }); + it('renders support contact with mailto link', () => { renderWithClient( , ); @@ -115,16 +146,54 @@ describe('AgentDetailContent', () => { it('falls back to owner contact when support contact is missing', () => { renderWithClient( , ); expect(screen.getByText('Owner User')).toBeInTheDocument(); expect(screen.queryByRole('link', { name: 'Owner User' })).not.toBeInTheDocument(); }); + + it('clears model spec display fields when starting an agent chat', async () => { + const user = userEvent.setup(); + mockConversation = { + conversationId: 'existing-conversation', + spec: 'ClickHouse Agent', + iconURL: '/images/clickhouse.svg', + modelLabel: 'ClickHouse Agent', + }; + + renderWithClient(); + await user.click(screen.getByRole('button', { name: 'Start chat' })); + + expect(mockNewConversation).toHaveBeenCalledWith({ + template: expect.objectContaining({ + endpoint: 'agents', + agent_id: 'agent-1', + spec: null, + iconURL: null, + modelLabel: null, + }), + preset: expect.objectContaining({ + spec: null, + iconURL: null, + modelLabel: null, + }), + }); + }); + + it('shows pin icon when agent is not a favorite and pin-off when it is', () => { + const { unmount } = renderWithClient(); + expect(screen.getByRole('button', { name: 'Pin' })).toBeInTheDocument(); + expect(screen.getByTestId('morph-icon')).toHaveAttribute('data-icon', 'pin'); + unmount(); + + mockIsFavoriteAgent.mockReturnValue(true); + renderWithClient(); + expect(screen.getByRole('button', { name: 'Unpin' })).toBeInTheDocument(); + expect(screen.getByTestId('morph-icon')).toHaveAttribute('data-icon', 'pin-off'); + }); }); diff --git a/client/src/components/Artifacts/DownloadArtifact.tsx b/client/src/components/Artifacts/DownloadArtifact.tsx index 3e2ff7b0e2a..ca48eebe7d4 100644 --- a/client/src/components/Artifacts/DownloadArtifact.tsx +++ b/client/src/components/Artifacts/DownloadArtifact.tsx @@ -1,6 +1,6 @@ import React, { useState } from 'react'; -import { Button } from '@librechat/client'; -import { Download, CircleCheckBig } from 'lucide-react'; +import { Download, CircleCheckBig } from 'lucide'; +import { Button, MorphIcon } from '@librechat/client'; import type { Artifact } from '~/common'; import { useAttachmentLink, @@ -94,11 +94,7 @@ const DownloadArtifact = ({ artifact }: { artifact: Artifact }) => { onClick={handleDownload} aria-label={localize('com_ui_download_artifact')} > - {isDownloaded ? ( -