clients
clients copied to clipboard
Published
20 hours ago •
bitwarden
bitwarden
[PM-7747] add timeout to safari sendMessageWithResponse
Type of change
- [x] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
Objective
chrome.runtime.sendMessage on safari doesn't have a timeout for a response. We are using the lack of a response to determine whether the popup is open or not.
Code changes
- file.ext: Description of what was changed and why
Screenshots
Before you submit
- Please add unit tests where it makes sense to do so (encouraged but not required)
- If this change requires a documentation update - notify the documentation team
- If this change has particular deployment requirements - notify the DevOps team
- Ensure that all UI additions follow WCAG AA requirements
Codecov Report
Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.
Project coverage is 27.63%. Comparing base (
5682e38) to head (68efc93). Report is 3 commits behind head on main.
| Files | Patch % | Lines |
|---|---|---|
| ...s/platform-utils/browser-platform-utils.service.ts | 0.00% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #9082 +/- ##
==========================================
- Coverage 27.63% 27.63% -0.01%
==========================================
Files 2416 2417 +1
Lines 69875 69881 +6
Branches 13012 13013 +1
==========================================
Hits 19309 19309
- Misses 49067 49073 +6
Partials 1499 1499
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Hm... if the issue described in this PR is actually the case, we likely need to think about this more exhaustively. This would present as a foundational issue in Safari that needs to be brought into consideration for other areas in the extension that depend on a sendResponse resolving...
Checkmarx One – Scan Summary & Details – 1daa0f5b-0eca-410e-9592-4c3caab97566
Fixed Issues
| Severity | Issue | Source File / Package |
|---|---|---|
 |
Client_DOM_Code_Injection | /apps/web/src/connectors/common.ts: 2 |
|
Client_DOM_Code_Injection | /apps/browser/src/autofill/services/collect-autofill-content.service.ts: 1054 |
|
Client_DOM_Stored_XSS | /apps/web/src/connectors/sso.ts: 33 |
|
Client_DOM_XSS | /apps/browser/src/auth/scripts/duo.js: 285 |
|
Client_DOM_XSS | /apps/browser/src/auth/scripts/duo.js: 285 |
|
Client_DOM_XSS | /apps/desktop/src/auth/scripts/duo.js: 285 |
|
Client_DOM_XSS | /apps/desktop/src/auth/scripts/duo.js: 285 |
|
Client_DOM_XSS | /apps/web/src/connectors/common.ts: 2 |
|
Client_DOM_XSS | /apps/web/src/connectors/common.ts: 2 |
|
Client_DOM_XSS | /apps/web/src/connectors/common.ts: 2 |
|
Client_DOM_XSS | /apps/web/src/connectors/common.ts: 2 |
|
Client_DOM_XSS | /apps/web/src/connectors/sso.ts: 21 |
|
Client_DOM_XSS | /apps/web/src/connectors/sso.ts: 19 |
|
Client_DOM_XSS | /apps/web/src/connectors/sso.ts: 15 |
 |
