Repository: electron/electron
Pull request: fix: lazily initialize safeStorage async encryptor

#### Description of Change

The `SafeStorage` constructor previously registered a browser observer that called `os_crypt_async()->GetInstance()` on app-ready. Because ESM named imports (`import { x } from 'electron'`) eagerly evaluate all electron module getters, simply importing electron in an ESM entrypoint would construct `SafeStorage` and touch the OS keychain on app-ready — even when `safeStorage` was never used.

This showed up as a macOS CI hang: the `esm-spec` `import-meta` fixture triggers a keychain access prompt that blocks the test runner until timeout, manifesting as flaky `macos-x64 / test (darwin, 2)` failures (~50% rate, ~14 occurrences over the last ~50 commits). Diagnosed via timeout screenshot artifacts showing the "Electron Safe Storage" keychain dialog.

**Trigger chain:**
1. `spec/fixtures/esm/import-meta/main.mjs` does `import { app, BrowserWindow } from 'electron'`
2. Node's ESM loader eagerly evaluates all electron lazy getters to build the namespace, including `safeStorage`
3. This calls `_linkedBinding('electron_browser_safe_storage')` → `SafeStorage()` constructor → `AddObserver(this)`
4. App reaches ready → `OnFinishLaunching` → `os_crypt_async()->GetInstance()` → `KeychainKeyProvider::GetKey()` → keychain prompt → hang

**Fix:** The async encryptor is now requested lazily on the first call to `encryptStringAsync`, `decryptStringAsync`, or `isAsyncEncryptionAvailable`. The constructor is now a no-op.

`isAsyncEncryptionAvailable()` now returns a `Promise<boolean>` that resolves once initialization completes, matching what the docs already stated (the previous implementation returned a sync `bool`, contradicting the docs).

Regression from #49054.

#### Checklist

- [x] PR description included
- [x] `npm test` passes
- [x] Relevant documentation updated
- [x] PR title follows semantic commit guidelines

#### Release Notes

Notes: Fixed an issue where importing `electron` via ESM would touch the OS keychain on app-ready even when `safeStorage` was never used. `safeStorage.isAsyncEncryptionAvailable()` now returns a Promise as documented.

Changed files:
filename: docs/api/safe-storage.md
patch: @@ -59,7 +59,12 @@ On Windows, returns true once the app has emitted the `ready` event.
 
 ### `safeStorage.isAsyncEncryptionAvailable()`
 
-Returns `Promise<Boolean>` - Whether encryption is available for asynchronous safeStorage operations.
+Returns `Promise<boolean>` - Resolves with whether encryption is available for
+asynchronous safeStorage operations.
+
+The asynchronous encryptor is initialized lazily the first time this method,
+`encryptStringAsync`, or `decryptStringAsync` is called after the app is ready.
+The returned promise resolves once initialization completes.
 
 ### `safeStorage.encryptString(plainText)`
 

filename: shell/browser/api/electron_api_safe_storage.cc
patch: @@ -54,17 +54,9 @@ gin_helper::Handle<SafeStorage> SafeStorage::Create(v8::Isolate* isolate) {
   return gin_helper::CreateHandle(isolate, new SafeStorage(isolate));
 }
 
-SafeStorage::SafeStorage(v8::Isolate* isolate) {
-  if (electron::Browser::Get()->is_ready()) {
-    OnFinishLaunching({});
-  } else {
-    Browser::Get()->AddObserver(this);
-  }
-}
+SafeStorage::SafeStorage(v8::Isolate* isolate) {}
 
