Fix P2/P3 review findings: DNR redirect format, runtime type guard, cache test setup

packages/extension/public/rules/phishing-rules.json

@@ -38,7 +38,7 @@
   {
     "id": 5,
     "priority": 2,
-    "action": { "type": "REDIRECT", "redirect": { "urlFilter": "chrome-extension://__MSG_@@extension_id__/popup.html" } },
+   "action": { "type": "redirect", "redirect": { "url": "chrome-extension://__MSG_@@extension_id__/popup.html" } },
     "condition": {
       "urlFilter": "*://*.tk/*",
       "resourceTypes": ["main_frame"],
@@ -48,7 +48,7 @@
   {
     "id": 6,
     "priority": 2,
-    "action": { "type": "REDIRECT", "redirect": { "urlFilter": "chrome-extension://__MSG_@@extension_id__/popup.html" } },
+    "action": { "type": "redirect", "redirect": { "url": "chrome-extension://__MSG_@@extension_id__/popup.html" } },
     "condition": {
       "urlFilter": "*://*.xyz/*",
       "resourceTypes": ["main_frame"],

packages/extension/src/background/index.ts

@@ -25,7 +25,7 @@ chrome.runtime.onInstalled.addListener(async () => {
   chrome.declarativeNetRequest.onRuleMatchedDebug.addListener((details) => {
     chrome.storage.local.get('blockedRequests').then((data) => {
       const blocked = data.blockedRequests || [];
-      blocked.push({ ruleId: details.ruleId, url: details.requestUrl, timestamp: Date.now() });
+      blocked.push({ ruleId: details.rule?.ruleId || 0, url: details.request?.url || '', timestamp: Date.now() });
       if (blocked.length > 100) blocked.shift();
       chrome.storage.local.set({ blockedRequests: blocked });
     });
@@ -207,7 +207,18 @@ async function handleMessage(
       return { settings: await settingsManager.update(message.payload as Partial<ExtensionSettings>) };
 
     case MessageType.REPORT_PHISHING: {
-      const report = message.payload as PhishingReport;
+      const payload = message.payload as Record<string, unknown> | undefined;
+      if (!payload || typeof payload.url !== 'string' || typeof payload.pageTitle !== 'string') {
+        return { success: false, error: 'Missing url or pageTitle' };
+      }
+      const report: PhishingReport = {
+        url: payload.url,
+        pageTitle: payload.pageTitle,
+        tabId: (payload.tabId as number) || 0,
+        timestamp: (payload.timestamp as number) || Date.now(),
+        reason: (payload.reason as string) || 'Manual report',
+        heuristics: (payload.heuristics as Record<string, unknown>) || {},
+      };
       const success = await shieldApiClient.submitPhishingReport(report);
       return { success };
     }

packages/extension/src/lib/cache.ts

@@ -44,7 +44,7 @@ export class UrlCache {
   }
 
   async loadFromStorage(): Promise<void> {
-    const data = await chrome.storage.local.get('urlCache');
+    const data = await chrome.storage.local.get('urlCache') as { urlCache: Record<string, { result: UrlCheckResult; expiresAt: number }> };
     if (data.urlCache) {
       const now = Date.now();
       for (const [key, entry] of Object.entries(data.urlCache)) {

packages/extension/tests/cache.test.ts

@@ -1,43 +1,59 @@
-import { describe, it, expect } from 'vitest';
-import { phishingDetector } from '../src/lib/phishing-detector';
-import { UrlVerdict, ThreatType } from '../src/types';
+import { describe, it, expect, beforeEach } from 'vitest';
+import { urlCache } from '../src/lib/cache';
+import { UrlCheckResult, UrlVerdict } from '../src/types';
 
-describe('PhishingDetector (cache test)', () => {
+describe('UrlCache', () => {
+  const sampleResult: UrlCheckResult = {
+    url: 'https://example.com',
+    domain: 'example.com',
+    verdict: UrlVerdict.SAFE,
+    confidence: 0.95,
+    threats: [],
+    cached: false,
+    latencyMs: 50,
+    timestamp: Date.now(),
+  };
 
-  describe('analyzeUrl', () => {
-    it('should return SAFE for legitimate URLs', () => {
-      const result = phishingDetector.analyzeUrl('https://www.google.com/search?q=test');
-      expect(result.verdict).toBe(UrlVerdict.SAFE);
-    });
+  beforeEach(async () => {
+    urlCache.clear();
+  });
 
-    it('should detect suspicious TLD', () => {
-      const result = phishingDetector.analyzeUrl('https://free-prize.tk/claim');
-      expect(result.threats.some((t) => t.type === ThreatType.DOMAIN_AGE)).toBe(true);
-    });
+  it('should return null for missing URL', async () => {
+    const result = await urlCache.get('https://missing.com');
+    expect(result).toBeNull();
+  });
 
-    it('should detect typosquatting', () => {
-      const result = phishingDetector.analyzeUrl('https://goggle.com/login');
-      expect(result.threats.some((t) => t.type === ThreatType.TYPOSQUAT)).toBe(true);
-    });
+  it('should store and retrieve cached result', async () => {
+    await urlCache.set('https://example.com', sampleResult);
+    const cached = await urlCache.get('https://example.com');
+    expect(cached).not.toBeNull();
+    expect(cached!.cached).toBe(true);
+    expect(cached!.verdict).toBe(UrlVerdict.SAFE);
+  });
 
-    it('should detect IP address hostname', () => {
-      const result = phishingDetector.analyzeUrl('http://192.168.1.100/admin');
-      expect(result.threats.some((t) => t.type === ThreatType.PHISHING_HEURISTIC)).toBe(true);
-    });
+  it('should normalize URLs by stripping hash and search', async () => {
+    await urlCache.set('https://example.com/page?foo=bar#section', sampleResult);
+    const cached = await urlCache.get('https://example.com/page');
+    expect(cached).not.toBeNull();
+  });
 
-    it('should detect phishing pattern in hostname', () => {
-      const result = phishingDetector.analyzeUrl('https://login-secure-portal.xyz/account');
-      expect(result.threats.some((t) => t.type === ThreatType.PHISHING_HEURISTIC)).toBe(true);
-    });
+  it('should persist and restore from storage', async () => {
+    await urlCache.set('https://test.com', sampleResult);
+    await urlCache.persistToStorage();
+    urlCache.clear();
+    await urlCache.loadFromStorage();
+    const cached = await urlCache.get('https://test.com');
+    expect(cached).not.toBeNull();
+  });
 
-    it('should detect HTTP protocol', () => {
-      const result = phishingDetector.analyzeUrl('http://example.com/login');
-      expect(result.threats.some((t) => t.type === ThreatType.MIXED_CONTENT)).toBe(true);
-    });
+  it('should evict oldest entry when at max capacity', async () => {
+    const stats = urlCache.getStats();
+    expect(stats.max).toBe(5000);
+  });
 
-    it('should return UNKNOWN for malformed URLs', () => {
-      const result = phishingDetector.analyzeUrl('not-a-real-url');
-      expect(result.verdict).toBe(UrlVerdict.UNKNOWN);
-    });
+  it('should handle malformed URLs gracefully', async () => {
+    await urlCache.set('not-a-url', sampleResult);
+    const cached = await urlCache.get('not-a-url');
+    expect(cached).not.toBeNull();
   });
 });

packages/extension/tests/setup.ts

@@ -0,0 +1,28 @@
+const mockStorage: Record<string, unknown> = {};
+
+const chromeMock = {
+  storage: {
+    local: {
+      set: async (data: Record<string, unknown>) => {
+        Object.assign(mockStorage, data);
+      },
+      get: async (key: string | string[]) => {
+        if (Array.isArray(key)) {
+          const result: Record<string, unknown> = {};
+          for (const k of key) result[k] = mockStorage[k];
+          return result;
+        }
+        return { [key]: mockStorage[key] };
+      },
+      remove: async (key: string | string[]) => {
+        const keys = Array.isArray(key) ? key : [key];
+        for (const k of keys) delete mockStorage[k];
+      },
+      clear: async () => {
+        Object.keys(mockStorage).forEach((k) => delete mockStorage[k]);
+      },
+    },
+  },
+};
+
+(global as any).chrome = chromeMock;

packages/extension/vitest.config.ts

@@ -5,5 +5,6 @@ export default defineConfig({
     globals: true,
     environment: 'node',
     include: ['tests/**/*.test.ts'],
+    setupFiles: ['./tests/setup.ts'],
   },
 });