Skip to content

Replaces keytar with Electron's safeStorage API#2995

Merged
gingi merged 3 commits intomainfrom
shpaster/replace-keytar
Dec 12, 2025
Merged

Replaces keytar with Electron's safeStorage API#2995
gingi merged 3 commits intomainfrom
shpaster/replace-keytar

Conversation

@gingi
Copy link
Member

@gingi gingi commented Dec 1, 2025

This pull request introduces improvements to secure credential storage, including enhanced encryption algorithms, better handling of legacy encrypted data, and user-facing notifications for when secure storage is unavailable. It also adds new tests and refactors the secure storage abstraction for better reliability and maintainability.

  • Refactored CryptoService to support both legacy (AES-256-CTR, 16-byte key) and current (AES-256-GCM, 32-byte key) encryption algorithms, ensuring backward compatibility and improved security. The encryption format now includes a version header for future extensibility.

  • Added a warning banner to the welcome screen when secure credential storage is unavailable, including clear instructions for Linux users to install libsecret.
    image

  • Replaced direct keytar usage with a new KeytarService that delegates to SafeStorageService, making credential storage easier to mock and more robust against platform differences.

  • Added comprehensive unit tests for KeytarService, verifying correct delegation and error handling in credential storage.

@codecov
Copy link

codecov bot commented Dec 1, 2025

Codecov Report

❌ Patch coverage is 76.38889% with 51 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.27%. Comparing base (c439b8c) to head (ddea64c).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...ore/secure-data-store/safe-storage-main.service.ts 13.04% 40 Missing ⚠️
...ktop/src/client/core/batch-explorer-application.ts 22.22% 7 Missing ⚠️
...rc/client/core/secure-data-store/crypto-service.ts 94.11% 1 Missing and 1 partial ⚠️
...client/core/secure-data-store/secure-data-store.ts 77.77% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2995      +/-   ##
==========================================
- Coverage   80.30%   80.27%   -0.03%     
==========================================
  Files        1462     1466       +4     
  Lines       46181    46366     +185     
  Branches     5785     5797      +12     
==========================================
+ Hits        37084    37222     +138     
- Misses       8910     8955      +45     
- Partials      187      189       +2     
Files with missing lines Coverage Δ
...top/src/@batch-flask/ui/banner/banner.component.ts 82.97% <100.00%> (+0.37%) ⬆️
...ktop/src/app/services/safe-storage.service.spec.ts 100.00% <100.00%> (ø)
desktop/src/app/services/safe-storage.service.ts 100.00% <100.00%> (ø)
...ient/core/secure-data-store/crypto-service.spec.ts 100.00% <100.00%> (ø)
desktop/src/client/core/secure-data-store/index.ts 100.00% <100.00%> (ø)
...ient/core/secure-data-store/keytar.service.spec.ts 100.00% <100.00%> (ø)
...rc/client/core/secure-data-store/keytar.service.ts 100.00% <100.00%> (+60.00%) ⬆️
...t/core/secure-data-store/secure-data-store.spec.ts 100.00% <100.00%> (ø)
desktop/src/common/constants/constants.ts 96.87% <ø> (ø)
...rc/client/core/secure-data-store/crypto-service.ts 89.28% <94.11%> (+1.05%) ⬆️
... and 3 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c439b8c...ddea64c. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gingi gingi force-pushed the shpaster/replace-keytar branch 6 times, most recently from 703fb1c to 065bceb Compare December 2, 2025 23:26
@gingi gingi force-pushed the shpaster/replace-keytar branch from 065bceb to ddea64c Compare December 3, 2025 17:31
@gingi gingi merged commit 0aaaddd into main Dec 12, 2025
8 checks passed
@gingi gingi deleted the shpaster/replace-keytar branch December 12, 2025 16:34
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.

2 participants