Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PM-5584] Set up a stay alive method to allow service worker in manifest v3 to stay alive indefinitely #8535

Conversation

cagonzalezcs
Copy link
Contributor

Type of change

- [ ] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [x] Other

Objective

This PR introduces code changes that facilitates a persistent service worker lifetime within the manifest v3 version of the extension. This is a temporary hack that we will use until we solidify our approach to handling the ephemeral nature of the background service worker in a more robust manner.

Code changes

  • apps/browser/src/platform/background.ts: Adjusting how we handle initializing the MainBackground class to ensure that we trigger a keep alive process when the extension is running within manifest v3.

@cagonzalezcs cagonzalezcs requested a review from a team as a code owner March 28, 2024 19:06
@github-actions github-actions bot added the needs-qa Marks a PR as requiring QA approval label Mar 28, 2024
Copy link

codecov bot commented Mar 28, 2024

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 26.90%. Comparing base (a201e9c) to head (38e6608).
Report is 1 commits behind head on main.

❗ Current head 38e6608 differs from pull request most recent head 8163622. Consider uploading reports for the commit 8163622 to get more accurate results

Files Patch % Lines
apps/browser/src/platform/background.ts 0.00% 11 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8535   +/-   ##
=======================================
  Coverage   26.89%   26.90%           
=======================================
  Files        2311     2311           
  Lines       67465    67436   -29     
  Branches    12642    12641    -1     
=======================================
- Hits        18148    18142    -6     
+ Misses      47924    47901   -23     
  Partials     1393     1393           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…me-to-allow-extension-to-stay-alive-indefinitely-in-manifest-v3
@cagonzalezcs cagonzalezcs requested review from MGibson1 and justindbaur and removed request for coroiu March 28, 2024 19:21
Copy link
Contributor

github-actions bot commented Mar 28, 2024

Logo
Checkmarx One – Scan Summary & Details841de4af-395b-4a49-b419-3f40de2217e6

New Issues

Severity Issue Source File / Package Checkmarx Insight
HIGH Client_DOM_XSS /apps/web/src/app/auth/settings/two-factor-verify.component.html: 3 Attack Vector
HIGH Client_DOM_XSS /bitwarden_license/bit-web/src/app/admin-console/providers/settings/account.component.html: 27 Attack Vector
HIGH Client_DOM_XSS /bitwarden_license/bit-web/src/app/admin-console/providers/settings/account.component.html: 27 Attack Vector
MEDIUM Client_Potential_XSS /apps/desktop/src/app/components/avatar.component.ts: 45 Attack Vector
MEDIUM Client_Potential_XSS /libs/components/src/avatar/avatar.component.ts: 48 Attack Vector
MEDIUM Client_Potential_XSS /apps/desktop/src/app/components/avatar.component.ts: 45 Attack Vector
LOW Use_Of_Hardcoded_Password /libs/common/src/tools/send/services/send.service.ts: 26 Attack Vector
LOW Use_Of_Hardcoded_Password /libs/common/src/tools/send/services/send.service.ts: 25 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/tools/export.command.ts: 119 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/tools/export.command.ts: 117 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/tools/export.command.ts: 118 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/tools/export.command.ts: 119 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/tools/export.command.ts: 118 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/tools/export.command.ts: 117 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 289 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 147 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 147 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 148 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 148 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 641 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 657 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 666 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 146 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 667 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 669 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 125 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 146 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 148 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 147 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 643 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 147 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 146 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 123 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 146 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 147 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 100 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 148 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 287 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 124 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 642 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 288 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 148 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 146 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 100 Attack Vector
LOW Use_Of_Hardcoded_Password /libs/auth/src/angular/user-verification/user-verification-form-input.component.ts: 71 Attack Vector
LOW Use_Of_Hardcoded_Password /libs/auth/src/angular/user-verification/user-verification-form-input.component.ts: 71 Attack Vector
LOW Use_Of_Hardcoded_Password /libs/auth/src/angular/user-verification/user-verification-form-input.component.ts: 71 Attack Vector
LOW Use_Of_Hardcoded_Password /libs/auth/src/angular/user-verification/user-verification-form-input.component.ts: 71 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 456 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 538 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 418 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 456 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 555 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 555 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 538 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 538 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 555 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 555 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 418 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 538 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 456 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 555 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 456 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 555 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 418 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 538 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 456 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 456 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 538 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 555 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 418 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 418 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 418 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 538 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 456 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 418 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 555 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/auth/commands/login.command.ts: 513 Attack Vector
LOW Use_Of_Hardcoded_Password

More results are available on AST platform

@willmartian
Copy link
Contributor

willmartian commented Mar 29, 2024

This is a temporary hack

Why is this the case? Does this approach have some limitation / unsupported edge case? Do we expect it to be patched?

@cagonzalezcs
Copy link
Contributor Author

cagonzalezcs commented Mar 29, 2024

This is a temporary hack

Why is this the case? Does this approach have some limitation / unsupported edge case? Do we expect it to be patched?

This is a supported work around to concerns that the extension development community has had regarding service worker lifetimes, and issues with having a non-persistent background script.

This is a temporary solution to a larger architectural issue that the extension is experiencing. To facilitate accommodation of a non-persistent runtime environment, we need to refactor the instantiation of background services to function through a dependency injection container. We also need to rework all browser extension event listeners to register at the top level of the background script execution context.

These code changes are a temporary way to ensure we can have a functioning extension in manifest v3 before those larger architectural changes are implemented. Also, it's possible that we will use this persistence "hack" in certain situations, even with the refactor. The scenarios that come to mind revolve around import/export of vault data, among other unknown use cases.

…me-to-allow-extension-to-stay-alive-indefinitely-in-manifest-v3
…me-to-allow-extension-to-stay-alive-indefinitely-in-manifest-v3
…me-to-allow-extension-to-stay-alive-indefinitely-in-manifest-v3
@cagonzalezcs cagonzalezcs removed the needs-qa Marks a PR as requiring QA approval label Apr 2, 2024
@cagonzalezcs cagonzalezcs enabled auto-merge (squash) April 2, 2024 15:21
…me-to-allow-extension-to-stay-alive-indefinitely-in-manifest-v3
@cagonzalezcs cagonzalezcs merged commit b9771c1 into main Apr 2, 2024
18 checks passed
@cagonzalezcs cagonzalezcs deleted the autofill/pm-5584-set-up-persistence-for-chrome-to-allow-extension-to-stay-alive-indefinitely-in-manifest-v3 branch April 2, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants