From 044c134ecfcf1f65084cc99fb19fbbc6a17e4568 Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Tue, 28 Jul 2026 04:38:37 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=8F=20fix:=20Filter=20Admin=20Config?= =?UTF-8?q?=20Reads=20by=20Section-Scoped=20Read=20Capability=20(#14472)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit listConfigs, getBaseConfig, and getConfig only checked the broad read:configs capability, so a caller holding nothing but read:configs:
grants got a blanket 403 on all three instead of a response filtered to the sections they hold. Any deployment using section-scoped config grants hits this. Adds hasAnyConfigReadAccess as a cheap pre-flight check covering broad and section-scoped read and manage grants (manage implies read), so a zero-access caller still 403s before a DB fetch while a section-scoped caller gets the response filtered to exactly what they hold. The same manage-implies-read rule is fixed at its root in getParentCapabilities so a manage-only caller sees the section they manage instead of having it stripped after passing the pre-flight. Resolves every section for a request in one batched getHeldCapabilities query via getReadableConfigSections instead of one round trip per section. Includes AppConfig field-renaming normalization (interfaceConfig, turnstileConfig, mcpConfig) so the filter checks the canonical section name rather than the renamed response field, and stops availableTools from bypassing the filter by gating it on its filteredTools/includedTools source sections. --- api/server/middleware/roles/capabilities.js | 21 +- api/server/routes/admin/config.js | 6 +- packages/api/src/admin/config.handler.spec.ts | 193 ++++++++++++++++++ packages/api/src/admin/config.ts | 159 ++++++++++++++- .../capabilities.integration.spec.ts | 111 +++++++++- packages/api/src/middleware/capabilities.ts | 95 +++++++-- .../src/methods/systemGrant.spec.ts | 109 ++++++++++ .../data-schemas/src/methods/systemGrant.ts | 84 +++++++- 8 files changed, 737 insertions(+), 41 deletions(-) diff --git a/api/server/middleware/roles/capabilities.js b/api/server/middleware/roles/capabilities.js index 6f2aa43e96..f2b1c5dd1c 100644 --- a/api/server/middleware/roles/capabilities.js +++ b/api/server/middleware/roles/capabilities.js @@ -1,9 +1,22 @@ const { generateCapabilityCheck, capabilityContextMiddleware } = require('@librechat/api'); -const { getUserPrincipals, hasCapabilityForPrincipals } = require('~/models'); - -const { hasCapability, requireCapability, hasConfigCapability } = generateCapabilityCheck({ +const { getUserPrincipals, + hasAnyConfigReadAccess, hasCapabilityForPrincipals, + getHeldCapabilities, +} = require('~/models'); + +const { + hasCapability, + requireCapability, + hasConfigCapability, + hasAnyConfigReadAccess: checkAnyConfigReadAccess, + getReadableConfigSections, +} = generateCapabilityCheck({ + getUserPrincipals, + hasAnyConfigReadAccess, + hasCapabilityForPrincipals, + getHeldCapabilities, }); module.exports = { @@ -11,4 +24,6 @@ module.exports = { requireCapability, hasConfigCapability, capabilityContextMiddleware, + hasAnyConfigReadAccess: checkAnyConfigReadAccess, + getReadableConfigSections, }; diff --git a/api/server/routes/admin/config.js b/api/server/routes/admin/config.js index ab7aa01a2b..9333495280 100644 --- a/api/server/routes/admin/config.js +++ b/api/server/routes/admin/config.js @@ -3,8 +3,10 @@ const { createAdminConfigHandlers } = require('@librechat/api'); const { SystemCapabilities } = require('@librechat/data-schemas'); const { hasCapability, - hasConfigCapability, requireCapability, + hasConfigCapability, + hasAnyConfigReadAccess, + getReadableConfigSections, } = require('~/server/middleware/roles/capabilities'); const { getAppConfig, invalidateConfigCaches } = require('~/server/services/Config'); const { requireJwtAuth } = require('~/server/middleware'); @@ -23,6 +25,8 @@ const handlers = createAdminConfigHandlers({ unsetConfigField: db.unsetConfigField, deleteConfig: db.deleteConfig, toggleConfigActive: db.toggleConfigActive, + hasAnyConfigReadAccess, + getReadableConfigSections, hasConfigCapability, hasCapability, getAppConfig, diff --git a/packages/api/src/admin/config.handler.spec.ts b/packages/api/src/admin/config.handler.spec.ts index d802c31f46..afb874d4b6 100644 --- a/packages/api/src/admin/config.handler.spec.ts +++ b/packages/api/src/admin/config.handler.spec.ts @@ -70,6 +70,7 @@ function createHandlers(overrides = {}) { deleteConfig: jest.fn().mockResolvedValue({ _id: 'c1' }), toggleConfigActive: jest.fn().mockResolvedValue({ _id: 'c1', isActive: false }), hasConfigCapability: jest.fn().mockResolvedValue(true), + hasAnyConfigReadAccess: jest.fn().mockResolvedValue(true), hasCapability: jest.fn().mockResolvedValue(true), getAppConfig: jest.fn().mockResolvedValue({ interface: { modelSelect: true } }), @@ -118,6 +119,7 @@ describe('createAdminConfigHandlers', () => { it('returns 403 before DB lookup when user lacks READ_CONFIGS', async () => { const { handlers, deps } = createHandlers({ hasConfigCapability: jest.fn().mockResolvedValue(false), + hasAnyConfigReadAccess: jest.fn().mockResolvedValue(false), }); const req = mockReq({ params: { principalType: 'role', principalId: 'admin' } }); const res = mockRes(); @@ -178,6 +180,195 @@ describe('createAdminConfigHandlers', () => { }); }); + describe('read handlers: section-scoped-only caller (no broad read:configs)', () => { + function sectionOnlyDeps(section: string, overrides: Record = {}) { + return { + hasConfigCapability: jest.fn( + async (_user: unknown, s: string | null, verb = 'manage') => + verb === 'read' && s === section, + ), + hasAnyConfigReadAccess: jest.fn().mockResolvedValue(true), + ...overrides, + }; + } + + it('getConfig: returns 200 with only the held section, other sections stripped', async () => { + const config = { + _id: 'c1', + principalType: 'role', + principalId: 'admin', + overrides: { memory: { charLimit: 500 }, endpoints: { allowedAddresses: ['10.0.0.1'] } }, + tombstones: ['memory.tokenLimit', 'endpoints.allowedAddresses'], + }; + const { handlers } = createHandlers( + sectionOnlyDeps('memory', { findConfigByPrincipal: jest.fn().mockResolvedValue(config) }), + ); + const req = mockReq({ params: { principalType: 'role', principalId: 'admin' } }); + const res = mockRes(); + + await handlers.getConfig(req, res); + + expect(res.statusCode).toBe(200); + const body = res.body!.config as { overrides: Record; tombstones: string[] }; + expect(body.overrides.memory).toEqual({ charLimit: 500 }); + expect(body.overrides.endpoints).toBeUndefined(); + expect(body.tombstones).toEqual(['memory.tokenLimit']); + }); + + it('listConfigs: strips non-held sections from every listed config', async () => { + const configs = [ + { _id: 'c1', principalType: 'role', principalId: 'admin', overrides: { memory: {} } }, + { + _id: 'c2', + principalType: 'user', + principalId: 'u1', + overrides: { endpoints: {}, memory: { charLimit: 10 } }, + }, + ]; + const { handlers } = createHandlers( + sectionOnlyDeps('memory', { listAllConfigs: jest.fn().mockResolvedValue(configs) }), + ); + const req = mockReq(); + const res = mockRes(); + + await handlers.listConfigs(req, res); + + expect(res.statusCode).toBe(200); + const body = res.body!.configs as Array<{ overrides: Record }>; + expect(body[0].overrides).toEqual({ memory: {} }); + expect(body[1].overrides).toEqual({ memory: { charLimit: 10 } }); + }); + + it('getBaseConfig: strips top-level sections and the nested config field to only the held section', async () => { + const appConfig = { + memory: { charLimit: 500 }, + endpoints: { allowedAddresses: ['10.0.0.1'] }, + fileStrategy: 's3', + config: { memory: { charLimit: 500 }, endpoints: { allowedAddresses: ['10.0.0.1'] } }, + paths: { uploads: '/tmp' }, + availableTools: { foo: {} }, + }; + const { handlers } = createHandlers( + sectionOnlyDeps('memory', { getAppConfig: jest.fn().mockResolvedValue(appConfig) }), + ); + const req = mockReq(); + const res = mockRes(); + + await handlers.getBaseConfig(req, res); + + expect(res.statusCode).toBe(200); + const body = res.body!.config as Record; + expect(body.memory).toEqual({ charLimit: 500 }); + expect(body.endpoints).toBeUndefined(); + expect(body.fileStrategy).toBeUndefined(); + expect((body.config as Record).memory).toEqual({ charLimit: 500 }); + expect((body.config as Record).endpoints).toBeUndefined(); + expect(body.paths).toEqual({ uploads: '/tmp' }); + expect(body.availableTools).toBeUndefined(); + }); + + it('getBaseConfig: strips availableTools when the caller holds neither of its source sections', async () => { + const appConfig = { + memory: { charLimit: 500 }, + filteredTools: ['dalle'], + includedTools: ['google'], + availableTools: { google: {} }, + paths: { uploads: '/tmp' }, + }; + const { handlers } = createHandlers( + sectionOnlyDeps('memory', { getAppConfig: jest.fn().mockResolvedValue(appConfig) }), + ); + const req = mockReq(); + const res = mockRes(); + + await handlers.getBaseConfig(req, res); + + expect(res.statusCode).toBe(200); + const body = res.body!.config as Record; + expect(body.availableTools).toBeUndefined(); + expect(body.filteredTools).toBeUndefined(); + expect(body.includedTools).toBeUndefined(); + }); + + it.each(['filteredTools', 'includedTools'])( + 'getBaseConfig: returns availableTools to a caller holding read:configs:%s', + async (section) => { + const appConfig = { + filteredTools: ['dalle'], + includedTools: ['google'], + availableTools: { google: {} }, + paths: { uploads: '/tmp' }, + }; + const { handlers } = createHandlers( + sectionOnlyDeps(section, { getAppConfig: jest.fn().mockResolvedValue(appConfig) }), + ); + const req = mockReq(); + const res = mockRes(); + + await handlers.getBaseConfig(req, res); + + expect(res.statusCode).toBe(200); + const body = res.body!.config as Record; + expect(body.availableTools).toEqual({ google: {} }); + }, + ); + + it('getBaseConfig: returns fileStrategy only to a caller holding read:configs:fileStrategy', async () => { + const appConfig = { + fileStrategy: 's3', + memory: { charLimit: 500 }, + paths: { uploads: '/tmp' }, + availableTools: {}, + }; + const { handlers } = createHandlers( + sectionOnlyDeps('fileStrategy', { getAppConfig: jest.fn().mockResolvedValue(appConfig) }), + ); + const req = mockReq(); + const res = mockRes(); + + await handlers.getBaseConfig(req, res); + + expect(res.statusCode).toBe(200); + const body = res.body!.config as Record; + expect(body.fileStrategy).toBe('s3'); + expect(body.memory).toBeUndefined(); + }); + + it('getBaseConfig: normalizes renamed top-level fields to their canonical section before checking read access', async () => { + // getAppConfig renames interface -> interfaceConfig, turnstile -> turnstileConfig, + // and mcpServers -> mcpConfig in the resolved payload. A caller holding + // read:configs:interface and read:configs:turnstile (but not mcpServers) must + // still see interfaceConfig/turnstileConfig, since checking the raw field name + // against a nonexistent "interfaceConfig"/"turnstileConfig" section would wrongly + // strip them. + const appConfig = { + interfaceConfig: { modelSelect: true }, + turnstileConfig: { siteKey: 'abc' }, + mcpConfig: { docs: {} }, + paths: { uploads: '/tmp' }, + availableTools: {}, + }; + const { handlers } = createHandlers({ + hasConfigCapability: jest.fn( + async (_user: unknown, s: string | null, verb = 'manage') => + verb === 'read' && (s === 'interface' || s === 'turnstile'), + ), + hasAnyConfigReadAccess: jest.fn().mockResolvedValue(true), + getAppConfig: jest.fn().mockResolvedValue(appConfig), + }); + const req = mockReq(); + const res = mockRes(); + + await handlers.getBaseConfig(req, res); + + expect(res.statusCode).toBe(200); + const body = res.body!.config as Record; + expect(body.interfaceConfig).toEqual({ modelSelect: true }); + expect(body.turnstileConfig).toEqual({ siteKey: 'abc' }); + expect(body.mcpConfig).toBeUndefined(); + }); + }); + describe('upsertConfigOverrides', () => { it('returns 201 when creating a new config (configVersion === 1)', async () => { const { handlers } = createHandlers({ @@ -2031,6 +2222,7 @@ describe('createAdminConfigHandlers', () => { it(`${name} returns 403 when user lacks capability`, async () => { const { handlers } = createHandlers({ hasConfigCapability: jest.fn().mockResolvedValue(false), + hasAnyConfigReadAccess: jest.fn().mockResolvedValue(false), }); const req = mockReq(reqOverrides); const res = mockRes(); @@ -2075,6 +2267,7 @@ describe('createAdminConfigHandlers', () => { it('returns 403 when user lacks READ_CONFIGS', async () => { const { handlers } = createHandlers({ hasConfigCapability: jest.fn().mockResolvedValue(false), + hasAnyConfigReadAccess: jest.fn().mockResolvedValue(false), }); const req = mockReq(); const res = mockRes(); diff --git a/packages/api/src/admin/config.ts b/packages/api/src/admin/config.ts index f1e3558f0d..8fad76708f 100644 --- a/packages/api/src/admin/config.ts +++ b/packages/api/src/admin/config.ts @@ -131,6 +131,13 @@ export interface AdminConfigDeps { section: ConfigSection | null, verb?: 'manage' | 'read', ) => Promise; + /** Pre-flight-only: whether the caller holds any config-read capability at all (broad or any section), so a zero-access caller 403s before a DB fetch. */ + hasAnyConfigReadAccess?: (user: CapabilityUser) => Promise; + /** Resolves which of a set of sections the caller can read in a single batched query. */ + getReadableConfigSections?: ( + user: CapabilityUser, + sections: ConfigSection[], + ) => Promise<{ broad: boolean; sections: Set }>; hasCapability?: (user: CapabilityUser, capability: SystemCapability) => Promise; getAppConfig?: (options?: { role?: string; @@ -183,6 +190,118 @@ function getCapabilityUser(req: ServerRequest): CapabilityUser | null { }; } +/** + * `AppConfig` keys exempt from the generic per-key `read:configs:
` + * lookup in `filterSectionsByReadAccess`, for three distinct reasons: + * - `paths` is a server-computed constant (resolved at module load), not a + * `TCustomConfig` section, so no `read:configs:
` grant could ever + * apply to it. + * - `config` is the nested container whose contents are filtered separately + * below; checking the outer key against a nonexistent `read:configs:config` + * grant would always fail and strip the whole object, including sections + * the caller legitimately holds. + * - `availableTools` is derived from the `filteredTools`/`includedTools` + * sections plus a filesystem scan, not itself a grantable section. It gets + * its own explicit check below, gated on those two source sections, rather + * than a lookup against the nonexistent `read:configs:availableTools`. + * Real `TCustomConfig` sections (e.g. `fileStrategy`) must never be added + * here: exempting one would return it to every caller regardless of grants. + */ +const STRUCTURAL_APP_CONFIG_KEYS = new Set(['paths', 'availableTools', 'config']); + +/** + * Top-level `AppConfig` response field → canonical `ConfigSection` name. + * `getAppConfig` renames a few sections in the resolved payload + * (`interface` → `interfaceConfig`, `turnstile` → `turnstileConfig`, + * `mcpServers` → `mcpConfig`). The read-grant capability is keyed by the + * canonical section name, so the top-level filter must normalize through + * this map before calling `canRead`. Otherwise a caller holding + * `read:configs:interface` gets `interfaceConfig` incorrectly stripped + * because no section named "interfaceConfig" exists to grant. + */ +const APP_CONFIG_FIELD_TO_SECTION: Readonly> = { + interfaceConfig: 'interface', + turnstileConfig: 'turnstile', + mcpConfig: 'mcpServers', +}; + +type ReadableSections = { broad: boolean; sections: ReadonlySet }; + +function canReadSection(readable: ReadableSections, section: string): boolean { + return readable.broad || readable.sections.has(section); +} + +/** Strips every top-level key not in `preserveKeys` that `canRead` rejects. */ +function filterSectionsByReadAccess>( + obj: T, + canRead: (section: string) => boolean, + preserveKeys: Set = new Set(), +): T { + const result: Record = { ...obj }; + for (const key of Object.keys(result)) { + if (!preserveKeys.has(key) && !canRead(key)) { + delete result[key]; + } + } + return result as T; +} + +function filterConfigDocForReadAccess(config: IConfig, readable: ReadableSections): IConfig { + const canRead = (section: string): boolean => canReadSection(readable, section); + const filteredOverrides = filterSectionsByReadAccess( + (config.overrides ?? {}) as Record, + canRead, + ); + + let filteredTombstones = config.tombstones; + if (config.tombstones?.length) { + filteredTombstones = config.tombstones.filter((path) => canRead(getTopLevelSection(path))); + } + + return { + ...config, + overrides: filteredOverrides as Partial, + tombstones: filteredTombstones, + } as IConfig; +} + +function filterAppConfigForReadAccess(appConfig: AppConfig, readable: ReadableSections): AppConfig { + const canRead = (section: string): boolean => canReadSection(readable, section); + const canReadTopLevelField = (field: string): boolean => + canRead(APP_CONFIG_FIELD_TO_SECTION[field] ?? field); + + const filtered = filterSectionsByReadAccess( + appConfig as unknown as Record, + canReadTopLevelField, + STRUCTURAL_APP_CONFIG_KEYS, + ); + if (!canRead('filteredTools') && !canRead('includedTools')) { + delete (filtered as { availableTools?: unknown }).availableTools; + } + const nestedConfig = (filtered as { config?: Record }).config; + if (nestedConfig != null && typeof nestedConfig === 'object') { + (filtered as { config?: unknown }).config = filterSectionsByReadAccess(nestedConfig, canRead); + } + return filtered as unknown as AppConfig; +} + +/** All section names an `IConfig` document's overrides/tombstones could reference. */ +function collectConfigSections(config: IConfig): string[] { + return [ + ...Object.keys(config.overrides ?? {}), + ...(config.tombstones ?? []).map(getTopLevelSection), + ]; +} + +/** All section names an `AppConfig` response could reference, normalized to canonical section names. */ +function collectAppConfigSections(appConfig: AppConfig): string[] { + const topLevel = Object.keys(appConfig) + .filter((key) => !STRUCTURAL_APP_CONFIG_KEYS.has(key)) + .map((key) => APP_CONFIG_FIELD_TO_SECTION[key] ?? key); + const nested = (appConfig as unknown as { config?: Record }).config; + return [...topLevel, ...(nested ? Object.keys(nested) : [])]; +} + function redactConfigForResponse(config: IConfig): IConfig { const safeConfig = JSON.parse(JSON.stringify(config)) as IConfig; if (safeConfig.overrides) { @@ -245,6 +364,16 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { deleteConfig, toggleConfigActive, hasConfigCapability, + hasAnyConfigReadAccess = async () => false, + getReadableConfigSections = async (u, sections) => { + if (await hasConfigCapability(u, null, 'read')) { + return { broad: true, sections: new Set(sections) }; + } + const held = await Promise.all( + sections.map((section) => hasConfigCapability(u, section, 'read')), + ); + return { broad: false, sections: new Set(sections.filter((_, i) => held[i])) }; + }, hasCapability = async () => false, getAppConfig, invalidateConfigCaches, @@ -260,12 +389,16 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { return res.status(401).json({ error: 'Authentication required' }); } - if (!(await hasConfigCapability(user, null, 'read'))) { + if (!(await hasAnyConfigReadAccess(user))) { return res.status(403).json({ error: 'Insufficient permissions' }); } const configs = await listAllConfigs(); - const safeConfigs = configs.map(redactConfigForResponse); + const sections = [...new Set(configs.flatMap(collectConfigSections))] as ConfigSection[]; + const readable = await getReadableConfigSections(user, sections); + const filtered = configs.map((config) => filterConfigDocForReadAccess(config, readable)); + + const safeConfigs = filtered.map(redactConfigForResponse); return res.status(200).json({ configs: safeConfigs }); } catch (error) { logger.error('[adminConfig] listConfigs error:', error); @@ -284,20 +417,24 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { return res.status(401).json({ error: 'Authentication required' }); } - if (!(await hasConfigCapability(user, null, 'read'))) { - return res.status(403).json({ error: 'Insufficient permissions' }); - } - if (!getAppConfig) { return res.status(501).json({ error: 'Base config endpoint not configured' }); } + if (!(await hasAnyConfigReadAccess(user))) { + return res.status(403).json({ error: 'Insufficient permissions' }); + } + const baseOnly = (req.query as Record).baseOnly === 'true'; const appConfig = await getAppConfig({ tenantId: user.tenantId, baseOnly, }); - return res.status(200).json({ config: redactAppConfigForResponse(appConfig) }); + const sections = collectAppConfigSections(appConfig) as ConfigSection[]; + const readable = await getReadableConfigSections(user, sections); + const filteredAppConfig = filterAppConfigForReadAccess(appConfig, readable); + + return res.status(200).json({ config: redactAppConfigForResponse(filteredAppConfig) }); } catch (error) { logger.error('[adminConfig] getBaseConfig error:', error); return res.status(500).json({ error: 'Failed to get base config' }); @@ -323,7 +460,7 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { return res.status(401).json({ error: 'Authentication required' }); } - if (!(await hasConfigCapability(user, null, 'read'))) { + if (!(await hasAnyConfigReadAccess(user))) { return res.status(403).json({ error: 'Insufficient permissions' }); } @@ -334,7 +471,11 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { return res.status(404).json({ error: 'Config not found' }); } - return res.status(200).json({ config: redactConfigForResponse(config) }); + const sections = collectConfigSections(config) as ConfigSection[]; + const readable = await getReadableConfigSections(user, sections); + const filteredConfig = filterConfigDocForReadAccess(config, readable); + + return res.status(200).json({ config: redactConfigForResponse(filteredConfig) }); } catch (error) { logger.error('[adminConfig] getConfig error:', error); return res.status(500).json({ error: 'Failed to get config' }); diff --git a/packages/api/src/middleware/capabilities.integration.spec.ts b/packages/api/src/middleware/capabilities.integration.spec.ts index cde5926b0b..7aa460fed2 100644 --- a/packages/api/src/middleware/capabilities.integration.spec.ts +++ b/packages/api/src/middleware/capabilities.integration.spec.ts @@ -7,7 +7,7 @@ import { SystemCapabilities, CapabilityImplications, } from '@librechat/data-schemas'; -import type { SystemCapability } from '@librechat/data-schemas'; +import type { SystemCapability, ConfigSection } from '@librechat/data-schemas'; import type { AllMethods } from '@librechat/data-schemas'; import { generateCapabilityCheck, @@ -237,6 +237,115 @@ describe('capabilities integration (real MongoDB)', () => { }); }); + describe('getReadableConfigSections', () => { + let getReadableConfigSections: ReturnType< + typeof generateCapabilityCheck + >['getReadableConfigSections']; + + beforeEach(() => { + ({ getReadableConfigSections } = generateCapabilityCheck({ + getUserPrincipals: methods.getUserPrincipals, + hasCapabilityForPrincipals: methods.hasCapabilityForPrincipals, + getHeldCapabilities: methods.getHeldCapabilities, + })); + }); + + it('reports broad access for a broad read:configs holder', async () => { + await methods.grantCapability({ + principalType: PrincipalType.USER, + principalId: regularUser.id, + capability: SystemCapabilities.READ_CONFIGS, + }); + + const readable = await getReadableConfigSections(regularUser, [ + 'endpoints', + 'balance', + ] as ConfigSection[]); + expect(readable.broad).toBe(true); + }); + + it('reports broad access for a broad manage:configs holder (manage implies read)', async () => { + await methods.grantCapability({ + principalType: PrincipalType.USER, + principalId: regularUser.id, + capability: SystemCapabilities.MANAGE_CONFIGS, + }); + + const readable = await getReadableConfigSections(regularUser, [ + 'endpoints', + ] as ConfigSection[]); + expect(readable.broad).toBe(true); + }); + + it('resolves only the sections held via section-scoped read grants', async () => { + await methods.grantCapability({ + principalType: PrincipalType.USER, + principalId: regularUser.id, + capability: 'read:configs:endpoints' as SystemCapability, + }); + + const readable = await getReadableConfigSections(regularUser, [ + 'endpoints', + 'balance', + ] as ConfigSection[]); + expect(readable.broad).toBe(false); + expect(readable.sections).toEqual(new Set(['endpoints'])); + }); + + it('resolves a section as readable for a caller holding only the same-section manage grant', async () => { + await methods.grantCapability({ + principalType: PrincipalType.USER, + principalId: regularUser.id, + capability: 'manage:configs:endpoints' as SystemCapability, + }); + + const readable = await getReadableConfigSections(regularUser, [ + 'endpoints', + 'balance', + ] as ConfigSection[]); + expect(readable.broad).toBe(false); + expect(readable.sections).toEqual(new Set(['endpoints'])); + }); + + it('resolves an empty set for a caller with no config access', async () => { + const readable = await getReadableConfigSections(regularUser, [ + 'endpoints', + 'balance', + ] as ConfigSection[]); + expect(readable.broad).toBe(false); + expect(readable.sections.size).toBe(0); + }); + + it('resolves all sections via a single getHeldCapabilities call regardless of section count', async () => { + await methods.grantCapability({ + principalType: PrincipalType.USER, + principalId: regularUser.id, + capability: 'read:configs:endpoints' as SystemCapability, + }); + + const getHeldCapabilities = jest.fn< + ReturnType, + Parameters + >(methods.getHeldCapabilities); + + const { getReadableConfigSections: batched } = generateCapabilityCheck({ + getUserPrincipals: methods.getUserPrincipals, + hasCapabilityForPrincipals: methods.hasCapabilityForPrincipals, + getHeldCapabilities, + }); + + const readable = await batched(regularUser, [ + 'endpoints', + 'balance', + 'interface', + 'mcpServers', + ] as ConfigSection[]); + + expect(readable.sections).toEqual(new Set(['endpoints'])); + expect(getHeldCapabilities).toHaveBeenCalledTimes(1); + }); + }); + describe('AsyncLocalStorage per-request caching', () => { it('caches getUserPrincipals within a single request context', async () => { await methods.seedSystemGrants(); diff --git a/packages/api/src/middleware/capabilities.ts b/packages/api/src/middleware/capabilities.ts index 0135f2c77e..02aea011b5 100644 --- a/packages/api/src/middleware/capabilities.ts +++ b/packages/api/src/middleware/capabilities.ts @@ -26,6 +26,15 @@ interface CapabilityDeps { capability: SystemCapability; tenantId?: string; }) => Promise; + hasAnyConfigReadAccess?: (params: { + principals: ResolvedPrincipal[]; + tenantId?: string; + }) => Promise; + getHeldCapabilities?: (params: { + principals: ResolvedPrincipal[]; + capabilities: SystemCapability[]; + tenantId?: string; + }) => Promise>; } export interface CapabilityUser { @@ -123,15 +132,76 @@ export function getCachedPrincipals(user: CapabilityUser): ResolvedPrincipal[] | * database methods. Follows the same dependency-injection pattern as * `generateCheckAccess`. */ +export type GetReadableConfigSectionsFn = ( + user: CapabilityUser, + sections: ConfigSection[], +) => Promise<{ broad: boolean; sections: Set }>; + export function generateCapabilityCheck(deps: CapabilityDeps): { hasCapability: HasCapabilityFn; requireCapability: RequireCapabilityFn; hasConfigCapability: HasConfigCapabilityFn; + hasAnyConfigReadAccess: (user: CapabilityUser) => Promise; + getReadableConfigSections: GetReadableConfigSectionsFn; } { - const { getUserPrincipals, hasCapabilityForPrincipals } = deps; + const { + getUserPrincipals, + hasCapabilityForPrincipals, + hasAnyConfigReadAccess: checkAny = async () => false, + getHeldCapabilities: getHeldCaps = async () => new Set(), + } = deps; let workerWarned = false; + async function resolvePrincipals(user: CapabilityUser): Promise { + const store = capabilityStore.getStore(); + const principalKey = `${user.id}:${user.role}:${user.tenantId ?? ''}`; + const cached = store?.principals.get(principalKey); + if (cached) { + return cached; + } + const principals = await getUserPrincipals({ + userId: user.id, + role: user.role, + idOnTheSource: user.idOnTheSource, + }); + store?.principals.set(principalKey, principals); + return principals; + } + + /** Whether the user holds any config-read capability at all, broad or section-scoped. */ + async function hasAnyConfigReadAccess(user: CapabilityUser): Promise { + const principals = await resolvePrincipals(user); + return checkAny({ principals, tenantId: user.tenantId }); + } + + /** + * Resolves which of `sections` the user can read in a single batched + * query, instead of one `hasConfigCapability` round trip per section. + */ + async function getReadableConfigSections( + user: CapabilityUser, + sections: ConfigSection[], + ): Promise<{ broad: boolean; sections: Set }> { + const principals = await resolvePrincipals(user); + const capsToCheck = [ + SystemCapabilities.READ_CONFIGS, + SystemCapabilities.MANAGE_CONFIGS, + ...sections.map(readConfigCapability), + ]; + const held = await getHeldCaps({ + principals, + capabilities: capsToCheck, + tenantId: user.tenantId, + }); + const broad = + held.has(SystemCapabilities.READ_CONFIGS) || held.has(SystemCapabilities.MANAGE_CONFIGS); + const readableSections = new Set( + broad ? sections : sections.filter((s) => held.has(readConfigCapability(s))), + ); + return { broad, sections: readableSections }; + } + async function hasCapability( user: CapabilityUser, capability: SystemCapability, @@ -153,20 +223,7 @@ export function generateCapabilityCheck(deps: CapabilityDeps): { return cached; } - const principalKey = `${user.id}:${user.role}:${user.tenantId ?? ''}`; - let principals: ResolvedPrincipal[]; - const cachedPrincipals = store?.principals.get(principalKey); - if (cachedPrincipals) { - principals = cachedPrincipals; - } else { - principals = await getUserPrincipals({ - userId: user.id, - role: user.role, - idOnTheSource: user.idOnTheSource, - }); - store?.principals.set(principalKey, principals); - } - + const principals = await resolvePrincipals(user); const result = await hasCapabilityForPrincipals({ principals, capability, @@ -237,5 +294,11 @@ export function generateCapabilityCheck(deps: CapabilityDeps): { }; } - return { hasCapability, requireCapability, hasConfigCapability }; + return { + hasCapability, + requireCapability, + hasConfigCapability, + hasAnyConfigReadAccess, + getReadableConfigSections, + }; } diff --git a/packages/data-schemas/src/methods/systemGrant.spec.ts b/packages/data-schemas/src/methods/systemGrant.spec.ts index b58a9431fa..c7d9fb9ae1 100644 --- a/packages/data-schemas/src/methods/systemGrant.spec.ts +++ b/packages/data-schemas/src/methods/systemGrant.spec.ts @@ -1473,5 +1473,114 @@ describe('systemGrant methods', () => { expect(held.size).toBe(0); }); + + it('resolves a section-scoped read capability when the principal holds the same-section manage grant', async () => { + const sectionManager = new Types.ObjectId(); + await methods.grantCapability({ + principalType: PrincipalType.USER, + principalId: sectionManager, + capability: 'manage:configs:endpoints' as SystemCapability, + }); + + const held = await methods.getHeldCapabilities({ + principals: [{ principalType: PrincipalType.USER, principalId: sectionManager }], + capabilities: ['read:configs:endpoints' as SystemCapability], + }); + + expect(held).toEqual(new Set(['read:configs:endpoints'])); + }); + + it("does not resolve a read capability from a different section's manage grant", async () => { + const otherSectionManager = new Types.ObjectId(); + await methods.grantCapability({ + principalType: PrincipalType.USER, + principalId: otherSectionManager, + capability: 'manage:configs:balance' as SystemCapability, + }); + + const held = await methods.getHeldCapabilities({ + principals: [{ principalType: PrincipalType.USER, principalId: otherSectionManager }], + capabilities: ['read:configs:endpoints' as SystemCapability], + }); + + expect(held.size).toBe(0); + }); + }); + + describe('hasAnyConfigReadAccess', () => { + it('returns true for a broad read:configs holder', async () => { + const userId = new Types.ObjectId(); + await methods.grantCapability({ + principalType: PrincipalType.USER, + principalId: userId, + capability: SystemCapabilities.READ_CONFIGS, + }); + + const result = await methods.hasAnyConfigReadAccess({ + principals: [{ principalType: PrincipalType.USER, principalId: userId }], + }); + expect(result).toBe(true); + }); + + it('returns true for a broad manage:configs holder, which implies read', async () => { + const userId = new Types.ObjectId(); + await methods.grantCapability({ + principalType: PrincipalType.USER, + principalId: userId, + capability: SystemCapabilities.MANAGE_CONFIGS, + }); + + const result = await methods.hasAnyConfigReadAccess({ + principals: [{ principalType: PrincipalType.USER, principalId: userId }], + }); + expect(result).toBe(true); + }); + + it('returns true for a section-scoped read:configs:
holder', async () => { + const userId = new Types.ObjectId(); + await methods.grantCapability({ + principalType: PrincipalType.USER, + principalId: userId, + capability: 'read:configs:endpoints' as SystemCapability, + }); + + const result = await methods.hasAnyConfigReadAccess({ + principals: [{ principalType: PrincipalType.USER, principalId: userId }], + }); + expect(result).toBe(true); + }); + + it('returns true for a section-scoped manage:configs:
holder', async () => { + const userId = new Types.ObjectId(); + await methods.grantCapability({ + principalType: PrincipalType.USER, + principalId: userId, + capability: 'manage:configs:endpoints' as SystemCapability, + }); + + const result = await methods.hasAnyConfigReadAccess({ + principals: [{ principalType: PrincipalType.USER, principalId: userId }], + }); + expect(result).toBe(true); + }); + + it('returns false for a caller with no config capability at all', async () => { + const userId = new Types.ObjectId(); + await methods.grantCapability({ + principalType: PrincipalType.USER, + principalId: userId, + capability: SystemCapabilities.READ_USAGE, + }); + + const result = await methods.hasAnyConfigReadAccess({ + principals: [{ principalType: PrincipalType.USER, principalId: userId }], + }); + expect(result).toBe(false); + }); + + it('returns false for an empty principals array', async () => { + const result = await methods.hasAnyConfigReadAccess({ principals: [] }); + expect(result).toBe(false); + }); }); }); diff --git a/packages/data-schemas/src/methods/systemGrant.ts b/packages/data-schemas/src/methods/systemGrant.ts index 6d82e9cd56..c030b6ed51 100644 --- a/packages/data-schemas/src/methods/systemGrant.ts +++ b/packages/data-schemas/src/methods/systemGrant.ts @@ -23,11 +23,13 @@ const baseCapabilityValues = new Set(Object.values(SystemCapabilities)); /** * For a section/assignment capability like `manage:configs:endpoints` or - * `assign:configs:user`, returns all base capabilities that subsume it: - * the direct parent (`manage:configs`) plus any that imply the parent - * via `reverseImplications` (`manage:configs` has no reverse, but - * `read:configs` is implied by `manage:configs`—so `read:configs:endpoints` - * is satisfied by holding `manage:configs`). + * `assign:configs:user`, returns all capabilities that subsume it: + * the direct parent (`manage:configs`), any base capability that implies + * the parent via `reverseImplications` (`read:configs` is implied by + * `manage:configs`, so `read:configs:endpoints` is satisfied by holding + * `manage:configs`), and, for a `read:configs:
` capability, the + * same-section `manage:configs:
` grant (manage implies read at + * the section level too, mirroring the broad-level implication). */ function getParentCapabilities(capability: string): string[] { const lastColon = capability.lastIndexOf(':'); @@ -35,13 +37,16 @@ function getParentCapabilities(capability: string): string[] { return []; } const parent = capability.slice(0, lastColon); - if (!baseCapabilityValues.has(parent)) { - return []; + const parents: string[] = []; + if (baseCapabilityValues.has(parent)) { + parents.push(parent); + const implied = reverseImplications[parent as keyof typeof reverseImplications]; + if (implied) { + parents.push(...implied); + } } - const parents = [parent]; - const implied = reverseImplications[parent as keyof typeof reverseImplications]; - if (implied) { - parents.push(...implied); + if (parent === SystemCapabilities.READ_CONFIGS) { + parents.push(`manage:configs:${capability.slice(lastColon + 1)}`); } return parents; } @@ -87,6 +92,13 @@ export function createSystemGrantMethods(mongoose: typeof import('mongoose')): { capability: SystemCapability; tenantId?: string; }) => Promise; + hasAnyConfigReadAccess: ({ + principals, + tenantId, + }: { + principals: Array<{ principalType: PrincipalType; principalId?: string | Types.ObjectId }>; + tenantId?: string; + }) => Promise; getHeldCapabilities: ({ principals, capabilities, @@ -134,6 +146,55 @@ export function createSystemGrantMethods(mongoose: typeof import('mongoose')): { : { tenantId: { $exists: false } }; } + const CONFIG_READ_ACCESS_PATTERN = /^(?:read|manage):configs(?::\w+)?$/; + + /** + * Whether any of the given principals holds *some* config-read + * capability: the broad `read:configs`/`manage:configs` (manage implies + * read) or any `read:configs:
`/`manage:configs:
`, + * without needing to know which sections in advance. Used to gate a + * cheap pre-flight 403 before fetching a config document, so a caller + * with zero read access never triggers a DB lookup, while a + * section-scoped-only caller still passes through to have the response + * filtered to what they actually hold. + * + * @param principals - Resolved principal list from getUserPrincipals + * @param tenantId - If present, checks tenant-scoped grant; if absent, checks platform-level + */ + async function hasAnyConfigReadAccess({ + principals, + tenantId, + }: { + principals: Array<{ principalType: PrincipalType; principalId?: string | Types.ObjectId }>; + tenantId?: string; + }): Promise { + const SystemGrant = mongoose.models.SystemGrant as Model; + const principalsQuery = principals + .filter( + (p): p is typeof p & { principalId: string | Types.ObjectId } => + p.principalType !== PrincipalType.PUBLIC && p.principalId != null, + ) + .map((p) => ({ + principalType: p.principalType, + principalId: normalizePrincipalId(p.principalId, p.principalType), + })); + + if (!principalsQuery.length) { + return false; + } + + const query: FilterQuery = { + $and: [ + { $or: principalsQuery }, + { capability: CONFIG_READ_ACCESS_PATTERN }, + tenantCondition(tenantId), + ], + }; + + const doc = await SystemGrant.exists(query); + return doc != null; + } + /** * Check if any of the given principals holds a specific capability. * Follows the same principal-resolution pattern as AclEntry: @@ -534,6 +595,7 @@ export function createSystemGrantMethods(mongoose: typeof import('mongoose')): { seedSystemGrants, revokeCapability, hasCapabilityForPrincipals, + hasAnyConfigReadAccess, getHeldCapabilities, listGrants, countGrants,