🪪 fix: Enforce VIEW ACL on Agent Edge References at Write and Runtime (#12246)

* 🛡️ fix: Enforce ACL checks on handoff edge and added-convo agent loading

Edge-linked agents and added-convo agents were fetched by ID via
getAgent without verifying the requesting user's access permissions.
This allowed an authenticated user to reference another user's private
agent in edges or addedConvo and have it initialized at runtime.

Add checkPermission(VIEW) gate in processAgent before initializing
any handoff agent, and in processAddedConvo for non-ephemeral added
agents. Unauthorized agents are logged and added to skippedAgentIds
so orphaned-edge filtering removes them cleanly.

* 🛡️ fix: Validate edge agent access at agent create/update time

Reject agent create/update requests that reference agents in edges
the requesting user cannot VIEW. This provides early feedback and
prevents storing unauthorized agent references as defense-in-depth
alongside the runtime ACL gate in processAgent.

Add collectEdgeAgentIds utility to extract all unique agent IDs from
an edge array, and validateEdgeAgentAccess helper in the v1 handler.

* 🧪 test: Improve ACL gate test coverage and correctness

- Add processAgent ACL gate tests for initializeClient (skip/allow handoff agents)
- Fix addedConvo.spec.js to mock loadAddedAgent directly instead of getAgent
- Seed permMap with ownedAgent VIEW bits in v1.spec.js update-403 test

* 🧹 chore: Remove redundant addedConvo ACL gate (now in middleware)

PR #12243 moved the addedConvo agent ACL check upstream into
canAccessAgentFromBody middleware, making the runtime check in
processAddedConvo and its spec redundant.

* 🧪 test: Rewrite processAgent ACL test with real DB and minimal mocking

Replace heavy mock-based test (12 mocks, Providers.XAI crash) with
MongoMemoryServer-backed integration test that exercises real getAgent,
checkPermission, and AclEntry — only external I/O (initializeAgent,
ToolService, AgentClient) remains mocked. Load edge utilities directly
from packages/api/src/agents/edges to sidestep the config.ts barrel.

* 🧪 fix: Use requireActual spread for @librechat/agents and @librechat/api mocks

The Providers.XAI crash was caused by mocking @librechat/agents with
a minimal replacement object, breaking the @librechat/api initialization
chain. Match the established pattern from client.test.js and
recordCollectedUsage.spec.js: spread jest.requireActual for both
packages, overriding only the functions under test.
This commit is contained in:
Danny Avila 2026-03-15 18:08:57 -04:00 committed by GitHub
parent 1312cd757c
commit bcf45519bd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 457 additions and 4 deletions

View file

@ -10,6 +10,8 @@ const {
createSequentialChainEdges,
} = require('@librechat/api');
const {
ResourceType,
PermissionBits,
EModelEndpoint,
isAgentsEndpoint,
getResponseSender,
@ -21,6 +23,7 @@ const {
} = require('~/server/controllers/agents/callbacks');
const { loadAgentTools, loadToolsForExecution } = require('~/server/services/ToolService');
const { getModelsConfig } = require('~/server/controllers/ModelController');
const { checkPermission } = require('~/server/services/PermissionService');
const AgentClient = require('~/server/controllers/agents/client');
const { getConvoFiles } = require('~/models/Conversation');
const { processAddedConvo } = require('./addedConvo');
@ -229,6 +232,22 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
return null;
}
const hasAccess = await checkPermission({
userId: req.user.id,
role: req.user.role,
resourceType: ResourceType.AGENT,
resourceId: agent._id,
requiredPermission: PermissionBits.VIEW,
});
if (!hasAccess) {
logger.warn(
`[processAgent] User ${req.user.id} lacks VIEW access to handoff agent ${agentId}, skipping`,
);
skippedAgentIds.add(agentId);
return null;
}
const validationResult = await validateAgentModel({
req,
res,

View file

@ -0,0 +1,201 @@
const mongoose = require('mongoose');
const {
ResourceType,
PermissionBits,
PrincipalType,
PrincipalModel,
} = require('librechat-data-provider');
const { MongoMemoryServer } = require('mongodb-memory-server');
const mockInitializeAgent = jest.fn();
const mockValidateAgentModel = jest.fn();
jest.mock('@librechat/agents', () => ({
...jest.requireActual('@librechat/agents'),
createContentAggregator: jest.fn(() => ({
contentParts: [],
aggregateContent: jest.fn(),
})),
}));
jest.mock('@librechat/api', () => ({
...jest.requireActual('@librechat/api'),
initializeAgent: (...args) => mockInitializeAgent(...args),
validateAgentModel: (...args) => mockValidateAgentModel(...args),
GenerationJobManager: { setCollectedUsage: jest.fn() },
getCustomEndpointConfig: jest.fn(),
createSequentialChainEdges: jest.fn(),
}));
jest.mock('~/server/controllers/agents/callbacks', () => ({
createToolEndCallback: jest.fn(() => jest.fn()),
getDefaultHandlers: jest.fn(() => ({})),
}));
jest.mock('~/server/services/ToolService', () => ({
loadAgentTools: jest.fn(),
loadToolsForExecution: jest.fn(),
}));
jest.mock('~/server/controllers/ModelController', () => ({
getModelsConfig: jest.fn().mockResolvedValue({}),
}));
let agentClientArgs;
jest.mock('~/server/controllers/agents/client', () => {
return jest.fn().mockImplementation((args) => {
agentClientArgs = args;
return {};
});
});
jest.mock('./addedConvo', () => ({
processAddedConvo: jest.fn().mockResolvedValue({ userMCPAuthMap: undefined }),
}));
jest.mock('~/cache', () => ({
logViolation: jest.fn(),
}));
const { initializeClient } = require('./initialize');
const { createAgent } = require('~/models/Agent');
const { User, AclEntry } = require('~/db/models');
const PRIMARY_ID = 'agent_primary';
const TARGET_ID = 'agent_target';
const AUTHORIZED_ID = 'agent_authorized';
describe('initializeClient — processAgent ACL gate', () => {
let mongoServer;
let testUser;
beforeAll(async () => {
mongoServer = await MongoMemoryServer.create();
await mongoose.connect(mongoServer.getUri());
});
afterAll(async () => {
await mongoose.disconnect();
await mongoServer.stop();
});
beforeEach(async () => {
await mongoose.connection.dropDatabase();
jest.clearAllMocks();
agentClientArgs = undefined;
testUser = await User.create({
email: 'test@example.com',
name: 'Test User',
username: 'testuser',
role: 'USER',
});
mockValidateAgentModel.mockResolvedValue({ isValid: true });
});
const makeReq = () => ({
user: { id: testUser._id.toString(), role: 'USER' },
body: { conversationId: 'conv_1', files: [] },
config: { endpoints: {} },
_resumableStreamId: null,
});
const makeEndpointOption = () => ({
agent: Promise.resolve({
id: PRIMARY_ID,
name: 'Primary',
provider: 'openai',
model: 'gpt-4',
tools: [],
}),
model_parameters: { model: 'gpt-4' },
endpoint: 'agents',
});
const makePrimaryConfig = (edges) => ({
id: PRIMARY_ID,
endpoint: 'agents',
edges,
toolDefinitions: [],
toolRegistry: new Map(),
userMCPAuthMap: null,
tool_resources: {},
resendFiles: true,
maxContextTokens: 4096,
});
it('should skip handoff agent and filter its edge when user lacks VIEW access', async () => {
await createAgent({
id: TARGET_ID,
name: 'Target Agent',
provider: 'openai',
model: 'gpt-4',
author: new mongoose.Types.ObjectId(),
tools: [],
});
const edges = [{ from: PRIMARY_ID, to: TARGET_ID, edgeType: 'handoff' }];
mockInitializeAgent.mockResolvedValue(makePrimaryConfig(edges));
await initializeClient({
req: makeReq(),
res: {},
signal: new AbortController().signal,
endpointOption: makeEndpointOption(),
});
expect(mockInitializeAgent).toHaveBeenCalledTimes(1);
expect(agentClientArgs.agent.edges).toEqual([]);
});
it('should initialize handoff agent and keep its edge when user has VIEW access', async () => {
const authorizedAgent = await createAgent({
id: AUTHORIZED_ID,
name: 'Authorized Agent',
provider: 'openai',
model: 'gpt-4',
author: new mongoose.Types.ObjectId(),
tools: [],
});
await AclEntry.create({
principalType: PrincipalType.USER,
principalId: testUser._id,
principalModel: PrincipalModel.USER,
resourceType: ResourceType.AGENT,
resourceId: authorizedAgent._id,
permBits: PermissionBits.VIEW,
grantedBy: testUser._id,
});
const edges = [{ from: PRIMARY_ID, to: AUTHORIZED_ID, edgeType: 'handoff' }];
const handoffConfig = {
id: AUTHORIZED_ID,
edges: [],
toolDefinitions: [],
toolRegistry: new Map(),
userMCPAuthMap: null,
tool_resources: {},
};
let callCount = 0;
mockInitializeAgent.mockImplementation(() => {
callCount++;
return callCount === 1
? Promise.resolve(makePrimaryConfig(edges))
: Promise.resolve(handoffConfig);
});
await initializeClient({
req: makeReq(),
res: {},
signal: new AbortController().signal,
endpointOption: makeEndpointOption(),
});
expect(mockInitializeAgent).toHaveBeenCalledTimes(2);
expect(agentClientArgs.agent.edges).toHaveLength(1);
expect(agentClientArgs.agent.edges[0].to).toBe(AUTHORIZED_ID);
});
});