Skip to content

Commit dd5829e

Browse files
authored
🏰 fix: Enforce Platform Grants for Code Workers (#15700)
* fix: Require Platform Grants for Code Workers * fix: Separate Platform Scope From Tenant Identity
1 parent 911b632 commit dd5829e

4 files changed

Lines changed: 97 additions & 7 deletions

File tree

api/server/routes/admin/code.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ const { requireJwtAuth } = require('~/server/middleware');
77

88
const router = express.Router();
99
const requireAdminAccess = requireCapability(SystemCapabilities.ACCESS_ADMIN);
10-
const requireCodeEnvironmentManage = requireCapability(SystemCapabilities.MANAGE_CODE_ENVIRONMENTS);
10+
const requireCodeEnvironmentManage = requireCapability(
11+
SystemCapabilities.MANAGE_CODE_ENVIRONMENTS,
12+
{ platformOnly: true },
13+
);
1114
const handlers = createAdminCodeEnvironmentHandlers({ getAppConfig });
1215

1316
router.use(requireJwtAuth, requireAdminAccess, requireCodeEnvironmentManage);

api/server/routes/admin/code.test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,5 +70,10 @@ describe('admin code environment routes', () => {
7070
expect(response.body).toEqual({ operation, environmentId: 'attached-vm' });
7171
expect(middlewareCalls).toEqual(['jwt', 'access:admin', 'manage:code_environments']);
7272
expect(mockHandlers[handlerName]).toHaveBeenCalledTimes(1);
73+
if (path === 'pairings') {
74+
expect(mockRequireCapability).toHaveBeenCalledWith('manage:code_environments', {
75+
platformOnly: true,
76+
});
77+
}
7378
});
7479
});

packages/api/src/middleware/capabilities.spec.ts

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ import {
55
readConfigCapability,
66
} from '@librechat/data-schemas';
77
import type { Response } from 'express';
8+
import type { CapabilityUser } from './capabilities';
89
import type { ServerRequest } from '~/types/http';
9-
import { generateCapabilityCheck } from './capabilities';
10+
import { capabilityContextMiddleware, generateCapabilityCheck } from './capabilities';
1011

