From f34adc5e820b0f787004abce997c66b703b31672 Mon Sep 17 00:00:00 2001 From: Michael Freno Date: Sat, 2 May 2026 13:01:02 -0400 Subject: [PATCH] Add null checks in feedback processing pipeline (FRE-4514) Co-Authored-By: Paperclip --- .../services/spamshield/spamshield.service.ts | 27 +++++++-- packages/integration-tests/package.json | 1 + .../src/services/spamshield.service.ts | 12 ++++ services/spamshield/test/spamshield.test.ts | 55 +++++++++++++++++++ 4 files changed, 91 insertions(+), 4 deletions(-) diff --git a/packages/api/src/services/spamshield/spamshield.service.ts b/packages/api/src/services/spamshield/spamshield.service.ts index 9dd8add..0f2f710 100644 --- a/packages/api/src/services/spamshield/spamshield.service.ts +++ b/packages/api/src/services/spamshield/spamshield.service.ts @@ -366,8 +366,27 @@ export class SpamFeedbackService { confidence?: number, metadata?: Record ): Promise { - // Validate metadata - const validation = this.validateMetadata(metadata); + // Defensive null checks for required fields + if (!userId || typeof userId !== 'string' || userId.trim().length === 0) { + throw new Error('Feedback: userId is required'); + } + + if (!phoneNumber || typeof phoneNumber !== 'string' || phoneNumber.trim().length === 0) { + throw new Error('Feedback: phoneNumber is required'); + } + + if (typeof isSpam !== 'boolean') { + throw new Error('Feedback: isSpam must be a boolean'); + } + + // Validate confidence range if provided + const validatedConfidence = confidence !== undefined && confidence !== null + ? (Number.isFinite(confidence) && confidence >= 0 && confidence <= 1 ? confidence : undefined) + : undefined; + + // Treat null metadata as undefined + const effectiveMetadata = metadata !== null ? metadata : undefined; + const validation = this.validateMetadata(effectiveMetadata); const validatedMetadata = validation.trimmedMetadata; // Only enable if feature flag is set @@ -379,7 +398,7 @@ export class SpamFeedbackService { phoneNumber, phoneNumberHash: this.hashPhoneNumber(phoneNumber), isSpam, - confidence, + confidence: validatedConfidence, feedbackType: 'user_confirmation' as const, metadata: validatedMetadata, createdAt: new Date(), @@ -395,7 +414,7 @@ export class SpamFeedbackService { phoneNumber, phoneNumberHash, isSpam, - confidence, + confidence: validatedConfidence, feedbackType: 'user_confirmation', metadata: validatedMetadata, }, diff --git a/packages/integration-tests/package.json b/packages/integration-tests/package.json index 9dac93d..2faf6c6 100644 --- a/packages/integration-tests/package.json +++ b/packages/integration-tests/package.json @@ -16,6 +16,7 @@ "@shieldai/shared-notifications": "workspace:*", "jest": "^29.7.0", "@types/jest": "^29.5.0", + "@jest/globals": "^29.7.0", "ts-jest": "^29.1.0", "typescript": "^5.0.0" }, diff --git a/services/spamshield/src/services/spamshield.service.ts b/services/spamshield/src/services/spamshield.service.ts index d73696c..aaa10bb 100644 --- a/services/spamshield/src/services/spamshield.service.ts +++ b/services/spamshield/src/services/spamshield.service.ts @@ -253,6 +253,18 @@ export class SpamShieldService { throw new Error('Feedback loop disabled via feature flag'); } + if (!userId || typeof userId !== 'string' || userId.trim().length === 0) { + throw new Error('Feedback: userId is required'); + } + + if (!phoneNumber || typeof phoneNumber !== 'string') { + throw new Error('Feedback: phoneNumber must be a non-empty string'); + } + + if (typeof isSpam !== 'boolean') { + throw new Error('Feedback: isSpam must be a boolean'); + } + const validated = this.validatePhoneNumber(phoneNumber); const encrypted = FieldEncryptionService.encrypt(validated); const hash = FieldEncryptionService.hashPhoneNumber(validated); diff --git a/services/spamshield/test/spamshield.test.ts b/services/spamshield/test/spamshield.test.ts index e3f05ec..a6271f5 100644 --- a/services/spamshield/test/spamshield.test.ts +++ b/services/spamshield/test/spamshield.test.ts @@ -418,6 +418,61 @@ describe('SpamShieldService', () => { }); }); + describe('recordFeedback null checks', () => { + it('throws when userId is null', async () => { + process.env.FLAG_ENABLEFEEDBACKLOOP = 'true'; + const result = service.recordFeedback(null as any, '+14155552671', true); + await expect(result).rejects.toThrow('Feedback: userId is required'); + }); + + it('throws when userId is empty string', async () => { + process.env.FLAG_ENABLEFEEDBACKLOOP = 'true'; + const result = service.recordFeedback('', '+14155552671', true); + await expect(result).rejects.toThrow('Feedback: userId is required'); + }); + + it('throws when phoneNumber is null', async () => { + process.env.FLAG_ENABLEFEEDBACKLOOP = 'true'; + const result = service.recordFeedback('user123', null as any, true); + await expect(result).rejects.toThrow('Feedback: phoneNumber must be a non-empty string'); + }); + + it('throws when isSpam is null', async () => { + process.env.FLAG_ENABLEFEEDBACKLOOP = 'true'; + const result = service.recordFeedback('user123', '+14155552671', null as any); + await expect(result).rejects.toThrow('Feedback: isSpam must be a boolean'); + }); + + it('throws when isSpam is undefined', async () => { + process.env.FLAG_ENABLEFEEDBACKLOOP = 'true'; + const result = service.recordFeedback('user123', '+14155552671', undefined as any); + await expect(result).rejects.toThrow('Feedback: isSpam must be a boolean'); + }); + + it('throws when userId is undefined', async () => { + process.env.FLAG_ENABLEFEEDBACKLOOP = 'true'; + const result = service.recordFeedback(undefined as any, '+14155552671', true); + await expect(result).rejects.toThrow('Feedback: userId is required'); + }); + + it('throws when phoneNumber is undefined', async () => { + process.env.FLAG_ENABLEFEEDBACKLOOP = 'true'; + const result = service.recordFeedback('user123', undefined as any, true); + await expect(result).rejects.toThrow('Feedback: phoneNumber must be a non-empty string'); + }); + + it('handles null metadata gracefully (falls back to default)', async () => { + process.env.FLAG_ENABLEFEEDBACKLOOP = 'true'; + const result = service.recordFeedback('user123', '+14155552671', true, undefined, null as any); + try { + await result; + } catch (e) { + expect((e as Error).message).not.toContain('userId is required'); + expect((e as Error).message).not.toContain('isSpam must be a boolean'); + } + }); + }); + describe('enableFeedbackLoop flag', () => { it('throws when feedback loop is disabled in recordFeedback', async () => { process.env.FLAG_ENABLEFEEDBACKLOOP = 'false';