-SafeStorage::~SafeStorage() {
-  Browser::Get()->RemoveObserver(this);
-}
+SafeStorage::~SafeStorage() = default;
 
 gin::ObjectTemplateBuilder SafeStorage::GetObjectTemplateBuilder(
     v8::Isolate* isolate) {
@@ -85,7 +77,11 @@ gin::ObjectTemplateBuilder SafeStorage::GetObjectTemplateBuilder(
       ;
 }
 
-void SafeStorage::OnFinishLaunching(base::DictValue launch_info) {
+void SafeStorage::EnsureAsyncEncryptorRequested() {
+  DCHECK(electron::Browser::Get()->is_ready());
+  if (encryptor_requested_)
+    return;
+  encryptor_requested_ = true;
   g_browser_process->os_crypt_async()->GetInstance(
       base::BindOnce(&SafeStorage::OnOsCryptReady, base::Unretained(this)),
       os_crypt_async::Encryptor::Option::kEncryptSyncCompat);
@@ -95,13 +91,21 @@ void SafeStorage::OnOsCryptReady(os_crypt_async::Encryptor encryptor) {
   encryptor_ = std::move(encryptor);
   is_available_ = true;
 
+  // This callback may fire from a posted task without an active V8 scope.
+  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
+  v8::HandleScope handle_scope(isolate);
+
+  for (auto& pending : pending_availability_checks_) {
+    pending.Resolve(true);
+  }
+  pending_availability_checks_.clear();
+
   for (auto& pending : pending_encrypts_) {
     std::string ciphertext;
     bool encrypted = encryptor_->EncryptString(pending.plaintext, &ciphertext);
     if (encrypted) {
       pending.promise.Resolve(
-          electron::Buffer::Copy(pending.promise.isolate(), ciphertext)
-              .ToLocalChecked());
+          electron::Buffer::Copy(isolate, ciphertext).ToLocalChecked());
     } else {
       pending.promise.RejectWithErrorMessage(
           "Error while encrypting the text provided to "
@@ -117,8 +121,6 @@ void SafeStorage::OnOsCryptReady(os_crypt_async::Encryptor encryptor) {
         encryptor_->DecryptString(pending.ciphertext, &plaintext, &flags);
 
     if (decrypted) {
-      v8::Isolate* isolate = pending.promise.isolate();
-      v8::HandleScope handle_scope(isolate);
       auto dict = gin_helper::Dictionary::CreateEmpty(isolate);
 
       dict.Set("shouldReEncrypt", flags.should_reencrypt);
@@ -155,16 +157,33 @@ bool SafeStorage::IsEncryptionAvailable() {
 #endif
 }
 
-bool SafeStorage::IsAsyncEncryptionAvailable() {
-  if (!electron::Browser::Get()->is_ready())
-    return false;
+v8::Local<v8::Promise> SafeStorage::IsAsyncEncryptionAvailable(
+    v8::Isolate* isolate) {
+  gin_helper::Promise<bool> promise(isolate);
+  v8::Local<v8::Promise> handle = promise.GetHandle();
+
+  if (!electron::Browser::Get()->is_ready()) {
+    promise.Resolve(false);
+    return handle;
+  }
+
 #if BUILDFLAG(IS_LINUX)
-  return is_available_ || (use_password_v10_ &&
-                           static_cast<BrowserProcessImpl*>(g_browser_process)
-                                   ->linux_storage_backend() == "basic_text");
-#else
-  return is_available_;
+  if (use_password_v10_ && static_cast<BrowserProcessImpl*>(g_browser_process)
+                                   ->linux_storage_backend() == "basic_text") {
+    promise.Resolve(true);
+    return handle;
+  }
 #endif
+
+  EnsureAsyncEncryptorRequested();
+
+  if (is_available_) {
+    promise.Resolve(true);
+    return handle;
+  }
+
+  pending_availability_checks_.push_back(std::move(promise));
+  return handle;
 }
 
 void SafeStorage::SetUsePasswordV10(bool use) {
@@ -270,6 +289,8 @@ v8::Local<v8::Promise> SafeStorage::encryptStringAsync(
     return handle;
   }
 
+  EnsureAsyncEncryptorRequested();
+
   if (is_available_) {
     std::string ciphertext;
     bool encrypted = encryptor_->EncryptString(plaintext, &ciphertext);
@@ -318,6 +339,8 @@ v8::Local<v8::Promise> SafeStorage::decryptStringAsync(
     return handle;
   }
 
+  EnsureAsyncEncryptorRequested();
+
   if (is_available_) {
     std::string plaintext;
     os_crypt_async::Encryptor::DecryptFlags flags;

filename: shell/browser/api/electron_api_safe_storage.h
patch: @@ -10,7 +10,6 @@
 
 #include "build/build_config.h"
 #include "components/os_crypt/async/common/encryptor.h"
-#include "shell/browser/browser_observer.h"
 #include "shell/browser/event_emitter_mixin.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/promise.h"
@@ -37,8 +36,7 @@ class Handle;
 namespace electron::api {
 
 class SafeStorage final : public gin_helper::DeprecatedWrappable<SafeStorage>,
-                          public gin_helper::EventEmitterMixin<SafeStorage>,
-                          private BrowserObserver {
+                          public gin_helper::EventEmitterMixin<SafeStorage> {
  public:
   static gin_helper::Handle<SafeStorage> Create(v8::Isolate* isolate);
 
@@ -57,14 +55,16 @@ class SafeStorage final : public gin_helper::DeprecatedWrappable<SafeStorage>,
   ~SafeStorage() override;
 
  private:
-  // BrowserObserver:
-  void OnFinishLaunching(base::DictValue launch_info) override;
+  // Lazily request the async encryptor on first use. ESM named imports
+  // eagerly evaluate all electron module getters, so requesting in the
+  // constructor would touch the OS keychain even when safeStorage is unused.
+  void EnsureAsyncEncryptorRequested();
 
   void OnOsCryptReady(os_crypt_async::Encryptor encryptor);
 
   bool IsEncryptionAvailable();
 
-  bool IsAsyncEncryptionAvailable();
+  v8::Local<v8::Promise> IsAsyncEncryptionAvailable(v8::Isolate* isolate);
 
   void SetUsePasswordV10(bool use);
 
@@ -85,6 +85,7 @@ class SafeStorage final : public gin_helper::DeprecatedWrappable<SafeStorage>,
 
   bool use_password_v10_ = false;
 
+  bool encryptor_requested_ = false;
   bool is_available_ = false;
 
   std::optional<os_crypt_async::Encryptor> encryptor_;
@@ -114,6 +115,8 @@ class SafeStorage final : public gin_helper::DeprecatedWrappable<SafeStorage>,
     std::string ciphertext;
   };
   std::vector<PendingDecrypt> pending_decrypts_;
+
+  std::vector<gin_helper::Promise<bool>> pending_availability_checks_;
 };
 
 }  // namespace electron::api

filename: spec/api-safe-storage-spec.ts
patch: @@ -81,8 +81,8 @@ describe('safeStorage module', () => {
   });
 
   describe('SafeStorage.isAsyncEncryptionAvailable()', () => {
-    it('should return true when async encryption is available', () => {
-      expect(safeStorage.isAsyncEncryptionAvailable()).to.equal(true);
+    it('should resolve true when async encryption is available', async () => {
+      expect(await safeStorage.isAsyncEncryptionAvailable()).to.equal(true);
     });
   });