fix(core): Require admin push when migrating legacy environments to projects folder (#30530)
Some checks are pending
CI: Master (Build, Test, Lint) / Build for Github Cache (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (22.x) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (24.13.1) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (25.x) (push) Waiting to run
CI: Master (Build, Test, Lint) / Lint (push) Waiting to run
CI: Master (Build, Test, Lint) / Performance (push) Waiting to run
CI: Master (Build, Test, Lint) / Notify Slack on failure (push) Blocked by required conditions

This commit is contained in:
Ali Elkhateeb 2026-05-20 12:13:48 +03:00 committed by GitHub
parent 5ce4ce36cf
commit 96a0380a6d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 315 additions and 57 deletions

View File

@ -4,8 +4,8 @@ import { simpleGit } from 'simple-git';
import type { SimpleGit } from 'simple-git';
import { SourceControlGitService } from '../source-control-git.service.ee';
import type { SourceControlPreferences } from '../types/source-control-preferences';
import type { SourceControlPreferencesService } from '../source-control-preferences.service.ee';
import type { SourceControlPreferences } from '../types/source-control-preferences';
const MOCK_BRANCHES = {
all: ['origin/master', 'origin/feature/branch'],
@ -19,6 +19,8 @@ const MOCK_BRANCHES = {
const mockGitInstance = {
branch: jest.fn().mockResolvedValue(MOCK_BRANCHES),
env: jest.fn().mockReturnThis(),
fetch: jest.fn().mockResolvedValue(undefined),
raw: jest.fn().mockResolvedValue(''),
};
jest.mock('simple-git', () => {
@ -507,4 +509,59 @@ describe('SourceControlGitService', () => {
});
});
});
describe('requiresAdminPushForProjectsMigration', () => {
beforeEach(() => {
mockSourceControlPreferencesService.getPreferences.mockReturnValue({
branchName: 'main',
} as SourceControlPreferences);
});
it('should return true when workflows exist on remote but projects do not', async () => {
mockGitInstance.raw.mockImplementation(async (args: string[]) => {
const path = args[2];
if (path === 'workflows') {
return '040000 tree abc\tworkflows\n';
}
if (path === 'projects') {
return '';
}
return '';
});
await expect(sourceControlGitService.requiresAdminPushForProjectsMigration()).resolves.toBe(
true,
);
});
it('should return false when both workflows and projects exist on remote', async () => {
mockGitInstance.raw.mockImplementation(async (args: string[]) => {
const path = args[2];
if (path === 'workflows' || path === 'projects') {
return `040000 tree abc\t${path}\n`;
}
return '';
});
await expect(sourceControlGitService.requiresAdminPushForProjectsMigration()).resolves.toBe(
false,
);
});
it('should return false when workflows do not exist on remote', async () => {
mockGitInstance.raw.mockResolvedValue('');
await expect(sourceControlGitService.requiresAdminPushForProjectsMigration()).resolves.toBe(
false,
);
});
it('should reject when remote directory check fails (fail closed for migration guard)', async () => {
mockGitInstance.raw.mockRejectedValue(new Error('network error'));
await expect(sourceControlGitService.requiresAdminPushForProjectsMigration()).rejects.toThrow(
'network error',
);
});
});
});

View File

