From fe10975927e1c6907cc436e1dd84b7eaf0fa8996 Mon Sep 17 00:00:00 2001 From: Paul Rastoin <45004772+prastoin@users.noreply.github.com> Date: Thu, 16 Jul 2026 16:26:23 +0200 Subject: [PATCH] fix(billing): guard workspace suspend against concurrent soft-delete (#22955) ## Context Follow-up to #22943, addressing the cubic review comments left on that PR (handled in a follow-up as agreed in the thread). In `BillingWebhookSubscriptionService.processStripeEvent`, the suspend/reactivate decision re-reads the workspace and then acts on it. A concurrent soft-delete landing in that window could transition an already soft-deleted workspace to `SUSPENDED`, contrary to the guarded-transition behavior (cubic P2). ## Fix Added `deletedAt: IsNull()` to the `suspendWorkspace` compare-and-swap WHERE clause, matching the guard already present in `reactivateWorkspace`. A concurrent soft-delete now blocks the suspension instead of transitioning a deleted workspace to `SUSPENDED`. ## On the delete guard (cubic P1) Cubic also flagged that the `PENDING_CREATION` hard-delete path could hard-delete a concurrently soft-deleted workspace. On review this is not worth guarding: - A `PENDING_CREATION` workspace has no DB schema and no records (activation is what creates them), so there is no data to lose. - The cleaner already hard-deletes soft-deleted workspaces by design (`cleaner.workspace-service.ts` soft-deletes a pending workspace, then hard-deletes it on a later run), so "hard delete an already soft-deleted workspace" is a supported transition, not corruption. So P1 is intentionally left out to keep `deleteWorkspace` and all its callers unchanged. ## Tests - `suspendWorkspace` update includes `deletedAt IS NULL` and reports whether the guarded update applied. Typecheck, lint, and format pass on the changed files. --- .../__tests__/workspace.service.spec.ts | 36 ++++++++++++++++++- .../workspace/services/workspace.service.ts | 1 + 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/packages/twenty-server/src/engine/core-modules/workspace/services/__tests__/workspace.service.spec.ts b/packages/twenty-server/src/engine/core-modules/workspace/services/__tests__/workspace.service.spec.ts index b30da46b96..5c8fcfbd0b 100644 --- a/packages/twenty-server/src/engine/core-modules/workspace/services/__tests__/workspace.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/workspace/services/__tests__/workspace.service.spec.ts @@ -1,7 +1,8 @@ import { Test, type TestingModule } from '@nestjs/testing'; import { getDataSourceToken, getRepositoryToken } from '@nestjs/typeorm'; -import { type Repository } from 'typeorm'; +import { WorkspaceActivationStatus } from 'twenty-shared/workspace'; +import { IsNull, Not, type Repository } from 'typeorm'; import { BillingSubscriptionService } from 'src/engine/core-modules/billing/services/billing-subscription.service'; import { BillingService } from 'src/engine/core-modules/billing/services/billing.service'; @@ -57,6 +58,7 @@ describe('WorkspaceService', () => { findOne: jest.fn(), softDelete: jest.fn(), delete: jest.fn(), + update: jest.fn(), manager: { connection: { driver: { options: { type: 'postgres' } } }, }, @@ -374,4 +376,36 @@ describe('WorkspaceService', () => { }); }); }); + + describe('suspendWorkspace', () => { + it('should only suspend workspaces that are not already suspended and not soft-deleted', async () => { + jest + .spyOn(workspaceRepository, 'update') + .mockResolvedValue({ affected: 1 } as never); + + const hasBeenSuspended = await service.suspendWorkspace('workspace-id'); + + expect(workspaceRepository.update).toHaveBeenCalledWith( + { + id: 'workspace-id', + activationStatus: Not(WorkspaceActivationStatus.SUSPENDED), + deletedAt: IsNull(), + }, + expect.objectContaining({ + activationStatus: WorkspaceActivationStatus.SUSPENDED, + }), + ); + expect(hasBeenSuspended).toBe(true); + }); + + it('should report no suspension when the guarded update affects no rows', async () => { + jest + .spyOn(workspaceRepository, 'update') + .mockResolvedValue({ affected: 0 } as never); + + const hasBeenSuspended = await service.suspendWorkspace('workspace-id'); + + expect(hasBeenSuspended).toBe(false); + }); + }); }); diff --git a/packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts b/packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts index 48707b50c6..bb6b722dfd 100644 --- a/packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts +++ b/packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts @@ -493,6 +493,7 @@ export class WorkspaceService { { id, activationStatus: Not(WorkspaceActivationStatus.SUSPENDED), + deletedAt: IsNull(), }, { activationStatus: WorkspaceActivationStatus.SUSPENDED,