Absolute_Path_Traversal | /apps/cli/src/commands/serve.command.ts: 312 |
|
Absolute_Path_Traversal | /apps/cli/src/commands/serve.command.ts: 344 |
|
Absolute_Path_Traversal | /apps/cli/src/commands/serve.command.ts: 312 |
|
Absolute_Path_Traversal | /apps/cli/src/commands/serve.command.ts: 344 |
|
Angular_Improper_Type_Pipe_Usage | /apps/browser/src/vault/popup/components/fido2/fido2-use-browser-link.component.html: 1 |
|
Angular_Improper_Type_Pipe_Usage | /apps/web/src/app/billing/organizations/adjust-subscription.component.html: 54 |
|
Angular_Improper_Type_Pipe_Usage | /apps/web/src/app/billing/organizations/adjust-subscription.component.html: 18 |
|
Client_Privacy_Violation | /apps/browser/src/background/runtime.background.ts: 308 |
|
Client_Privacy_Violation | /apps/web/src/app/tools/reports/pages/breach-report.component.html: 14 |
|
Client_Privacy_Violation | /apps/browser/src/auth/popup/account-switching/account.component.ts: 12 |
|
Client_Privacy_Violation | /apps/browser/src/auth/popup/account-switching/account.component.ts: 12 |
|
Client_Privacy_Violation | /apps/browser/src/auth/popup/account-switching/account.component.ts: 12 |
|
Client_Privacy_Violation | /libs/components/src/color-password/color-password.component.ts: 25 |
|
Client_Privacy_Violation | /libs/components/src/color-password/color-password.component.ts: 26 |
|
Client_Privacy_Violation | /apps/desktop/src/auth/lock.component.html: 32 |
|
Client_Privacy_Violation | /apps/web/src/app/auth/lock.component.html: 18 |
|
Client_Privacy_Violation | /apps/web/src/app/billing/shared/add-credit.component.ts: 70 |
|
Client_Privacy_Violation | /apps/web/src/app/billing/shared/add-credit.component.ts: 30 |
|
Client_Privacy_Violation | /apps/web/src/app/billing/shared/add-credit.component.ts: 135 |
|
Client_Privacy_Violation | /apps/web/src/app/billing/shared/add-credit.component.ts: 146 |
|
Client_Privacy_Violation | /apps/web/src/app/billing/shared/add-credit.component.ts: 80 |
|
Client_Privacy_Violation | /apps/web/src/app/auth/lock.component.html: 18 |
|
Client_Privacy_Violation | /apps/desktop/src/auth/lock.component.html: 32 |
|
Client_Privacy_Violation | /apps/web/src/app/auth/recover-two-factor.component.html: 37 |
|
Client_Privacy_Violation | /apps/web/src/app/billing/shared/add-credit.component.html: 46 |
|
Client_Privacy_Violation | /apps/desktop/src/vault/app/vault/view.component.html: 534 |
|
Client_Privacy_Violation | /apps/web/src/connectors/webauthn-fallback.ts: 116 |
|
Client_Privacy_Violation | /bitwarden_license/bit-web/src/app/auth/sso/sso.component.ts: 161 |
|
Client_Privacy_Violation | /bitwarden_license/bit-web/src/app/auth/sso/sso.component.ts: 161 |
|
Client_Privacy_Violation | /libs/components/src/color-password/color-password.component.ts: 14 |
|
Client_Privacy_Violation | /apps/desktop/src/vault/app/vault/view.component.html: 60 |
|
Client_Privacy_Violation | /apps/desktop/src/vault/app/vault/view.component.html: 56 |
|
Client_Privacy_Violation | /apps/browser/src/tools/popup/generator/password-generator-history.component.html: 26 |
|
Client_Privacy_Violation | /apps/browser/src/vault/popup/components/vault/password-history.component.html: 18 |
|
Client_Privacy_Violation | /apps/desktop/src/app/tools/password-generator-history.component.html: 15 |
|
Client_Privacy_Violation | /apps/desktop/src/vault/app/vault/password-history.component.html: 12 |
|
Client_Privacy_Violation | /apps/desktop/src/vault/app/vault/view.component.html: 50 |
|
Client_Privacy_Violation | /libs/components/src/color-password/color-password.component.ts: 14 |
|
Client_Privacy_Violation | /apps/browser/src/tools/popup/generator/password-generator-history.component.html: 26 |
|
Client_Privacy_Violation | /apps/browser/src/vault/popup/components/vault/password-history.component.html: 18 |
|
Client_Privacy_Violation | /apps/desktop/src/app/tools/password-generator-history.component.html: 15 |
|
Client_Privacy_Violation | /apps/desktop/src/vault/app/vault/password-history.component.html: 12 |
|
Missing_HSTS_Header | /apps/cli/src/auth/commands/login.command.ts: 705 |
|
SSRF | /libs/importer/src/importers/lastpass/access/services/rest-client.ts: 69 |
|
SSRF | /libs/importer/src/importers/lastpass/access/services/rest-client.ts: 69 |
 |
