feat(security): harden the API, trust Hiboutik for deletions, lock syncs (#3)

src/services/hiboutik/brand.hiboutik.service.ts

@@ -25,11 +25,7 @@ export class BrandHiboutikService {
       (brand) => `Hiboutik brand ${brand.hiboutikId}`,
     );
 
-    // Safety net: an empty answer from Hiboutik most likely means an API issue
-    if (hiboutikBrands.length === 0) {
-      this.logger.warn('Hiboutik returned no brands: deletion skipped.');
-      return;
-    }
+    // Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
     const nbDeleted = await this.brandDatabaseRepository.deleteMissing(hiboutikBrands.map((dto) => Number(dto.brand_id)));
     this.logger.log(
       `Brands sync ended: ${hiboutikBrands.length - nbFailed}/${hiboutikBrands.length} synced, ${nbFailed} failed, ${nbDeleted} deleted.`,

src/services/hiboutik/category.hiboutik.service.spec.ts

@@ -79,11 +79,19 @@ describe('CategoryHiboutikService', () => {
     expect(categoryDatabaseRepository.deleteMissing).toHaveBeenCalledWith([1, 2]);
   });
 
-  it('does not delete anything when Hiboutik returns no categories', async () => {
+  it('deletes every category when Hiboutik answers with an empty list', async () => {
     categoryHiboutikRepository.getCategories.mockResolvedValue([]);
 
     await service.syncCategories();
 
+    expect(categoryDatabaseRepository.deleteMissing).toHaveBeenCalledWith([]);
+  });
+
+  it('deletes nothing when the Hiboutik call fails', async () => {
+    categoryHiboutikRepository.getCategories.mockRejectedValue(new Error('Request failed with status code 500'));
+
+    await expect(service.syncCategories()).rejects.toThrow('500');
+
     expect(categoryDatabaseRepository.deleteMissing).not.toHaveBeenCalled();
   });
 });

src/services/hiboutik/category.hiboutik.service.ts

@@ -37,11 +37,7 @@ export class CategoryHiboutikService {
       (dto) => `parent of Hiboutik category ${dto.category_id}`,
     );
 
-    // Safety net: an empty answer from Hiboutik most likely means an API issue
-    if (hiboutikCategories.length === 0) {
-      this.logger.warn('Hiboutik returned no categories: deletion skipped.');
-      return;
-    }
+    // Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
     const nbDeleted = await this.categoryDatabaseRepository.deleteMissing(hiboutikCategories.map((dto) => Number(dto.category_id)));
     this.logger.log(
       `Categories sync ended: ${hiboutikCategories.length - nbFailed}/${hiboutikCategories.length} synced, ${nbFailed} failed, ` +

src/services/hiboutik/customer.hiboutik.service.spec.ts

@@ -48,9 +48,17 @@ describe('CustomerHiboutikService', () => {
     expect(customerDatabaseRepository.upsert).toHaveBeenCalledTimes(2);
   });
 
-  it('does not delete customers when Hiboutik returns none', async () => {
+  it('deletes every customer when Hiboutik answers with an empty list', async () => {
     await service.syncCustomers();
 
+    expect(customerDatabaseRepository.deleteMissing).toHaveBeenCalledWith([]);
+  });
+
+  it('deletes nothing when the Hiboutik call fails', async () => {
+    customerHiboutikRepository.getCustomers.mockRejectedValue(new Error('Request failed with status code 500'));
+
+    await expect(service.syncCustomers()).rejects.toThrow('500');
+
     expect(customerDatabaseRepository.deleteMissing).not.toHaveBeenCalled();
   });
 

src/services/hiboutik/customer.hiboutik.service.ts

@@ -37,13 +37,8 @@ export class CustomerHiboutikService {
   async syncCustomers() {
     const hiboutikCustomers = await this.customerHiboutikRepository.getCustomers();
 
-    // Safety net: an empty answer from Hiboutik most likely means an API issue
-    let nbDeleted = 0;
-    if (hiboutikCustomers.length === 0) {
-      this.logger.warn('Hiboutik returned no customers: deletion skipped.');
-    } else {
-      nbDeleted = await this.customerDatabaseRepository.deleteMissing(hiboutikCustomers.map((dto) => Number(dto.customers_id)));
-    }
+    // Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
+    const nbDeleted = await this.customerDatabaseRepository.deleteMissing(hiboutikCustomers.map((dto) => Number(dto.customers_id)));
 
     let nbFailed = 0;
     let nbFailedAddresses = 0;

src/services/hiboutik/hiboutik.service.spec.ts

@@ -7,7 +7,10 @@ describe('HiboutikService', () => {
   const supplierService = { syncSuppliers: jest.fn() };
   const productService = { syncProducts: jest.fn() };
   const customerService = { syncCustomers: jest.fn() };
+  const syncLock = { tryAcquire: jest.fn() };
+  const release = jest.fn();
   let service: HiboutikService;
+  const flush = () => new Promise((resolve) => setImmediate(resolve));
 
   beforeAll(() => {
     jest.spyOn(Logger.prototype, 'log').mockImplementation(() => undefined);
@@ -22,12 +25,15 @@ describe('HiboutikService', () => {
     supplierService.syncSuppliers.mockResolvedValue(undefined);
     productService.syncProducts.mockResolvedValue(undefined);
     customerService.syncCustomers.mockResolvedValue(undefined);
+    release.mockResolvedValue(undefined);
+    syncLock.tryAcquire.mockResolvedValue(release);
     service = new HiboutikService(
       brandService as never,
       categoryService as never,
       customerService as never,
       productService as never,
       supplierService as never,
+      syncLock as never,
     );
   });
 
@@ -56,13 +62,44 @@ describe('HiboutikService', () => {
   it('refuses a second synchronization while one is running, and unlocks at the end', async () => {
     let finishBrands: () => void = () => undefined;
     brandService.syncBrands.mockReturnValue(new Promise<void>((resolve) => (finishBrands = resolve)));
+    // The lock is the only guard: the second request does not get it, even from the same instance
+    syncLock.tryAcquire.mockResolvedValueOnce(release).mockResolvedValueOnce(null);
 
-    expect(service.startSync()).toBe(true);
-    expect(service.isSyncInProgress).toBe(true);
-    expect(service.startSync()).toBe(false);
+    await expect(service.startSync()).resolves.toBe(true);
+    await expect(service.startSync()).resolves.toBe(false);
+    expect(brandService.syncBrands).toHaveBeenCalledTimes(1);
+    expect(release).not.toHaveBeenCalled();
 
     finishBrands();
-    await new Promise((resolve) => setImmediate(resolve));
-    expect(service.isSyncInProgress).toBe(false);
+    await flush();
+    expect(release).toHaveBeenCalledTimes(1);
+  });
+
+  it('starts nothing when another instance holds the lock', async () => {
+    syncLock.tryAcquire.mockResolvedValue(null);
+
+    await expect(service.startSync()).resolves.toBe(false);
+
+    expect(brandService.syncBrands).not.toHaveBeenCalled();
+    expect(release).not.toHaveBeenCalled();
+  });
+
+  it('starts nothing and can be retried when the lock cannot be requested', async () => {
+    syncLock.tryAcquire.mockRejectedValueOnce(new Error('database unreachable'));
+
+    await expect(service.startSync()).rejects.toThrow('database unreachable');
+    expect(brandService.syncBrands).not.toHaveBeenCalled();
+
+    await expect(service.startSync()).resolves.toBe(true);
+    await flush();
+  });
+
+  it('releases the lock even when the synchronization crashes', async () => {
+    jest.spyOn(service as never, 'syncAll').mockRejectedValue(new Error('crash'));
+
+    await expect(service.startSync()).resolves.toBe(true);
+    await flush();
+
+    expect(release).toHaveBeenCalledTimes(1);
   });
 });

src/services/hiboutik/hiboutik.service.ts

@@ -4,13 +4,12 @@ import { CategoryHiboutikService } from './category.hiboutik.service';
 import { CustomerHiboutikService } from './customer.hiboutik.service';
 import { SupplierHiboutikService } from './supplier.hiboutik.service';
 import { ProductHiboutikService } from './product.hiboutik.service';
+import { ReleaseLock, SyncLockDatabaseRepository } from '../../database/syncLock.database.repository';
 
 /** Entry point of the Hiboutik → database synchronization: runs each entity sync in dependency order. */
 @Injectable()
 export class HiboutikService {
   private readonly logger = new Logger('HiboutikService');
-  /** In-memory lock: only valid while the app runs as a single instance. */
-  private syncInProgress = false;
 
   constructor(
     private readonly brandHiboutikService: BrandHiboutikService,
@@ -18,28 +17,37 @@ export class HiboutikService {
     private readonly customerHiboutikService: CustomerHiboutikService,
     private readonly productHiboutikService: ProductHiboutikService,
     private readonly supplierHiboutikService: SupplierHiboutikService,
+    private readonly syncLock: SyncLockDatabaseRepository,
   ) {}
 
-  get isSyncInProgress(): boolean {
-    return this.syncInProgress;
-  }
-
   /**
-   * Starts a full synchronization in the background.
+   * Starts a full synchronization in the background, if no other one is running on this instance or any other.
+   * The Postgres advisory lock is the only guard: each request takes it on its own connection (its own session),
+   * so it also refuses concurrent calls on this very instance. It is held until the synchronization ends.
    * @returns false if a synchronization is already running (nothing is started).
+   * @throws if the lock cannot be requested (e.g. database unreachable).
    */
-  startSync(): boolean {
-    if (this.syncInProgress) return false;
+  async startSync(): Promise<boolean> {
+    const release = await this.syncLock.tryAcquire();
+    if (!release) return false;
 
-    this.syncInProgress = true;
-    void this.syncAll()
-      .catch((error) => this.logger.error('Global Hiboutik synchronization crashed', (error as Error)?.stack))
-      .finally(() => (this.syncInProgress = false));
+    void this.runLocked(release);
     return true;
   }
 
+  /** Runs the synchronization, then always releases the lock. */
+  private async runLocked(release: ReleaseLock): Promise<void> {
+    try {
+      await this.syncAll();
+    } catch (error) {
+      this.logger.error('Global Hiboutik synchronization crashed', (error as Error)?.stack);
+    } finally {
+      await release();
+    }
+  }
+
   /** Runs every sync step in order. A failing step is logged and does not stop the next ones. */
-  async syncAll() {
+  private async syncAll() {
     this.logger.log('Starting global Hiboutik synchronization');
     // Order matters: products connect to brands, categories and suppliers by hiboutikId.
     const steps: [string, () => Promise<void>][] = [

src/services/hiboutik/product.hiboutik.service.spec.ts

@@ -89,12 +89,21 @@ describe('ProductHiboutikService', () => {
     expect(calls[0]).toBe('deleteProducts');
   });
 
-  it('does not delete anything when Hiboutik returns no products', async () => {
+  it('deletes every product when Hiboutik answers with an empty list', async () => {
     productHiboutikRepository.getProducts.mockResolvedValue([]);
     productDatabaseRepository.getAll.mockResolvedValue([{ id: 'uuid-1', hiboutikId: 1 }]);
 
     await service.syncProducts();
 
+    expect(productDatabaseRepository.deleteAll).toHaveBeenCalledWith(['uuid-1']);
+  });
+
+  it('deletes nothing when the Hiboutik call fails', async () => {
+    productHiboutikRepository.getProducts.mockRejectedValue(new Error('Request failed with status code 500'));
+    productDatabaseRepository.getAll.mockResolvedValue([{ id: 'uuid-1', hiboutikId: 1 }]);
+
+    await expect(service.syncProducts()).rejects.toThrow('500');
+
     expect(productDatabaseRepository.deleteAll).not.toHaveBeenCalled();
   });
 

src/services/hiboutik/product.hiboutik.service.ts

@@ -27,7 +27,6 @@ export class ProductHiboutikService {
 
   /** Mirrors Hiboutik products, their versions (sizes) and stocks. Products missing from Hiboutik are deleted. */
   async syncProducts() {
-    this.logger.log('Starting products sync');
     const [productsHiboutik, hiboutikStocks, existingProducts] = await Promise.all([
       this.productHiboutikRepository.getProducts(),
       this.stockHiboutikRepository.getStocks(),
@@ -56,12 +55,7 @@ export class ProductHiboutikService {
 
   /** @returns the number of deleted products. */
   private async deleteMissingProducts(productsHiboutik: ProductHiboutikDto[], existingProducts: Product[]): Promise<number> {
-    // Safety net: an empty answer from Hiboutik most likely means an API issue, not an empty catalog
-    if (productsHiboutik.length === 0 && existingProducts.length > 0) {
-      this.logger.warn(`Hiboutik returned no products while ${existingProducts.length} exist in database: deletion skipped.`);
-      return 0;
-    }
-
+    // Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
     const hiboutikIds = new Set(productsHiboutik.map((product) => Number(product.product_id)));
     const missingProductIds = existingProducts.filter((product) => !hiboutikIds.has(product.hiboutikId)).map((product) => product.id);
     return this.productDatabaseRepository.deleteAll(missingProductIds);

src/services/hiboutik/supplier.hiboutik.service.ts

@@ -25,11 +25,7 @@ export class SupplierHiboutikService {
       (supplier) => `Hiboutik supplier ${supplier.hiboutikId}`,
     );
 
-    // Safety net: an empty answer from Hiboutik most likely means an API issue
-    if (hiboutikSuppliers.length === 0) {
-      this.logger.warn('Hiboutik returned no suppliers: deletion skipped.');
-      return;
-    }
+    // Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
     const nbDeleted = await this.supplierDatabaseRepository.deleteMissing(hiboutikSuppliers.map((dto) => Number(dto.supplier_id)));
     this.logger.log(
       `Suppliers sync ended: ${hiboutikSuppliers.length - nbFailed}/${hiboutikSuppliers.length} synced, ${nbFailed} failed, ${nbDeleted} deleted.`,