fix(editor): Split global variable scopes to be added to custom roles separately (#38724)

This commit is contained in:
James Martin 2026-09-18 08:50:32 +00:00 committed by GitHub
parent 0e7ccf6f74
commit 7b059d64f1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 148 additions and 2 deletions

View File

@ -117,6 +117,37 @@ describe('custom role scope whitelists', () => {
expect(manage.length).toBeGreaterThan(view.length);
});
it('exposes "Variables: View" as exactly the global variable list/read pair', () => {
expect(GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.variable.View).toEqual([
'variable:list',
'variable:read',
]);
});
it('keeps "Variables: View" within GLOBAL_MEMBER_SCOPES', () => {
// A custom role built to mirror Member must never end up with more than Member.
for (const scope of GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.variable.View) {
expect(GLOBAL_MEMBER_SCOPES).toContain(scope);
}
});
it('keeps variable "Manage" a strict superset of variable "View"', () => {
// The editor's implied/downgrade arithmetic (SUPERSEDED_BY: View -> Manage)
// only holds while Manage contains everything View grants.
const view = GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.variable.View;
const manage = GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.variable.Manage;
expect(view.every((scope) => (manage as readonly string[]).includes(scope))).toBe(true);
expect(manage.length).toBeGreaterThan(view.length);
});
it('keeps variable scopes out of the settings.Manage bundle', () => {
// Global variables have their own group, so granting instance Settings no
// longer grants them. Every scope must live under exactly one group.
const bundle = GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS.settings.Manage as readonly string[];
expect(bundle).not.toContain('variable:list');
expect(bundle).not.toContain('variable:read');
});
it('keeps "Tags: View" within GLOBAL_MEMBER_SCOPES', () => {
// "Tags: View" is granted to every instance role by default (see
// instanceRoleScopes.ts), so it must never exceed what the built-in Member

View File

@ -100,8 +100,6 @@ export const GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS = {
'eventBusDestination:delete',
'eventBusDestination:list',
'eventBusDestination:test',
'variable:list',
'variable:read',
'dataTable:list',
'aiPreference:create', // Context (instance-wide AI preferences)
'aiPreference:read',
@ -169,6 +167,21 @@ export const GLOBAL_CUSTOM_ROLE_SCOPE_GROUPS = {
View: ['tag:read', 'tag:list'],
Manage: ['tag:read', 'tag:list', 'tag:create', 'tag:update', 'tag:delete'],
},
variable: {
// Global (instance-level) variables only; project variables are granted per
// project via `projectVariable:*` on a project role. View mirrors
// GLOBAL_MEMBER_SCOPES exactly, so a custom role built to match Member never
// exceeds it. The list response carries each variable's value, so View reads
// values and is not only discovery — hence it is opt-in, not mandatory.
View: ['variable:list', 'variable:read'],
Manage: [
'variable:list',
'variable:read',
'variable:create',
'variable:update',
'variable:delete',
],
},
project: {
Create: ['project:create'],
},

View File

@ -10,6 +10,7 @@ import {
getVariableByKey,
} from '@test-integration/db/variables';
import { createCustomRoleWithScopeSlugs } from './shared/db/roles';
import { createOwner, createUser } from './shared/db/users';
import type { SuperAgentTest } from './shared/types';
import * as utils from './shared/utils/';
@ -119,6 +120,78 @@ describe('GET /variables/:id', () => {
});
});
// ----------------------------------------
// Custom instance roles - global variables
// ----------------------------------------
describe('GET /variables - custom instance roles', () => {
// A custom instance role only sees global variables when it holds `variable:list`.
// `variable:list` is now its own permission option, so a role can read global
// variables without also holding every instance settings scope.
let authNoVariableScopesAgent: SuperAgentTest;
let authVariableViewAgent: SuperAgentTest;
let globalVariable: Variables;
let projectVariable: Variables;
beforeAll(async () => {
const roleWithoutVariableScopes = await createCustomRoleWithScopeSlugs(['user:list'], {
roleType: 'global',
});
const roleWithVariableView = await createCustomRoleWithScopeSlugs(
['user:list', 'variable:list', 'variable:read'],
{ roleType: 'global' },
);
const userWithoutVariableScopes = await createUser({ role: roleWithoutVariableScopes });
const userWithVariableView = await createUser({ role: roleWithVariableView });
await Promise.all([
linkUserToProject(userWithoutVariableScopes, project, 'project:admin'),
linkUserToProject(userWithVariableView, project, 'project:admin'),
]);
authNoVariableScopesAgent = testServer.authAgentFor(userWithoutVariableScopes);
authVariableViewAgent = testServer.authAgentFor(userWithVariableView);
});
beforeEach(async () => {
[globalVariable, projectVariable] = await Promise.all([
createVariable('globalVar', 'globalValue'),
createProjectVariable('projectVar', 'projectValue', project),
]);
});
test('should filter out global variables for a role without variable:list', async () => {
const response = await authNoVariableScopesAgent.get('/variables');
// Filtered out silently, not a 403 - the list still returns the project variables.
expect(response.statusCode).toBe(200);
expect(response.body.data.map((variable: Variables) => variable.key)).toEqual([
projectVariable.key,
]);
});
test('should return global variables for a role with variable:list', async () => {
const response = await authVariableViewAgent.get('/variables');
expect(response.statusCode).toBe(200);
expect(response.body.data.map((variable: Variables) => variable.key).sort()).toEqual(
[globalVariable.key, projectVariable.key].sort(),
);
});
test('should return a single global variable for a role with variable:read', async () => {
const response = await authVariableViewAgent.get(`/variables/${globalVariable.id}`);
expect(response.statusCode).toBe(200);
expect(response.body.data.key).toBe(globalVariable.key);
});
test('should deny a single global variable to a role without variable:read', async () => {
const response = await authNoVariableScopesAgent.get(`/variables/${globalVariable.id}`);
expect(response.statusCode).toBe(403);
});
});
// ----------------------------------------
// POST /variables - create a new variable
// ----------------------------------------

View File

@ -3030,6 +3030,7 @@
"instanceRoles.resource.role": "Roles",
"instanceRoles.resource.apiKey": "API keys",
"instanceRoles.resource.tag": "Tags",
"instanceRoles.resource.variable": "Variables",
"instanceRoles.resource.project": "Projects",
"instanceRoles.resource.insights": "Insights",
"instanceRoles.option.view": "View",
@ -3059,6 +3060,8 @@
"instanceRoles.description.apiKey.manageAll": "View and delete other users' API keys.",
"instanceRoles.description.tag.view": "Users always see and can select any tags associated with workflows they have access to.",
"instanceRoles.description.tag.manage": "Create, edit, and delete tags",
"instanceRoles.description.variable.view": "View global variables and their values",
"instanceRoles.description.variable.manage": "Create, edit, and delete global variables",
"instanceRoles.description.project.create": "Create new projects",
"instanceRoles.description.insights.view": "View instance-level insights",
"instanceRoles.warning.manageMembers": "A user with this permission can give themselves or an account they invite permissions they don't currently hold. {link}",

View File

@ -82,6 +82,7 @@ describe('instanceRoleScopes config', () => {
it('exposes the configured option labels per resource', () => {
expect(Object.keys(INSTANCE_SCOPE_GROUPS.apiKey)).toEqual(['Manage own', 'Manage all']);
expect(Object.keys(INSTANCE_SCOPE_GROUPS.tag)).toEqual(['View', 'Manage']);
expect(Object.keys(INSTANCE_SCOPE_GROUPS.variable)).toEqual(['View', 'Manage']);
expect(Object.keys(INSTANCE_SCOPE_GROUPS.role)).toEqual(['Manage project roles', 'Manage']);
expect(Object.keys(INSTANCE_SCOPE_GROUPS.project)).toEqual(['Create']);
expect(Object.keys(INSTANCE_SCOPE_GROUPS.settings)).toEqual([
@ -154,6 +155,14 @@ describe('isOptionImplied', () => {
it('returns false when the superseding option is present but "Manage own" is checked via own scopes only', () => {
expect(isOptionImplied(manageOwn, apiKeyGroup.options, ownScopes)).toBe(false);
});
it('renders variable "View" as implied under a fully-checked variable "Manage"', () => {
const variableGroup = INSTANCE_SCOPE_GROUP_LIST.find((g) => g.resource === 'variable')!;
const view = variableGroup.options.find((o) => o.key === 'View')!;
const manage = variableGroup.options.find((o) => o.key === 'Manage')!;
expect(isOptionImplied(view, variableGroup.options, [...manage.scopes])).toBe(true);
expect(isOptionImplied(view, variableGroup.options, [...view.scopes])).toBe(false);
});
});
describe('resolveOptionState', () => {
@ -306,6 +315,17 @@ describe('toggleOptionInGroup', () => {
expect(result).toEqual(['user:read']);
});
it('downgrades to "View" when unchecking variable "Manage"', () => {
const variableGroup = INSTANCE_SCOPE_GROUP_LIST.find((g) => g.resource === 'variable')!;
const manageVariables = variableGroup.options.find((o) => o.key === 'Manage')!;
const result = toggleOptionInGroup(
['user:list', ...manageVariables.scopes],
manageVariables,
variableGroup.options,
);
expect(result).toEqual(['user:list', 'variable:list', 'variable:read']);
});
it('downgrades to "View" when unchecking tag "Manage", keeping the mandatory scopes', () => {
const tagGroup = INSTANCE_SCOPE_GROUP_LIST.find((g) => g.resource === 'tag')!;
const manageTags = tagGroup.options.find((o) => o.key === 'Manage')!;

View File

@ -20,6 +20,7 @@ export const INSTANCE_RESOURCE_ORDER: InstanceResource[] = [
'role',
'apiKey',
'tag',
'variable',
'project',
'insights',
];
@ -31,6 +32,7 @@ export const INSTANCE_RESOURCE_LABEL_KEYS: Record<InstanceResource, BaseTextKey>
role: 'instanceRoles.resource.role',
apiKey: 'instanceRoles.resource.apiKey',
tag: 'instanceRoles.resource.tag',
variable: 'instanceRoles.resource.variable',
project: 'instanceRoles.resource.project',
insights: 'instanceRoles.resource.insights',
};
@ -95,6 +97,10 @@ export const INSTANCE_OPTION_DESCRIPTION_KEYS: Partial<
View: 'instanceRoles.description.tag.view',
Manage: 'instanceRoles.description.tag.manage',
},
variable: {
View: 'instanceRoles.description.variable.view',
Manage: 'instanceRoles.description.variable.manage',
},
project: { Create: 'instanceRoles.description.project.create' },
insights: { View: 'instanceRoles.description.insights.view' },
};