fix: Node low-severity security hardening bundle (timing oracle, sharp DoS, dependabot, deps, audit)

.github/workflows/dependabot.yml

@@ -14,18 +14,14 @@ jobs:
       - name: Dependabot metadata
         id: metadata
         uses: dependabot/fetch-metadata@v2
-      - name: Approve patch and minor updates
-        if: |
-          steps.metadata.outputs.update-type == 'version-update:semver-patch' ||
-          steps.metadata.outputs.update-type == 'version-update:semver-minor'
+      - name: Approve patch updates
+        if: steps.metadata.outputs.update-type == 'version-update:semver-patch'
         run: gh pr review --approve "$PR_URL"
         env:
           PR_URL: ${{ github.event.pull_request.html_url }}
           GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-      - name: Enable auto-merge for patch and minor updates
-        if: |
-          steps.metadata.outputs.update-type == 'version-update:semver-patch' ||
-          steps.metadata.outputs.update-type == 'version-update:semver-minor'
+      - name: Enable auto-merge for patch updates
+        if: steps.metadata.outputs.update-type == 'version-update:semver-patch'
         run: gh pr merge --auto --squash "$PR_URL"
         env:
           PR_URL: ${{ github.event.pull_request.html_url }}

ERRORS.md

@@ -29,3 +29,4 @@ Use this file as a compact memory of recurring AI mistakes.
 - [2026-05-05] repository: Repository.update(doc) doing `new Model(doc).save()` rewrites the full document from in-memory state, silently clobbering any concurrent partial update that landed after the read -> always use `Model.updateOne({ _id }, { $set: ... })` or `findOneAndUpdate({ _id }, { $set: ... })` for partial updates to avoid race conditions; see comes-io/trawl_node#1115 comes-io/trawl_node#1116 comes-io/trawl_node#1118 + pierreb-devkit/Node#3605
 - [2026-05-31] billing/stripe: reading `price.metadata.planId` in `customer.subscription.updated` webhook handler -> field is EMPTY in real Stripe webhook payloads (planId lives on the Product, not the Price); use a `priceId → plan` map built at boot from `config.stripe.prices` instead; see pierreb-devkit/Node#3742
 - [2026-06-04] repository: top-level `const Foo = mongoose.model('Foo')` in a repository file -> this is evaluated at import time; safe in an HTTP server (loadModels() runs first) but silently crashes standalone scripts (crons, migrations) with `MissingSchemaError` when import order differs; tests miss it because jest mocks intercept the module entirely; fix = lazy getter `const Foo = () => mongoose.model('Foo')` (call sites: `Foo().find(...)`) or dynamic import after `loadModels()` in the entrypoint; see comes-io/trawl_node#1337 comes-io/trawl_node#1338 pierreb-devkit/Node#3789
+- [2026-06-15] deps/audit: leaving `npm audit` advisories unaddressed on the assumption they need a major bump -> run `npm audit fix` (never `--force`) first; the runtime-tree DoS/ReDoS items (`qs`, `path-to-regexp`, `brace-expansion`) all fixed via in-range bumps, no residual. These are DoS-class but NOT attacker-reachable in this stack: Express route patterns are static (no user-controlled `path-to-regexp` input) and `qs`/`brace-expansion` only parse server-side query strings under fixed code paths — still bump them to keep the tree clean and avoid scanner noise.

config/defaults/production.config.js

@@ -53,6 +53,15 @@ const config = {
       standardHeaders: true,
       legacyHeaders: false,
     },