Angular_Usage_of_Unsafe_DOM_Sanitizer | /libs/components/src/avatar/avatar.component.ts: 80 |
|
Angular_Usage_of_Unsafe_DOM_Sanitizer | /apps/desktop/src/app/components/avatar.component.ts: 75 |
|
Angular_Usage_of_Unsafe_DOM_Sanitizer | /libs/components/src/icon/icon.component.ts: 18 |
|
Angular_Usage_of_Unsafe_DOM_Sanitizer | /libs/components/src/icon/icon.component.ts: 18 |
|
Client_DOM_Open_Redirect | /apps/desktop/src/auth/accessibility-cookie.component.html: 18 |
|
Client_DOM_Open_Redirect | /apps/web/src/connectors/common.ts: 2 |
|
Client_DOM_Open_Redirect | /apps/web/src/connectors/common.ts: 2 |
|
Client_DOM_Open_Redirect | /apps/web/src/connectors/common.ts: 2 |
|
Client_DOM_Open_Redirect | /apps/web/src/connectors/sso.ts: 21 |
|
Client_DOM_Open_Redirect | /apps/web/src/connectors/common.ts: 2 |
|
Client_DOM_Open_Redirect | /apps/web/src/connectors/sso.ts: 19 |
|
Client_DOM_Open_Redirect | /apps/web/src/connectors/common.ts: 2 |
|
Client_DOM_Open_Redirect | /apps/web/src/connectors/sso.ts: 15 |
|
Client_DOM_Open_Redirect | /apps/browser/src/tools/popup/generator/password-generator-history.component.ts: 18 |
|
Client_DOM_Open_Redirect | /apps/browser/src/auth/popup/login-via-auth-request.component.ts: 54 |
|
Client_DOM_Open_Redirect | /apps/browser/src/auth/popup/login-via-auth-request.component.ts: 54 |
|
Client_DOM_Open_Redirect | /apps/desktop/src/auth/login/login-via-auth-request.component.ts: 62 |
|
Client_DOM_Open_Redirect | /apps/desktop/src/auth/login/login-via-auth-request.component.ts: 62 |
|
Client_DOM_Open_Redirect | /apps/browser/src/auth/popup/account-switching/current-account.component.ts: 31 |
|
Client_DOM_Open_Redirect | /apps/browser/src/auth/popup/login-via-auth-request.component.ts: 54 |
|
Client_DOM_Open_Redirect | /apps/browser/src/auth/popup/login-via-auth-request.component.ts: 54 |
|
Client_DOM_Open_Redirect | /apps/desktop/src/auth/login/login-via-auth-request.component.ts: 62 |
|
Client_DOM_Open_Redirect | /apps/desktop/src/auth/login/login-via-auth-request.component.ts: 62 |
|
Client_DOM_Open_Redirect | /apps/browser/src/auth/popup/account-switching/account.component.ts: 25 |
|
Client_DOM_Open_Redirect | /apps/browser/src/vault/popup/components/vault/password-history.component.ts: 21 |
|
Client_DOM_Open_Redirect | /apps/browser/src/billing/popup/settings/premium.component.ts: 27 |
|
Client_DOM_Open_Redirect | /apps/browser/src/vault/popup/components/vault/attachments.component.ts: 32 |
|
Client_DOM_Open_Redirect | /libs/common/src/auth/iframe-component.ts: 49 |
|
Client_DOM_Open_Redirect | /apps/browser/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /apps/browser/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /apps/desktop/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /apps/desktop/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /libs/common/src/auth/webauthn-iframe.ts: 25 |
|
Client_DOM_Open_Redirect | /apps/desktop/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /apps/desktop/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /apps/browser/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /apps/browser/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /apps/desktop/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /apps/desktop/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /apps/browser/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /apps/browser/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /libs/common/src/auth/webauthn-iframe.ts: 25 |
|
Client_DOM_Open_Redirect | /apps/desktop/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /apps/desktop/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /apps/browser/src/auth/scripts/duo.js: 277 |
|
Client_DOM_Open_Redirect | /apps/browser/src/auth/scripts/duo.js: 277 |
|
Client_Hardcoded_Domain | /apps/web/src/app/billing/shared/payment.component.ts: 56 |
|
Client_Hardcoded_Domain | /apps/web/src/app/billing/shared/payment.component.ts: 56 |
|
Client_Hardcoded_Domain | /apps/web/src/connectors/captcha.ts: 57 |
|
Client_Use_Of_Iframe_Without_Sandbox | /apps/browser/src/autofill/content/notification-bar.ts: 868 |
|
Client_Use_Of_Iframe_Without_Sandbox | /apps/browser/src/autofill/overlay/iframe-content/autofill-overlay-iframe.service.ts: 90 |
|
Client_Use_Of_Iframe_Without_Sandbox | /apps/web/src/connectors/duo.ts: 8 |
|
Client_Use_Of_Iframe_Without_Sandbox | /apps/web/src/connectors/duo.ts: 8 |
|
Client_Weak_Cryptographic_Hash | /libs/common/src/platform/services/web-crypto-function.service.ts: 142 |
|
Client_Weak_Cryptographic_Hash | /apps/desktop/src/proxy/ipc.ts: 24 |
|
Missing_CSP_Header | /apps/cli/src/auth/commands/login.command.ts: 705 |
|
Unprotected_Cookie | /apps/web/src/app/auth/two-factor.component.ts: 143 |
|
Unprotected_Cookie | /apps/web/src/connectors/duo-redirect.ts: 57 |
|
Unprotected_Cookie | /apps/web/src/connectors/duo-redirect.ts: 112 |
|
Unprotected_Cookie | /apps/web/src/connectors/sso.ts: 33 |
|
Unprotected_Cookie | /apps/web/src/app/auth/sso.component.ts: 137 |
|
Unsafe_Use_Of_Target_blank | /apps/web/src/app/auth/settings/two-factor-recovery.component.ts: 25 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/auth/background/service-factories/device-trust-service.factory.ts: 82 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/auth/background/service-factories/device-trust-service.factory.ts: 83 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/auth/background/service-factories/auth-request-service.factory.ts: 54 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/auth/background/service-factories/login-strategy-service.factory.ts: 125 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/auth/background/service-factories/pin-crypto-service.factory.ts: 47 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/tools/background/service_factories/import-service.factory.ts: 58 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/platform/background/service-factories/key-generation-service.factory.ts: 23 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/auth/background/service-factories/user-verification-service.factory.ts: 77 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/auth/background/service-factories/auth-service.factory.ts: 51 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/auth/background/service-factories/key-connector-service.factory.ts: 70 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/background/service-factories/send-service.factory.ts: 50 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/background/service-factories/vault-timeout-settings-service.factory.ts: 55 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/platform/background/service-factories/crypto-service.factory.ts: 72 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/vault/background/service_factories/cipher-service.factory.ts: 75 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/vault/background/service_factories/collection-service.factory.ts: 37 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/browser/src/vault/background/service_factories/totp-service.factory.ts: 34 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm | /apps/cli/src/platform/services/node-env-secure-storage.service.ts: 62 |
|
Use_of_Broken_or_Risky_Cryptographic_Algorithm |
More results are available on AST platform
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@cagonzalezcs I can move this solution down a level if so, I was just worried about the hardcoded timeout introducing regressions. Do you know what those other areas are that depend on the message resolving? This should only be a problem when the listener might not exist at the time the message is sent.
@jlf0dev
Yeah that's what I'm thinking we'd want to consider, either moving this into the BrowserApi.sendMessageWithResponse method or dig a bit further and see if we need to change that method in a different way to address Safari's behavior.
Other locations that use this kind of messaging are the inline menu, the notification bar, the vault's add-edit component... but beyond those considerations, this issue presents a problem with "how extensions are supposed to work".
I'm taking a couple of minutes this morning to dig into this a bit... I think it'd be fine to introduce your scoped solution for now though, but we'd want to address this more holistically sooner rather than later.
Switched safari to use the pre-Mv3 version way of detecting our popup, chrome.extension.getViews, after chatting with @cagonzalezcs. This API should stay available on safari after Mv3.