fix: set macOS crypto keychain name earlier (#35795)

* fix: set macOS crypto keychain name earlier

* spec: ensure arm64 mac tests are cleaned up

* fixup for password name difference in 19-x-y

Co-authored-by: Samuel Attard <sattard@salesforce.com>
Co-authored-by: Samuel Attard <samuel.r.attard@gmail.com>
Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>

.circleci/config/base.yml

@@ -216,6 +216,7 @@ step-maybe-cleanup-arm64-mac: &step-maybe-cleanup-arm64-mac
         rm -rf ~/Library/Application\ Support/electron*
         security delete-generic-password -l "Chromium Safe Storage" || echo "✓ Keychain does not contain password from tests"
         security delete-generic-password -l "Electron Test Main Safe Storage" || echo "✓ Keychain does not contain password from tests"
+        security delete-generic-password -a "electron-safe-storage" || echo "✓ Keychain does not contain password from tests"
       elif [ "$TARGET_ARCH" == "arm" ] || [ "$TARGET_ARCH" == "arm64" ]; then
         XVFB=/usr/bin/Xvfb
         /sbin/start-stop-daemon --stop --exec $XVFB || echo "Xvfb not running"

shell/browser/electron_browser_main_parts.cc

@@ -90,6 +90,7 @@
 #endif
 
 #if BUILDFLAG(IS_MAC)
+#include "components/os_crypt/keychain_password_mac.h"
 #include "services/device/public/cpp/geolocation/geolocation_manager.h"
 #include "shell/browser/ui/cocoa/views_delegate_mac.h"
 #else
@@ -481,6 +482,9 @@ void ElectronBrowserMainParts::WillRunMainMessageLoop(
 }
 
 void ElectronBrowserMainParts::PostCreateMainMessageLoop() {
+#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC)
+  std::string app_name = electron::Browser::Get()->GetName();
+#endif
 #if BUILDFLAG(IS_LINUX)
   auto shutdown_cb =
       base::BindOnce(base::RunLoop::QuitCurrentWhenIdleClosureDeprecated());
@@ -491,7 +495,6 @@ void ElectronBrowserMainParts::PostCreateMainMessageLoop() {
   // Set up crypt config. This needs to be done before anything starts the
   // network service, as the raw encryption key needs to be shared with the
   // network service for encrypted cookie storage.
-  std::string app_name = electron::Browser::Get()->GetName();
   const base::CommandLine& command_line =
       *base::CommandLine::ForCurrentProcess();
   std::unique_ptr<os_crypt::Config> config =
@@ -508,6 +511,10 @@ void ElectronBrowserMainParts::PostCreateMainMessageLoop() {
   base::PathService::Get(chrome::DIR_USER_DATA, &config->user_data_path);
   OSCrypt::SetConfig(std::move(config));
 #endif
+#if BUILDFLAG(IS_MAC)
+  KeychainPassword::GetServiceName() = app_name + " Safe Storage";
+  KeychainPassword::GetAccountName() = app_name;
+#endif
 #if BUILDFLAG(IS_POSIX)
   // Exit in response to SIGINT, SIGTERM, etc.
   InstallShutdownSignalHandlers(

shell/browser/net/system_network_context_manager.cc

@@ -42,10 +42,6 @@
 #include "shell/common/options_switches.h"
 #include "url/gurl.h"
 
-#if BUILDFLAG(IS_MAC)
-#include "components/os_crypt/keychain_password_mac.h"
-#endif
-
 #if BUILDFLAG(IS_LINUX)
 #include "components/os_crypt/key_storage_config_linux.h"
 #endif
@@ -288,12 +284,6 @@ void SystemNetworkContextManager::OnNetworkServiceCreated(
       base::FeatureList::IsEnabled(features::kAsyncDns),
       default_secure_dns_mode, doh_config, additional_dns_query_types_enabled);
 
-  std::string app_name = electron::Browser::Get()->GetName();
-#if BUILDFLAG(IS_MAC)
-  KeychainPassword::GetServiceName() = app_name + " Safe Storage";
-  KeychainPassword::GetAccountName() = app_name;
-#endif
-
   // The OSCrypt keys are process bound, so if network service is out of
   // process, send it the required key.
   if (content::IsOutOfProcessNetworkService() &&

spec-main/api-safe-storage-spec.ts

@@ -4,7 +4,7 @@ import { safeStorage } from 'electron/main';
 import { expect } from 'chai';
 import { emittedOnce } from './events-helpers';
 import { ifdescribe } from './spec-helpers';
-import * as fs from 'fs';
+import * as fs from 'fs-extra';
 
 /* isEncryptionAvailable returns false in Linux when running CI due to a mocked dbus. This stops
 * Chrome from reaching the system's keyring or libsecret. When running the tests with config.store
@@ -36,8 +36,8 @@ describe('safeStorage module', () => {
 ifdescribe(process.platform !== 'linux')('safeStorage module', () => {
   after(async () => {
     const pathToEncryptedString = path.resolve(__dirname, 'fixtures', 'api', 'safe-storage', 'encrypted.txt');
-    if (fs.existsSync(pathToEncryptedString)) {
-      await fs.unlinkSync(pathToEncryptedString);
+    if (await fs.pathExists(pathToEncryptedString)) {
+      await fs.remove(pathToEncryptedString);
     }
   });