From 6777a59cd85c511d3182c8de09ade0d0f8f3cd21 Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Tue, 19 May 2026 02:38:46 -0500 Subject: [PATCH] fix(customer/search): escape SQL LIKE metacharacters in the user-supplied substring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GET /v1/customer/search?q=…` constructs an ILIKE pattern as `%${q}%` and hands it to Sequelize's `Op.iLike`. Sequelize does NOT auto-escape SQL LIKE metacharacters for that operator — it treats the pattern as verbatim user-controlled SQL. So a request like `q=%` produces `%%%` which matches **every** customer in scope; `q=__@` matches anything-2-chars-then-@; `q=\\` lands a raw backslash in the pattern. Same security boundary (still company-scoped — non-master keys can't reach beyond their own company), but the substring contract silently breaks: a user typing `100%` into a search-as-you-type input gets all customers back, not customers whose name contains "100%". Add a small `escapeLikeWildcards(s)` helper that prefixes the three LIKE metacharacters (`%`, `_`, `\`) with `\`. Use it on `q` before constructing the pattern. Helper is exposed via `exports._helpers` for unit testing and so future siblings (invoice number search, etc.) can reuse it. Four new tests cover: the three metacharacters individually, ordinary text passthrough, multi-meta strings (e.g. `100%_done`, `path\\to\\file`), and defensive non-string coercion. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/controllers/customercontroller.js | 29 +++++++++++++++++++++-- tests/api/customer-search.test.js | 33 +++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) 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