diff --git a/docs/api/safe-storage.md b/docs/api/safe-storage.md index e96c18bbf0..471351c9c1 100644 --- a/docs/api/safe-storage.md +++ b/docs/api/safe-storage.md @@ -18,8 +18,8 @@ The `safeStorage` module has the following methods: Returns `boolean` - Whether encryption is available. -On Linux, returns true if the secret key is -available. On MacOS, returns true if Keychain is available. +On Linux, returns true if the app has emitted the `ready` event and the secret key is available. +On MacOS, returns true if Keychain is available. On Windows, returns true once the app has emitted the `ready` event. ### `safeStorage.encryptString(plainText)` diff --git a/shell/browser/api/electron_api_safe_storage.cc b/shell/browser/api/electron_api_safe_storage.cc index ba1396cf28..d0e3a73bcd 100644 --- a/shell/browser/api/electron_api_safe_storage.cc +++ b/shell/browser/api/electron_api_safe_storage.cc @@ -31,12 +31,24 @@ void SetElectronCryptoReady(bool ready) { #endif bool IsEncryptionAvailable() { +#if BUILDFLAG(IS_LINUX) + // Calling IsEncryptionAvailable() before the app is ready results in a crash + // on Linux. + // Refs: https://github.com/electron/electron/issues/32206. + if (!Browser::Get()->is_ready()) + return false; +#endif return OSCrypt::IsEncryptionAvailable(); } v8::Local EncryptString(v8::Isolate* isolate, const std::string& plaintext) { - if (!OSCrypt::IsEncryptionAvailable()) { + if (!IsEncryptionAvailable()) { + if (!Browser::Get()->is_ready()) { + gin_helper::ErrorThrower(isolate).ThrowError( + "safeStorage cannot be used before app is ready"); + return v8::Local(); + } gin_helper::ErrorThrower(isolate).ThrowError( "Error while decrypting the ciphertext provided to " "safeStorage.decryptString. " @@ -59,7 +71,12 @@ v8::Local EncryptString(v8::Isolate* isolate, } std::string DecryptString(v8::Isolate* isolate, v8::Local buffer) { - if (!OSCrypt::IsEncryptionAvailable()) { + if (!IsEncryptionAvailable()) { + if (!Browser::Get()->is_ready()) { + gin_helper::ErrorThrower(isolate).ThrowError( + "safeStorage cannot be used before app is ready"); + return ""; + } gin_helper::ErrorThrower(isolate).ThrowError( "Error while decrypting the ciphertext provided to " "safeStorage.decryptString. " diff --git a/spec-main/api-safe-storage-spec.ts b/spec-main/api-safe-storage-spec.ts index b89f909513..098095e2fd 100644 --- a/spec-main/api-safe-storage-spec.ts +++ b/spec-main/api-safe-storage-spec.ts @@ -12,8 +12,27 @@ import * as fs from 'fs'; * * Because all encryption methods are gated by isEncryptionAvailable, the methods will never return the correct values * when run on CI and linux. +* Refs: https://github.com/electron/electron/issues/30424. */ +describe('safeStorage module', () => { + it('safeStorage before and after app is ready', async () => { + const appPath = path.join(__dirname, 'fixtures', 'crash-cases', 'safe-storage'); + const appProcess = cp.spawn(process.execPath, [appPath]); + + let output = ''; + appProcess.stdout.on('data', data => { output += data; }); + appProcess.stderr.on('data', data => { output += data; }); + + const code = (await emittedOnce(appProcess, 'exit'))[0] ?? 1; + + if (code !== 0 && output) { + console.log(output); + } + expect(code).to.equal(0); + }); +}); + ifdescribe(process.platform !== 'linux')('safeStorage module', () => { after(async () => { const pathToEncryptedString = path.resolve(__dirname, 'fixtures', 'api', 'safe-storage', 'encrypted.txt'); diff --git a/spec-main/fixtures/crash-cases/safe-storage/index.js b/spec-main/fixtures/crash-cases/safe-storage/index.js new file mode 100644 index 0000000000..151751820a --- /dev/null +++ b/spec-main/fixtures/crash-cases/safe-storage/index.js @@ -0,0 +1,39 @@ +const { app, safeStorage } = require('electron'); +const { expect } = require('chai'); + +(async () => { + if (!app.isReady()) { + // isEncryptionAvailable() returns false before the app is ready on + // Linux: https://github.com/electron/electron/issues/32206 + // and + // Windows: https://github.com/electron/electron/issues/33640. + expect(safeStorage.isEncryptionAvailable()).to.equal(process.platform === 'darwin'); + if (safeStorage.isEncryptionAvailable()) { + const plaintext = 'plaintext'; + const ciphertext = safeStorage.encryptString(plaintext); + expect(Buffer.isBuffer(ciphertext)).to.equal(true); + expect(safeStorage.decryptString(ciphertext)).to.equal(plaintext); + } else { + expect(() => safeStorage.encryptString('plaintext')).to.throw(/safeStorage cannot be used before app is ready/); + expect(() => safeStorage.decryptString(Buffer.from(''))).to.throw(/safeStorage cannot be used before app is ready/); + } + } + await app.whenReady(); + // isEncryptionAvailable() will always return false on CI due to a mocked + // dbus as mentioned above. + expect(safeStorage.isEncryptionAvailable()).to.equal(process.platform !== 'linux'); + if (safeStorage.isEncryptionAvailable()) { + const plaintext = 'plaintext'; + const ciphertext = safeStorage.encryptString(plaintext); + expect(Buffer.isBuffer(ciphertext)).to.equal(true); + expect(safeStorage.decryptString(ciphertext)).to.equal(plaintext); + } else { + expect(() => safeStorage.encryptString('plaintext')).to.throw(/Encryption is not available/); + expect(() => safeStorage.decryptString(Buffer.from(''))).to.throw(/Decryption is not available/); + } +})() + .then(app.quit) + .catch((err) => { + console.error(err); + app.exit(1); + });