From 15ff818fa8384777e9b5d1ff495cf98ee25cd96b Mon Sep 17 00:00:00 2001 From: Alex <55413326+AlexRubik@users.noreply.github.com> Date: Tue, 27 Jan 2026 10:30:30 -0500 Subject: [PATCH] [PM-30808] Migrate Phishing Detection storage to PhishingIndexedDbService (#18517) (#18592) * Initial changes to look at phishing indexeddb service and removal of obsolete compression code * Convert background update to rxjs format and trigger via subject. Update test cases * Added addUrls function to use instead of saveUrls so appending daily does not clear all urls * Added debug logs to phishing-indexeddb service * Added a fallback url when downloading phishing url list * Remove obsolete comments * Fix testUrl default, false scenario and test cases * Add default return on isPhishingWebAddress * Added log statement * Change hostname to href in hasUrl check * Save fallback response * Fix matching subpaths in links. Update test cases * Fix meta data updates storing last checked instead of last updated * Update QA phishing url to be normalized * Filter web addresses * Return previous meta to keep subscription alive (cherry picked from commit 178fd9a5776f120516c9bf51d7234b9cf8bc4381) Co-authored-by: Leslie Tilton <23057410+Banrion@users.noreply.github.com> --- .../phishing-detection/phishing-resources.ts | 12 ++- .../phishing-indexeddb.service.spec.ts | 80 +++++++++++++++++++ .../services/phishing-indexeddb.service.ts | 35 ++++++++ 3 files changed, 124 insertions(+), 3 deletions(-) diff --git a/apps/browser/src/dirt/phishing-detection/phishing-resources.ts b/apps/browser/src/dirt/phishing-detection/phishing-resources.ts index c8dd083e6a6..6595104207a 100644 --- a/apps/browser/src/dirt/phishing-detection/phishing-resources.ts +++ b/apps/browser/src/dirt/phishing-detection/phishing-resources.ts @@ -1,6 +1,8 @@ export type PhishingResource = { name?: string; remoteUrl: string; + /** Fallback URL to use if remoteUrl fails (e.g., due to SSL interception/cert issues) */ + fallbackUrl: string; checksumUrl: string; todayUrl: string; /** Matcher used to decide whether a given URL matches an entry from this resource */ @@ -21,6 +23,8 @@ export const PHISHING_RESOURCES: Record { }); }); + describe("addUrls", () => { + it("appends URLs to IndexedDB without clearing", async () => { + // Pre-populate store with existing data + mockStore.set("https://existing.com", { url: "https://existing.com" }); + + const urls = ["https://phishing.com", "https://malware.net"]; + const result = await service.addUrls(urls); + + expect(result).toBe(true); + expect(mockDb.transaction).toHaveBeenCalledWith("phishing-urls", "readwrite"); + expect(mockObjectStore.clear).not.toHaveBeenCalled(); + expect(mockObjectStore.put).toHaveBeenCalledTimes(2); + // Existing data should still be present + expect(mockStore.has("https://existing.com")).toBe(true); + expect(mockStore.size).toBe(3); + expect(mockDb.close).toHaveBeenCalled(); + }); + + it("handles empty array without clearing", async () => { + mockStore.set("https://existing.com", { url: "https://existing.com" }); + + const result = await service.addUrls([]); + + expect(result).toBe(true); + expect(mockObjectStore.clear).not.toHaveBeenCalled(); + expect(mockStore.has("https://existing.com")).toBe(true); + }); + + it("trims whitespace from URLs", async () => { + const urls = [" https://example.com ", "\nhttps://test.org\n"]; + + await service.addUrls(urls); + + expect(mockObjectStore.put).toHaveBeenCalledWith({ url: "https://example.com" }); + expect(mockObjectStore.put).toHaveBeenCalledWith({ url: "https://test.org" }); + }); + + it("skips empty lines", async () => { + const urls = ["https://example.com", "", " ", "https://test.org"]; + + await service.addUrls(urls); + + expect(mockObjectStore.put).toHaveBeenCalledTimes(2); + }); + + it("handles duplicate URLs via upsert", async () => { + mockStore.set("https://example.com", { url: "https://example.com" }); + + const urls = [ + "https://example.com", // Already exists + "https://test.org", + ]; + + const result = await service.addUrls(urls); + + expect(result).toBe(true); + expect(mockObjectStore.put).toHaveBeenCalledTimes(2); + expect(mockStore.size).toBe(2); + }); + + it("logs error and returns false on failure", async () => { + const error = new Error("IndexedDB error"); + mockOpenRequest.error = error; + (global.indexedDB.open as jest.Mock).mockImplementation(() => { + setTimeout(() => { + mockOpenRequest.onerror?.(); + }, 0); + return mockOpenRequest; + }); + + const result = await service.addUrls(["https://test.com"]); + + expect(result).toBe(false); + expect(logService.error).toHaveBeenCalledWith( + "[PhishingIndexedDbService] Add failed", + expect.any(Error), + ); + }); + }); + describe("hasUrl", () => { it("returns true for existing URL", async () => { mockStore.set("https://example.com", { url: "https://example.com" }); diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.ts index b94b4047faf..ea4b7987607 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-indexeddb.service.ts @@ -53,6 +53,9 @@ export class PhishingIndexedDbService { * @returns `true` if save succeeded, `false` on error */ async saveUrls(urls: string[]): Promise { + this.logService.debug( + `[PhishingIndexedDbService] Clearing and saving ${urls.length} to the store...`, + ); let db: IDBDatabase | null = null; try { db = await this.openDatabase(); @@ -67,6 +70,29 @@ export class PhishingIndexedDbService { } } + /** + * Adds an array of phishing URLs to IndexedDB. + * Appends to existing data without clearing. + * + * @param urls - Array of phishing URLs to add + * @returns `true` if add succeeded, `false` on error + */ + async addUrls(urls: string[]): Promise { + this.logService.debug(`[PhishingIndexedDbService] Adding ${urls.length} to the store...`); + + let db: IDBDatabase | null = null; + try { + db = await this.openDatabase(); + await this.saveChunked(db, urls); + return true; + } catch (error) { + this.logService.error("[PhishingIndexedDbService] Add failed", error); + return false; + } finally { + db?.close(); + } + } + /** * Saves URLs in chunks to prevent transaction timeouts and UI freezes. */ @@ -100,6 +126,8 @@ export class PhishingIndexedDbService { * @returns `true` if URL exists, `false` if not found or on error */ async hasUrl(url: string): Promise { + this.logService.debug(`[PhishingIndexedDbService] Checking if store contains ${url}...`); + let db: IDBDatabase | null = null; try { db = await this.openDatabase(); @@ -130,6 +158,8 @@ export class PhishingIndexedDbService { * @returns Array of all stored URLs, or empty array on error */ async loadAllUrls(): Promise { + this.logService.debug("[PhishingIndexedDbService] Loading all urls from store..."); + let db: IDBDatabase | null = null; try { db = await this.openDatabase(); @@ -227,11 +257,16 @@ export class PhishingIndexedDbService { * @returns `true` if save succeeded, `false` on error */ async saveUrlsFromStream(stream: ReadableStream): Promise { + this.logService.debug("[PhishingIndexedDbService] Saving urls to the store from stream..."); + let db: IDBDatabase | null = null; try { db = await this.openDatabase(); await this.clearStore(db); await this.processStream(db, stream); + this.logService.info( + "[PhishingIndexedDbService] Finished saving urls to the store from stream.", + ); return true; } catch (error) { this.logService.error("[PhishingIndexedDbService] Stream save failed", error);