feat(security): harden the API and lock synchronizations across instances
- Validate the environment at startup (src/config): required variables must be present, HIBOUTIK_BASE_URL must be HTTPS. - Global ValidationPipe (whitelist, forbidNonWhitelisted, transform). - Helmet, CORS disabled unless CORS_ORIGINS is set, TRUST_PROXY support, HTTP header/request timeouts, shutdown hooks. - Rate limiting: 60 req/min/IP globally, 5 req/min on POST /sync. - AllExceptionsFilter: unexpected errors are logged (message + stack, never the raw error object) and answered with a generic 500. - Postgres advisory lock (dedicated connection) so only one synchronization runs at a time across all instances; released by Postgres itself if the process dies. - Fix the Jest moduleNameMapper (unescaped dot also matched .cjs files). - Document the input validation and security rules in CLAUDE.md. - Add class-validator, class-transformer, helmet and @nestjs/throttler; npm audit fix refreshed package-lock.json. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
20
CLAUDE.md
20
CLAUDE.md
@@ -36,6 +36,10 @@ The client is generated into `prisma/generated/` (generator `prisma-client`, not
|
||||
|
||||
Read from `.env` (`@nestjs/config`, global): `DATABASE_URL` (used by Prisma and `PrismaService`), `HIBOUTIK_BASE_URL`, `HIBOUTIK_USERNAME`, `HIBOUTIK_PASSWORD` (HTTP Basic auth). HTTP calls to Hiboutik time out after 30 s (`HttpModule.register` in `AppModule`).
|
||||
|
||||
The environment is validated at startup (`src/config/env.validation.ts`, wired in `ConfigModule.forRoot`): a missing or malformed required variable stops the app, and `HIBOUTIK_BASE_URL` must be HTTPS (Basic auth). Optional variables: `PORT`, `CORS_ORIGINS` (comma-separated; CORS stays disabled when unset) and `TRUST_PROXY` (number of reverse proxies, so rate limiting sees the real client IP). Any new variable goes in `EnvironmentVariables`; the e2e test gets dummy values from `test/setup-env.ts`.
|
||||
|
||||
Security defaults in `main.ts`/`AppModule`: Helmet, global `ValidationPipe`, `ThrottlerGuard` on every route (60 req/min/IP; `@Throttle` for stricter limits, e.g. `POST /sync` at 5/min), `AllExceptionsFilter` (unexpected errors are logged with message + stack and answered with a generic 500; deliberate `HttpException`s and 4xx HTTP-layer errors pass through), HTTP header/request timeouts (15 s / 30 s), shutdown hooks. Long work must run in the background (like `POST /sync`), never inside a request.
|
||||
|
||||
## Architecture
|
||||
|
||||
Everything is registered as providers in a single `AppModule` (`src/app.module.ts`); new repositories/converters/services must be added there. `PrismaModule` (in `prisma/`, outside `src/`) is global.
|
||||
@@ -50,7 +54,7 @@ Sync pipeline, one entity per layer:
|
||||
|
||||
Domain notes:
|
||||
- Synced models carry a unique `hiboutikId` (join key with Hiboutik), except `Order` (nullable `hiboutikSaleId`), `ProductVersion` (unique `[productId, hiboutikSizeId]`) and `OrderLine`; the internal `id` is a UUID. Empty Hiboutik barcodes are stored as `null`. `ProductVersion.barcode` is indexed but **not unique** (Hiboutik may reuse a barcode).
|
||||
- Hiboutik is the source of truth: brands, categories, suppliers, products and customers missing from it are deleted (skipped if Hiboutik returns an empty list). Products are deleted before the upserts (so a recreated product can reuse its barcode); their versions go by cascade, and versions missing from a product are deleted before its versions are upserted. Customer addresses go by cascade with their customer; other relations (product brand/category/supplier, sub-categories, order customer) are set to null and orders are kept.
|
||||
- Hiboutik is the source of truth: brands, categories, suppliers, products and customers missing from it are deleted. Hiboutik is trusted: a 2xx answer is the truth, even an empty list (everything is then deleted). Deletion only requires that the call did not fail: `HiboutikClient` throws on HTTP errors (4xx/5xx) and `getList`/`getAllPages` also throw on a 2xx body that is not a list (a 2xx without body is an empty list), so a failed step deletes nothing. List endpoints must go through `getList`/`getAllPages`, never a bare `get`. Products are deleted before the upserts (so a recreated product can reuse its barcode); their versions go by cascade, and versions missing from a product are deleted before its versions are upserted. Customer addresses go by cascade with their customer; other relations (product brand/category/supplier, sub-categories, order customer) are set to null and orders are kept.
|
||||
- Brands, categories and suppliers are upserted through `forEachSafely` (`src/utils/sync.ts`): a failing row is logged and skipped. Each sync step and each product/customer is in a try/catch: failures are logged, the sync goes on, and a summary is logged at the end.
|
||||
- Categories form a tree: parents are linked in a second pass (`setParent`). `enabled` mirrors `category_enabled` (POS), `webEnabled` mirrors `category_enabled_www`.
|
||||
- Products: `ProductVersion` rows come from `product_size_details` (barcode, price/discount overrides from `product_specific_rules`, stock from the stocks endpoint); no sizes means one version with `hiboutikSizeId: 0`. `globalStock` is the sum of available stock. `archived` mirrors `product_arch`.
|
||||
@@ -59,6 +63,20 @@ Domain notes:
|
||||
- Orders come from the customer's `sales`: sales already stored as `COMPLETED` are skipped, the others are fetched and upserted on `hiboutikSaleId` (`source: STORE` on creation only). `status` is `COMPLETED` or `PENDING`, `orderedAt` is the completion date else the creation date. Lines are replaced on each upsert and snapshot name, version, barcode, price and Hiboutik ids; the `ProductVersion` link is optional (`SetNull`). `unitPrice` is the paid price, taxes and discount included. Quotations are skipped, anonymous sales are never synced (only customers' sales are read), and orders are never deleted.
|
||||
- Shared Prisma payload types (e.g. `ProductWithVersions`) live in `prisma/customObjects/`.
|
||||
|
||||
## Input validation
|
||||
|
||||
All data coming from API clients (`@Body()`, `@Query()`, `@Param()`) must be validated with NestJS `ValidationPipe` and `class-validator` DTO classes.
|
||||
|
||||
- Register `ValidationPipe` globally in `main.ts` with `whitelist: true`, `forbidNonWhitelisted: true` and `transform: true` (no `if` checks by hand in controllers/services).
|
||||
- Every endpoint that reads a body, query string or route param takes a dedicated input DTO class decorated with `class-validator` rules (type, length, range, format). Cap pagination (`limit`/`take`) with `@Max`. For nested payloads use `@ValidateNested()` + `@Type()`.
|
||||
- Never use Prisma models or Hiboutik DTOs as input types: the `src/models/hiboutikDTO/` types describe data coming from Hiboutik, not from API clients.
|
||||
|
||||
## Security rules
|
||||
|
||||
- **Logs**: never log a raw error object, a request/response or an axios config: they carry the Hiboutik `Authorization` header (credentials). Log `(error as Error).message` and `.stack` only, as the sync services do. No personal data (emails, names, addresses, phone numbers, tax numbers) in logs: ids only.
|
||||
- **SQL**: use the Prisma query API. `$queryRaw`/`$executeRaw` are allowed only as tagged templates with parameters (`` $queryRaw`... ${value}` ``); never `$queryRawUnsafe`/`$executeRawUnsafe` nor string concatenation or interpolation into SQL text.
|
||||
- **Sync lock**: only one synchronization runs at a time across all instances. `HiboutikService.startSync` takes a Postgres advisory lock (`SyncLockDatabaseRepository`, dedicated `pg` connection held for the whole sync, released by Postgres itself if the process dies). Start syncs only through `startSync`, never call `syncAll` directly from a controller or a scheduler.
|
||||
|
||||
## API
|
||||
|
||||
If the Postman plugin is installed, load its `postman:api-engineer` skill before any non-trivial API engineering task (designing, implementing, mocking, testing, monitoring, documenting, deploying). Pure questions and trivial one-line edits do not need it. The local `postman/` and `.postman/` folders are generated by `postman init` and are git-ignored.
|
||||
|
||||
778
package-lock.json
generated
778
package-lock.json
generated
File diff suppressed because it is too large
Load Diff
@@ -26,9 +26,13 @@
|
||||
"@nestjs/config": "^12.0.0",
|
||||
"@nestjs/core": "^11.0.1",
|
||||
"@nestjs/platform-express": "^11.0.1",
|
||||
"@nestjs/throttler": "^6.7.1",
|
||||
"@prisma/adapter-pg": "^7.10.0",
|
||||
"@prisma/client": "^7.10.0",
|
||||
"axios": "1.20.0",
|
||||
"class-transformer": "^0.5.1",
|
||||
"class-validator": "^0.15.1",
|
||||
"helmet": "^8.3.0",
|
||||
"pg": "^8.23.0",
|
||||
"reflect-metadata": "^0.2.2",
|
||||
"rxjs": "^7.8.1"
|
||||
@@ -90,7 +94,7 @@
|
||||
"/node_modules/(?!@nestjs/config/)"
|
||||
],
|
||||
"moduleNameMapper": {
|
||||
"^(.{1,2}/.*).js$": "$1"
|
||||
"^(\\.{1,2}/.*)\\.js$": "$1"
|
||||
}
|
||||
},
|
||||
"allowScripts": {
|
||||
|
||||
@@ -16,14 +16,14 @@ describe('AppController', () => {
|
||||
controller = module.get(AppController);
|
||||
});
|
||||
|
||||
it('starts a synchronization', () => {
|
||||
hiboutikService.startSync.mockReturnValue(true);
|
||||
expect(controller.hiboutikSync()).toEqual({ message: 'Hiboutik synchronization started' });
|
||||
it('starts a synchronization', async () => {
|
||||
hiboutikService.startSync.mockResolvedValue(true);
|
||||
await expect(controller.hiboutikSync()).resolves.toEqual({ message: 'Hiboutik synchronization started' });
|
||||
expect(hiboutikService.startSync).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('rejects a synchronization when one is already running', () => {
|
||||
hiboutikService.startSync.mockReturnValue(false);
|
||||
expect(() => controller.hiboutikSync()).toThrow(ConflictException);
|
||||
it('rejects a synchronization when one is already running', async () => {
|
||||
hiboutikService.startSync.mockResolvedValue(false);
|
||||
await expect(controller.hiboutikSync()).rejects.toThrow(ConflictException);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,15 +1,20 @@
|
||||
import { ConflictException, Controller, HttpCode, HttpStatus, Post } from '@nestjs/common';
|
||||
import { Throttle } from '@nestjs/throttler';
|
||||
import { HiboutikService } from './services/hiboutik/hiboutik.service';
|
||||
|
||||
@Controller()
|
||||
export class AppController {
|
||||
constructor(private readonly hiboutikService: HiboutikService) {}
|
||||
|
||||
/** Starts the synchronization in the background and answers immediately: 202, or 409 if one is already running. */
|
||||
/**
|
||||
* Starts the synchronization in the background and answers immediately: 202, or 409 if one is already running.
|
||||
* Rate limited to 5 calls per minute: each one can trigger thousands of Hiboutik API calls.
|
||||
*/
|
||||
@Post('sync')
|
||||
@Throttle({ default: { limit: 5, ttl: 60_000 } })
|
||||
@HttpCode(HttpStatus.ACCEPTED)
|
||||
hiboutikSync() {
|
||||
if (!this.hiboutikService.startSync()) {
|
||||
async hiboutikSync() {
|
||||
if (!(await this.hiboutikService.startSync())) {
|
||||
throw new ConflictException('A Hiboutik synchronization is already in progress');
|
||||
}
|
||||
return { message: 'Hiboutik synchronization started' };
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
import { APP_FILTER, APP_GUARD } from '@nestjs/core';
|
||||
import { AllExceptionsFilter } from './filters/all-exceptions.filter';
|
||||
import { ThrottlerGuard, ThrottlerModule } from '@nestjs/throttler';
|
||||
import { AppController } from './app.controller';
|
||||
import { ConfigModule } from '@nestjs/config';
|
||||
import { validateEnv } from './config/env.validation';
|
||||
import { HiboutikService } from './services/hiboutik/hiboutik.service';
|
||||
import { CategoryHiboutikRepository } from './hiboutik/category.hiboutik.repository';
|
||||
import { CategoryConverter } from './models/converter/category.converter';
|
||||
@@ -32,17 +36,23 @@ import { CategoryHiboutikService } from './services/hiboutik/category.hiboutik.s
|
||||
import { CustomerHiboutikService } from './services/hiboutik/customer.hiboutik.service';
|
||||
import { SupplierHiboutikService } from './services/hiboutik/supplier.hiboutik.service';
|
||||
import { ProductHiboutikService } from './services/hiboutik/product.hiboutik.service';
|
||||
import { SyncLockDatabaseRepository } from './database/syncLock.database.repository';
|
||||
|
||||
@Module({
|
||||
imports: [
|
||||
ConfigModule.forRoot({
|
||||
isGlobal: true,
|
||||
validate: validateEnv,
|
||||
}),
|
||||
// Default rate limit for every route: 60 requests per minute per IP (stricter limits via @Throttle)
|
||||
ThrottlerModule.forRoot([{ ttl: 60_000, limit: 60 }]),
|
||||
HttpModule.register({ timeout: 30_000 }),
|
||||
PrismaModule,
|
||||
],
|
||||
controllers: [AppController],
|
||||
providers: [
|
||||
{ provide: APP_GUARD, useClass: ThrottlerGuard },
|
||||
{ provide: APP_FILTER, useClass: AllExceptionsFilter },
|
||||
BrandConverter,
|
||||
BrandDatabaseRepository,
|
||||
BrandHiboutikRepository,
|
||||
@@ -70,6 +80,7 @@ import { ProductHiboutikService } from './services/hiboutik/product.hiboutik.ser
|
||||
StockHiboutikRepository,
|
||||
SupplierConverter,
|
||||
SupplierDatabaseRepository,
|
||||
SyncLockDatabaseRepository,
|
||||
SupplierHiboutikRepository,
|
||||
SupplierHiboutikService,
|
||||
],
|
||||
|
||||
39
src/config/env.validation.spec.ts
Normal file
39
src/config/env.validation.spec.ts
Normal file
@@ -0,0 +1,39 @@
|
||||
import 'reflect-metadata';
|
||||
import { validateEnv } from './env.validation';
|
||||
|
||||
describe('validateEnv', () => {
|
||||
const valid = {
|
||||
DATABASE_URL: 'postgresql://user:secret@localhost:5432/db',
|
||||
HIBOUTIK_BASE_URL: 'https://shop.hiboutik.com/api',
|
||||
HIBOUTIK_USERNAME: 'user',
|
||||
HIBOUTIK_PASSWORD: 'secret-password',
|
||||
};
|
||||
|
||||
it('accepts a valid configuration and keeps every variable', () => {
|
||||
const env = { ...valid, PORT: '3000', CORS_ORIGINS: 'https://shop.example.com', TRUST_PROXY: '1', OTHER: 'x' };
|
||||
expect(validateEnv(env)).toBe(env);
|
||||
});
|
||||
|
||||
it('rejects a missing required variable, naming it', () => {
|
||||
expect(() => validateEnv({ ...valid, HIBOUTIK_PASSWORD: undefined })).toThrow(/HIBOUTIK_PASSWORD/);
|
||||
});
|
||||
|
||||
it('rejects a non-HTTPS Hiboutik URL', () => {
|
||||
expect(() => validateEnv({ ...valid, HIBOUTIK_BASE_URL: 'http://shop.hiboutik.com/api' })).toThrow(/HIBOUTIK_BASE_URL/);
|
||||
});
|
||||
|
||||
it('rejects an invalid port', () => {
|
||||
expect(() => validateEnv({ ...valid, PORT: 'abc' })).toThrow(/PORT/);
|
||||
expect(() => validateEnv({ ...valid, PORT: '70000' })).toThrow(/PORT/);
|
||||
});
|
||||
|
||||
it('never puts secret values in the error message', () => {
|
||||
expect.assertions(2);
|
||||
try {
|
||||
validateEnv({ ...valid, HIBOUTIK_BASE_URL: 'http://insecure', DATABASE_URL: '' });
|
||||
} catch (error) {
|
||||
expect((error as Error).message).not.toContain('secret-password');
|
||||
expect((error as Error).message).not.toContain('secret');
|
||||
}
|
||||
});
|
||||
});
|
||||
51
src/config/env.validation.ts
Normal file
51
src/config/env.validation.ts
Normal file
@@ -0,0 +1,51 @@
|
||||
import { plainToInstance } from 'class-transformer';
|
||||
import { IsInt, IsNotEmpty, IsOptional, IsString, IsUrl, Max, Min, validateSync } from 'class-validator';
|
||||
|
||||
/** Environment variables read by the app. A missing or malformed required one stops the startup. */
|
||||
export class EnvironmentVariables {
|
||||
@IsString()
|
||||
@IsNotEmpty()
|
||||
DATABASE_URL!: string;
|
||||
|
||||
/** HTTPS only: the Hiboutik credentials are sent with HTTP Basic auth. */
|
||||
@IsUrl({ protocols: ['https'], require_protocol: true })
|
||||
HIBOUTIK_BASE_URL!: string;
|
||||
|
||||
@IsString()
|
||||
@IsNotEmpty()
|
||||
HIBOUTIK_USERNAME!: string;
|
||||
|
||||
@IsString()
|
||||
@IsNotEmpty()
|
||||
HIBOUTIK_PASSWORD!: string;
|
||||
|
||||
@IsOptional()
|
||||
@IsInt()
|
||||
@Min(1)
|
||||
@Max(65535)
|
||||
PORT?: number;
|
||||
|
||||
/** Comma-separated list of origins allowed by CORS. Empty or unset: CORS stays disabled. */
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
CORS_ORIGINS?: string;
|
||||
|
||||
/** Number of reverse proxies in front of the app (so the real client IP is used for rate limiting). Unset: no proxy. */
|
||||
@IsOptional()
|
||||
@IsInt()
|
||||
@Min(1)
|
||||
@Max(10)
|
||||
TRUST_PROXY?: number;
|
||||
}
|
||||
|
||||
/** `ConfigModule` validation hook: fails fast with the names of the faulty variables (never their values, they may be secrets). */
|
||||
export function validateEnv(config: Record<string, unknown>): Record<string, unknown> {
|
||||
const validated = plainToInstance(EnvironmentVariables, config, { enableImplicitConversion: true });
|
||||
const errors = validateSync(validated, { whitelist: false });
|
||||
|
||||
if (errors.length > 0) {
|
||||
const details = errors.map((error) => `${error.property}: ${Object.values(error.constraints ?? {}).join(', ')}`).join('\n ');
|
||||
throw new Error(`Invalid environment configuration:\n ${details}`);
|
||||
}
|
||||
return config;
|
||||
}
|
||||
74
src/database/syncLock.database.repository.spec.ts
Normal file
74
src/database/syncLock.database.repository.spec.ts
Normal file
@@ -0,0 +1,74 @@
|
||||
import { Logger } from '@nestjs/common';
|
||||
import { Client } from 'pg';
|
||||
import { SyncLockDatabaseRepository } from './syncLock.database.repository';
|
||||
|
||||
jest.mock('pg', () => ({ Client: jest.fn() }));
|
||||
|
||||
describe('SyncLockDatabaseRepository', () => {
|
||||
const client = { on: jest.fn(), connect: jest.fn(), query: jest.fn(), end: jest.fn() };
|
||||
const configService = { get: jest.fn().mockReturnValue('postgresql://user:pass@localhost:5432/db') };
|
||||
let repository: SyncLockDatabaseRepository;
|
||||
|
||||
beforeAll(() => {
|
||||
jest.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined);
|
||||
jest.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined);
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
(Client as unknown as jest.Mock).mockImplementation(() => client);
|
||||
client.connect.mockResolvedValue(undefined);
|
||||
client.end.mockResolvedValue(undefined);
|
||||
client.query.mockResolvedValue({ rows: [{ locked: true }] });
|
||||
repository = new SyncLockDatabaseRepository(configService as never);
|
||||
});
|
||||
|
||||
it('takes the advisory lock on a dedicated connection and keeps it open', async () => {
|
||||
const release = await repository.tryAcquire();
|
||||
|
||||
expect(release).toEqual(expect.any(Function));
|
||||
expect(Client).toHaveBeenCalledWith({ connectionString: 'postgresql://user:pass@localhost:5432/db' });
|
||||
expect(client.query).toHaveBeenCalledWith('SELECT pg_try_advisory_lock($1) AS locked', [expect.any(Number)]);
|
||||
expect(client.end).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns null and closes the connection when the lock is already held', async () => {
|
||||
client.query.mockResolvedValue({ rows: [{ locked: false }] });
|
||||
|
||||
await expect(repository.tryAcquire()).resolves.toBeNull();
|
||||
|
||||
expect(client.end).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('unlocks then closes the connection on release', async () => {
|
||||
const release = await repository.tryAcquire();
|
||||
|
||||
await release!();
|
||||
|
||||
expect(client.query).toHaveBeenLastCalledWith('SELECT pg_advisory_unlock($1)', [expect.any(Number)]);
|
||||
expect(client.end).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('still closes the connection, and does not throw, when the explicit unlock fails', async () => {
|
||||
const release = await repository.tryAcquire();
|
||||
client.query.mockRejectedValueOnce(new Error('connection lost'));
|
||||
|
||||
await expect(release!()).resolves.toBeUndefined();
|
||||
|
||||
expect(client.end).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('closes the connection and rethrows when the lock query fails', async () => {
|
||||
client.query.mockRejectedValue(new Error('boom'));
|
||||
|
||||
await expect(repository.tryAcquire()).rejects.toThrow('boom');
|
||||
|
||||
expect(client.end).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('logs connection errors instead of leaving them unhandled', async () => {
|
||||
await repository.tryAcquire();
|
||||
|
||||
expect(client.on).toHaveBeenCalledWith('error', expect.any(Function));
|
||||
});
|
||||
});
|
||||
52
src/database/syncLock.database.repository.ts
Normal file
52
src/database/syncLock.database.repository.ts
Normal file
@@ -0,0 +1,52 @@
|
||||
import { Injectable, Logger } from '@nestjs/common';
|
||||
import { ConfigService } from '@nestjs/config';
|
||||
import { Client } from 'pg';
|
||||
|
||||
/** Arbitrary constant identifying the Hiboutik synchronization among the Postgres advisory locks of the database. */
|
||||
const SYNC_LOCK_KEY = 727_001;
|
||||
|
||||
/** Function releasing a lock acquired with `tryAcquire`. Never throws. */
|
||||
export type ReleaseLock = () => Promise<void>;
|
||||
|
||||
/**
|
||||
* Cross-instance lock for the synchronization, based on a Postgres session-level advisory lock.
|
||||
* It lives on its own dedicated connection (not the Prisma pool, where lock and unlock could land on different
|
||||
* connections) kept open for as long as the lock is held. If the process dies, Postgres closes the connection and
|
||||
* releases the lock by itself: there is never a stale lock to clean up.
|
||||
*/
|
||||
@Injectable()
|
||||
export class SyncLockDatabaseRepository {
|
||||
private readonly logger = new Logger('SyncLockDatabaseRepository');
|
||||
|
||||
constructor(private readonly configService: ConfigService) {}
|
||||
|
||||
/** @returns the function releasing the lock, or `null` if another instance (or process) already holds it. */
|
||||
async tryAcquire(): Promise<ReleaseLock | null> {
|
||||
const client = new Client({ connectionString: this.configService.get<string>('DATABASE_URL') });
|
||||
// A lost connection means Postgres released the lock: log it instead of crashing the process on an unhandled 'error'
|
||||
client.on('error', (error) => this.logger.error(`Synchronization lock connection error: ${error.message}`));
|
||||
await client.connect();
|
||||
|
||||
try {
|
||||
const { rows } = await client.query<{ locked: boolean }>('SELECT pg_try_advisory_lock($1) AS locked', [SYNC_LOCK_KEY]);
|
||||
if (!rows[0]?.locked) {
|
||||
await client.end();
|
||||
return null;
|
||||
}
|
||||
} catch (error) {
|
||||
await client.end().catch(() => undefined);
|
||||
throw error;
|
||||
}
|
||||
|
||||
return async () => {
|
||||
try {
|
||||
await client.query('SELECT pg_advisory_unlock($1)', [SYNC_LOCK_KEY]);
|
||||
} catch (error) {
|
||||
// Ending the connection below releases the lock anyway
|
||||
this.logger.warn(`Failed to unlock the synchronization lock explicitly: ${(error as Error)?.message}`);
|
||||
} finally {
|
||||
await client.end().catch(() => undefined);
|
||||
}
|
||||
};
|
||||
}
|
||||
}
|
||||
58
src/filters/all-exceptions.filter.spec.ts
Normal file
58
src/filters/all-exceptions.filter.spec.ts
Normal file
@@ -0,0 +1,58 @@
|
||||
import { BadRequestException, Logger } from '@nestjs/common';
|
||||
import { AllExceptionsFilter } from './all-exceptions.filter';
|
||||
|
||||
describe('AllExceptionsFilter', () => {
|
||||
const applicationRef = { reply: jest.fn(), isHeadersSent: jest.fn().mockReturnValue(false), end: jest.fn() };
|
||||
const response = {};
|
||||
const host = { getArgByIndex: jest.fn().mockReturnValue(response) };
|
||||
let filter: AllExceptionsFilter;
|
||||
let logError: jest.SpyInstance;
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
logError = jest.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined);
|
||||
filter = new AllExceptionsFilter(applicationRef as never);
|
||||
});
|
||||
|
||||
it('answers a generic 500 for an unexpected error, without leaking its message', () => {
|
||||
filter.catch(new Error('relation "customer" does not exist'), host as never);
|
||||
|
||||
expect(applicationRef.reply).toHaveBeenCalledTimes(1);
|
||||
const [, body, status] = applicationRef.reply.mock.calls[0] as [unknown, { message: string }, number];
|
||||
expect(status).toBe(500);
|
||||
expect(JSON.stringify(body)).not.toContain('customer');
|
||||
});
|
||||
|
||||
it('logs the message and stack of an unexpected error, never the raw object', () => {
|
||||
const error = Object.assign(new Error('boom'), { config: { headers: { Authorization: 'Basic c2VjcmV0' } } });
|
||||
|
||||
filter.catch(error, host as never);
|
||||
|
||||
expect(logError).toHaveBeenCalledWith('Unhandled exception: boom', error.stack);
|
||||
expect(JSON.stringify(logError.mock.calls)).not.toContain('c2VjcmV0');
|
||||
});
|
||||
|
||||
it('keeps the status and body of deliberate HTTP exceptions', () => {
|
||||
filter.catch(new BadRequestException('limit must be an integer'), host as never);
|
||||
|
||||
const [, body, status] = applicationRef.reply.mock.calls[0] as [unknown, { message: string }, number];
|
||||
expect(status).toBe(400);
|
||||
expect(body.message).toBe('limit must be an integer');
|
||||
expect(logError).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('keeps the 4xx of HTTP-layer errors such as a payload too large', () => {
|
||||
filter.catch(Object.assign(new Error('request entity too large'), { statusCode: 413 }), host as never);
|
||||
|
||||
const [, , status] = applicationRef.reply.mock.calls[0] as [unknown, unknown, number];
|
||||
expect(status).toBe(413);
|
||||
});
|
||||
|
||||
it('treats a 5xx HTTP-layer error as an unexpected one', () => {
|
||||
filter.catch(Object.assign(new Error('upstream detail'), { statusCode: 502 }), host as never);
|
||||
|
||||
const [, body, status] = applicationRef.reply.mock.calls[0] as [unknown, unknown, number];
|
||||
expect(status).toBe(500);
|
||||
expect(JSON.stringify(body)).not.toContain('upstream');
|
||||
});
|
||||
});
|
||||
29
src/filters/all-exceptions.filter.ts
Normal file
29
src/filters/all-exceptions.filter.ts
Normal file
@@ -0,0 +1,29 @@
|
||||
import { ArgumentsHost, Catch, HttpException, InternalServerErrorException, Logger } from '@nestjs/common';
|
||||
import { BaseExceptionFilter } from '@nestjs/core';
|
||||
|
||||
/** Errors raised by the HTTP layer itself (malformed JSON, payload too large...) carry a 4xx `statusCode` but are not `HttpException`s. */
|
||||
function isClientHttpError(exception: unknown): boolean {
|
||||
const statusCode = (exception as { statusCode?: unknown } | null)?.statusCode;
|
||||
return typeof statusCode === 'number' && statusCode >= 400 && statusCode < 500;
|
||||
}
|
||||
|
||||
/**
|
||||
* Last line of defence: anything that is not a deliberate HTTP error (Prisma, axios, bug...) is logged with its stack
|
||||
* (message and stack only, never the raw error object: it may carry credentials) and answered with a generic 500,
|
||||
* so no internal detail (SQL, table names, URLs, stack) ever reaches the client.
|
||||
*/
|
||||
@Catch()
|
||||
export class AllExceptionsFilter extends BaseExceptionFilter {
|
||||
private readonly logger = new Logger('ExceptionsFilter');
|
||||
|
||||
catch(exception: unknown, host: ArgumentsHost): void {
|
||||
if (exception instanceof HttpException || isClientHttpError(exception)) {
|
||||
super.catch(exception, host);
|
||||
return;
|
||||
}
|
||||
|
||||
const error = exception as Error | undefined;
|
||||
this.logger.error(`Unhandled exception: ${error?.message}`, error?.stack);
|
||||
super.catch(new InternalServerErrorException(), host);
|
||||
}
|
||||
}
|
||||
32
src/main.ts
32
src/main.ts
@@ -1,8 +1,36 @@
|
||||
import { ValidationPipe } from '@nestjs/common';
|
||||
import { ConfigService } from '@nestjs/config';
|
||||
import { NestFactory } from '@nestjs/core';
|
||||
import { NestExpressApplication } from '@nestjs/platform-express';
|
||||
import helmet from 'helmet';
|
||||
import { AppModule } from './app.module';
|
||||
|
||||
async function bootstrap() {
|
||||
const app = await NestFactory.create(AppModule);
|
||||
await app.listen(process.env.PORT ?? 3000);
|
||||
const app = await NestFactory.create<NestExpressApplication>(AppModule);
|
||||
const config = app.get(ConfigService);
|
||||
|
||||
// Security headers (also removes X-Powered-By)
|
||||
app.use(helmet());
|
||||
|
||||
// Behind a reverse proxy, trust its X-Forwarded-For so rate limiting sees the real client IP
|
||||
const trustProxy = config.get<number>('TRUST_PROXY');
|
||||
if (trustProxy) app.set('trust proxy', trustProxy);
|
||||
|
||||
// CORS is disabled unless origins are explicitly allowed
|
||||
const origins = (config.get<string>('CORS_ORIGINS') ?? '')
|
||||
.split(',')
|
||||
.map((origin) => origin.trim())
|
||||
.filter(Boolean);
|
||||
if (origins.length > 0) app.enableCors({ origin: origins });
|
||||
|
||||
app.useGlobalPipes(new ValidationPipe({ whitelist: true, forbidNonWhitelisted: true, transform: true }));
|
||||
|
||||
// Disconnect Prisma cleanly on SIGTERM/SIGINT
|
||||
app.enableShutdownHooks();
|
||||
|
||||
const server = await app.listen(config.get<number>('PORT') ?? 3000);
|
||||
// Drop slow clients holding sockets open (Slowloris): every endpoint answers fast, POST /sync runs in the background
|
||||
server.headersTimeout = 15_000;
|
||||
server.requestTimeout = 30_000;
|
||||
}
|
||||
void bootstrap();
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -57,12 +63,58 @@ describe('HiboutikService', () => {
|
||||
let finishBrands: () => void = () => undefined;
|
||||
brandService.syncBrands.mockReturnValue(new Promise<void>((resolve) => (finishBrands = resolve)));
|
||||
|
||||
expect(service.startSync()).toBe(true);
|
||||
await expect(service.startSync()).resolves.toBe(true);
|
||||
expect(service.isSyncInProgress).toBe(true);
|
||||
expect(service.startSync()).toBe(false);
|
||||
await expect(service.startSync()).resolves.toBe(false);
|
||||
expect(syncLock.tryAcquire).toHaveBeenCalledTimes(1);
|
||||
expect(release).not.toHaveBeenCalled();
|
||||
|
||||
finishBrands();
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
await flush();
|
||||
expect(release).toHaveBeenCalledTimes(1);
|
||||
expect(service.isSyncInProgress).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses concurrent calls on the same instance while the lock is being requested', async () => {
|
||||
let grantLock: (value: typeof release) => void = () => undefined;
|
||||
syncLock.tryAcquire.mockReturnValue(new Promise((resolve) => (grantLock = resolve)));
|
||||
|
||||
const first = service.startSync();
|
||||
await expect(service.startSync()).resolves.toBe(false);
|
||||
|
||||
grantLock(release);
|
||||
await expect(first).resolves.toBe(true);
|
||||
await flush();
|
||||
expect(syncLock.tryAcquire).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('starts nothing when another instance holds the lock', async () => {
|
||||
syncLock.tryAcquire.mockResolvedValue(null);
|
||||
|
||||
await expect(service.startSync()).resolves.toBe(false);
|
||||
|
||||
expect(service.isSyncInProgress).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(service.isSyncInProgress).toBe(false);
|
||||
|
||||
await expect(service.startSync()).resolves.toBe(true);
|
||||
await flush();
|
||||
});
|
||||
|
||||
it('releases the lock even when the synchronization crashes', async () => {
|
||||
jest.spyOn(service, 'syncAll').mockRejectedValue(new Error('crash'));
|
||||
|
||||
await expect(service.startSync()).resolves.toBe(true);
|
||||
await flush();
|
||||
|
||||
expect(release).toHaveBeenCalledTimes(1);
|
||||
expect(service.isSyncInProgress).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4,12 +4,13 @@ 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. */
|
||||
/** Cheap in-process guard, checked before the database lock (which is the one that holds across instances). */
|
||||
private syncInProgress = false;
|
||||
|
||||
constructor(
|
||||
@@ -18,6 +19,7 @@ export class HiboutikService {
|
||||
private readonly customerHiboutikService: CustomerHiboutikService,
|
||||
private readonly productHiboutikService: ProductHiboutikService,
|
||||
private readonly supplierHiboutikService: SupplierHiboutikService,
|
||||
private readonly syncLock: SyncLockDatabaseRepository,
|
||||
) {}
|
||||
|
||||
get isSyncInProgress(): boolean {
|
||||
@@ -25,19 +27,44 @@ export class HiboutikService {
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
* (Postgres advisory lock, 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 {
|
||||
async startSync(): Promise<boolean> {
|
||||
if (this.syncInProgress) return false;
|
||||
|
||||
// Set before the first await so that concurrent calls on this instance are refused while the lock is requested
|
||||
this.syncInProgress = true;
|
||||
void this.syncAll()
|
||||
.catch((error) => this.logger.error('Global Hiboutik synchronization crashed', (error as Error)?.stack))
|
||||
.finally(() => (this.syncInProgress = false));
|
||||
|
||||
let release: ReleaseLock | null;
|
||||
try {
|
||||
release = await this.syncLock.tryAcquire();
|
||||
} catch (error) {
|
||||
this.syncInProgress = false;
|
||||
throw error;
|
||||
}
|
||||
if (!release) {
|
||||
this.syncInProgress = false;
|
||||
return 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();
|
||||
this.syncInProgress = false;
|
||||
}
|
||||
}
|
||||
|
||||
/** Runs every sync step in order. A failing step is logged and does not stop the next ones. */
|
||||
async syncAll() {
|
||||
this.logger.log('Starting global Hiboutik synchronization');
|
||||
|
||||
@@ -27,12 +27,12 @@ describe('AppController (e2e)', () => {
|
||||
});
|
||||
|
||||
it('POST /sync starts a synchronization (202)', () => {
|
||||
hiboutikService.startSync.mockReturnValue(true);
|
||||
hiboutikService.startSync.mockResolvedValue(true);
|
||||
return request(app.getHttpServer()).post('/sync').expect(202).expect({ message: 'Hiboutik synchronization started' });
|
||||
});
|
||||
|
||||
it('POST /sync answers 409 when a synchronization is already running', () => {
|
||||
hiboutikService.startSync.mockReturnValue(false);
|
||||
hiboutikService.startSync.mockResolvedValue(false);
|
||||
return request(app.getHttpServer()).post('/sync').expect(409);
|
||||
});
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
"rootDir": ".",
|
||||
"testEnvironment": "node",
|
||||
"testRegex": ".e2e-spec.ts$",
|
||||
"setupFiles": ["<rootDir>/setup-env.ts"],
|
||||
"transform": {
|
||||
"^.+.ts$": "ts-jest",
|
||||
"^.+.js$": [
|
||||
@@ -18,6 +19,6 @@
|
||||
},
|
||||
"transformIgnorePatterns": ["/node_modules/(?!@nestjs/config/)"],
|
||||
"moduleNameMapper": {
|
||||
"^(.{1,2}/.*).js$": "$1"
|
||||
"^(\\.{1,2}/.*)\\.js$": "$1"
|
||||
}
|
||||
}
|
||||
|
||||
5
test/setup-env.ts
Normal file
5
test/setup-env.ts
Normal file
@@ -0,0 +1,5 @@
|
||||
// The e2e test needs neither a database nor Hiboutik, but the app validates its environment at startup: provide dummy values.
|
||||
process.env.DATABASE_URL ??= 'postgresql://test:test@localhost:5432/test';
|
||||
process.env.HIBOUTIK_BASE_URL ??= 'https://example.hiboutik.test/api';
|
||||
process.env.HIBOUTIK_USERNAME ??= 'test';
|
||||
process.env.HIBOUTIK_PASSWORD ??= 'test';
|
||||
Reference in New Issue
Block a user