@ -7,6 +7,7 @@ import type { EventService } from '@/events/event.service';
import type { SourceControlPreferencesService } from '../source-control-preferences.service.ee';
import { SourceControlController } from '../source-control.controller.ee';
import type { SourceControlScopedService } from '../source-control-scoped.service';
import type { SourceControlService } from '../source-control.service.ee';
import type { SourceControlRequest } from '../types/requests';
import type { SourceControlGetStatus } from '../types/source-control-get-status';
@ -15,6 +16,7 @@ describe('SourceControlController', () => {
let controller: SourceControlController;
let sourceControlService: SourceControlService;
let sourceControlPreferencesService: SourceControlPreferencesService;
let sourceControlScopedService: SourceControlScopedService;
let eventService: EventService;
beforeEach(() => {
@ -23,15 +25,19 @@ describe('SourceControlController', () => {
pullWorkfolder: jest.fn().mockResolvedValue({ statusCode: 200 }),
getStatus: jest.fn().mockResolvedValue([]),
setGitUserDetails: jest.fn(),
sanityCheck: jest.fn().mockResolvedValue(undefined),
} as unknown as SourceControlService;
sourceControlPreferencesService = mock<SourceControlPreferencesService>();
sourceControlScopedService = {
ensureIsAllowedToPush: jest.fn().mockResolvedValue(undefined),
} as unknown as SourceControlScopedService;
eventService = mock<EventService>();
controller = new SourceControlController(
sourceControlService,
sourceControlPreferencesService,
mock(),
sourceControlScopedService,
eventService,
);
});

View File

@ -6,6 +6,7 @@ import { mock } from 'jest-mock-extended';
import { InstanceSettings } from 'n8n-core';
import type { PushResult } from 'simple-git';
import { SOURCE_CONTROL_ADMIN_PUSH_REQUIRED_MESSAGE } from '@/environments.ee/source-control/constants';
import { SourceControlPreferencesService } from '@/environments.ee/source-control/source-control-preferences.service.ee';
import { SourceControlService } from '@/environments.ee/source-control/source-control.service.ee';
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
@ -59,6 +60,7 @@ describe('SourceControlService', () => {
beforeEach(() => {
jest.resetAllMocks();
jest.spyOn(sourceControlService, 'sanityCheck').mockResolvedValue(undefined);
gitService.requiresAdminPushForProjectsMigration.mockResolvedValue(false);
// Reset mock implementations
mockStatusService.getStatus.mockReset();
});
@ -268,6 +270,45 @@ describe('SourceControlService', () => {
});
});
it('should throw ForbiddenError when projects migration requires admin push', async () => {
const user = Object.assign(new User(), {
role: GLOBAL_MEMBER_ROLE,
});
gitService.requiresAdminPushForProjectsMigration.mockResolvedValueOnce(true);
await expect(
sourceControlService.pushWorkfolder(user, {
fileNames: [],
commitMessage: 'Test',
}),
).rejects.toThrow(new ForbiddenError(SOURCE_CONTROL_ADMIN_PUSH_REQUIRED_MESSAGE));
expect(mockStatusService.getStatus).not.toHaveBeenCalled();
});
it('should allow instance admin to push when projects migration requires admin push', async () => {
const user = Object.assign(new User(), {
role: GLOBAL_ADMIN_ROLE,
});
gitService.requiresAdminPushForProjectsMigration.mockResolvedValueOnce(true);
mockStatusService.getStatus.mockResolvedValueOnce([]);
sourceControlExportService.exportCredentialsToWorkFolder.mockResolvedValueOnce({
count: 0,
missingIds: [],
folder: '',
files: [],
});
gitService.push.mockResolvedValueOnce(mock<PushResult>());
await sourceControlService.pushWorkfolder(user, {
fileNames: [],
commitMessage: 'Test',
});
expect(gitService.requiresAdminPushForProjectsMigration).toHaveBeenCalled();
expect(mockStatusService.getStatus).toHaveBeenCalled();
});
it('should throw an error if file path validation fails', async () => {
const user = mock<User>();
(isContainedWithin as jest.Mock).mockReturnValueOnce(false);

View File

@ -17,3 +17,5 @@ export const SOURCE_CONTROL_README = `
`;
export const SOURCE_CONTROL_DEFAULT_NAME = 'n8n user';
export const SOURCE_CONTROL_DEFAULT_EMAIL = 'n8n@example.com';
export const SOURCE_CONTROL_ADMIN_PUSH_REQUIRED_MESSAGE =
'The connected Git repository is missing the projects folder. An instance owner or instance admin must perform the next push so that projects are synced correctly with environments.';

View File

@ -22,6 +22,8 @@ import {
SOURCE_CONTROL_DEFAULT_EMAIL,
SOURCE_CONTROL_DEFAULT_NAME,
SOURCE_CONTROL_ORIGIN,
SOURCE_CONTROL_PROJECT_EXPORT_FOLDER,
SOURCE_CONTROL_WORKFLOW_EXPORT_FOLDER,
} from './constants';
import { sourceControlFoldersExistCheck } from './source-control-helper.ee';
import { SourceControlPreferencesService } from './source-control-preferences.service.ee';
@ -491,4 +493,39 @@ export class SourceControlGitService {
);
}
}
/**
* Instances that used environments before n8n 1.118.0 have workflows on the remote repository
* but no projects directory yet. The first push after upgrading must be done by an instance admin.
*/
async requiresAdminPushForProjectsMigration(): Promise<boolean> {
const workflowsExist = await this.remoteDirectoryExists(SOURCE_CONTROL_WORKFLOW_EXPORT_FOLDER);
if (!workflowsExist) {
return false;
}
const projectsExist = await this.remoteDirectoryExists(SOURCE_CONTROL_PROJECT_EXPORT_FOLDER);
return !projectsExist;
}
/**
* Checks whether a directory exists on the remote tracking branch of the connected repository.
* On git or network failure, throws callers must not treat errors as "directory absent", or
* non-admins could bypass the legacy migration admin push requirement (see LIGO-326).
*/
private async remoteDirectoryExists(directory: string): Promise<boolean> {
if (!this.git) {
throw new UnexpectedError('Git is not initialized (remoteDirectoryExists)');
}
const branchName = this.sourceControlPreferencesService.getPreferences().branchName;
if (!branchName) {
return false;
}
await this.fetch();
const remoteRef = `${SOURCE_CONTROL_ORIGIN}/${branchName}`;
const result = await this.git.raw(['ls-tree', remoteRef, directory]);
return result.trim().length > 0;
}
}

View File

@ -7,12 +7,14 @@ import { Logger } from '@n8n/backend-common';
import { type User } from '@n8n/db';
import { OnPubSubEvent } from '@n8n/decorators';
import { Service } from '@n8n/di';
import { hasGlobalScope } from '@n8n/permissions';
import { writeFileSync } from 'fs';
import { UnexpectedError, UserError, jsonParse } from 'n8n-workflow';
import * as path from 'path';
import type { PushResult } from 'simple-git';
import {
SOURCE_CONTROL_ADMIN_PUSH_REQUIRED_MESSAGE,
SOURCE_CONTROL_DEFAULT_EMAIL,
SOURCE_CONTROL_DEFAULT_NAME,
SOURCE_CONTROL_README,
@ -243,8 +245,11 @@ export class SourceControlService {
return await this.gitService.setBranch(branch);
}
// will reset the branch to the remote branch and pull
// this will discard all local changes
/**
* Resets the current local branch to match its remote counterpart, discarding all local changes.
* This operation overwrites any uncommitted or divergent local changes with the state from the remote branch.
* Use with caution, as this action cannot be undone.
*/
async resetWorkfolder(): Promise<ImportResult | undefined> {
if (!this.gitService.git) {
await this.initGitService();
@ -275,6 +280,8 @@ export class SourceControlService {
throw new BadRequestError('Cannot push onto read-only branch.');
}
await this.ensureCanPushWorkfolder(user);
const context = new SourceControlContext(user);
let filesToPush: SourceControlledFile[] = options.fileNames.map((file) => {
@ -497,6 +504,11 @@ export class SourceControlService {
async getStatus(user: User, options: SourceControlGetStatus) {
await this.sanityCheck();
if (options.direction === 'push') {
await this.ensureCanPushWorkfolder(user);
}
return await this.sourceControlStatusService.getStatus(user, options);
}
@ -542,4 +554,12 @@ export class SourceControlService {
throw new BadRequestError(`Unsupported file type: ${type}`);
}
}
private async ensureCanPushWorkfolder(user: User): Promise<void> {
if (await this.gitService.requiresAdminPushForProjectsMigration()) {
if (!hasGlobalScope(user, 'sourceControl:push')) {
throw new ForbiddenError(SOURCE_CONTROL_ADMIN_PUSH_REQUIRED_MESSAGE);
}
}
}
}

View File

@ -12,17 +12,14 @@ import {
WorkflowEntity,
} from '@n8n/db';
import { Container } from '@n8n/di';
import { createCredentials } from '@test-integration/db/credentials';
import { createFolder } from '@test-integration/db/folders';
import { assignTagToWorkflow, createTag, updateTag } from '@test-integration/db/tags';
import { createUser } from '@test-integration/db/users';
import * as fastGlob from 'fast-glob';
import { mock } from 'jest-mock-extended';
import { mock, type MockProxy } from 'jest-mock-extended';
import { Cipher } from 'n8n-core';
import fsp from 'node:fs/promises';
import { basename, isAbsolute } from 'node:path';
import {
SOURCE_CONTROL_ADMIN_PUSH_REQUIRED_MESSAGE,
SOURCE_CONTROL_CREDENTIAL_EXPORT_FOLDER,
SOURCE_CONTROL_FOLDERS_EXPORT_FILE,
SOURCE_CONTROL_TAGS_EXPORT_FILE,
@ -42,6 +39,10 @@ import type { RemoteResourceOwner } from '@/environments.ee/source-control/types
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
import { EventService } from '@/events/event.service';
import { createCredentials } from '@test-integration/db/credentials';
import { createFolder } from '@test-integration/db/folders';
import { assignTagToWorkflow, createTag, updateTag } from '@test-integration/db/tags';
import { createUser } from '@test-integration/db/users';
jest.mock('fast-glob');
@ -175,7 +176,7 @@ describe('SourceControlService', () => {
let deletedOutOfScopeCredential: CredentialsEntity;
let deletedInScopeCredential: CredentialsEntity;
let gitService: SourceControlGitService;
let gitService: MockProxy<SourceControlGitService>;
let service: SourceControlService;
let statusService: SourceControlStatusService;
@ -426,6 +427,7 @@ describe('SourceControlService', () => {
};
gitService = mock<SourceControlGitService>();
gitService.requiresAdminPushForProjectsMigration.mockResolvedValue(false);
statusService = Container.get(SourceControlStatusService);
service = new SourceControlService(
@ -810,6 +812,7 @@ describe('SourceControlService', () => {
});
beforeEach(() => {
gitService.requiresAdminPushForProjectsMigration.mockResolvedValue(false);
fsWriteFile.mockImplementation(async (path, data) => {
updatedFiles[path as string] = data as string;
});
@ -1130,6 +1133,52 @@ describe('SourceControlService', () => {
});
});
describe('legacy environments repository without projects folder', () => {
beforeEach(() => {
gitService.requiresAdminPushForProjectsMigration.mockResolvedValue(true);
});
afterEach(() => {
gitService.requiresAdminPushForProjectsMigration.mockResolvedValue(false);
});
it('should deny push for project admin', async () => {
// getStatus(direction: 'push') also enforces migration; load changes without that guard first
gitService.requiresAdminPushForProjectsMigration.mockResolvedValue(false);
const allChanges = (await service.getStatus(projectAdmin, {
direction: 'push',
preferLocalVersion: true,
verbose: false,
})) as SourceControlledFile[];
gitService.requiresAdminPushForProjectsMigration.mockResolvedValue(true);
await expect(
service.pushWorkfolder(projectAdmin, {
fileNames: allChanges,
commitMessage: 'Test',
force: true,
}),
).rejects.toThrow(new ForbiddenError(SOURCE_CONTROL_ADMIN_PUSH_REQUIRED_MESSAGE));
});
it('should allow push for global admin', async () => {
const allChanges = (await service.getStatus(globalAdmin, {
direction: 'push',
preferLocalVersion: true,
verbose: false,
})) as SourceControlledFile[];
const result = await service.pushWorkfolder(globalAdmin, {
fileNames: allChanges,
commitMessage: 'Test',
force: true,
});
expect(result.statusCode).toBe(200);
expect(gitService.push).toBeCalled();
});
});
describe('global:member', () => {
it('should deny all changes', async () => {
const allChanges = (await service.getStatus(globalAdmin, {

View File

@ -28,6 +28,7 @@ import { ResourceType } from '@/features/collaboration/projects/projects.utils';
import { useProjectsStore } from '@/features/collaboration/projects/projects.store';
import { useSettingsStore } from '@/app/stores/settings.store';
import { useSourceControlStore } from '@/features/integrations/sourceControl.ee/sourceControl.store';
import { useSourceControlModalRouting } from '@/features/integrations/sourceControl.ee/useSourceControlModalRouting';
import { useTagsStore } from '@/features/shared/tags/tags.store';
import { useUIStore } from '@/app/stores/ui.store';
import { useUsersStore } from '@/features/settings/users/users.store';
@ -117,6 +118,7 @@ const i18n = useI18n();
const router = useRouter();
const route = useRoute();
const { openPushModal } = useSourceControlModalRouting();
const locale = useI18n();
const telemetry = useTelemetry();
@ -617,27 +619,9 @@ async function onWorkflowMenuSelect(action: WORKFLOW_MENU_ACTIONS): Promise<void
case WORKFLOW_MENU_ACTIONS.PUSH: {
try {
await onSaveButtonClick();
// Navigate to route with sourceControl param - modal will handle data loading and loading states
void router.push({
query: {
...route.query,
sourceControl: 'push',
},
});
await openPushModal();
} catch (error) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
switch (error.message) {
case 'source_control_not_connected':
toast.showError(
{ ...error, message: '' },
locale.baseText('settings.sourceControl.error.not.connected.title'),
locale.baseText('settings.sourceControl.error.not.connected.message'),
);
break;
default:
toast.showError(error, locale.baseText('error'));
}
toast.showError(error, locale.baseText('error'));
}
break;

View File

@ -31,6 +31,14 @@ vi.mock('vue-router', () => ({
RouterLink: vi.fn(),
}));
const mockShowError = vi.fn();
vi.mock('@/app/composables/useToast', () => ({
useToast: () => ({
showError: mockShowError,
}),
}));
const renderComponent = createComponentRenderer(MainSidebarSourceControl);
describe('MainSidebarSourceControl', () => {
@ -57,6 +65,7 @@ describe('MainSidebarSourceControl', () => {
sourceControlStore = useSourceControlStore();
vi.spyOn(sourceControlStore, 'isEnterpriseSourceControlEnabled', 'get').mockReturnValue(true);
mockShowError.mockReset();
});
it('should render nothing when not instance owner', async () => {

View File

@ -4,8 +4,8 @@ import { useI18n } from '@n8n/i18n';
import { hasPermission } from '@/app/utils/rbac/permissions';
import { getResourcePermissions } from '@n8n/permissions';
import { useSourceControlStore } from '@/features/integrations/sourceControl.ee/sourceControl.store';
import { useSourceControlModalRouting } from '@/features/integrations/sourceControl.ee/useSourceControlModalRouting';
import { useProjectsStore } from '@/features/collaboration/projects/projects.store';
import { useRoute, useRouter } from 'vue-router';
import { N8nButton, N8nIcon, N8nTooltip } from '@n8n/design-system';
defineProps<{
@ -15,8 +15,7 @@ defineProps<{
const sourceControlStore = useSourceControlStore();
const projectStore = useProjectsStore();
const i18n = useI18n();
const route = useRoute();
const router = useRouter();
const { openPushModal, openPullModal } = useSourceControlModalRouting();
const tooltipOpenDelay = ref(300);
const currentBranch = computed(() => {
@ -43,26 +42,6 @@ const sourceControlAvailable = computed(
sourceControlStore.isEnterpriseSourceControlEnabled &&
(hasPullPermission.value || hasPushPermission.value),
);
async function pushWorkfolder() {
// Navigate to route with sourceControl param - modal will handle data loading and loading states
void router.push({
query: {
...route.query,
sourceControl: 'push',
},
});
}
function pullWorkfolder() {
// Navigate to route with sourceControl param - modal will handle the pull operation
void router.push({
query: {
...route.query,
sourceControl: 'pull',
},
});
}
</script>
<template>
@ -112,7 +91,7 @@ function pullWorkfolder() {
size="mini"
:square="isCollapsed"
:label="isCollapsed ? '' : i18n.baseText('settings.sourceControl.button.pull')"
@click="pullWorkfolder"
@click="openPullModal"
/>
</N8nTooltip>
<N8nTooltip
@ -139,7 +118,7 @@ function pullWorkfolder() {
icon="arrow-up"
type="tertiary"
size="mini"
@click="pushWorkfolder"
@click="openPushModal"
/>
</N8nTooltip>
</div>

View File

@ -83,7 +83,9 @@ async function loadSourceControlStatus() {
loadingService.setLoadingText(i18n.baseText('settings.sourceControl.loading.checkingForChanges'));
try {
const freshStatus = await sourceControlStore.getAggregatedStatus();
const freshStatus =
sourceControlStore.takePrefetchedPushStatus() ??
(await sourceControlStore.getAggregatedStatus());
if (!freshStatus.length) {
toast.showMessage({

View File

@ -180,6 +180,33 @@ describe('useSourceControlStore', () => {
});
});
describe('prefetchPushStatus', () => {
it('should fetch push status and cache it for the modal', async () => {
const mockStatus: SourceControlledFile[] = [
{
id: 'workflow1',
name: 'Test Workflow',
type: 'workflow' as const,
status: 'modified' as const,
location: 'local' as const,
conflict: false,
file: '/path/to/workflow.json',
updatedAt: '2024-01-01T00:00:00.000Z',
pushed: false,
},
];
const mockGetAggregatedStatus = vi.mocked(vcApi.getAggregatedStatus);
mockGetAggregatedStatus.mockResolvedValue(mockStatus);
const result = await sourceControlStore.prefetchPushStatus();
expect(mockGetAggregatedStatus).toHaveBeenCalledWith({});
expect(result).toEqual(mockStatus);
expect(sourceControlStore.takePrefetchedPushStatus()).toEqual(mockStatus);
});
});
describe('pushWorkfolder', () => {
it('should call API with correct parameters', async () => {
const data = {

View File

@ -1,4 +1,4 @@
import { computed, reactive } from 'vue';
import { computed, reactive, ref } from 'vue';
import { defineStore } from 'pinia';
import { EnterpriseEditionFeature } from '@/app/constants';
import { useSettingsStore } from '@/app/stores/settings.store';
@ -103,6 +103,20 @@ export const useSourceControlStore = defineStore('sourceControl', () => {
return await vcApi.getAggregatedStatus(rootStore.restApiContext);
};
const prefetchedPushStatus = ref<SourceControlledFile[] | null>(null);
const prefetchPushStatus = async () => {
const status = await getAggregatedStatus();
prefetchedPushStatus.value = status;
return status;
};
const takePrefetchedPushStatus = () => {
const status = prefetchedPushStatus.value;
prefetchedPushStatus.value = null;
return status;
};
const getRemoteWorkflow = async (workflowId: string) => {
return await vcApi.getRemoteWorkflow(rootStore.restApiContext, workflowId);
};
@ -111,6 +125,8 @@ export const useSourceControlStore = defineStore('sourceControl', () => {
isEnterpriseSourceControlEnabled,
state,
preferences,
prefetchPushStatus,
takePrefetchedPushStatus,
pushWorkfolder,
pullWorkfolder,
getPreferences,

View File

@ -0,0 +1,29 @@
import { useRoute, useRouter } from 'vue-router';
type SourceControlModalDirection = 'push' | 'pull';
export function useSourceControlModalRouting() {
const route = useRoute();
const router = useRouter();
async function navigateToSourceControlModal(direction: SourceControlModalDirection) {
// Navigate to route with sourceControl param - modal will handle data loading,
// loading states, and backend errors (e.g. admin push required for legacy repos).
return await router.push({
query: {
...route.query,
sourceControl: direction,
},
});
}
async function openPullModal(): Promise<void> {
await navigateToSourceControlModal('pull');
}
async function openPushModal(): Promise<void> {
await navigateToSourceControlModal('push');
}
return { openPushModal, openPullModal };
}