From 2733d422bb4a4ac1a7275319203c2ebec20236b7 Mon Sep 17 00:00:00 2001 From: Chris Kehayias Date: Sun, 17 May 2026 08:33:24 -0400 Subject: [PATCH] fix(security): expand filter sanitization to remaining GUID sites and LIKE wildcards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups to PR #57: - Apply sanitizeGuid() to userService.getUserProfile() and contact-logs actions (both User_GUID filters), bringing all GUID interpolations under the same validation as contactService.getContactByGuid(). - Add sanitizeLikeValue() that also escapes %, _, and \ in addition to single quotes. Use it in contactService.contactSearch() with an ESCAPE '\' clause so wildcards in user input are treated literally. - Fix misleading "UUID v4 pattern" doc comment in filter-sanitize.ts — the regex accepts any UUID variant (MP GUIDs are not guaranteed v4). - Update userService and contact-logs action tests to use valid-format GUID fixtures (required now that sanitizeGuid validates them) and add an invalid-GUID throws case for getUserProfile(). - Rename test constant in contactService.test.ts not-found case to validButUnknownGuid for clarity. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/components/contact-logs/actions.test.ts | 4 ++- src/components/contact-logs/actions.ts | 5 +-- .../utils/filter-sanitize.test.ts | 32 ++++++++++++++++++- .../utils/filter-sanitize.ts | 21 +++++++++++- src/services/contactService.test.ts | 3 +- src/services/contactService.ts | 8 +++-- src/services/userService.test.ts | 22 +++++++++---- src/services/userService.ts | 3 +- 8 files changed, 82 insertions(+), 16 deletions(-) diff --git a/src/components/contact-logs/actions.test.ts b/src/components/contact-logs/actions.test.ts index 74fc5e2a..34b5881c 100644 --- a/src/components/contact-logs/actions.test.ts +++ b/src/components/contact-logs/actions.test.ts @@ -62,8 +62,10 @@ import { getContactLogById, } from './actions'; +const validUserGuid = 'a1b2c3d4-e5f6-7890-abcd-ef1234567890'; + const mockAuthSession = { - user: { id: 'internal-id', userGuid: 'user-guid-123' }, + user: { id: 'internal-id', userGuid: validUserGuid }, }; describe('contact-logs actions', () => { diff --git a/src/components/contact-logs/actions.ts b/src/components/contact-logs/actions.ts index 0ca145d3..a196a1c2 100644 --- a/src/components/contact-logs/actions.ts +++ b/src/components/contact-logs/actions.ts @@ -4,6 +4,7 @@ import { ContactLog } from "@/lib/providers/ministry-platform/models/ContactLog" import { ContactLogTypes } from "@/lib/providers/ministry-platform/models/ContactLogTypes"; import { ContactLogInput } from "@/lib/providers/ministry-platform/models/ContactLogSchema"; import { ContactLogService } from "@/services/contactLogService"; +import { sanitizeGuid } from "@/lib/providers/ministry-platform/utils/filter-sanitize"; import { auth } from "@/lib/auth"; import { headers } from "next/headers"; @@ -50,7 +51,7 @@ export async function createContactLog( const users = await mp.getTableRecords<{ User_ID: number }>({ table: "dp_Users", - filter: `User_GUID = '${userGuid}'`, + filter: `User_GUID = '${sanitizeGuid(userGuid)}'`, select: "User_ID", top: 1 }); @@ -102,7 +103,7 @@ export async function updateContactLog( const users = await mp.getTableRecords<{ User_ID: number }>({ table: "dp_Users", - filter: `User_GUID = '${userGuid}'`, + filter: `User_GUID = '${sanitizeGuid(userGuid)}'`, select: "User_ID", top: 1 }); diff --git a/src/lib/providers/ministry-platform/utils/filter-sanitize.test.ts b/src/lib/providers/ministry-platform/utils/filter-sanitize.test.ts index cbae24bf..5433e31e 100644 --- a/src/lib/providers/ministry-platform/utils/filter-sanitize.test.ts +++ b/src/lib/providers/ministry-platform/utils/filter-sanitize.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { sanitizeFilterValue, sanitizeGuid } from './filter-sanitize'; +import { sanitizeFilterValue, sanitizeLikeValue, sanitizeGuid } from './filter-sanitize'; describe('sanitizeFilterValue', () => { it('should return plain strings unchanged', () => { @@ -23,6 +23,36 @@ describe('sanitizeFilterValue', () => { }); }); +describe('sanitizeLikeValue', () => { + it('should return plain strings unchanged', () => { + expect(sanitizeLikeValue('John')).toBe('John'); + }); + + it('should escape single quotes', () => { + expect(sanitizeLikeValue("O'Brien")).toBe("O''Brien"); + }); + + it('should escape percent wildcards', () => { + expect(sanitizeLikeValue('100%')).toBe('100\\%'); + }); + + it('should escape underscore wildcards', () => { + expect(sanitizeLikeValue('a_b')).toBe('a\\_b'); + }); + + it('should escape backslashes before other escapes', () => { + expect(sanitizeLikeValue('a\\b')).toBe('a\\\\b'); + }); + + it('should escape all special characters together', () => { + expect(sanitizeLikeValue("100%_o'reilly\\")).toBe("100\\%\\_o''reilly\\\\"); + }); + + it('should handle empty string', () => { + expect(sanitizeLikeValue('')).toBe(''); + }); +}); + describe('sanitizeGuid', () => { it('should return a valid lowercase GUID unchanged', () => { const guid = '12345678-1234-1234-1234-123456789abc'; diff --git a/src/lib/providers/ministry-platform/utils/filter-sanitize.ts b/src/lib/providers/ministry-platform/utils/filter-sanitize.ts index 5d7e64d2..077b86c6 100644 --- a/src/lib/providers/ministry-platform/utils/filter-sanitize.ts +++ b/src/lib/providers/ministry-platform/utils/filter-sanitize.ts @@ -9,14 +9,33 @@ * Escapes a string value for safe interpolation inside a single-quoted filter value. * Doubles single quotes (SQL standard escaping) so that input like O'Brien * becomes O''Brien and cannot break out of the quoted context. + * + * Use for equality comparisons: `Column = '${sanitizeFilterValue(value)}'`. + * For LIKE patterns, use {@link sanitizeLikeValue} instead. */ export function sanitizeFilterValue(value: string): string { return value.replace(/'/g, "''"); } +/** + * Escapes a string value for safe interpolation inside a LIKE pattern. + * Escapes the SQL LIKE wildcards (`%`, `_`) and the backslash escape character + * itself, then doubles single quotes for string-literal escaping. Callers MUST + * include `ESCAPE '\'` in the LIKE clause for the escapes to be honored, e.g. + * `Column LIKE '%${sanitizeLikeValue(value)}%' ESCAPE '\\'`. + */ +export function sanitizeLikeValue(value: string): string { + return value + .replace(/\\/g, "\\\\") + .replace(/%/g, "\\%") + .replace(/_/g, "\\_") + .replace(/'/g, "''"); +} + /** * Validates a GUID/UUID string format and returns the sanitized value. - * Throws if the value does not match the expected UUID v4 pattern. + * Accepts any UUID variant (v1–v5) — Ministry Platform GUIDs are not guaranteed + * to be v4. Throws if the value does not match the canonical 8-4-4-4-12 hex format. */ export function sanitizeGuid(guid: string): string { const guidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; diff --git a/src/services/contactService.test.ts b/src/services/contactService.test.ts index 600798d1..88ec4827 100644 --- a/src/services/contactService.test.ts +++ b/src/services/contactService.test.ts @@ -84,6 +84,7 @@ describe('ContactService', () => { describe('getContactByGuid', () => { const validGuid = 'a1b2c3d4-e5f6-7890-abcd-ef1234567890'; + const validButUnknownGuid = 'b2c3d4e5-f678-9012-3456-7890abcdef12'; it('should return contact when found', async () => { const mockContact = { Contact_ID: 1, Contact_GUID: validGuid, First_Name: 'John' }; @@ -105,7 +106,7 @@ describe('ContactService', () => { mockGetTableRecords.mockResolvedValueOnce([]); const service = await ContactService.getInstance(); - const result = await service.getContactByGuid(validGuid); + const result = await service.getContactByGuid(validButUnknownGuid); expect(result).toBeNull(); }); diff --git a/src/services/contactService.ts b/src/services/contactService.ts index a0c70271..ce150f89 100644 --- a/src/services/contactService.ts +++ b/src/services/contactService.ts @@ -1,6 +1,6 @@ import { ContactSearch } from "@/lib/dto"; import { MPHelper } from "@/lib/providers/ministry-platform"; -import { sanitizeFilterValue, sanitizeGuid } from "@/lib/providers/ministry-platform/utils/filter-sanitize"; +import { sanitizeLikeValue, sanitizeGuid } from "@/lib/providers/ministry-platform/utils/filter-sanitize"; /** * ContactService - Singleton service for managing contact-related operations @@ -53,9 +53,13 @@ export class ContactService { * @returns Promise - Array of matching contacts (limited to 20 results) */ public async contactSearch(search: string): Promise { + const term = sanitizeLikeValue(search); + const filter = ["First_Name", "Last_Name", "Nickname", "Email_Address", "Mobile_Phone"] + .map((col) => `${col} LIKE '%${term}%' ESCAPE '\\'`) + .join(" OR "); const records = await this.mp!.getTableRecords({ table: "Contacts", - filter: `First_Name LIKE '%${sanitizeFilterValue(search)}%' OR Last_Name LIKE '%${sanitizeFilterValue(search)}%' OR Nickname LIKE '%${sanitizeFilterValue(search)}%' OR Email_Address LIKE '%${sanitizeFilterValue(search)}%' OR Mobile_Phone LIKE '%${sanitizeFilterValue(search)}%'`, + filter, select: "Contact_ID, Contact_GUID,First_Name,Nickname,Last_Name,Email_Address,Mobile_Phone,dp_fileUniqueId AS Image_GUID", top: 20 }); diff --git a/src/services/userService.test.ts b/src/services/userService.test.ts index 9c82745f..1454b291 100644 --- a/src/services/userService.test.ts +++ b/src/services/userService.test.ts @@ -28,10 +28,13 @@ describe('UserService', () => { }); describe('getUserProfile', () => { + const validGuid = 'a1b2c3d4-e5f6-7890-abcd-ef1234567890'; + const otherValidGuid = 'b2c3d4e5-f678-9012-3456-7890abcdef12'; + it('should fetch user profile with roles and groups', async () => { const mockProfile = { User_ID: 1, - User_GUID: 'test-guid-123', + User_GUID: validGuid, Contact_ID: 100, First_Name: 'John', Nickname: 'Johnny', @@ -46,12 +49,12 @@ describe('UserService', () => { .mockResolvedValueOnce([{ User_Group_Name: 'Staff' }]); const service = await UserService.getInstance(); - const result = await service.getUserProfile('test-guid-123'); + const result = await service.getUserProfile(validGuid); expect(mockGetTableRecords).toHaveBeenCalledTimes(3); expect(mockGetTableRecords).toHaveBeenCalledWith({ table: 'dp_Users', - filter: "User_GUID = 'test-guid-123'", + filter: `User_GUID = '${validGuid}'`, select: expect.stringContaining('User_ID'), top: 1, }); @@ -76,7 +79,7 @@ describe('UserService', () => { mockGetTableRecords.mockResolvedValueOnce([]); const service = await UserService.getInstance(); - const result = await service.getUserProfile('nonexistent-guid'); + const result = await service.getUserProfile(validGuid); expect(result).toBeUndefined(); expect(mockGetTableRecords).toHaveBeenCalledTimes(1); @@ -85,7 +88,7 @@ describe('UserService', () => { it('should return empty arrays when user has no roles or groups', async () => { const mockProfile = { User_ID: 2, - User_GUID: 'test-guid-456', + User_GUID: otherValidGuid, Contact_ID: 200, First_Name: 'Jane', Nickname: 'Jane', @@ -100,7 +103,7 @@ describe('UserService', () => { .mockResolvedValueOnce([]); const service = await UserService.getInstance(); - const result = await service.getUserProfile('test-guid-456'); + const result = await service.getUserProfile(otherValidGuid); expect(result).toEqual({ ...mockProfile, @@ -113,7 +116,12 @@ describe('UserService', () => { mockGetTableRecords.mockRejectedValueOnce(new Error('API error')); const service = await UserService.getInstance(); - await expect(service.getUserProfile('test-guid')).rejects.toThrow('API error'); + await expect(service.getUserProfile(validGuid)).rejects.toThrow('API error'); + }); + + it('should throw on invalid GUID format', async () => { + const service = await UserService.getInstance(); + await expect(service.getUserProfile('not-a-guid')).rejects.toThrow('Invalid GUID format'); }); }); }); diff --git a/src/services/userService.ts b/src/services/userService.ts index 7c61edd9..91801d8d 100644 --- a/src/services/userService.ts +++ b/src/services/userService.ts @@ -1,5 +1,6 @@ import { MPUserProfile } from "@/lib/providers/ministry-platform/types"; import { MPHelper } from "@/lib/providers/ministry-platform"; +import { sanitizeGuid } from "@/lib/providers/ministry-platform/utils/filter-sanitize"; /** * UserService - Singleton service for managing user-related operations @@ -60,7 +61,7 @@ export class UserService { public async getUserProfile(id: string): Promise { const records = await this.mp!.getTableRecords({ table: "dp_Users", - filter: `User_GUID = '${id}'`, + filter: `User_GUID = '${sanitizeGuid(id)}'`, select: "User_ID, User_GUID, Contact_ID_TABLE.First_Name,Contact_ID_TABLE.Nickname,Contact_ID_TABLE.Last_Name,Contact_ID_TABLE.Email_Address,Contact_ID_TABLE.Mobile_Phone,Contact_ID_TABLE.dp_fileUniqueId AS Image_GUID", top: 1 });