1112
jest.mock('@librechat/data-schemas', () => ({
1213
...jest.requireActual('@librechat/data-schemas'),
@@ -149,6 +150,77 @@ describe('generateCapabilityCheck', () => {
149150
expect(statusMock).not.toHaveBeenCalled();
150151
});
151152

153+
it('omits tenant scope for platform-only capability checks', async () => {
154+
mockReq.user = {
155+
id: 'user-123',
156+
role: 'ADMIN',
157+
tenantId: 'tenant-1',
158+
} as ServerRequest['user'];
159+
mockGetUserPrincipals.mockResolvedValue(adminPrincipals);
160+
mockHasCapabilityForPrincipals.mockResolvedValue(true);
161+
162+
const middleware = requireCapability(SystemCapabilities.MANAGE_CODE_ENVIRONMENTS, {
163+
platformOnly: true,
164+
});
165+
await middleware(mockReq as ServerRequest, mockRes as Response, mockNext);
166+
167+
expect(mockNext).toHaveBeenCalled();
168+
expect(mockHasCapabilityForPrincipals).toHaveBeenCalledWith({
169+
capability: SystemCapabilities.MANAGE_CODE_ENVIRONMENTS,
170+
principals: [adminPrincipals[0]],
171+
tenantId: undefined,
172+
});
173+
});
174+
175+
it('rejects a tenant-only grant for a platform-only capability check', async () => {
176+
mockReq.user = {
177+
id: 'user-123',
178+
role: 'ADMIN',
179+
tenantId: 'tenant-1',
180+
} as ServerRequest['user'];
181+
mockGetUserPrincipals.mockResolvedValue(adminPrincipals);
182+
mockHasCapabilityForPrincipals.mockImplementation(({ tenantId }) =>
183+
Promise.resolve(tenantId === 'tenant-1'),
184+
);
185+
186+
const middleware = requireCapability(SystemCapabilities.MANAGE_CODE_ENVIRONMENTS, {
187+
platformOnly: true,
188+
});
189+
await middleware(mockReq as ServerRequest, mockRes as Response, mockNext);
190+
191+
expect(mockNext).not.toHaveBeenCalled();
192+
expect(statusMock).toHaveBeenCalledWith(403);
193+
});
194+
195+
it('reuses tenant principal resolution across tenant and platform checks', async () => {
196+
mockReq.user = {
197+
id: 'user-123',
198+
role: 'ADMIN',
199+
tenantId: 'tenant-1',
200+
} as ServerRequest['user'];
201+
mockGetUserPrincipals.mockResolvedValue(adminPrincipals);
202+
mockHasCapabilityForPrincipals.mockResolvedValue(true);
203+
204+
await new Promise<void>((resolve, reject) => {
205+
capabilityContextMiddleware(mockReq as ServerRequest, mockRes as Response, () => {
206+
void (async () => {
207+
try {
208+
await hasCapability(mockReq.user as CapabilityUser, SystemCapabilities.ACCESS_ADMIN);
209+
const middleware = requireCapability(SystemCapabilities.MANAGE_CODE_ENVIRONMENTS, {
210+
platformOnly: true,
211+
});
212+
await middleware(mockReq as ServerRequest, mockRes as Response, mockNext);
213+
resolve();
214+
} catch (error) {
215+
reject(error);
216+
}
217+
})();
218+
});
219+
});
220+
221+
expect(mockGetUserPrincipals).toHaveBeenCalledTimes(1);
222+
});
223+
152224
it('returns 403 when user lacks the capability', async () => {
153225
mockReq.user = { id: 'user-456', role: 'USER' } as ServerRequest['user'];
154226
mockGetUserPrincipals.mockResolvedValue(userPrincipals);

packages/api/src/middleware/capabilities.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { isMainThread } from 'node:worker_threads';
22
import { AsyncLocalStorage } from 'node:async_hooks';
3+
import { PrincipalType } from 'librechat-data-provider';
34
import {
45
logger,
56
configCapability,
@@ -61,6 +62,7 @@ export type HasCapabilityFn = (
6162

6263
export type RequireCapabilityFn = (
6364
capability: SystemCapability,
65+
options?: { platformOnly?: boolean },
6466
) => (req: ServerRequest, res: Response, next: NextFunction) => Promise<void>;
6567

6668
export type HasConfigCapabilityFn = (
@@ -214,6 +216,7 @@ export function generateCapabilityCheck(deps: CapabilityDeps): {
214216
async function hasCapability(
215217
user: CapabilityUser,
216218
capability: SystemCapability,
219+
{ platformOnly = false }: { platformOnly?: boolean } = {},
217220
): Promise<boolean> {
218221
if (!isMainThread && !workerWarned) {
219222
workerWarned = true;
@@ -226,17 +229,21 @@ export function generateCapabilityCheck(deps: CapabilityDeps): {
226229

227230
const store = capabilityStore.getStore();
228231

229-
const resultKey = `${user.id}:${user.tenantId ?? ''}:${capability}`;
232+
const resultKey = `${user.id}:${user.tenantId ?? ''}:${capability}:${platformOnly ? 'platform' : 'tenant'}`;
230233
const cached = store?.results.get(resultKey);
231234
if (cached !== undefined) {
232235
return cached;
233236
}
234237

235-
const principals = await resolvePrincipals(user);
238+
const resolvedPrincipals = await resolvePrincipals(user);
239+
const principals =
240+
platformOnly && user.tenantId
241+
? resolvedPrincipals.filter(({ principalType }) => principalType === PrincipalType.USER)
242+
: resolvedPrincipals;
236243
const result = await hasCapabilityForPrincipals({
237244
principals,
238245
capability,
239-
tenantId: user.tenantId,
246+
tenantId: platformOnly ? undefined : user.tenantId,
240247
});
241248
store?.results.set(resultKey, result);
242249
return result;
@@ -265,7 +272,10 @@ export function generateCapabilityCheck(deps: CapabilityDeps): {
265272
return hasCapability(user, sectionCap);
266273
}
267274

268-
function requireCapability(capability: SystemCapability) {
275+
function requireCapability(
276+
capability: SystemCapability,
277+
{ platformOnly = false }: { platformOnly?: boolean } = {},
278+
) {
269279
return async (req: ServerRequest, res: Response, next: NextFunction) => {
270280
try {
271281
if (!req.user) {
@@ -286,7 +296,7 @@ export function generateCapabilityCheck(deps: CapabilityDeps): {
286296
idOnTheSource: req.user.idOnTheSource ?? null,
287297
};
288298

289-
if (await hasCapability(user, capability)) {
299+
if (await hasCapability(user, capability, { platformOnly })) {
290300
next();
291301
return;
292302
}

0 commit comments

Comments
 (0)