+    // Public, unauthenticated image route that runs the full sharp pipeline on
+    // every request. Stricter prod cap to harden against CPU-exhaustion DoS.
+    publicImage: {
+      windowMs: 60 * 1000,
+      max: 60,
+      message: { message: 'Too many requests, please try again later.' },
+      standardHeaders: true,
+      legacyHeaders: false,
+    },
   },
   log: {
     format: 'custom',

config/defaults/test.config.js

@@ -40,6 +40,9 @@ const config = {
     billingPlans: {
       max: Number.MAX_SAFE_INTEGER, // disable rate limiting in tests
     },
+    publicImage: {
+      max: Number.MAX_SAFE_INTEGER, // disable rate limiting in tests
+    },
   },
   uploads: {
     avatar: {

lib/middlewares/tests/rateLimiter.baseLayer.unit.tests.js

@@ -5,6 +5,7 @@ import { describe, test, expect } from '@jest/globals';
 import organizationsDevConfig from '../../../modules/organizations/config/organizations.development.config.js';
 import billingDevConfig from '../../../modules/billing/config/billing.development.config.js';
 import authDevConfig from '../../../modules/auth/config/auth.development.config.js';
+import uploadsDevConfig from '../../../modules/uploads/config/uploads.development.config.js';
 
 /**
  * Config-layering regression guard for the rate-limiter env-gate defect.
@@ -50,4 +51,11 @@ describe('rate-limiter base-layer profiles (env-gate config-layering):', () => {
   test('billingPlans profile lives in the billing base layer (always merges)', () => {
     expectUsableProfile(billingDevConfig.rateLimit.billingPlans);
   });
+
+  test('publicImage profile lives in the uploads base layer (always merges)', () => {
+    // Guards the unauthenticated /api/uploads/images/:imageName route, which runs
+    // the full sharp pipeline on every request. The profile must be present in
+    // the base layer so the limiter is active under every NODE_ENV, not only prod.
+    expectUsableProfile(uploadsDevConfig.rateLimit.publicImage);
+  });
 });

lib/middlewares/tests/rateLimiter.unit.tests.js

@@ -95,6 +95,21 @@ describe('rateLimiter middleware unit tests:', () => {
     });
   });
 
+  describe('named profile resolution', () => {
+    test('publicImage resolves to a real limiter (not passthrough) when the profile is present', () => {
+      mockRateLimitConfig = {
+        publicImage: { windowMs: 60 * 1000, max: 60 },
+      };
+      const callsBefore = calls.length;
+      const middleware = limiters.publicImage;
+      // A real limiter is created via the mocked rateLimit() factory, which
+      // stamps _rateLimitOpts; a passthrough no-op would not.
+      expect(calls.length).toBe(callsBefore + 1);
+      expect(middleware._rateLimitOpts).toBeDefined();
+      expect(middleware._rateLimitOpts.max).toBe(60);
+    });
+  });
+
   describe('caching', () => {
     test('should cache limiter and not recreate on second access', () => {
       mockRateLimitConfig = {

modules/auth/services/auth.service.js

@@ -10,6 +10,13 @@ import AppError from '../../../lib/helpers/AppError.js';
 import { hashPassword, comparePassword } from '../../../lib/helpers/password.js';
 import UserService from '../../users/services/users.service.js';
 
+// Precomputed valid bcrypt hash (cost=10, 60 chars) used as a constant-work
+// sentinel on the unknown-user login path. Running a real compare against it
+// equalises response timing with the password-mismatch path so the endpoint
+// cannot be used as an account-enumeration oracle. Mirrors the forgot-password
+// dummy-compare pattern.
+const DUMMY_PASSWORD_HASH = '$2b$10$wotSb2xPb8VF8lpBkulk0.e2xvYvDedhjMyuG/7w3GdPl1vdvogY2';
+
 /**
  * @desc Local function to removeSensitive data from user
  * @param {Object} user
@@ -84,7 +91,13 @@ const recordSuccessfulLogin = async (user) => {
  */
 const authenticate = async (email, password) => {
   const user = await UserService.getBrut({ email });
-  if (!user) throw new AppError('invalid user or password.', { code: 'SERVICE_ERROR' });
+  if (!user) {
+    // Unknown account — run a dummy compare against the sentinel hash to keep
+    // the work (and thus the response timing) indistinguishable from a real
+    // password mismatch, then throw the same generic error (anti-enumeration).
+    await comparePassword(password, DUMMY_PASSWORD_HASH);
+    throw new AppError('invalid user or password.', { code: 'SERVICE_ERROR' });
+  }
 
   // Check lockout before attempting password comparison
   await checkLockout(user);

modules/auth/tests/auth.unit.tests.js

@@ -97,6 +97,20 @@ describe('Auth service unit tests:', () => {
       await expect(AuthService.authenticate('a@b.com', 'pass')).rejects.toThrow('invalid user or password.');
     });
 
+    test('should run a dummy password compare on the unknown-user path to equalise timing (anti-enumeration)', async () => {
+      // Unknown email: the service must still run a bcrypt compare against a
+      // sentinel hash so response timing does not reveal whether the account
+      // exists, THEN throw the same generic error as a wrong-password attempt.
+      mockGetBrut.mockResolvedValueOnce(null);
+      mockBcryptCompare.mockResolvedValueOnce(false);
+      await expect(AuthService.authenticate('ghost@b.com', 'pass')).rejects.toThrow('invalid user or password.');
+      expect(mockBcryptCompare).toHaveBeenCalledTimes(1);
+      expect(mockBcryptCompare).toHaveBeenCalledWith(
+        'pass',
+        expect.stringMatching(/^\$2[aby]\$\d{2}\$.{53}$/)
+      );
+    });
+
     test('should return sanitised user when credentials are valid', async () => {
       const storedUser = { _id: '1', email: 'a@b.com', firstName: 'Joe', password: 'hashed', roles: ['user'], provider: 'local', save: jest.fn() };
       mockGetBrut.mockResolvedValueOnce(storedUser);

modules/uploads/config/uploads.development.config.js

@@ -17,6 +17,20 @@ const config = {
       },
     },
   },
+  rateLimit: {
+    // Guards the unauthenticated /api/uploads/images/:imageName route, which runs
+    // the full sharp resize/transform pipeline on every guest request. Lives in
+    // this base layer so the profile is present — and the limiter active — under
+    // EVERY env, not only literal `production`. A stricter cap is applied as an
+    // override in config/defaults/production.config.js.
+    publicImage: {
+      windowMs: 60 * 1000, // 1 min
+      max: 600, // lenient in dev; production overrides to a stricter cap
+      message: { message: 'Too many requests, please try again later.' },
+      standardHeaders: true,
+      legacyHeaders: false,
+    },
+  },
 };
 
 export default config;

modules/uploads/routes/uploads.routes.js

@@ -4,10 +4,13 @@
 import passport from 'passport';
 
 import policy from '../../../lib/middlewares/policy.js';
+import limiters from '../../../lib/middlewares/rateLimiter.js';
 import uploads from '../controllers/uploads.controller.js';
 
 /**
- * Routes
+ * Register uploads routes.
+ * @param {import('express').Application} app - Express app instance
+ * @returns {void}
  */
 export default (app) => {
   // classic crud — ownership is enforced by CASL conditions in isAllowed
@@ -17,11 +20,12 @@ export default (app) => {
     .get(uploads.get)
     .delete(uploads.remove); // delete
 
-  // public image access (guest-readable, no JWT required)
+  // public image access (guest-readable, no JWT required). Rate-limited because
+  // it runs the full sharp pipeline on every guest request (CPU-exhaustion DoS).
   app
     .route('/api/uploads/images/:imageName')
     .all(policy.isAllowed)
-    .get(uploads.getSharp);
+    .get(limiters.publicImage, uploads.getSharp);
 
   // Finish by binding the task middleware
   app.param('uploadName', uploads.uploadByName);