From 535b958f9e4edf5f0132b4859801bba6347f1dc1 Mon Sep 17 00:00:00 2001 From: Daniel Riera Date: Thu, 15 Jan 2026 12:09:55 -0500 Subject: [PATCH] [PM-29523] Remove ts strict ignore in browser context menu clicked handler (#18264) * early return if no cipher before switch case * explicit null checks within switch cases for early returns * lower cipher check and add to explicit checks * add test cases for null values * format spec file --- .../context-menu-clicked-handler.spec.ts | 90 +++++++++++++++++++ .../browser/context-menu-clicked-handler.ts | 29 +++++- 2 files changed, 116 insertions(+), 3 deletions(-) diff --git a/apps/browser/src/autofill/browser/context-menu-clicked-handler.spec.ts b/apps/browser/src/autofill/browser/context-menu-clicked-handler.spec.ts index 61d6b9dc480..5c2b266f829 100644 --- a/apps/browser/src/autofill/browser/context-menu-clicked-handler.spec.ts +++ b/apps/browser/src/autofill/browser/context-menu-clicked-handler.spec.ts @@ -4,8 +4,10 @@ import { of } from "rxjs"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { AUTOFILL_ID, + COPY_IDENTIFIER_ID, COPY_PASSWORD_ID, COPY_USERNAME_ID, COPY_VERIFICATION_CODE_ID, @@ -85,6 +87,7 @@ describe("ContextMenuClickedHandler", () => { accountService = mockAccountServiceWith(mockUserId as UserId); totpService = mock(); eventCollectionService = mock(); + userVerificationService = mock(); sut = new ContextMenuClickedHandler( copyToClipboard, @@ -102,6 +105,93 @@ describe("ContextMenuClickedHandler", () => { afterEach(() => jest.resetAllMocks()); describe("run", () => { + beforeEach(() => { + authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Unlocked); + userVerificationService.hasMasterPasswordAndMasterKeyHash.mockResolvedValue(false); + }); + + const runWithUrl = (data: chrome.contextMenus.OnClickData) => + sut.run(data, { url: "https://test.com" } as any); + + describe("early returns", () => { + it.each([ + { + name: "tab id is missing", + data: createData(COPY_IDENTIFIER_ID), + tab: { url: "https://test.com" } as any, + expectNotCalled: () => expect(copyToClipboard).not.toHaveBeenCalled(), + }, + { + name: "tab url is missing", + data: createData(`${COPY_USERNAME_ID}_${NOOP_COMMAND_SUFFIX}`, COPY_USERNAME_ID), + tab: {} as any, + expectNotCalled: () => { + expect(cipherService.getAllDecryptedForUrl).not.toHaveBeenCalled(); + expect(copyToClipboard).not.toHaveBeenCalled(); + }, + }, + ])("returns early when $name", async ({ data, tab, expectNotCalled }) => { + await expect(sut.run(data, tab)).resolves.toBeUndefined(); + expectNotCalled(); + }); + }); + + describe("missing cipher", () => { + it.each([ + { + label: "AUTOFILL", + parentId: AUTOFILL_ID, + extra: () => expect(autofill).not.toHaveBeenCalled(), + }, + { label: "username", parentId: COPY_USERNAME_ID, extra: () => {} }, + { label: "password", parentId: COPY_PASSWORD_ID, extra: () => {} }, + { + label: "totp", + parentId: COPY_VERIFICATION_CODE_ID, + extra: () => expect(totpService.getCode$).not.toHaveBeenCalled(), + }, + ])("breaks silently when cipher is missing for $label", async ({ parentId, extra }) => { + cipherService.getAllDecrypted.mockResolvedValue([]); + + await expect(runWithUrl(createData(`${parentId}_1`, parentId))).resolves.toBeUndefined(); + + expect(copyToClipboard).not.toHaveBeenCalled(); + extra(); + }); + }); + + describe("missing login properties", () => { + it.each([ + { + label: "username", + parentId: COPY_USERNAME_ID, + unset: (c: CipherView): void => (c.login.username = undefined), + }, + { + label: "password", + parentId: COPY_PASSWORD_ID, + unset: (c: CipherView): void => (c.login.password = undefined), + }, + { + label: "totp", + parentId: COPY_VERIFICATION_CODE_ID, + unset: (c: CipherView): void => (c.login.totp = undefined), + isTotp: true, + }, + ])("breaks silently when $label property is missing", async ({ parentId, unset, isTotp }) => { + const cipher = createCipher(); + unset(cipher); + cipherService.getAllDecrypted.mockResolvedValue([cipher]); + + await expect(runWithUrl(createData(`${parentId}_1`, parentId))).resolves.toBeUndefined(); + + expect(copyToClipboard).not.toHaveBeenCalled(); + if (isTotp) { + expect(totpService.getCode$).not.toHaveBeenCalled(); + } + }); + }); + it("can generate password", async () => { await sut.run(createData(GENERATE_PASSWORD_ID), { id: 5 } as any); diff --git a/apps/browser/src/autofill/browser/context-menu-clicked-handler.ts b/apps/browser/src/autofill/browser/context-menu-clicked-handler.ts index 6f0979d4fd5..aa01ada0838 100644 --- a/apps/browser/src/autofill/browser/context-menu-clicked-handler.ts +++ b/apps/browser/src/autofill/browser/context-menu-clicked-handler.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { firstValueFrom } from "rxjs"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; @@ -72,6 +70,10 @@ export class ContextMenuClickedHandler { await this.generatePasswordToClipboard(tab); break; case COPY_IDENTIFIER_ID: + if (!tab.id) { + return; + } + this.copyToClipboard({ text: await this.getIdentifier(tab, info), tab: tab }); break; default: @@ -120,6 +122,10 @@ export class ContextMenuClickedHandler { if (isCreateCipherAction) { // pass; defer to logic below } else if (menuItemId === NOOP_COMMAND_SUFFIX) { + if (!tab.url) { + return; + } + const additionalCiphersToGet = info.parentMenuItemId === AUTOFILL_IDENTITY_ID ? [CipherType.Identity] @@ -158,6 +164,10 @@ export class ContextMenuClickedHandler { break; } + if (!cipher) { + break; + } + if (await this.isPasswordRepromptRequired(cipher)) { await openVaultItemPasswordRepromptPopout(tab, { cipherId: cipher.id, @@ -176,6 +186,10 @@ export class ContextMenuClickedHandler { break; } + if (!cipher || !cipher.login?.username) { + break; + } + this.copyToClipboard({ text: cipher.login.username, tab: tab }); break; case COPY_PASSWORD_ID: @@ -184,6 +198,10 @@ export class ContextMenuClickedHandler { break; } + if (!cipher || !cipher.login?.password) { + break; + } + if (await this.isPasswordRepromptRequired(cipher)) { await openVaultItemPasswordRepromptPopout(tab, { cipherId: cipher.id, @@ -205,6 +223,10 @@ export class ContextMenuClickedHandler { break; } + if (!cipher || !cipher.login?.totp) { + break; + } + if (await this.isPasswordRepromptRequired(cipher)) { await openVaultItemPasswordRepromptPopout(tab, { cipherId: cipher.id, @@ -240,9 +262,10 @@ export class ContextMenuClickedHandler { } private async getIdentifier(tab: chrome.tabs.Tab, info: chrome.contextMenus.OnClickData) { + const tabId = tab.id!; return new Promise((resolve, reject) => { BrowserApi.sendTabsMessage( - tab.id, + tabId, { command: "getClickedElement" }, { frameId: info.frameId }, (identifier: string) => {