feat: Synchronize deletions when pulling from source control

packages/cli/src/commands/ldap/reset.ts

@@ -110,7 +110,7 @@ export class Reset extends BaseCommand {
 		}
 
 		for (const credential of ownedCredentials) {
-			await Container.get(CredentialsService).delete(credential);
+			await Container.get(CredentialsService).delete(owner, credential.id);
 		}
 
 		await Container.get(AuthProviderSyncHistoryRepository).delete({ providerType: 'ldap' });

packages/cli/src/controllers/users.controller.ts

@@ -240,7 +240,7 @@ export class UsersController {
 		}
 
 		for (const credential of ownedCredentials) {
-			await this.credentialsService.delete(credential);
+			await this.credentialsService.delete(userToDelete, credential.id);
 		}
 
 		await this.userService.getManager().transaction(async (trx) => {

packages/cli/src/credentials/credentials.controller.ts

@@ -251,7 +251,7 @@ export class CredentialsController {
 			);
 		}
 
-		await this.credentialsService.delete(credential);
+		await this.credentialsService.delete(req.user, credential.id);
 
 		this.eventService.emit('credentials-deleted', {
 			user: req.user,

packages/cli/src/credentials/credentials.service.ts

@@ -406,10 +406,26 @@ export class CredentialsService {
 		return result;
 	}
 
-	async delete(credentials: CredentialsEntity) {
-		await this.externalHooks.run('credentials.delete', [credentials.id]);
+	/**
+	 * Deletes a credential.
+	 *
+	 * If the user does not have permission to delete the credential this does
+	 * nothing and returns void.
+	 */
+	async delete(user: User, credentialId: string) {
+		await this.externalHooks.run('credentials.delete', [credentialId]);
+
+		const credential = await this.sharedCredentialsRepository.findCredentialForUser(
+			credentialId,
+			user,
+			['credential:delete'],
+		);
+
+		if (!credential) {
+			return;
+		}
 
-		await this.credentialsRepository.remove(credentials);
+		await this.credentialsRepository.remove(credential);
 	}
 
 	async test(user: User, credentials: ICredentialsDecrypted) {

packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts

@@ -26,6 +26,9 @@ describe('SourceControlImportService', () => {
 		mock(),
 		workflowRepository,
 		mock(),
+		mock(),
+		mock(),
+		mock(),
 		mock<InstanceSettings>({ n8nFolder: '/mock/n8n' }),
 	);
 

packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts

@@ -1,10 +1,19 @@
+import type { SourceControlledFile } from '@n8n/api-types';
 import { Container } from '@n8n/di';
 import { mock } from 'jest-mock-extended';
 import { InstanceSettings } from 'n8n-core';
 
+import type { TagEntity } from '@/databases/entities/tag-entity';
+import type { User } from '@/databases/entities/user';
+import type { Variables } from '@/databases/entities/variables';
+import type { TagRepository } from '@/databases/repositories/tag.repository';
 import { SourceControlPreferencesService } from '@/environments.ee/source-control/source-control-preferences.service.ee';
 import { SourceControlService } from '@/environments.ee/source-control/source-control.service.ee';
 
+import type { SourceControlImportService } from '../source-control-import.service.ee';
+import type { ExportableCredential } from '../types/exportable-credential';
+import type { SourceControlWorkflowVersionId } from '../types/source-control-workflow-version-id';
+
 describe('SourceControlService', () => {
 	const preferencesService = new SourceControlPreferencesService(
 		Container.get(InstanceSettings),
@@ -13,20 +22,95 @@ describe('SourceControlService', () => {
 		mock(),
 		mock(),
 	);
+	const sourceControlImportService = mock<SourceControlImportService>();
+	const tagRepository = mock<TagRepository>();
 	const sourceControlService = new SourceControlService(
 		mock(),
 		mock(),
 		preferencesService,
 		mock(),
-		mock(),
-		mock(),
+		sourceControlImportService,
+		tagRepository,
 		mock(),
 	);
 
+	beforeEach(() => {
+		jest.resetAllMocks();
+		jest.spyOn(sourceControlService, 'sanityCheck').mockResolvedValue(undefined);
+	});
+
 	describe('pushWorkfolder', () => {
 		it('should throw an error if a file is given that is not in the workfolder', async () => {
-			jest.spyOn(sourceControlService, 'sanityCheck').mockResolvedValue(undefined);
+			await expect(
+				sourceControlService.pushWorkfolder({
+					fileNames: [
+						{
+							file: '/etc/passwd',
+							id: 'test',
+							name: 'secret-file',
+							type: 'file',
+							status: 'modified',
+							location: 'local',
+							conflict: false,
+							updatedAt: new Date().toISOString(),
+							pushed: false,
+						},
+					],
+				}),
+			).rejects.toThrow('File path /etc/passwd is invalid');
+		});
+	});
+
+	describe('pullWorkfolder', () => {
+		it('does not filter locally created credentials', async () => {
+			// ARRANGE
+			const user = mock<User>();
+			const statuses = [
+				mock<SourceControlledFile>({
+					status: 'created',
+					location: 'local',
+					type: 'credential',
+				}),
+				mock<SourceControlledFile>({
+					status: 'created',
+					location: 'local',
+					type: 'workflow',
+				}),
+			];
+			jest.spyOn(sourceControlService, 'getStatus').mockResolvedValueOnce(statuses);
 
+			// ACT
+			const result = await sourceControlService.pullWorkfolder(user, {});
+
+			// ASSERT
+			expect(result).toMatchObject({ statusCode: 409, statusResult: statuses });
+		});
+
+		it('does not filter remotely deleted credentials', async () => {
+			// ARRANGE
+			const user = mock<User>();
+			const statuses = [
+				mock<SourceControlledFile>({
+					status: 'deleted',
+					location: 'remote',
+					type: 'credential',
+				}),
+				mock<SourceControlledFile>({
+					status: 'created',
+					location: 'local',
+					type: 'workflow',
+				}),
+			];
+			jest.spyOn(sourceControlService, 'getStatus').mockResolvedValueOnce(statuses);
+
+			// ACT
+			const result = await sourceControlService.pullWorkfolder(user, {});
+
+			// ASSERT
+			expect(result).toMatchObject({ statusCode: 409, statusResult: statuses });
+		});
+
+		it('should throw an error if a file is given that is not in the workfolder', async () => {
 			await expect(
 				sourceControlService.pushWorkfolder({
 					fileNames: [
@@ -46,4 +130,85 @@ describe('SourceControlService', () => {
 			).rejects.toThrow('File path /etc/passwd is invalid');
 		});
 	});
+
+	describe('getStatus', () => {
+		it('conflict depends on the value of `direction`', async () => {
+			// ARRANGE
+
+			// Define a credential that does only exist locally.
+			// Pulling this would delete it so it should be marked as a conflict.
+			// Pushing this is conflict free.
+			sourceControlImportService.getRemoteVersionIdsFromFiles.mockResolvedValue([]);
+			sourceControlImportService.getLocalVersionIdsFromDb.mockResolvedValue([
+				mock<SourceControlWorkflowVersionId>(),
+			]);
+
+			// Define a credential that does only exist locally.
+			// Pulling this would delete it so it should be marked as a conflict.
+			// Pushing this is conflict free.
+			sourceControlImportService.getRemoteCredentialsFromFiles.mockResolvedValue([]);
+			sourceControlImportService.getLocalCredentialsFromDb.mockResolvedValue([
+				mock<ExportableCredential & { filename: string }>(),
+			]);
+
+			// Define a variable that does only exist locally.
+			// Pulling this would delete it so it should be marked as a conflict.
+			// Pushing this is conflict free.
+			sourceControlImportService.getRemoteVariablesFromFile.mockResolvedValue([]);
+			sourceControlImportService.getLocalVariablesFromDb.mockResolvedValue([mock<Variables>()]);
+
+			// Define a tag that does only exist locally.
+			// Pulling this would delete it so it should be marked as a conflict.
+			// Pushing this is conflict free.
+			const tag = mock<TagEntity>({ updatedAt: new Date() });
+			tagRepository.find.mockResolvedValue([tag]);
+			sourceControlImportService.getRemoteTagsAndMappingsFromFile.mockResolvedValue({
+				tags: [],
+				mappings: [],
+			});
+			sourceControlImportService.getLocalTagsAndMappingsFromDb.mockResolvedValue({
+				tags: [tag],
+				mappings: [],
+			});
+
+			// ACT
+			const pullResult = await sourceControlService.getStatus({
+				direction: 'pull',
+				verbose: false,
+				preferLocalVersion: false,
+			});
+
+			const pushResult = await sourceControlService.getStatus({
+				direction: 'push',
+				verbose: false,
+				preferLocalVersion: false,
+			});
+
+			// ASSERT
+			console.log(pullResult);
+			console.log(pushResult);
+
+			if (!Array.isArray(pullResult)) {
+				fail('Expected pullResult to be an array.');
+			}
+			if (!Array.isArray(pushResult)) {
+				fail('Expected pushResult to be an array.');
+			}
+
+			expect(pullResult).toHaveLength(4);
+			expect(pushResult).toHaveLength(4);
+
+			expect(pullResult.find((i) => i.type === 'workflow')).toHaveProperty('conflict', true);
+			expect(pushResult.find((i) => i.type === 'workflow')).toHaveProperty('conflict', false);
+
+			expect(pullResult.find((i) => i.type === 'credential')).toHaveProperty('conflict', true);
+			expect(pushResult.find((i) => i.type === 'credential')).toHaveProperty('conflict', false);
+
+			expect(pullResult.find((i) => i.type === 'variables')).toHaveProperty('conflict', true);
+			expect(pushResult.find((i) => i.type === 'variables')).toHaveProperty('conflict', false);
+
+			expect(pullResult.find((i) => i.type === 'tags')).toHaveProperty('conflict', true);
+			expect(pushResult.find((i) => i.type === 'tags')).toHaveProperty('conflict', false);
+		});
+	});
 });

packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts

@@ -9,9 +9,11 @@ import { readFile as fsReadFile } from 'node:fs/promises';
 import path from 'path';
 
 import { ActiveWorkflowManager } from '@/active-workflow-manager';
+import { CredentialsService } from '@/credentials/credentials.service';
 import type { Project } from '@/databases/entities/project';
 import { SharedCredentials } from '@/databases/entities/shared-credentials';
 import type { TagEntity } from '@/databases/entities/tag-entity';
+import type { User } from '@/databases/entities/user';
 import type { Variables } from '@/databases/entities/variables';
 import type { WorkflowTagMapping } from '@/databases/entities/workflow-tag-mapping';
 import { CredentialsRepository } from '@/databases/repositories/credentials.repository';
@@ -25,7 +27,9 @@ import { WorkflowTagMappingRepository } from '@/databases/repositories/workflow-
 import { WorkflowRepository } from '@/databases/repositories/workflow.repository';
 import type { IWorkflowToImport } from '@/interfaces';
 import { isUniqueConstraintError } from '@/response-helper';
+import { TagService } from '@/services/tag.service';
 import { assertNever } from '@/utils';
+import { WorkflowService } from '@/workflows/workflow.service';
 
 import {
 	SOURCE_CONTROL_CREDENTIAL_EXPORT_FOLDER,
@@ -62,6 +66,9 @@ export class SourceControlImportService {
 		private readonly variablesRepository: VariablesRepository,
 		private readonly workflowRepository: WorkflowRepository,
 		private readonly workflowTagMappingRepository: WorkflowTagMappingRepository,
+		private readonly workflowService: WorkflowService,
+		private readonly credentialsService: CredentialsService,
+		private readonly tagService: TagService,
 		instanceSettings: InstanceSettings,
 	) {
 		this.gitFolder = path.join(instanceSettings.n8nFolder, SOURCE_CONTROL_GIT_FOLDER);
@@ -500,6 +507,30 @@ export class SourceControlImportService {
 		return result;
 	}
 
+	async deleteWorkflowsNotInWorkfolder(user: User, candidates: SourceControlledFile[]) {
+		for (const candidate of candidates) {
+			await this.workflowService.delete(user, candidate.id);
+		}
+	}
+
+	async deleteCredentialsNotInWorkfolder(user: User, candidates: SourceControlledFile[]) {
+		for (const candidate of candidates) {
+			await this.credentialsService.delete(user, candidate.id);
+		}
+	}
+
+	async deleteVariablesNotInWorkfolder(candidates: SourceControlledFile[]) {
+		for (const candidate of candidates) {
+			await this.variablesService.delete(candidate.id);
+		}
+	}
+
+	async deleteTagsNotInWorkfolder(candidates: SourceControlledFile[]) {
+		for (const candidate of candidates) {
+			await this.tagService.delete(candidate.id);
+		}
+	}
+
 	private async findOrCreateOwnerProject(owner: ResourceOwner): Promise<Project | null> {
 		if (typeof owner === 'string' || owner.type === 'personal') {
 			const email = typeof owner === 'string' ? owner : owner.personalEmail;

packages/cli/src/environments.ee/source-control/source-control.controller.ee.ts

@@ -191,7 +191,7 @@ export class SourceControlController {
 		@Body payload: PullWorkFolderRequestDto,
 	): Promise<SourceControlledFile[] | ImportResult | PullResult | undefined> {
 		try {
-			const result = await this.sourceControlService.pullWorkfolder(req.user.id, payload);
+			const result = await this.sourceControlService.pullWorkfolder(req.user, payload);
 			res.statusCode = result.statusCode;
 			return result.statusResult;
 		} catch (error) {