From 3dcee2ef5de12dfcb092e52887c72d7e8c5f441c Mon Sep 17 00:00:00 2001 From: Robyn MacCallum Date: Thu, 29 Jan 2026 10:08:14 -0500 Subject: [PATCH 1/6] Fix DDG build action file list (#18390) * Fix file list * Add ddg-alert-files-list branch to test PR triggers * Update branches for pull request trigger Restrict pull request monitoring to the main branch only. --- .github/workflows/alert-ddg-files-modified.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/alert-ddg-files-modified.yml b/.github/workflows/alert-ddg-files-modified.yml index 35eb0515c10..49918563644 100644 --- a/.github/workflows/alert-ddg-files-modified.yml +++ b/.github/workflows/alert-ddg-files-modified.yml @@ -73,7 +73,7 @@ jobs: _MONITORED_FILES: ${{ steps.changed-files.outputs.monitored_files }} with: script: | - const changedFiles = `$_MONITORED_FILES`.split(' ').filter(file => file.trim() !== ''); + const changedFiles = process.env._MONITORED_FILES.split(' ').filter(file => file.trim() !== ''); const message = ` ⚠️🦆 **DuckDuckGo Integration files have been modified in this PR:** From 96ce13760b86348f147133e2369ef634a2fc9863 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com> Date: Thu, 29 Jan 2026 16:14:41 +0100 Subject: [PATCH 2/6] [PM-30307] Session key retrieval redesign for the local backed session storage (#18493) * session key retrieval redesign for the local backed session storage * typo * incorrect substring * get cache edge cases incorrectly handling to null values after removal * test coverage * internal `SessionKeyResolveService` --- .../browser/src/background/main.background.ts | 44 +-- .../services/browser-local-storage.service.ts | 16 + ...cal-backed-session-storage.service.spec.ts | 281 +++++++++++++----- .../local-backed-session-storage.service.ts | 133 +++++++-- 4 files changed, 351 insertions(+), 123 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 8d741039b31..eb6d26357eb 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -139,8 +139,6 @@ import { IpcService, IpcSessionRepository } from "@bitwarden/common/platform/ipc import { Message, MessageListener, MessageSender } from "@bitwarden/common/platform/messaging"; // eslint-disable-next-line no-restricted-imports -- Used for dependency creation import { SubjectMessageSender } from "@bitwarden/common/platform/messaging/internal"; -import { Lazy } from "@bitwarden/common/platform/misc/lazy"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { ServerNotificationsService } from "@bitwarden/common/platform/server-notifications"; // eslint-disable-next-line no-restricted-imports -- Needed for service creation import { @@ -565,36 +563,18 @@ export default class MainBackground { this.memoryStorageService = this.memoryStorageForStateProviders; } + this.encryptService = new EncryptServiceImplementation( + this.cryptoFunctionService, + this.logService, + true, + ); + if (BrowserApi.isManifestVersion(3)) { - // Creates a session key for mv3 storage of large memory items - const sessionKey = new Lazy(async () => { - // Key already in session storage - const sessionStorage = new BrowserMemoryStorageService(); - const existingKey = await sessionStorage.get("session-key"); - if (existingKey) { - if (sessionStorage.valuesRequireDeserialization) { - return SymmetricCryptoKey.fromJSON(existingKey); - } - return existingKey; - } - - // New key - const { derivedKey } = await this.keyGenerationService.createKeyWithPurpose( - 128, - "ephemeral", - "bitwarden-ephemeral", - ); - await sessionStorage.save("session-key", derivedKey.toJSON()); - return derivedKey; - }); - this.largeObjectMemoryStorageForStateProviders = new LocalBackedSessionStorageService( - sessionKey, + new BrowserMemoryStorageService(), this.storageService, - // For local backed session storage, we expect that the encrypted data on disk will persist longer than the encryption key in memory - // and failures to decrypt because of that are completely expected. For this reason, we pass in `false` to the `EncryptServiceImplementation` - // so that MAC failures are not logged. - new EncryptServiceImplementation(this.cryptoFunctionService, this.logService, false), + this.keyGenerationService, + this.encryptService, this.platformUtilsService, this.logService, ); @@ -629,12 +609,6 @@ export default class MainBackground { storageServiceProvider, ); - this.encryptService = new EncryptServiceImplementation( - this.cryptoFunctionService, - this.logService, - true, - ); - this.singleUserStateProvider = new DefaultSingleUserStateProvider( storageServiceProvider, stateEventRegistrarService, diff --git a/apps/browser/src/platform/services/browser-local-storage.service.ts b/apps/browser/src/platform/services/browser-local-storage.service.ts index 30454cf6a77..e34c3ac4904 100644 --- a/apps/browser/src/platform/services/browser-local-storage.service.ts +++ b/apps/browser/src/platform/services/browser-local-storage.service.ts @@ -15,6 +15,22 @@ export default class BrowserLocalStorageService extends AbstractChromeStorageSer return await this.getWithRetries(key, 0); } + /** + * Retrieves all storage keys. + * + * Returns all keys stored in local storage when the browser supports the getKeys API (Chrome 130+). + * Returns an empty array on older browser versions where this feature is unavailable. + * + * @returns Array of storage keys, or empty array if the feature is not supported + */ + async getKeys(): Promise { + // getKeys function is only available since Chrome 130 + if ("getKeys" in this.chromeStorageApi) { + return this.chromeStorageApi.getKeys(); + } + return []; + } + private async getWithRetries(key: string, retryNum: number): Promise { // See: https://github.com/EFForg/privacybadger/pull/2980 const MAX_RETRIES = 5; diff --git a/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts b/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts index 947fecb5aac..26b51e2fea7 100644 --- a/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts +++ b/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts @@ -1,20 +1,89 @@ import { mock, MockProxy } from "jest-mock-extended"; +import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { Lazy } from "@bitwarden/common/platform/misc/lazy"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; -import { FakeStorageService, makeEncString } from "@bitwarden/common/spec"; +import { FakeStorageService, makeEncString, makeSymmetricCryptoKey } from "@bitwarden/common/spec"; +import { StorageService } from "@bitwarden/storage-core"; -import { LocalBackedSessionStorageService } from "./local-backed-session-storage.service"; +import BrowserLocalStorageService from "./browser-local-storage.service"; +import { + LocalBackedSessionStorageService, + SessionKeyResolveService, +} from "./local-backed-session-storage.service"; + +describe("SessionKeyResolveService", () => { + let storageService: FakeStorageService; + let keyGenerationService: MockProxy; + let sut: SessionKeyResolveService; + + const mockKey = makeSymmetricCryptoKey(); + + beforeEach(() => { + storageService = new FakeStorageService(); + keyGenerationService = mock(); + sut = new SessionKeyResolveService(storageService, keyGenerationService); + }); + + describe("get", () => { + it("returns null when no session key exists", async () => { + const result = await sut.get(); + expect(result).toBeNull(); + }); + + it("returns the session key from storage", async () => { + await storageService.save("session-key", mockKey); + const result = await sut.get(); + expect(result).toEqual(mockKey); + }); + + it("deserializes the session key when storage requires deserialization", async () => { + const mockStorageService = mock(); + Object.defineProperty(mockStorageService, "valuesRequireDeserialization", { + get: () => true, + }); + mockStorageService.get.mockResolvedValue(mockKey.toJSON()); + + const deserializableSut = new SessionKeyResolveService( + mockStorageService, + keyGenerationService, + ); + + const result = await deserializableSut.get(); + + expect(result).toBeInstanceOf(SymmetricCryptoKey); + expect(result?.toJSON()).toEqual(mockKey.toJSON()); + }); + }); + + describe("create", () => { + it("creates a new session key and saves it to storage", async () => { + keyGenerationService.createKeyWithPurpose.mockResolvedValue({ + salt: "salt", + material: new Uint8Array(16) as any, + derivedKey: mockKey, + }); + + const result = await sut.create(); + + expect(keyGenerationService.createKeyWithPurpose).toHaveBeenCalledWith( + 128, + "ephemeral", + "bitwarden-ephemeral", + ); + expect(result).toEqual(mockKey); + expect(await storageService.get("session-key")).toEqual(mockKey.toJSON()); + }); + }); +}); describe("LocalBackedSessionStorage", () => { - const sessionKey = new SymmetricCryptoKey( - Utils.fromUtf8ToArray("00000000000000000000000000000000"), - ); - let localStorage: FakeStorageService; + const sessionKey = makeSymmetricCryptoKey(); + let memoryStorage: MockProxy; + let keyGenerationService: MockProxy; + let localStorage: MockProxy; let encryptService: MockProxy; let platformUtilsService: MockProxy; let logService: MockProxy; @@ -22,14 +91,23 @@ describe("LocalBackedSessionStorage", () => { let sut: LocalBackedSessionStorageService; beforeEach(() => { - localStorage = new FakeStorageService(); + memoryStorage = mock(); + keyGenerationService = mock(); + localStorage = mock(); encryptService = mock(); platformUtilsService = mock(); logService = mock(); + // Default: session key exists + memoryStorage.get.mockResolvedValue(sessionKey); + Object.defineProperty(memoryStorage, "valuesRequireDeserialization", { + get: () => true, + }); + sut = new LocalBackedSessionStorageService( - new Lazy(async () => sessionKey), + memoryStorage, localStorage, + keyGenerationService, encryptService, platformUtilsService, logService, @@ -37,57 +115,79 @@ describe("LocalBackedSessionStorage", () => { }); describe("get", () => { - it("return the cached value when one is cached", async () => { + const encString = makeEncString("encrypted"); + + it("returns the cached value when one is cached", async () => { sut["cache"]["test"] = "cached"; const result = await sut.get("test"); expect(result).toEqual("cached"); }); - it("returns a decrypted value when one is stored in local storage", async () => { - const encrypted = makeEncString("encrypted"); - localStorage.internalStore["session_test"] = encrypted.encryptedString; - encryptService.decryptString.mockResolvedValue(JSON.stringify("decrypted")); - const result = await sut.get("test"); - // FIXME: Remove when updating file. Eslint update - // eslint-disable-next-line @typescript-eslint/no-unused-expressions - (expect(encryptService.decryptString).toHaveBeenCalledWith(encrypted, sessionKey), - expect(result).toEqual("decrypted")); - }); + it("returns null when both cache and storage are null", async () => { + sut["cache"]["test"] = null; + localStorage.get.mockResolvedValue(null); - it("caches the decrypted value when one is stored in local storage", async () => { - const encrypted = makeEncString("encrypted"); - localStorage.internalStore["session_test"] = encrypted.encryptedString; - encryptService.decryptString.mockResolvedValue(JSON.stringify("decrypted")); - await sut.get("test"); - expect(sut["cache"]["test"]).toEqual("decrypted"); + const result = await sut.get("test"); + + expect(result).toBeNull(); + expect(localStorage.get).toHaveBeenCalledWith("session_test"); }); it("returns a decrypted value when one is stored in local storage", async () => { - const encrypted = makeEncString("encrypted"); - localStorage.internalStore["session_test"] = encrypted.encryptedString; + localStorage.get.mockResolvedValue(encString.encryptedString); encryptService.decryptString.mockResolvedValue(JSON.stringify("decrypted")); + const result = await sut.get("test"); - // FIXME: Remove when updating file. Eslint update - // eslint-disable-next-line @typescript-eslint/no-unused-expressions - (expect(encryptService.decryptString).toHaveBeenCalledWith(encrypted, sessionKey), - expect(result).toEqual("decrypted")); + + expect(encryptService.decryptString).toHaveBeenCalledWith(encString, sessionKey); + expect(result).toEqual("decrypted"); + expect(sut["cache"]["test"]).toEqual("decrypted"); }); - it("caches the decrypted value when one is stored in local storage", async () => { - const encrypted = makeEncString("encrypted"); - localStorage.internalStore["session_test"] = encrypted.encryptedString; - encryptService.decryptString.mockResolvedValue(JSON.stringify("decrypted")); - await sut.get("test"); - expect(sut["cache"]["test"]).toEqual("decrypted"); + it("returns the cached value when cache is populated during storage retrieval", async () => { + localStorage.get.mockImplementation(async () => { + sut["cache"]["test"] = "cached-during-read"; + return encString.encryptedString; + }); + encryptService.decryptString.mockResolvedValue(JSON.stringify("decrypted-from-storage")); + + const result = await sut.get("test"); + + expect(result).toEqual("cached-during-read"); + }); + + it("returns the cached value when storage returns null but cache was filled", async () => { + localStorage.get.mockImplementation(async () => { + sut["cache"]["test"] = "cached-during-read"; + return null; + }); + + const result = await sut.get("test"); + + expect(result).toEqual("cached-during-read"); + }); + + it("creates new session key, clears old data, and returns null when session key is missing", async () => { + const newSessionKey = makeSymmetricCryptoKey(); + const clearSpy = jest.spyOn(sut as any, "clear"); + memoryStorage.get.mockResolvedValue(null); + keyGenerationService.createKeyWithPurpose.mockResolvedValue({ + salt: "salt", + material: new Uint8Array(16) as any, + derivedKey: newSessionKey, + }); + localStorage.get.mockResolvedValue(null); + localStorage.getKeys.mockResolvedValue([]); + + const result = await sut.get("test"); + + expect(keyGenerationService.createKeyWithPurpose).toHaveBeenCalled(); + expect(clearSpy).toHaveBeenCalled(); + expect(result).toBeNull(); }); }); describe("has", () => { - it("returns false when the key is not in cache", async () => { - const result = await sut.has("test"); - expect(result).toBe(false); - }); - it("returns true when the key is in cache", async () => { sut["cache"]["test"] = "cached"; const result = await sut.has("test"); @@ -95,21 +195,17 @@ describe("LocalBackedSessionStorage", () => { }); it("returns true when the key is in local storage", async () => { - localStorage.internalStore["session_test"] = makeEncString("encrypted").encryptedString; + const encString = makeEncString("encrypted"); + localStorage.get.mockResolvedValue(encString.encryptedString); encryptService.decryptString.mockResolvedValue(JSON.stringify("decrypted")); const result = await sut.has("test"); expect(result).toBe(true); }); - it.each([null, undefined])("returns false when %s is cached", async (nullish) => { - sut["cache"]["test"] = nullish; - await expect(sut.has("test")).resolves.toBe(false); - }); - it.each([null, undefined])( - "returns false when null is stored in local storage", - async (nullish) => { - localStorage.internalStore["session_test"] = nullish; + "returns false when the key does not exist in local storage (%s)", + async (value) => { + localStorage.get.mockResolvedValue(value); await expect(sut.has("test")).resolves.toBe(false); expect(encryptService.decryptString).not.toHaveBeenCalled(); }, @@ -118,6 +214,7 @@ describe("LocalBackedSessionStorage", () => { describe("save", () => { const encString = makeEncString("encrypted"); + beforeEach(() => { encryptService.encryptString.mockResolvedValue(encString); }); @@ -137,29 +234,44 @@ describe("LocalBackedSessionStorage", () => { }); it("removes the key when saving a null value", async () => { - const spy = jest.spyOn(sut, "remove"); + const removeSpy = jest.spyOn(sut, "remove"); await sut.save("test", null); - expect(spy).toHaveBeenCalledWith("test"); + expect(removeSpy).toHaveBeenCalledWith("test"); }); - it("saves the value to cache", async () => { + it("uses the session key when encrypting", async () => { await sut.save("test", "value"); - expect(sut["cache"]["test"]).toEqual("value"); - }); - it("encrypts and saves the value to local storage", async () => { - await sut.save("test", "value"); + expect(memoryStorage.get).toHaveBeenCalledWith("session-key"); expect(encryptService.encryptString).toHaveBeenCalledWith( JSON.stringify("value"), sessionKey, ); - expect(localStorage.internalStore["session_test"]).toEqual(encString.encryptedString); }); it("emits an update", async () => { - const spy = jest.spyOn(sut["updatesSubject"], "next"); + const updateSpy = jest.spyOn(sut["updatesSubject"], "next"); await sut.save("test", "value"); - expect(spy).toHaveBeenCalledWith({ key: "test", updateType: "save" }); + expect(updateSpy).toHaveBeenCalledWith({ key: "test", updateType: "save" }); + }); + + it("creates a new session key when session key is missing before saving", async () => { + const newSessionKey = makeSymmetricCryptoKey(); + memoryStorage.get.mockResolvedValue(null); + keyGenerationService.createKeyWithPurpose.mockResolvedValue({ + salt: "salt", + material: new Uint8Array(16) as any, + derivedKey: newSessionKey, + }); + localStorage.getKeys.mockResolvedValue([]); + + await sut.save("test", "value"); + + expect(keyGenerationService.createKeyWithPurpose).toHaveBeenCalled(); + expect(encryptService.encryptString).toHaveBeenCalledWith( + JSON.stringify("value"), + newSessionKey, + ); }); }); @@ -171,15 +283,50 @@ describe("LocalBackedSessionStorage", () => { }); it("removes the key from local storage", async () => { - localStorage.internalStore["session_test"] = makeEncString("encrypted").encryptedString; await sut.remove("test"); - expect(localStorage.internalStore["session_test"]).toBeUndefined(); + expect(localStorage.remove).toHaveBeenCalledWith("session_test"); }); it("emits an update", async () => { - const spy = jest.spyOn(sut["updatesSubject"], "next"); + const updateSpy = jest.spyOn(sut["updatesSubject"], "next"); await sut.remove("test"); - expect(spy).toHaveBeenCalledWith({ key: "test", updateType: "remove" }); + expect(updateSpy).toHaveBeenCalledWith({ key: "test", updateType: "remove" }); + }); + }); + + describe("sessionStorageKey", () => { + it("prefixes keys with session_ prefix", () => { + expect(sut["sessionStorageKey"]("test")).toBe("session_test"); + }); + }); + + describe("clear", () => { + it("only removes keys with session_ prefix", async () => { + const removeSpy = jest.spyOn(sut, "remove"); + localStorage.getKeys.mockResolvedValue([ + "session_data1", + "session_data2", + "regular_key", + "another_key", + "session_data3", + "my_session_key", + "mysession", + "sessiondata", + "user_session", + ]); + + await sut["clear"](); + + expect(removeSpy).toHaveBeenCalledWith("data1"); + expect(removeSpy).toHaveBeenCalledWith("data2"); + expect(removeSpy).toHaveBeenCalledWith("data3"); + expect(removeSpy).not.toHaveBeenCalledWith("regular_key"); + expect(removeSpy).not.toHaveBeenCalledWith("another_key"); + expect(removeSpy).not.toHaveBeenCalledWith("my_session_key"); + expect(removeSpy).not.toHaveBeenCalledWith("mysession"); + expect(removeSpy).not.toHaveBeenCalledWith("sessiondata"); + expect(removeSpy).not.toHaveBeenCalledWith("user_session"); + expect(removeSpy).toHaveBeenCalledTimes(3); }); }); }); diff --git a/apps/browser/src/platform/services/local-backed-session-storage.service.ts b/apps/browser/src/platform/services/local-backed-session-storage.service.ts index d0613ee644c..63a51d28e35 100644 --- a/apps/browser/src/platform/services/local-backed-session-storage.service.ts +++ b/apps/browser/src/platform/services/local-backed-session-storage.service.ts @@ -2,6 +2,7 @@ // @ts-strict-ignore import { Subject } from "rxjs"; +import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -12,33 +13,94 @@ import { StorageUpdate, } from "@bitwarden/common/platform/abstractions/storage.service"; import { compareValues } from "@bitwarden/common/platform/misc/compare-values"; -import { Lazy } from "@bitwarden/common/platform/misc/lazy"; import { StorageOptions } from "@bitwarden/common/platform/models/domain/storage-options"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { StorageService } from "@bitwarden/storage-core"; import { BrowserApi } from "../browser/browser-api"; import { MemoryStoragePortMessage } from "../storage/port-messages"; import { portName } from "../storage/port-name"; +import BrowserLocalStorageService from "./browser-local-storage.service"; + +const SESSION_KEY_PREFIX = "session_"; + +/** + * Manages an ephemeral session key for encrypting session storage items persisted in local storage. + * + * The session key is stored in session storage and automatically cleared when the browser session ends + * (e.g., browser restart, extension reload). When the session key is unavailable, any encrypted items + * in local storage cannot be decrypted and must be cleared to maintain data consistency. + * + * This provides session-scoped security for sensitive data while using persistent local storage as the backing store. + * + * @internal Internal implementation detail. Exported only for testing purposes. + * Do not use this class directly outside of tests. Use LocalBackedSessionStorageService instead. + */ +export class SessionKeyResolveService { + constructor( + private readonly storageService: StorageService, + private readonly keyGenerationService: KeyGenerationService, + ) {} + + /** + * Retrieves the session key from storage. + * + * @return session key or null when not in storage + */ + async get(): Promise { + const key = await this.storageService.get("session-key"); + if (key) { + if (this.storageService.valuesRequireDeserialization) { + return SymmetricCryptoKey.fromJSON(key); + } + return key; + } + return null; + } + + /** + * Creates new session key and adds it to underlying storage. + * + * @return newly created session key + */ + async create(): Promise { + const { derivedKey } = await this.keyGenerationService.createKeyWithPurpose( + 128, + "ephemeral", + "bitwarden-ephemeral", + ); + await this.storageService.save("session-key", derivedKey.toJSON()); + return derivedKey; + } +} + export class LocalBackedSessionStorageService extends AbstractStorageService implements ObservableStorageService { + readonly valuesRequireDeserialization = true; private ports: Set = new Set([]); private cache: Record = {}; private updatesSubject = new Subject(); - readonly valuesRequireDeserialization = true; updates$ = this.updatesSubject.asObservable(); + private readonly sessionKeyResolveService: SessionKeyResolveService; constructor( - private readonly sessionKey: Lazy>, - private readonly localStorage: AbstractStorageService, + private readonly memoryStorage: StorageService, + private readonly localStorage: BrowserLocalStorageService, + private readonly keyGenerationService: KeyGenerationService, private readonly encryptService: EncryptService, private readonly platformUtilsService: PlatformUtilsService, private readonly logService: LogService, ) { super(); + this.sessionKeyResolveService = new SessionKeyResolveService( + this.memoryStorage, + this.keyGenerationService, + ); + BrowserApi.addListener(chrome.runtime.onConnect, (port) => { if (port.name !== portName(chrome.storage.session)) { return; @@ -70,20 +132,20 @@ export class LocalBackedSessionStorageService } async get(key: string, options?: StorageOptions): Promise { - if (this.cache[key] !== undefined) { + if (this.cache[key] != null) { return this.cache[key] as T; } - const value = await this.getLocalSessionValue(await this.sessionKey.get(), key); + const value = await this.getLocalSessionValue(await this.getSessionKey(), key); - if (this.cache[key] === undefined && value !== undefined) { + if (this.cache[key] == null && value != null) { // Cache is still empty and we just got a value from local/session storage, cache it. this.cache[key] = value; return value as T; - } else if (this.cache[key] === undefined && value === undefined) { + } else if (this.cache[key] == null && value == null) { // Cache is still empty and we got nothing from local/session storage, no need to modify cache. return value as T; - } else if (this.cache[key] !== undefined && value !== undefined) { + } else if (this.cache[key] != null && value != null) { // Conflict, somebody wrote to the cache while we were reading from storage // but we also got a value from storage. We assume the cache is more up to date // and use that value. @@ -91,7 +153,7 @@ export class LocalBackedSessionStorageService `Conflict while reading from local session storage, both cache and storage have values. Key: ${key}. Using cached value.`, ); return this.cache[key] as T; - } else if (this.cache[key] !== undefined && value === undefined) { + } else if (this.cache[key] != null && value == null) { // Cache was filled after the local/session storage read completed. We got null // from the storage read, but we have a value from the cache, use that. this.logService.warning( @@ -136,6 +198,44 @@ export class LocalBackedSessionStorageService this.updatesSubject.next({ key, updateType: "remove" }); } + protected broadcastMessage(data: Omit) { + this.ports.forEach((port) => { + this.sendMessageTo(port, data); + }); + } + + private async getSessionKey(): Promise { + const sessionKey = await this.sessionKeyResolveService.get(); + if (sessionKey != null) { + return sessionKey; + } + + // Session key is missing (browser restart/extension reload), so all stored session data + // cannot be decrypted. Clear all items before creating a new session key. + await this.clear(); + + return await this.sessionKeyResolveService.create(); + } + + /** + * Removes all stored session data. + * + * Called when the session key is unavailable (typically after browser restart or extension reload), + * making all encrypted session data unrecoverable. Prevents orphaned encrypted data from accumulating. + */ + private async clear() { + const keys = (await this.localStorage.getKeys()).filter((key) => + key.startsWith(SESSION_KEY_PREFIX), + ); + this.logService.debug( + `[LocalBackedSessionStorageService] Clearing local session storage. Found ${keys}`, + ); + for (const key of keys) { + const keyWithoutPrefix = key.substring(SESSION_KEY_PREFIX.length); + await this.remove(keyWithoutPrefix); + } + } + private async getLocalSessionValue(encKey: SymmetricCryptoKey, key: string): Promise { const local = await this.localStorage.get(this.sessionStorageKey(key)); if (local == null) { @@ -159,10 +259,7 @@ export class LocalBackedSessionStorageService } const valueJson = JSON.stringify(value); - const encValue = await this.encryptService.encryptString( - valueJson, - await this.sessionKey.get(), - ); + const encValue = await this.encryptService.encryptString(valueJson, await this.getSessionKey()); await this.localStorage.save(this.sessionStorageKey(key), encValue.encryptedString); } @@ -197,12 +294,6 @@ export class LocalBackedSessionStorageService }); } - protected broadcastMessage(data: Omit) { - this.ports.forEach((port) => { - this.sendMessageTo(port, data); - }); - } - private sendMessageTo( port: chrome.runtime.Port, data: Omit, @@ -214,7 +305,7 @@ export class LocalBackedSessionStorageService } private sessionStorageKey(key: string) { - return `session_${key}`; + return `${SESSION_KEY_PREFIX}${key}`; } private compareValues(value1: T, value2: T): boolean { From 2ada60d1060e1d0a1b2818267a50bb28812fbb64 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com> Date: Thu, 29 Jan 2026 16:16:39 +0100 Subject: [PATCH 3/6] GH Testing workflow separation to fix OOM kill, timeout issues (#18594) * testing workflow oom kill fix * testing workflow oom kill fix * Adding job to verify all tests complete successfully (cherry picked from commit 845bafeb95fa59cf8d1508d298ccf4dc2684551d) * needs code cov --------- Co-authored-by: Andy Pixley <3723676+pixman20@users.noreply.github.com> --- .github/workflows/test.yml | 147 +++++++++++++++++++++++++++++++++---- 1 file changed, 134 insertions(+), 13 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 41b75c5a31d..a0f783bbb36 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -14,13 +14,11 @@ permissions: {} jobs: - testing: - name: Run tests + typecheck: + name: Run typechecking runs-on: ubuntu-22.04 permissions: - checks: write contents: read - pull-requests: write steps: - name: Check out repo @@ -56,17 +54,81 @@ jobs: - name: Run typechecking run: npm run test:types - - name: Run tests + testing: + name: Run tests - ${{ matrix.test-group.name }} + runs-on: ubuntu-22.04 + permissions: + checks: write + contents: read + pull-requests: write + + strategy: + fail-fast: false + matrix: + test-group: + - name: Browser + paths: apps/browser bitwarden_license/bit-browser + artifact: jest-coverage-browser + junit: junit-browser.xml + - name: Web + paths: apps/web bitwarden_license/bit-web + artifact: jest-coverage-web + junit: junit-web.xml + - name: Desktop + paths: apps/desktop + artifact: jest-coverage-desktop + junit: junit-desktop.xml + - name: CLI + paths: apps/cli bitwarden_license/bit-cli + artifact: jest-coverage-cli + junit: junit-cli.xml + - name: Libs + paths: libs bitwarden_license/bit-common + artifact: jest-coverage-libs + junit: junit-libs.xml + + steps: + - name: Check out repo + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + with: + persist-credentials: false + + - name: Get Node Version + id: retrieve-node-version + run: | + NODE_NVMRC=$(cat .nvmrc) + NODE_VERSION=${NODE_NVMRC/v/''} + echo "node_version=$NODE_VERSION" >> "$GITHUB_OUTPUT" + + - name: Set up Node + uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 + with: + cache: 'npm' + cache-dependency-path: '**/package-lock.json' + node-version: ${{ steps.retrieve-node-version.outputs.node_version }} + + - name: Print environment + run: | + node --version + npm --version + + - name: Install Node dependencies + run: npm ci + + - name: Run tests - ${{ matrix.test-group.name }} # maxWorkers is a workaround for a memory leak that crashes tests in CI: # https://github.com/facebook/jest/issues/9430#issuecomment-1149882002 - run: npm test -- --coverage --maxWorkers=3 + # Reduced to 2 workers and split tests across parallel jobs to prevent OOM kills + run: npm test -- ${{ matrix.test-group.paths }} --coverage --maxWorkers=2 + env: + JEST_JUNIT_OUTPUT_NAME: ${{ matrix.test-group.junit }} - name: Report test results uses: dorny/test-reporter@b082adf0eced0765477756c2a610396589b8c637 # v2.5.0 if: ${{ github.event.pull_request.head.repo.full_name == github.repository && !cancelled() }} with: - name: Test Results - path: "junit.xml" + name: Test Results - ${{ matrix.test-group.name }} + path: ${{ matrix.test-group.junit }} reporter: jest-junit fail-on-error: true @@ -78,7 +140,7 @@ jobs: - name: Upload test coverage uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: - name: jest-coverage + name: ${{ matrix.test-group.artifact }} path: ./coverage/lcov.info rust: @@ -183,11 +245,35 @@ jobs: with: persist-credentials: false - - name: Download jest coverage + - name: Download Browser coverage uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: - name: jest-coverage - path: ./ + name: jest-coverage-browser + path: ./jest-coverage-browser + + - name: Download Web coverage + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 + with: + name: jest-coverage-web + path: ./jest-coverage-web + + - name: Download Desktop coverage + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 + with: + name: jest-coverage-desktop + path: ./jest-coverage-desktop + + - name: Download CLI coverage + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 + with: + name: jest-coverage-cli + path: ./jest-coverage-cli + + - name: Download Libs coverage + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 + with: + name: jest-coverage-libs + path: ./jest-coverage-libs - name: Download rust coverage uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 @@ -199,5 +285,40 @@ jobs: uses: codecov/codecov-action@671740ac38dd9b0130fbe1cec585b89eea48d3de # v5.5.2 with: files: | - ./lcov.info + ./jest-coverage-browser/lcov.info + ./jest-coverage-web/lcov.info + ./jest-coverage-desktop/lcov.info + ./jest-coverage-cli/lcov.info + ./jest-coverage-libs/lcov.info ./apps/desktop/desktop_native/lcov.info + + run-tests: # Verifies all required tests complete successfully + name: Run tests + runs-on: ubuntu-24.04 + if: always() + needs: + - typecheck + - testing + - rust + - rust-coverage + - upload-codecov + permissions: + contents: read + + steps: + - name: Check job results + env: + NEEDS: ${{ toJSON(needs) }} + run: | + # Print status of all jobs + echo "$NEEDS" | jq -r 'to_entries[] | "\(.key): \(.value.result)"' + + # Collect failed jobs + failed_jobs=$(echo "$NEEDS" | jq -r 'to_entries[] | select(.value.result != "success") | .key' | tr '\n' ' ') + + if [ -n "$failed_jobs" ]; then + echo "::error::The following jobs failed:$failed_jobs" + exit 1 + fi + + echo "All required jobs passed successfully!" From a4355dbcab2d782d3d506adc4e0adb21fb10b457 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Thu, 29 Jan 2026 16:21:46 +0100 Subject: [PATCH 4/6] Ensure "MyVault" is not identified as an organaization (#18643) When creating a new vault item in the My Vault filter owner would not be set. --- apps/desktop/src/vault/app/vault-v3/vault.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/desktop/src/vault/app/vault-v3/vault.component.ts b/apps/desktop/src/vault/app/vault-v3/vault.component.ts index efb7e4de70f..698c442dd8f 100644 --- a/apps/desktop/src/vault/app/vault-v3/vault.component.ts +++ b/apps/desktop/src/vault/app/vault-v3/vault.component.ts @@ -953,7 +953,7 @@ export class VaultComponent implements OnInit, OnDestroy, CopyClickListener { this.addOrganizationId = collections[0].organizationId; this.addCollectionIds = [this.activeFilter.collectionId]; } - } else if (this.activeFilter.organizationId) { + } else if (this.activeFilter.organizationId && this.activeFilter.organizationId !== "MyVault") { this.addOrganizationId = this.activeFilter.organizationId; } else { // clear out organizationId when the user switches to a personal vault filter From b51d3bf9dad4e8477d268dfaebe2ca962c934d74 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Thu, 29 Jan 2026 09:58:31 -0600 Subject: [PATCH 5/6] [PM-29271] Add referrer checking for vault messages (#18346) * update message from vault handling to check against accounts or message sender * update valid vault referrer logic to check all configured environments --- .../src/background/runtime.background.ts | 59 +++++++++++++------ 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index eba6b01fe90..7483e71f87f 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -291,15 +291,21 @@ export default class RuntimeBackground { } break; case "openPopup": - await this.openPopup(); + await this.executeMessageActionOrOpenPopup(msg, this.openPopup.bind(this)); break; case VaultMessages.OpenAtRiskPasswords: { - await this.main.openAtRisksPasswordsPage(); + await this.executeMessageActionOrOpenPopup( + msg, + this.main.openAtRisksPasswordsPage.bind(this), + ); this.announcePopupOpen(); break; } case VaultMessages.OpenBrowserExtensionToUrl: { - await this.main.openTheExtensionToPage(msg.url); + await this.executeMessageActionOrOpenPopup( + msg, + this.main.openTheExtensionToPage.bind(this, msg.url), + ); this.announcePopupOpen(); break; } @@ -374,40 +380,55 @@ export default class RuntimeBackground { * @param message * @returns true if message fails validation */ - private async shouldRejectManyOriginMessage(message: { - webExtSender: chrome.runtime.MessageSender; - }): Promise { + private async executeMessageActionOrOpenPopup( + message: { + webExtSender: chrome.runtime.MessageSender; + }, + messageAction: () => Promise, + ): Promise { + const hasAccounts = await firstValueFrom( + this.accountService.accounts$.pipe(map((a) => Object.keys(a).length > 0)), + ); + + // When there are no accounts associated with the extension, only allow opening the popup + if (!hasAccounts) { + await this.openPopup(); + return; + } + const isValidVaultReferrer = await this.isValidVaultReferrer( Utils.getHostname(message?.webExtSender?.origin), ); - if (isValidVaultReferrer) { - return false; + // When the referrer is not a known vault and the message is external, reject the message + if (!isValidVaultReferrer && isExternalMessage(message)) { + return; } - return isExternalMessage(message); + await messageAction(); } /** - * Validates a message's referrer matches the configured web vault hostname. + * Validates that a referrer hostname matches any of the available regions' and current environment web vault URLs. * - * @param referrer - hostname from message source - * @returns true if referrer matches web vault + * @param referrer - hostname from message source (should not include protocol or path) + * @returns true if referrer matches any known vault hostname, false otherwise */ private async isValidVaultReferrer(referrer: string | null | undefined): Promise { if (!referrer) { return false; } - const env = await firstValueFrom(this.environmentService.environment$); - const vaultUrl = env.getWebVaultUrl(); - const vaultHostname = Utils.getHostname(vaultUrl); + const environment = await firstValueFrom(this.environmentService.environment$); - if (!vaultHostname) { - return false; - } + const regions = this.environmentService.availableRegions(); + const regionVaultUrls = regions.map((r) => r.urls.webVault ?? r.urls.base); + const environmentWebVaultUrl = environment.getWebVaultUrl(); + const messageIsFromKnownVault = [...regionVaultUrls, environmentWebVaultUrl].some( + (webVaultUrl) => Utils.getHostname(webVaultUrl) === referrer, + ); - return vaultHostname === referrer; + return messageIsFromKnownVault; } private async autofillPage(tabToAutoFill: chrome.tabs.Tab) { From 544dcc6757ad943afb2e2b9d2c18af027c7686b6 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Thu, 29 Jan 2026 09:58:57 -0600 Subject: [PATCH 6/6] remove unarchive button when a cipher is deleted (#18575) --- .../vault-v2/view-v2/view-v2.component.html | 2 +- .../vault-v2/view-v2/view-v2.component.spec.ts | 14 +++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.html b/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.html index 03eb701704f..8ac6de75997 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.html @@ -26,7 +26,7 @@ } - @if ((archiveFlagEnabled$ | async) && cipher.isArchived) { + @if ((archiveFlagEnabled$ | async) && cipher.isArchived && !cipher.isDeleted) {