Add null checks in feedback processing pipeline (FRE-4514)

Co-Authored-By: Paperclip <noreply@paperclip.ing>

packages/api/src/services/spamshield/spamshield.service.ts

@@ -366,8 +366,27 @@ export class SpamFeedbackService {
     confidence?: number,
     metadata?: Record<string, any>
   ): Promise<SpamFeedback> {
-    // Validate metadata
+    // Defensive null checks for required fields
-    const validation = this.validateMetadata(metadata);
+    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,
       },

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"
   },

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);

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';