diff --git a/packages/api/src/admin/config.handler.spec.ts b/packages/api/src/admin/config.handler.spec.ts index b3f8e9ee3b..7649d10ae9 100644 --- a/packages/api/src/admin/config.handler.spec.ts +++ b/packages/api/src/admin/config.handler.spec.ts @@ -647,6 +647,174 @@ describe('createAdminConfigHandlers', () => { }); }); + describe('patchConfigField: section-scoped priority preservation', () => { + it('ignores request-supplied priority when caller lacks broad manage:configs', async () => { + const { handlers, deps } = createHandlers({ + hasConfigCapability: jest.fn(async (_user, section) => section === 'memory'), + findConfigByPrincipal: jest + .fn() + .mockResolvedValue({ _id: 'c1', priority: 7, overrides: {} }), + }); + const req = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { + entries: [{ fieldPath: 'memory.context', value: 'updated' }], + priority: 999, + }, + }); + const res = mockRes(); + + await handlers.patchConfigField(req, res); + + expect(res.statusCode).toBe(200); + const [, , , , priorityArg] = deps.patchConfigFields.mock.calls[0]; + expect(priorityArg).toBe(7); + }); + + it('falls back to default priority when no existing doc and caller lacks broad manage', async () => { + const { handlers, deps } = createHandlers({ + hasConfigCapability: jest.fn(async (_user, section) => section === 'memory'), + findConfigByPrincipal: jest.fn().mockResolvedValue(null), + }); + const req = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { + entries: [{ fieldPath: 'memory.context', value: 'updated' }], + priority: 999, + }, + }); + const res = mockRes(); + + await handlers.patchConfigField(req, res); + + expect(res.statusCode).toBe(200); + const [, , , , priorityArg] = deps.patchConfigFields.mock.calls[0]; + expect(priorityArg).toBe(10); + }); + + it('honors request-supplied priority when caller holds broad manage:configs', async () => { + const { handlers, deps } = createHandlers({ + hasConfigCapability: jest.fn().mockResolvedValue(true), + }); + const req = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { + entries: [{ fieldPath: 'memory.context', value: 'updated' }], + priority: 999, + }, + }); + const res = mockRes(); + + await handlers.patchConfigField(req, res); + + expect(res.statusCode).toBe(200); + const [, , , , priorityArg] = deps.patchConfigFields.mock.calls[0]; + expect(priorityArg).toBe(999); + expect(deps.findConfigByPrincipal).not.toHaveBeenCalled(); + }); + + it('preserves priority 0 when broad caller supplies it', async () => { + const { handlers, deps } = createHandlers({ + hasConfigCapability: jest.fn().mockResolvedValue(true), + }); + const req = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { + entries: [{ fieldPath: 'memory.context', value: 'updated' }], + priority: 0, + }, + }); + const res = mockRes(); + + await handlers.patchConfigField(req, res); + + expect(res.statusCode).toBe(200); + const [, , , , priorityArg] = deps.patchConfigFields.mock.calls[0]; + expect(priorityArg).toBe(0); + }); + + it('preserves existing priority 0 for section-scoped callers', async () => { + const { handlers, deps } = createHandlers({ + hasConfigCapability: jest.fn(async (_user, section) => section === 'memory'), + findConfigByPrincipal: jest + .fn() + .mockResolvedValue({ _id: 'c1', priority: 0, overrides: {} }), + }); + const req = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { + entries: [{ fieldPath: 'memory.context', value: 'updated' }], + priority: 999, + }, + }); + const res = mockRes(); + + await handlers.patchConfigField(req, res); + + expect(res.statusCode).toBe(200); + const [, , , , priorityArg] = deps.patchConfigFields.mock.calls[0]; + expect(priorityArg).toBe(0); + }); + }); + + describe('tombstoneConfigField: section-scoped priority preservation', () => { + it('ignores request-supplied priority when caller lacks broad manage:configs', async () => { + const { handlers, deps } = createHandlers({ + hasConfigCapability: jest.fn(async (_user, section) => section === 'memory'), + findConfigByPrincipal: jest + .fn() + .mockResolvedValue({ _id: 'c1', priority: 7, overrides: {} }), + }); + const req = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { fieldPath: 'memory.context', priority: 999 }, + }); + const res = mockRes(); + + await handlers.tombstoneConfigField(req, res); + + expect(res.statusCode).toBe(200); + const [, , , , priorityArg] = deps.tombstoneConfigField.mock.calls[0]; + expect(priorityArg).toBe(7); + }); + + it('falls back to default priority when no existing doc and caller lacks broad manage', async () => { + const { handlers, deps } = createHandlers({ + hasConfigCapability: jest.fn(async (_user, section) => section === 'memory'), + findConfigByPrincipal: jest.fn().mockResolvedValue(null), + }); + const req = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { fieldPath: 'memory.context', priority: 999 }, + }); + const res = mockRes(); + + await handlers.tombstoneConfigField(req, res); + + expect(res.statusCode).toBe(200); + const [, , , , priorityArg] = deps.tombstoneConfigField.mock.calls[0]; + expect(priorityArg).toBe(10); + }); + + it('honors request-supplied priority when caller holds broad manage:configs', async () => { + const { handlers, deps } = createHandlers({ + hasConfigCapability: jest.fn().mockResolvedValue(true), + }); + const req = mockReq({ + params: { principalType: 'role', principalId: 'admin' }, + body: { fieldPath: 'memory.context', priority: 999 }, + }); + const res = mockRes(); + + await handlers.tombstoneConfigField(req, res); + + expect(res.statusCode).toBe(200); + const [, , , , priorityArg] = deps.tombstoneConfigField.mock.calls[0]; + expect(priorityArg).toBe(999); + expect(deps.findConfigByPrincipal).not.toHaveBeenCalled(); + }); + }); + describe('upsertConfigOverrides — Bug 2 regression', () => { it('returns 403 for empty overrides when user lacks MANAGE_CONFIGS', async () => { const { handlers } = createHandlers({ diff --git a/packages/api/src/admin/config.ts b/packages/api/src/admin/config.ts index 1a79e01384..de0b57b525 100644 --- a/packages/api/src/admin/config.ts +++ b/packages/api/src/admin/config.ts @@ -466,14 +466,16 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { return true; }); + const hasBroadManage = await hasConfigCapability(user, null, 'manage'); + if (validEntries.length === 0) { - if (!(await hasConfigCapability(user, null, 'manage'))) { + if (!hasBroadManage) { return res.status(403).json({ error: 'Insufficient permissions' }); } return res.status(200).json({ message: 'No actionable field entries provided' }); } - if (!(await hasConfigCapability(user, null, 'manage'))) { + if (!hasBroadManage) { const sections = [...new Set(validEntries.map((e) => getTopLevelSection(e.fieldPath)))]; const allowed = await Promise.all( sections.map((s) => hasConfigCapability(user, s as ConfigSection, 'manage')), @@ -496,8 +498,15 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { fields[entry.fieldPath] = entry.value; } + if (priority != null && !hasBroadManage) { + logger.warn( + `[adminConfig] Ignoring caller-supplied priority on section-scoped patch to ${principalType}/${principalId}: only broad manage:configs may modify document priority`, + ); + } + const requestedPriority = hasBroadManage ? priority : undefined; + const existing = - priority == null + requestedPriority == null ? await findConfigByPrincipal(principalType, principalId, { includeInactive: true }) : null; @@ -506,7 +515,7 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { principalId, principalModel(principalType), fields, - priority ?? existing?.priority ?? DEFAULT_PRIORITY, + requestedPriority ?? existing?.priority ?? DEFAULT_PRIORITY, ); invalidateConfigCaches?.(user.tenantId)?.catch((err) => @@ -557,7 +566,11 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { const section = getTopLevelSection(fieldPath); - if (!(await hasConfigCapability(user, section as ConfigSection, 'manage'))) { + const hasBroadManage = await hasConfigCapability(user, null, 'manage'); + if ( + !hasBroadManage && + !(await hasConfigCapability(user, section as ConfigSection, 'manage')) + ) { return res.status(403).json({ error: `Insufficient permissions for config section: ${section}`, }); @@ -570,8 +583,15 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { return res.status(200).json({ message: 'No actionable field path provided' }); } + if (priority != null && !hasBroadManage) { + logger.warn( + `[adminConfig] Ignoring caller-supplied priority on section-scoped tombstone for ${principalType}/${principalId}: only broad manage:configs may modify document priority`, + ); + } + const requestedPriority = hasBroadManage ? priority : undefined; + const existing = - priority == null + requestedPriority == null ? await findConfigByPrincipal(principalType, principalId, { includeInactive: true }) : null; @@ -580,7 +600,7 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): { principalId, principalModel(principalType), fieldPath, - priority ?? existing?.priority ?? DEFAULT_PRIORITY, + requestedPriority ?? existing?.priority ?? DEFAULT_PRIORITY, ); invalidateConfigCaches?.(user.tenantId)?.catch((err) =>