🪜 fix: Strip Caller-Supplied Priority from Assign-Only Config Upserts (#13911)

* fix: prevent assign-only config priority changes

* fix: preserve assign-only config priority atomically

* style: format config priority guard

* fix: type preserve priority upsert option
This commit is contained in:
Danny Avila 2026-06-23 08:31:26 -04:00 committed by GitHub
parent edc0aebdb9
commit 725a14e409
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 100 additions and 9 deletions

View file

@ -994,7 +994,45 @@ describe('createAdminConfigHandlers', () => {
await handlers.upsertConfigOverrides(req, res);
expect(res.statusCode).toBe(201);
expect(deps.upsertConfig).toHaveBeenCalled();
expect(deps.upsertConfig).toHaveBeenCalledWith(
'role',
'admin',
expect.anything(),
{},
10,
undefined,
{ expectEmpty: true, preservePriority: true },
);
});
it('requests atomic priority preservation for ASSIGN_CONFIGS-only empty-overrides upsert', async () => {
const findConfigByPrincipal = jest
.fn()
.mockResolvedValue({ _id: 'c1', priority: 7, overrides: {} });
const { handlers, deps } = createHandlers({
hasConfigCapability: jest.fn().mockResolvedValue(false),
hasCapability: jest.fn().mockResolvedValue(true),
findConfigByPrincipal,
});
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
body: { overrides: {}, priority: 999 },
});
const res = mockRes();
await handlers.upsertConfigOverrides(req, res);
expect(res.statusCode).toBe(201);
expect(deps.upsertConfig).toHaveBeenCalledWith(
'role',
'admin',
expect.anything(),
{},
10,
undefined,
{ expectEmpty: true, preservePriority: true },
);
expect(findConfigByPrincipal).not.toHaveBeenCalled();
});
it('rejects non-empty overrides for ASSIGN_CONFIGS-only caller', async () => {
@ -1113,7 +1151,15 @@ describe('createAdminConfigHandlers', () => {
await handlers.upsertConfigOverrides(req, res);
expect(res.statusCode).toBe(201);
expect(deps.upsertConfig).toHaveBeenCalled();
expect(deps.upsertConfig).toHaveBeenCalledWith(
'role',
'admin',
expect.anything(),
{},
10,
undefined,
{ expectEmpty: true, preservePriority: true },
);
});
it('rejects upsert when parameterized grant targets a different principalType', async () => {
@ -1245,7 +1291,7 @@ describe('createAdminConfigHandlers', () => {
expect.anything(),
expect.anything(),
undefined,
{ expectEmpty: true },
{ expectEmpty: true, preservePriority: true },
);
});

View file

@ -79,7 +79,7 @@ export interface AdminConfigDeps {
overrides: Partial<TCustomConfig>,
priority: number,
session?: ClientSession,
options?: { expectEmpty?: boolean },
options?: { expectEmpty?: boolean; preservePriority?: boolean },
) => Promise<IConfig | null>;
patchConfigFields: (
principalType: PrincipalType,
@ -393,14 +393,25 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps): {
return res.status(403).json({ error: 'Insufficient permissions' });
}
if (priority != null && !hasBroadManage) {
logger.warn(
`[adminConfig] Ignoring caller-supplied priority on assign-only scope lifecycle upsert to ${principalType}/${principalId}: only broad manage:configs may modify document priority`,
);
}
const requestedPriority = hasBroadManage ? (priority ?? DEFAULT_PRIORITY) : DEFAULT_PRIORITY;
const upsertOptions = hasBroadManage
? { expectEmpty: false }
: { expectEmpty: true, preservePriority: true };
const config = await upsertConfig(
principalType,
principalId,
principalModel(principalType),
filteredOverrides,
priority ?? DEFAULT_PRIORITY,
requestedPriority,
undefined,
{ expectEmpty: !hasBroadManage },
upsertOptions,
);
if (!config && !hasBroadManage) {
return res.status(403).json({ error: 'Insufficient permissions' });

View file

@ -587,6 +587,39 @@ describe('expectEmpty atomic guard', () => {
expect(result!.priority).toBe(99);
});
it('upsertConfig with preservePriority inserts with the requested priority', async () => {
const result = await methods.upsertConfig(
PrincipalType.ROLE,
'admin',
PrincipalModel.ROLE,
{},
10,
undefined,
{ expectEmpty: true, preservePriority: true },
);
expect(result).toBeTruthy();
expect(result!.priority).toBe(10);
});
it('upsertConfig with preservePriority keeps an empty existing doc priority', async () => {
await methods.upsertConfig(PrincipalType.ROLE, 'admin', PrincipalModel.ROLE, {}, 5);
const result = await methods.upsertConfig(
PrincipalType.ROLE,
'admin',
PrincipalModel.ROLE,
{},
99,
undefined,
{ expectEmpty: true, preservePriority: true },
);
expect(result).toBeTruthy();
expect(result!.priority).toBe(5);
expect(result!.configVersion).toBe(2);
});
it('upsertConfig with expectEmpty returns null when existing doc has non-empty overrides', async () => {
await methods.upsertConfig(
PrincipalType.ROLE,

View file

@ -37,7 +37,7 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): {
overrides: Partial<TCustomConfig>,
priority: number,
session?: ClientSession,
options?: { expectEmpty?: boolean },
options?: { expectEmpty?: boolean; preservePriority?: boolean },
) => Promise<IConfig | null>;
patchConfigFields: (
principalType: PrincipalType,
@ -149,7 +149,7 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): {
overrides: Partial<TCustomConfig>,
priority: number,
session?: ClientSession,
options?: { expectEmpty?: boolean },
options?: { expectEmpty?: boolean; preservePriority?: boolean },
): Promise<IConfig | null> {
const Config = mongoose.models.Config as Model<IConfig>;
@ -168,9 +168,10 @@ export function createConfigMethods(mongoose: typeof import('mongoose')): {
$set: {
principalModel,
overrides,
priority,
...(options?.preservePriority ? {} : { priority }),
isActive: true,
},
...(options?.preservePriority ? { $setOnInsert: { priority } } : {}),
$inc: { configVersion: 1 },
};