From 6b3c4f87c70543b3f2599f72acf8a10053006a15 Mon Sep 17 00:00:00 2001
From: Vicki League <vleague@bitwarden.com>
Date: Wed, 29 Oct 2025 16:52:28 -0400
Subject: [PATCH 1/3] [CL-807] Improve aria a11y of nav group (#17078)

---
 .../src/navigation/nav-group.component.html   |  6 ++--
 .../src/navigation/nav-group.component.ts     | 32 +++++++++++++++++--
 .../src/navigation/nav-item.component.html    | 11 ++++---
 .../src/navigation/nav-item.component.ts      | 10 +++++-
 4 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/libs/components/src/navigation/nav-group.component.html b/libs/components/src/navigation/nav-group.component.html
index 195569292f6..bcf6ae2b5b7 100644
--- a/libs/components/src/navigation/nav-group.component.html
+++ b/libs/components/src/navigation/nav-group.component.html
@@ -8,7 +8,8 @@
     [routerLinkActiveOptions]="routerLinkActiveOptions()"
     (mainContentClicked)="handleMainContentClicked()"
     [ariaLabel]="ariaLabel()"
-    [hideActiveStyles]="parentHideActiveStyles"
+    [hideActiveStyles]="parentHideActiveStyles()"
+    [ariaCurrentWhenActive]="ariaCurrent()"
   >
     <ng-template #button>
       <button
@@ -18,7 +19,6 @@
         [buttonType]="'nav-contrast'"
         (click)="toggle($event)"
         size="small"
-        aria-haspopup="true"
         [attr.aria-expanded]="open().toString()"
         [attr.aria-controls]="contentId"
         [label]="['toggleCollapse' | i18n, text()].join(' ')"
@@ -30,7 +30,7 @@
     </ng-container>
   </bit-nav-item>
   <!-- [attr.aria-controls] of the above button expects a unique ID on the controlled element -->
-  @if (sideNavService.open$ | async) {
+  @if (sideNavOpen()) {
     @if (open()) {
       <div
         [attr.id]="contentId"
diff --git a/libs/components/src/navigation/nav-group.component.ts b/libs/components/src/navigation/nav-group.component.ts
index e3bb02bb75a..3408af3d734 100644
--- a/libs/components/src/navigation/nav-group.component.ts
+++ b/libs/components/src/navigation/nav-group.component.ts
@@ -9,7 +9,10 @@ import {
   input,
   model,
   contentChildren,
+  computed,
 } from "@angular/core";
+import { toSignal } from "@angular/core/rxjs-interop";
+import { RouterLinkActive } from "@angular/router";
 
 import { I18nPipe } from "@bitwarden/ui-common";
 
@@ -33,10 +36,33 @@ import { SideNavService } from "./side-nav.service";
 export class NavGroupComponent extends NavBaseComponent {
   readonly nestedNavComponents = contentChildren(NavBaseComponent, { descendants: true });
 
+  readonly sideNavOpen = toSignal(this.sideNavService.open$);
+
+  readonly sideNavAndGroupOpen = computed(() => {
+    return this.open() && this.sideNavOpen();
+  });
+
   /** When the side nav is open, the parent nav item should not show active styles when open. */
-  protected get parentHideActiveStyles(): boolean {
-    return this.hideActiveStyles() || (this.open() && this.sideNavService.open);
-  }
+  readonly parentHideActiveStyles = computed(() => {
+    return this.hideActiveStyles() || this.sideNavAndGroupOpen();
+  });
+
+  /**
+   * Allow overriding of the RouterLink['ariaCurrentWhenActive'] property.
+   *
+   * By default, assuming that the nav group navigates to its first child page instead of its
+   * own page, the nav group will be `current` when the side nav is collapsed or the nav group
+   * is collapsed (since child pages don't show in either collapsed view) and not `current`
+   * when the side nav and nav group are open (since the child page will show as `current`).
+   *
+   * If the nav group navigates to its own page, use this property to always set it to announce
+   * as `current` by passing in `"page"`.
+   */
+  readonly ariaCurrentWhenActive = input<RouterLinkActive["ariaCurrentWhenActive"]>();
+
+  readonly ariaCurrent = computed(() => {
+    return this.ariaCurrentWhenActive() ?? (this.sideNavAndGroupOpen() ? undefined : "page");
+  });
 
   /**
    * UID for `[attr.aria-controls]`
diff --git a/libs/components/src/navigation/nav-item.component.html b/libs/components/src/navigation/nav-item.component.html
index f7d55a33362..37a5d82aa1b 100644
--- a/libs/components/src/navigation/nav-item.component.html
+++ b/libs/components/src/navigation/nav-item.component.html
@@ -11,7 +11,7 @@
       ]"
     >
       <div class="tw-relative tw-flex tw-items-center tw-h-full">
-        <ng-container *ngIf="route; then isAnchor; else isButton"></ng-container>
+        <ng-container *ngIf="route(); then isAnchor; else isButton"></ng-container>
 
         <!-- Main content of `NavItem` -->
         <ng-template #anchorAndButtonContent>
@@ -31,7 +31,7 @@
           </div>
         </ng-template>
 
-        <!-- Show if a value was passed to `this.to` -->
+        <!-- Show if a value was passed to `this.route` -->
         <ng-template #isAnchor>
           <!-- The `data-fvw` attribute passes focus to `this.focusVisibleWithin$` -->
           <!-- The following `class` field should match the `#isButton` class field below -->
@@ -43,7 +43,7 @@
             [attr.aria-label]="ariaLabel() || text()"
             routerLinkActive
             [routerLinkActiveOptions]="routerLinkActiveOptions()"
-            [ariaCurrentWhenActive]="'page'"
+            [ariaCurrentWhenActive]="ariaCurrentWhenActive()"
             (isActiveChange)="setIsActive($event)"
             (click)="mainContentClicked.emit()"
           >
@@ -51,12 +51,13 @@
           </a>
         </ng-template>
 
-        <!-- Show if `this.to` is falsy -->
+        <!-- Show if `this.route` is falsy -->
         <ng-template #isButton>
           <!-- Class field should match `#isAnchor` class field above -->
           <button
             type="button"
-            class="tw-size-full tw-px-4 tw-pe-3 tw-truncate tw-border-none tw-bg-transparent tw-text-start !tw-text-alt2 hover:tw-text-alt2 hover:tw-no-underline focus:tw-outline-none [&_i]:tw-leading-[1.5rem]"
+            class="tw-size-full tw-px-4 tw-truncate tw-border-none tw-bg-transparent tw-text-start !tw-text-alt2 hover:tw-text-alt2 hover:tw-no-underline focus:tw-outline-none [&_i]:tw-leading-[1.5rem]"
+            [ngClass]="open ? 'tw-pe-3' : 'tw-pe-4'"
             data-fvw
             (click)="mainContentClicked.emit()"
           >
diff --git a/libs/components/src/navigation/nav-item.component.ts b/libs/components/src/navigation/nav-item.component.ts
index 5b5709ebebb..de757f1aec4 100644
--- a/libs/components/src/navigation/nav-item.component.ts
+++ b/libs/components/src/navigation/nav-item.component.ts
@@ -1,6 +1,6 @@
 import { CommonModule } from "@angular/common";
 import { Component, HostListener, Optional, input } from "@angular/core";
-import { RouterModule } from "@angular/router";
+import { RouterLinkActive, RouterModule } from "@angular/router";
 import { BehaviorSubject, map } from "rxjs";
 
 import { IconButtonModule } from "../icon-button";
@@ -39,6 +39,14 @@ export class NavItemComponent extends NavBaseComponent {
     return this.forceActiveStyles() || (this._isActive && !this.hideActiveStyles());
   }
 
+  /**
+   * Allow overriding of the RouterLink['ariaCurrentWhenActive'] property.
+   *
+   * Useful for situations like nav-groups that navigate to their first child page and should
+   * not be marked `current` while the child page is marked as `current`
+   */
+  readonly ariaCurrentWhenActive = input<RouterLinkActive["ariaCurrentWhenActive"]>("page");
+
   /**
    * The design spec calls for the an outline to wrap the entire element when the template's
    * anchor/button has :focus-visible. Usually, we would use :focus-within for this. However, that

From 9fca0b01380cfe05506925421c174755773c32c2 Mon Sep 17 00:00:00 2001
From: Daniel James Smith <2670567+djsmith85@users.noreply.github.com>
Date: Wed, 29 Oct 2025 21:56:55 +0100
Subject: [PATCH 2/3] [PM-26731] Fix feature flag check for
 "pm-25855-chromium-importer-abe" (#17102)

* Fix feature flag check for "pm-25855-chromium-importer-abe"

The old lofgic actually removed all chromium support when the flag was disabled. It should only remove those browser if the flag is disabled and when on Windows.

* Extend tests

* Update comment

* Remove duplicate test

* Add test for when device cannot be detected and throws and error

* Add descriptive comment to feature flag test case assertions

* Better test assertion

---------

Co-authored-by: Daniel James Smith <djsmith85@users.noreply.github.com>
---
 .../default-import-metadata.service.ts        | 19 +++---
 .../services/import-metadata.service.spec.ts  | 61 ++++++++++++++-----
 2 files changed, 55 insertions(+), 25 deletions(-)

diff --git a/libs/importer/src/services/default-import-metadata.service.ts b/libs/importer/src/services/default-import-metadata.service.ts
index 05b8472869d..e9c526fb6c0 100644
--- a/libs/importer/src/services/default-import-metadata.service.ts
+++ b/libs/importer/src/services/default-import-metadata.service.ts
@@ -61,27 +61,28 @@ export class DefaultImportMetadataService implements ImportMetadataServiceAbstra
     importers: ImportersMetadata,
     type: ImportType,
     client: ClientType,
-    enabled: boolean,
+    withABESupport: boolean,
   ): DataLoader[] | undefined {
     let loaders = availableLoaders(importers, type, client);
-    let includeABE = false;
 
-    if (enabled && (type === "bravecsv" || type === "chromecsv" || type === "edgecsv")) {
+    if (withABESupport) {
+      return loaders;
+    }
+
+    // Special handling for Brave, Chrome, and Edge CSV imports on Windows Desktop
+    if (type === "bravecsv" || type === "chromecsv" || type === "edgecsv") {
       try {
         const device = this.system.environment.getDevice();
         const isWindowsDesktop = device === DeviceType.WindowsDesktop;
         if (isWindowsDesktop) {
-          includeABE = true;
+          // Exclude the Chromium loader if on Windows Desktop without ABE support
+          loaders = loaders?.filter((loader) => loader !== Loader.chromium);
         }
       } catch {
-        includeABE = true;
+        loaders = loaders?.filter((loader) => loader !== Loader.chromium);
       }
     }
 
-    // If the browser is unsupported, remove the chromium loader
-    if (!includeABE) {
-      loaders = loaders?.filter((loader) => loader !== Loader.chromium);
-    }
     return loaders;
   }
 }
diff --git a/libs/importer/src/services/import-metadata.service.spec.ts b/libs/importer/src/services/import-metadata.service.spec.ts
index 908ce6ad476..e16965a69f8 100644
--- a/libs/importer/src/services/import-metadata.service.spec.ts
+++ b/libs/importer/src/services/import-metadata.service.spec.ts
@@ -55,6 +55,25 @@ describe("ImportMetadataService", () => {
 
       // Recreate the service with the updated mocks for logging tests
       sut = new DefaultImportMetadataService(systemServiceProvider);
+
+      // Set up importers to include bravecsv and chromecsv with chromium loader
+      sut["importers"] = {
+        chromecsv: {
+          type: "chromecsv",
+          loaders: [Loader.file, Loader.chromium],
+          instructions: Instructions.chromium,
+        },
+        bravecsv: {
+          type: "bravecsv",
+          loaders: [Loader.file, Loader.chromium],
+          instructions: Instructions.chromium,
+        },
+        edgecsv: {
+          type: "edgecsv",
+          loaders: [Loader.file, Loader.chromium],
+          instructions: Instructions.chromium,
+        },
+      } as ImportersMetadata;
     });
 
     afterEach(() => {
@@ -112,6 +131,7 @@ describe("ImportMetadataService", () => {
     });
 
     it("should update when feature flag changes", async () => {
+      environment.getDevice.mockReturnValue(DeviceType.WindowsDesktop);
       const testType: ImportType = "bravecsv"; // Use bravecsv which supports chromium loader
       const emissions: ImporterMetadata[] = [];
 
@@ -126,13 +146,15 @@ describe("ImportMetadataService", () => {
       await new Promise((resolve) => setTimeout(resolve, 0));
 
       expect(emissions).toHaveLength(2);
+      // Disable ABE - chromium loader should be excluded
       expect(emissions[0].loaders).not.toContain(Loader.chromium);
-      expect(emissions[1].loaders).toContain(Loader.file);
+      // Enabled ABE - chromium loader should be included
+      expect(emissions[1].loaders).toContain(Loader.chromium);
 
       subscription.unsubscribe();
     });
 
-    it("should exclude chromium loader when ABE is disabled but on Windows Desktop", async () => {
+    it("should exclude chromium loader when ABE is disabled and on Windows Desktop", async () => {
       environment.getDevice.mockReturnValue(DeviceType.WindowsDesktop);
       const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders
       featureFlagSubject.next(false);
@@ -146,10 +168,12 @@ describe("ImportMetadataService", () => {
       expect(result.loaders).toContain(Loader.file);
     });
 
-    it("should exclude chromium loader when ABE is enabled but not on Windows Desktop", async () => {
-      environment.getDevice.mockReturnValue(DeviceType.MacOsDesktop);
-      const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders
-      featureFlagSubject.next(true);
+    it("should exclude chromium loader when ABE is disabled and getDevice throws error", async () => {
+      environment.getDevice.mockImplementation(() => {
+        throw new Error("Device detection failed");
+      });
+      const testType: ImportType = "bravecsv";
+      featureFlagSubject.next(false);
 
       const metadataPromise = firstValueFrom(sut.metadata$(typeSubject));
       typeSubject.next(testType);
@@ -160,17 +184,22 @@ describe("ImportMetadataService", () => {
       expect(result.loaders).toContain(Loader.file);
     });
 
-    it("should include chromium loader when ABE is enabled and on Windows Desktop", async () => {
-      // Set up importers to include bravecsv with chromium loader
-      sut["importers"] = {
-        bravecsv: {
-          type: "bravecsv",
-          loaders: [Loader.file, Loader.chromium],
-          instructions: Instructions.chromium,
-        },
-      } as ImportersMetadata;
+    it("should include chromium loader when ABE is disabled and not on Windows Desktop", async () => {
+      environment.getDevice.mockReturnValue(DeviceType.MacOsDesktop);
+      const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders
+      featureFlagSubject.next(false);
 
-      environment.getDevice.mockReturnValue(DeviceType.WindowsDesktop);
+      const metadataPromise = firstValueFrom(sut.metadata$(typeSubject));
+      typeSubject.next(testType);
+
+      const result = await metadataPromise;
+
+      expect(result.loaders).toContain(Loader.chromium);
+      expect(result.loaders).toContain(Loader.file);
+    });
+
+    it("should include chromium loader when ABE is enabled regardless of device", async () => {
+      environment.getDevice.mockReturnValue(DeviceType.MacOsDesktop);
       const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders
       featureFlagSubject.next(true);
 

From 51a557514f112717dad2cb068cd770d60a284b8b Mon Sep 17 00:00:00 2001
From: Shane Melton <smelton@bitwarden.com>
Date: Wed, 29 Oct 2025 14:47:55 -0700
Subject: [PATCH 3/3] [PM-20379] Fix At-risk password task permission bug
 (#17110)

* [PM-20379] Fix at risk password task permission checks

* [PM-20379] Fix at risk password component specs

* [PM-20379] Cleanup FIXMEs

* [PM-20379] Update to OnPush

* [PM-20379] Add tests for pendingTasks$

* [PM-20379] Reduce test boilerplate / redundancy

* [PM-20379] Cleanup as any

* [PM-20379] Remove redundant "should" language
---
 .../at-risk-passwords.component.spec.ts       |  67 +++--
 .../at-risk-passwords.component.ts            |  14 +-
 .../at-risk-password-callout.service.spec.ts  | 244 +++++++++++++++++-
 .../at-risk-password-callout.service.ts       |   2 +
 4 files changed, 298 insertions(+), 29 deletions(-)

diff --git a/apps/browser/src/vault/popup/components/at-risk-passwords/at-risk-passwords.component.spec.ts b/apps/browser/src/vault/popup/components/at-risk-passwords/at-risk-passwords.component.spec.ts
index c58798d9d12..96c597113a5 100644
--- a/apps/browser/src/vault/popup/components/at-risk-passwords/at-risk-passwords.component.spec.ts
+++ b/apps/browser/src/vault/popup/components/at-risk-passwords/at-risk-passwords.component.spec.ts
@@ -1,4 +1,4 @@
-import { Component, Input } from "@angular/core";
+import { ChangeDetectionStrategy, Component, input } from "@angular/core";
 import { ComponentFixture, TestBed } from "@angular/core/testing";
 import { By } from "@angular/platform-browser";
 import { mock } from "jest-mock-extended";
@@ -37,43 +37,32 @@ import { AtRiskCarouselDialogResult } from "../at-risk-carousel-dialog/at-risk-c
 import { AtRiskPasswordPageService } from "./at-risk-password-page.service";
 import { AtRiskPasswordsComponent } from "./at-risk-passwords.component";
 
-// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
-// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
 @Component({
   selector: "popup-header",
   template: `<ng-content></ng-content>`,
+  changeDetection: ChangeDetectionStrategy.OnPush,
 })
 class MockPopupHeaderComponent {
-  // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals
-  // eslint-disable-next-line @angular-eslint/prefer-signals
-  @Input() pageTitle: string | undefined;
-  // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals
-  // eslint-disable-next-line @angular-eslint/prefer-signals
-  @Input() backAction: (() => void) | undefined;
+  readonly pageTitle = input<string | undefined>(undefined);
+  readonly backAction = input<(() => void) | undefined>(undefined);
 }
 
-// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
-// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
 @Component({
   selector: "popup-page",
   template: `<ng-content></ng-content>`,
+  changeDetection: ChangeDetectionStrategy.OnPush,
 })
 class MockPopupPageComponent {
-  // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals
-  // eslint-disable-next-line @angular-eslint/prefer-signals
-  @Input() loading: boolean | undefined;
+  readonly loading = input<boolean | undefined>(undefined);
 }
 
-// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
-// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
 @Component({
   selector: "app-vault-icon",
   template: `<ng-content></ng-content>`,
+  changeDetection: ChangeDetectionStrategy.OnPush,
 })
 class MockAppIcon {
-  // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals
-  // eslint-disable-next-line @angular-eslint/prefer-signals
-  @Input() cipher: CipherView | undefined;
+  readonly cipher = input<CipherView | undefined>(undefined);
 }
 
 describe("AtRiskPasswordsComponent", () => {
@@ -109,11 +98,15 @@ describe("AtRiskPasswordsComponent", () => {
         id: "cipher",
         organizationId: "org",
         name: "Item 1",
+        edit: true,
+        viewPassword: true,
       } as CipherView,
       {
         id: "cipher2",
         organizationId: "org",
         name: "Item 2",
+        edit: true,
+        viewPassword: true,
       } as CipherView,
     ]);
     mockOrgs$ = new BehaviorSubject<Organization[]>([
@@ -235,6 +228,38 @@ describe("AtRiskPasswordsComponent", () => {
           organizationId: "org",
           name: "Item 1",
           isDeleted: true,
+          edit: true,
+          viewPassword: true,
+        } as CipherView,
+      ]);
+
+      const items = await firstValueFrom(component["atRiskItems$"]);
+      expect(items).toHaveLength(0);
+    });
+
+    it("should not show tasks when cipher does not have edit permission", async () => {
+      mockCiphers$.next([
+        {
+          id: "cipher",
+          organizationId: "org",
+          name: "Item 1",
+          edit: false,
+          viewPassword: true,
+        } as CipherView,
+      ]);
+
+      const items = await firstValueFrom(component["atRiskItems$"]);
+      expect(items).toHaveLength(0);
+    });
+
+    it("should not show tasks when cipher does not have viewPassword permission", async () => {
+      mockCiphers$.next([
+        {
+          id: "cipher",
+          organizationId: "org",
+          name: "Item 1",
+          edit: true,
+          viewPassword: false,
         } as CipherView,
       ]);
 
@@ -288,11 +313,15 @@ describe("AtRiskPasswordsComponent", () => {
           id: "cipher",
           organizationId: "org",
           name: "Item 1",
+          edit: true,
+          viewPassword: true,
         } as CipherView,
         {
           id: "cipher2",
           organizationId: "org2",
           name: "Item 2",
+          edit: true,
+          viewPassword: true,
         } as CipherView,
       ]);
 
diff --git a/apps/browser/src/vault/popup/components/at-risk-passwords/at-risk-passwords.component.ts b/apps/browser/src/vault/popup/components/at-risk-passwords/at-risk-passwords.component.ts
index 3eeb2d1917b..94fdb00f566 100644
--- a/apps/browser/src/vault/popup/components/at-risk-passwords/at-risk-passwords.component.ts
+++ b/apps/browser/src/vault/popup/components/at-risk-passwords/at-risk-passwords.component.ts
@@ -1,5 +1,12 @@
 import { CommonModule } from "@angular/common";
-import { Component, DestroyRef, inject, OnInit, signal } from "@angular/core";
+import {
+  Component,
+  DestroyRef,
+  inject,
+  OnInit,
+  signal,
+  ChangeDetectionStrategy,
+} from "@angular/core";
 import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
 import { Router } from "@angular/router";
 import {
@@ -58,8 +65,6 @@ import {
 
 import { AtRiskPasswordPageService } from "./at-risk-password-page.service";
 
-// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
-// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
 @Component({
   imports: [
     PopupPageComponent,
@@ -82,6 +87,7 @@ import { AtRiskPasswordPageService } from "./at-risk-password-page.service";
   ],
   selector: "vault-at-risk-passwords",
   templateUrl: "./at-risk-passwords.component.html",
+  changeDetection: ChangeDetectionStrategy.OnPush,
 })
 export class AtRiskPasswordsComponent implements OnInit {
   private taskService = inject(TaskService);
@@ -158,6 +164,8 @@ export class AtRiskPasswordsComponent implements OnInit {
             t.type === SecurityTaskType.UpdateAtRiskCredential &&
             t.cipherId != null &&
             ciphers[t.cipherId] != null &&
+            ciphers[t.cipherId].edit &&
+            ciphers[t.cipherId].viewPassword &&
             !ciphers[t.cipherId].isDeleted,
         )
         .map((t) => ciphers[t.cipherId!]),
diff --git a/libs/vault/src/services/at-risk-password-callout.service.spec.ts b/libs/vault/src/services/at-risk-password-callout.service.spec.ts
index 47b83f4a903..5b687970c4b 100644
--- a/libs/vault/src/services/at-risk-password-callout.service.spec.ts
+++ b/libs/vault/src/services/at-risk-password-callout.service.spec.ts
@@ -28,6 +28,8 @@ class MockCipherView {
   constructor(
     public id: string,
     private deleted: boolean,
+    public edit: boolean = true,
+    public viewPassword: boolean = true,
   ) {}
   get isDeleted() {
     return this.deleted;
@@ -65,33 +67,261 @@ describe("AtRiskPasswordCalloutService", () => {
     service = TestBed.inject(AtRiskPasswordCalloutService);
   });
 
+  describe("pendingTasks$", () => {
+    it.each([
+      {
+        description:
+          "returns tasks filtered by UpdateAtRiskCredential type with valid cipher permissions",
+        tasks: [
+          {
+            id: "t1",
+            cipherId: "c1",
+            type: SecurityTaskType.UpdateAtRiskCredential,
+            status: SecurityTaskStatus.Pending,
+          } as SecurityTask,
+          {
+            id: "t2",
+            cipherId: "c2",
+            type: SecurityTaskType.UpdateAtRiskCredential,
+            status: SecurityTaskStatus.Pending,
+          } as SecurityTask,
+        ],
+        ciphers: [
+          new MockCipherView("c1", false, true, true),
+          new MockCipherView("c2", false, true, true),
+        ],
+        expectedLength: 2,
+        expectedFirstId: "t1",
+      },
+      {
+        description: "filters out tasks with wrong task type",
+        tasks: [
+          {
+            id: "t1",
+            cipherId: "c1",
+            type: SecurityTaskType.UpdateAtRiskCredential,
+            status: SecurityTaskStatus.Pending,
+          } as SecurityTask,
+          {
+            id: "t2",
+            cipherId: "c2",
+            type: 999 as SecurityTaskType,
+            status: SecurityTaskStatus.Pending,
+          } as SecurityTask,
+        ],
+        ciphers: [
+          new MockCipherView("c1", false, true, true),
+          new MockCipherView("c2", false, true, true),
+        ],
+        expectedLength: 1,
+        expectedFirstId: "t1",
+      },
+      {
+        description: "filters out tasks with missing associated cipher",
+        tasks: [
+          {
+            id: "t1",
+            cipherId: "c1",
+            type: SecurityTaskType.UpdateAtRiskCredential,
+            status: SecurityTaskStatus.Pending,
+          } as SecurityTask,
+          {
+            id: "t2",
+            cipherId: "c-nonexistent",
+            type: SecurityTaskType.UpdateAtRiskCredential,
+            status: SecurityTaskStatus.Pending,
+          } as SecurityTask,
+        ],
+        ciphers: [new MockCipherView("c1", false, true, true)],
+        expectedLength: 1,
+        expectedFirstId: "t1",
+      },
+      {
+        description: "filters out tasks when cipher edit permission is false",
+        tasks: [
+          {
+            id: "t1",
+            cipherId: "c1",
+            type: SecurityTaskType.UpdateAtRiskCredential,
+            status: SecurityTaskStatus.Pending,
+          } as SecurityTask,
+          {
+            id: "t2",
+            cipherId: "c2",
+            type: SecurityTaskType.UpdateAtRiskCredential,
+            status: SecurityTaskStatus.Pending,
+          } as SecurityTask,
+        ],
+        ciphers: [
+          new MockCipherView("c1", false, true, true),
+          new MockCipherView("c2", false, false, true),
+        ],
+        expectedLength: 1,
+        expectedFirstId: "t1",
+      },
+      {
+        description: "filters out tasks when cipher viewPassword permission is false",
+        tasks: [
+          {
+            id: "t1",
+            cipherId: "c1",
+            type: SecurityTaskType.UpdateAtRiskCredential,
+            status: SecurityTaskStatus.Pending,
+          } as SecurityTask,
+          {
+            id: "t2",
+            cipherId: "c2",
+            type: SecurityTaskType.UpdateAtRiskCredential,
+            status: SecurityTaskStatus.Pending,
+          } as SecurityTask,
+        ],
+        ciphers: [
+          new MockCipherView("c1", false, true, true),
+          new MockCipherView("c2", false, true, false),
+        ],
+        expectedLength: 1,
+        expectedFirstId: "t1",
+      },
+      {
+        description: "filters out tasks when cipher is deleted",
+        tasks: [
+          {
+            id: "t1",
+            cipherId: "c1",
+            type: SecurityTaskType.UpdateAtRiskCredential,
+            status: SecurityTaskStatus.Pending,
+          } as SecurityTask,
+          {
+            id: "t2",
+            cipherId: "c2",
+            type: SecurityTaskType.UpdateAtRiskCredential,
+            status: SecurityTaskStatus.Pending,
+          } as SecurityTask,
+        ],
+        ciphers: [
+          new MockCipherView("c1", false, true, true),
+          new MockCipherView("c2", true, true, true),
+        ],
+        expectedLength: 1,
+        expectedFirstId: "t1",
+      },
+    ])("$description", async ({ tasks, ciphers, expectedLength, expectedFirstId }) => {
+      jest.spyOn(mockTaskService, "pendingTasks$").mockReturnValue(of(tasks));
+      jest.spyOn(mockCipherService, "cipherViews$").mockReturnValue(of(ciphers));
+
+      const result = await firstValueFrom(service.pendingTasks$(userId));
+
+      expect(result).toHaveLength(expectedLength);
+      if (expectedFirstId) {
+        expect(result[0].id).toBe(expectedFirstId);
+      }
+    });
+
+    it("correctly filters mixed valid and invalid tasks", async () => {
+      const tasks: SecurityTask[] = [
+        {
+          id: "t1",
+          cipherId: "c1",
+          type: SecurityTaskType.UpdateAtRiskCredential,
+          status: SecurityTaskStatus.Pending,
+        } as SecurityTask,
+        {
+          id: "t2",
+          cipherId: "c2",
+          type: SecurityTaskType.UpdateAtRiskCredential,
+          status: SecurityTaskStatus.Pending,
+        } as SecurityTask,
+        {
+          id: "t3",
+          cipherId: "c3",
+          type: SecurityTaskType.UpdateAtRiskCredential,
+          status: SecurityTaskStatus.Pending,
+        } as SecurityTask,
+        {
+          id: "t4",
+          cipherId: "c4",
+          type: SecurityTaskType.UpdateAtRiskCredential,
+          status: SecurityTaskStatus.Pending,
+        } as SecurityTask,
+        {
+          id: "t5",
+          cipherId: "c5",
+          type: SecurityTaskType.UpdateAtRiskCredential,
+          status: SecurityTaskStatus.Pending,
+        } as SecurityTask,
+      ];
+      const ciphers = [
+        new MockCipherView("c1", false, true, true), // valid
+        new MockCipherView("c2", false, false, true), // no edit
+        new MockCipherView("c3", true, true, true), // deleted
+        new MockCipherView("c4", false, true, false), // no viewPassword
+        // c5 missing
+      ];
+
+      jest.spyOn(mockTaskService, "pendingTasks$").mockReturnValue(of(tasks));
+      jest.spyOn(mockCipherService, "cipherViews$").mockReturnValue(of(ciphers));
+
+      const result = await firstValueFrom(service.pendingTasks$(userId));
+
+      expect(result).toHaveLength(1);
+      expect(result[0].id).toBe("t1");
+    });
+
+    it.each([
+      {
+        description: "returns empty array when no tasks match filter criteria",
+        tasks: [
+          {
+            id: "t1",
+            cipherId: "c1",
+            type: SecurityTaskType.UpdateAtRiskCredential,
+            status: SecurityTaskStatus.Pending,
+          } as SecurityTask,
+        ],
+        ciphers: [new MockCipherView("c1", true, true, true)], // deleted
+      },
+      {
+        description: "returns empty array when no pending tasks exist",
+        tasks: [],
+        ciphers: [new MockCipherView("c1", false, true, true)],
+      },
+    ])("$description", async ({ tasks, ciphers }) => {
+      jest.spyOn(mockTaskService, "pendingTasks$").mockReturnValue(of(tasks));
+      jest.spyOn(mockCipherService, "cipherViews$").mockReturnValue(of(ciphers));
+
+      const result = await firstValueFrom(service.pendingTasks$(userId));
+
+      expect(result).toHaveLength(0);
+    });
+  });
+
   describe("completedTasks$", () => {
-    it(" should return true if completed tasks exist", async () => {
+    it("returns true if completed tasks exist", async () => {
       const tasks: SecurityTask[] = [
         {
           id: "t1",
           cipherId: "c1",
           type: SecurityTaskType.UpdateAtRiskCredential,
           status: SecurityTaskStatus.Completed,
-        } as any,
+        } as SecurityTask,
         {
           id: "t2",
           cipherId: "c2",
           type: SecurityTaskType.UpdateAtRiskCredential,
           status: SecurityTaskStatus.Pending,
-        } as any,
+        } as SecurityTask,
         {
           id: "t3",
           cipherId: "nope",
           type: SecurityTaskType.UpdateAtRiskCredential,
           status: SecurityTaskStatus.Completed,
-        } as any,
+        } as SecurityTask,
         {
           id: "t4",
           cipherId: "c3",
           type: SecurityTaskType.UpdateAtRiskCredential,
           status: SecurityTaskStatus.Completed,
-        } as any,
+        } as SecurityTask,
       ];
 
       jest.spyOn(mockTaskService, "completedTasks$").mockReturnValue(of(tasks));
@@ -110,7 +340,7 @@ describe("AtRiskPasswordCalloutService", () => {
       jest.spyOn(mockCipherService, "cipherViews$").mockReturnValue(of([]));
     });
 
-    it("should return false if banner has been dismissed", async () => {
+    it("returns false if banner has been dismissed", async () => {
       const state: AtRiskPasswordCalloutData = {
         hasInteractedWithTasks: true,
         tasksBannerDismissed: true,
@@ -123,7 +353,7 @@ describe("AtRiskPasswordCalloutService", () => {
       expect(result).toBe(false);
     });
 
-    it("should return true when has completed tasks, no pending tasks, and banner not dismissed", async () => {
+    it("returns true when has completed tasks, no pending tasks, and banner not dismissed", async () => {
       const completedTasks = [
         {
           id: "t1",
diff --git a/libs/vault/src/services/at-risk-password-callout.service.ts b/libs/vault/src/services/at-risk-password-callout.service.ts
index d3af4f8421e..214a061399e 100644
--- a/libs/vault/src/services/at-risk-password-callout.service.ts
+++ b/libs/vault/src/services/at-risk-password-callout.service.ts
@@ -45,6 +45,8 @@ export class AtRiskPasswordCalloutService {
           return (
             t.type === SecurityTaskType.UpdateAtRiskCredential &&
             associatedCipher &&
+            associatedCipher.edit &&
+            associatedCipher.viewPassword &&
             !associatedCipher.isDeleted
           );
         });