fix(policy): startup warn for missing SubjectRegistration + mailer test coverage (#3397)

* fix(policy): warn at startup if module has abilities without SubjectRegistration (#3395)

Track hasSubjectRegistration per entry during discoverPolicies; emit a logger.warn
when abilities/guestAbilities are exported but no *SubjectRegistration function is
found, so misconfigured modules are caught at startup instead of silently failing
CASL document-level checks.

* test(mailer): add Buffer attachment and empty filename coverage (#3396)

Add test for Buffer content encoding to base64 in ResendProvider, and a test
that validates sendMail rejects attachments with an empty string filename.

* fix(jest): exclude tests/fixtures/ dirs from testPathIgnorePatterns

* fix(jest): exclude tests/fixtures from coverage collection

* fix(policy): improve warn message wording, add JSDoc to fixtures, prefix unused params

Author: PierreBrisorgueil

jest.config.js

@@ -57,6 +57,8 @@ export default {
     '!<rootDir>/modules/**/migrations/**',
     // Exclude static upload config — just object literals, like config/defaults
     '!<rootDir>/modules/uploads/config/config.uploads.js',
+    // Exclude test fixtures — helper stubs used by tests, not production code
+    '!<rootDir>/**/tests/fixtures/**',
   ],
   // The directory where Jest should output its coverage files
   coverageDirectory: 'coverage',
@@ -180,7 +182,7 @@ export default {
   testMatch: ['<rootDir>/modules/*/tests/**/*.js', '<rootDir>/lib/**/tests/**/*.js'],
 
   // An array of regexp pattern strings that are matched against all test paths, matched tests are skipped
-  testPathIgnorePatterns: ['/node_modules/'],
+  testPathIgnorePatterns: ['/node_modules/', '/tests/fixtures/'],
 
   // The regexp pattern Jest uses to detect test files
   // testRegex: "",

lib/helpers/mailer/tests/mailer.unit.tests.js

@@ -131,6 +131,18 @@ describe('mailer index with resend provider unit tests:', () => {
     ).rejects.toThrow('exceeds 25 MB limit');
   });
 
+  test('should throw when attachment filename is an empty string', async () => {
+    await expect(
+      mailer.sendMail({
+        to: 'user@example.com',
+        subject: 'Test',
+        template: 'welcome',
+        params: { name: 'Bob' },
+        attachments: [{ filename: '', content: 'data' }],
+      }),
+    ).rejects.toThrow('Attachment filename must be a non-empty string');
+  });
+
   test('should throw when attachment filename is not a string', async () => {
     await expect(
       mailer.sendMail({

lib/helpers/mailer/tests/provider.resend.unit.tests.js

@@ -84,6 +84,26 @@ describe('ResendProvider unit tests:', () => {
     expect(result).toEqual(mockData);
   });
 
+  test('should encode Buffer attachment content to base64', async () => {
+    const buffer = Buffer.from('binary data');
+    const mockData = { id: 'email_buf' };
+    sendMock.mockResolvedValue({ data: mockData, error: null });
+
+    await provider.send({
+      from: 'from@test.com',
+      to: 'to@test.com',
+      subject: 'Test',
+      html: '<p>test</p>',
+      attachments: [{ filename: 'data.bin', content: buffer }],
+    });
+
+    expect(sendMock).toHaveBeenCalledWith(
+      expect.objectContaining({
+        attachments: [{ filename: 'data.bin', content: Buffer.from(buffer).toString('base64') }],
+      }),
+    );
+  });
+
   test('should throw on Resend API error', async () => {
     sendMock.mockResolvedValue({ data: null, error: { message: 'Invalid API key' } });
 

lib/middlewares/policy.js

@@ -3,6 +3,7 @@
  */
 import path from 'path';
 import responses from '../helpers/responses.js';
+import logger from '../services/logger.js';
 
 const methodToAction = {
   get: 'read', post: 'create', put: 'update', patch: 'update', delete: 'delete',
@@ -255,9 +256,16 @@ const discoverPolicies = async (policyPaths) => {
       }
       // Call subject registration functions exported by module policy files
       if (normalizedKey.endsWith('subjectregistration')) {
+        entry.hasSubjectRegistration = true;
         value({ registerDocumentSubject, registerPathSubject });
       }
     }
+    // Warn if the module exports abilities/guestAbilities but no SubjectRegistration — document subjects will not be resolved
+    if ((entry.abilities || entry.guestAbilities) && !entry.hasSubjectRegistration) {
+      logger.warn(
+        `[policy] ${path.basename(policyPath)}: exports abilities/guestAbilities but no SubjectRegistration — document subjects will not be resolved`,
+      );
+    }
     // Also support old invokeRolesPolicies pattern during transition — skip if new-style exports found
     if (entry.abilities || entry.guestAbilities) {
       registerAbilities(entry);

lib/middlewares/tests/fixtures/policy-abilities-only.js

@@ -0,0 +1,12 @@
+// Fixture: policy file with abilities but no SubjectRegistration
+/**
+ * Define task abilities for authenticated users (fixture without SubjectRegistration).
+ * @param {Object} _user - The authenticated user (unused in fixture)
+ * @param {Object} _membership - Optional organization membership (unused in fixture)
+ * @param {Object} context - CASL ability builder context
+ * @param {Function} context.can - Function to grant abilities
+ * @returns {void}
+ */
+export function taskAbilities(_user, _membership, { can }) {
+  can('read', 'Task');
+}

lib/middlewares/tests/fixtures/policy-guest-abilities-only.js

@@ -0,0 +1,10 @@
+// Fixture: policy file with guestAbilities but no SubjectRegistration
+/**
+ * Define task abilities for guest users (fixture without SubjectRegistration).
+ * @param {Object} context - CASL ability builder context
+ * @param {Function} context.can - Function to grant abilities
+ * @returns {void}
+ */
+export function taskGuestAbilities({ can }) {
+  can('read', 'Task');
+}

lib/middlewares/tests/fixtures/policy-registration-only.js

@@ -0,0 +1,10 @@
+// Fixture: policy file with SubjectRegistration only (no abilities)
+/**
+ * Register task document subject for CASL resolution.
+ * @param {Object} context - Registration context
+ * @param {Function} context.registerDocumentSubject - Function to register document subjects
+ * @returns {void}
+ */
+export function taskSubjectRegistration({ registerDocumentSubject }) {
+  registerDocumentSubject('task', 'Task');
+}

lib/middlewares/tests/fixtures/policy-with-registration.js

@@ -0,0 +1,22 @@
+// Fixture: policy file with abilities AND SubjectRegistration
+/**
+ * Define task abilities for authenticated users.
+ * @param {Object} _user - The authenticated user (unused in fixture)
+ * @param {Object} _membership - Optional organization membership (unused in fixture)
+ * @param {Object} context - CASL ability builder context
+ * @param {Function} context.can - Function to grant abilities
+ * @returns {void}
+ */
+export function taskAbilities(_user, _membership, { can }) {
+  can('read', 'Task');
+}
+
+/**
+ * Register task document subject for CASL resolution.
+ * @param {Object} context - Registration context
+ * @param {Function} context.registerDocumentSubject - Function to register document subjects
+ * @returns {void}
+ */
+export function taskSubjectRegistration({ registerDocumentSubject }) {
+  registerDocumentSubject('task', 'Task');
+}

lib/middlewares/tests/policy.unit.tests.js

@@ -0,0 +1,72 @@
+/**
+ * Module dependencies.
+ */
+import { jest, describe, test, expect, beforeEach } from '@jest/globals';
+import { fileURLToPath } from 'url';
+import path from 'path';
+
+const __dirname = path.dirname(fileURLToPath(import.meta.url));
+
+// Mock logger before importing policy
+jest.unstable_mockModule('../../services/logger.js', () => ({
+  default: {
+    warn: jest.fn(),
+    info: jest.fn(),
+    error: jest.fn(),
+  },
+}));
+
+// Mock @casl/ability
+jest.unstable_mockModule('@casl/ability', () => ({
+  AbilityBuilder: jest.fn().mockImplementation(() => ({
+    can: jest.fn(),
+    cannot: jest.fn(),
+    build: jest.fn().mockReturnValue({ can: jest.fn().mockReturnValue(true) }),
+  })),
+  Ability: jest.fn(),
+  subject: jest.fn((type, doc) => doc),
+}));
+
+const { default: logger } = await import('../../services/logger.js');
+const { default: policy } = await import('../policy.js');
+
+/**
+ * Build absolute path to a test fixture file.
+ * @param {string} name - Fixture filename
+ * @returns {string} Absolute path to the fixture
+ */
+const fixture = (name) => path.join(__dirname, 'fixtures', name);
+
+describe('policy discoverPolicies unit tests:', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  test('should warn when a policy file exports abilities/guestAbilities but no SubjectRegistration', async () => {
+    await policy.discoverPolicies([fixture('policy-abilities-only.js')]);
+
+    expect(logger.warn).toHaveBeenCalledWith(
+      expect.stringContaining('exports abilities/guestAbilities but no SubjectRegistration'),
+    );
+  });
+
+  test('should not warn when a policy file exports both abilities and SubjectRegistration', async () => {
+    await policy.discoverPolicies([fixture('policy-with-registration.js')]);
+
+    expect(logger.warn).not.toHaveBeenCalled();
+  });
+
+  test('should not warn when a policy file exports only SubjectRegistration without abilities', async () => {
+    await policy.discoverPolicies([fixture('policy-registration-only.js')]);
+
+    expect(logger.warn).not.toHaveBeenCalled();
+  });
+
+  test('should warn when a policy file exports guestAbilities but no SubjectRegistration', async () => {
+    await policy.discoverPolicies([fixture('policy-guest-abilities-only.js')]);
+
+    expect(logger.warn).toHaveBeenCalledWith(
+      expect.stringContaining('exports abilities/guestAbilities but no SubjectRegistration'),
+    );
+  });
+});