diff --git a/app/controllers/customercontroller.js b/app/controllers/customercontroller.js index 9c4b578..37e32e6 100644 --- a/app/controllers/customercontroller.js +++ b/app/controllers/customercontroller.js @@ -18,6 +18,24 @@ const Customer = db.Customer; const IsMaster = auth.isMaster; const GetCompanyId = auth.getCompanyId; +/** + * Escape the three SQL LIKE/ILIKE metacharacters in a substring so + * the calling code can wrap it in `%…%` without the user-supplied + * portion of the pattern doing wildcard work on its own. + * + * % matches any sequence of characters + * _ matches any single character + * \ the escape character (must come first in the replacement + * regex so we don't double-escape our own escape) + * + * Used by exports.search; documented as test-only via _helpers so + * the unit test can pin the contract without taking a hard + * dependency on the function reference moving. + */ +function escapeLikeWildcards(s) { + return String(s).replace(/[\\%_]/g, '\\$&'); +} + /** * GET /v1/customer/:id * @@ -475,7 +493,14 @@ exports.search = async (req, res) => { } // ILIKE on Postgres; the `%` wildcards come from us, not the user. - const pattern = `%${q}%`; + // Escape SQL LIKE metacharacters in `q` so a user sending `%` or + // `_` or `\` doesn't turn the substring search into a pattern + // search. Without this, `q=%` produces `%%%` which matches every + // customer in the scope — same security boundary (still + // company-scoped) but the substring contract is silently broken. + // Sequelize does NOT auto-escape these for Op.iLike; the + // package treats the pattern as verbatim user-controlled SQL. + const pattern = `%${escapeLikeWildcards(q)}%`; const where = { custCompId: effectiveCompanyId, [Op.or]: [ @@ -566,4 +591,4 @@ function CompaniesMatch(int1, int2) { } // Exported for testing -exports._helpers = { IsMaster, GetCompanyId, GetCustomerCompanyId, CompaniesMatch }; +exports._helpers = { IsMaster, GetCompanyId, GetCustomerCompanyId, CompaniesMatch, escapeLikeWildcards }; diff --git a/tests/api/customer-search.test.js b/tests/api/customer-search.test.js index b1883c8..88e1974 100644 --- a/tests/api/customer-search.test.js +++ b/tests/api/customer-search.test.js @@ -88,6 +88,39 @@ describe('GET /v1/customer/search query validation', () => { }); }); +describe('escapeLikeWildcards helper', () => { + // Pinning the escape because Sequelize's Op.iLike does NOT + // auto-escape SQL LIKE metacharacters. A future helper rewrite + // that accidentally drops the escape on `%` would silently turn + // the substring search into a pattern search — same auth scope + // still applies, but the intent of "match substring q" breaks. + let escapeLikeWildcards; + beforeAll(async () => { + const ctrl = await import('../../app/controllers/customercontroller.js'); + escapeLikeWildcards = ctrl._helpers.escapeLikeWildcards; + }); + test('escapes the three LIKE metacharacters: %, _, backslash', () => { + expect(escapeLikeWildcards('%')).toBe('\\%'); + expect(escapeLikeWildcards('_')).toBe('\\_'); + expect(escapeLikeWildcards('\\')).toBe('\\\\'); + }); + test('leaves ordinary text alone', () => { + expect(escapeLikeWildcards('Acme Corp')).toBe('Acme Corp'); + expect(escapeLikeWildcards('john.doe@example.com')).toBe('john.doe@example.com'); + }); + test('escapes multiple metacharacters in one string', () => { + expect(escapeLikeWildcards('100%_done')).toBe('100\\%\\_done'); + expect(escapeLikeWildcards('path\\to\\file')).toBe('path\\\\to\\\\file'); + }); + test('coerces non-string to string before escaping (defensive)', () => { + // Schema validation already enforces string, but the helper + // defensively coerces so a runtime mis-call doesn't throw. + expect(escapeLikeWildcards(null)).toBe('null'); + expect(escapeLikeWildcards(undefined)).toBe('undefined'); + expect(escapeLikeWildcards(42)).toBe('42'); + }); +}); + describe('GET /v1/customer/search route mounting', () => { test('route is mounted; not treated as /v1/customer/:id', async () => { // If the route ordering were wrong, "search" would be parsed as