feat(security): encrypt stored credentials at rest with keychain-backed master key by sunxiaobin89 · Pull Request #114 · GOODBOY008/r-shell

sunxiaobin89 · 2026-08-26T10:30:59Z

Summary

Encrypts connection secrets (SSH password, key passphrase, proxy password, VNC password) at rest. Previously these were stored in plaintext in the app's WebView localStorage; the export feature also wrote them into shareable files.

Approach

Single app-level master key + AES-256-GCM:

A random 32-byte master key is generated on first use and stored in the OS keychain (Rust keyring crate, one entry for the whole app → one authorization prompt, never per-credential).
Secrets are persisted as v1:<b64 nonce>:<b64 ciphertext>. Plaintext only exists in memory transiently at connect time — never written to localStorage.
persistConnections() is the single write path and strips any non-v1: plaintext secret as a defensive layer.
Startup migration seals legacy plaintext already in storage (keeps the plaintext copy on failure so the only copy is never destroyed).
Edit dialog never echoes a stored secret: the field stays empty with a hint ("leave blank to keep the saved password"); blank = keep stored, typed = replace.
Export strips all secret fields (plaintext and ciphertext) from shared config bundles.

Verification

636 frontend tests + 152 Rust tests pass.
tsc --noEmit clean; i18n key parity (en/zh-CN) holds.
Manual test of save / edit / connect / reconnect / migration round-tripped end to end.

Fixes #100

sunxiaobin89 · 2026-08-31T02:50:46Z

@GOODBOY008 Friendly ping on this one — happy to adjust or rebase as needed. 🙂

One thing I noticed: since this PR was opened, main gained SSH tunnel / jump-host support (#79), which introduces a new credential field (tunnel password). If the overall approach here looks acceptable, I'd suggest including that field in the encryption scope as well — I'm happy to fold it in together with the rebase onto the latest main.

sunxiaobin89 · 2026-09-01T07:14:19Z

Rebased onto the latest main (v2.9.0) — the conflicts with the new quit-guard commands and the SSH tunnel tab are resolved.

While rebasing, I also folded the new SSH tunnel (jump host) fields from #79 into the encryption scope: tunnelPassword and tunnelPassphrase are now sealed like the other secrets, never echoed back in the edit dialog, and stripped from exported configs. Any legacy plaintext tunnel credentials migrate on startup together with the other secret fields.

Verification: 718 frontend tests + 170 Rust tests pass; tsc --noEmit, cargo clippy and i18n key parity are clean.

sunxiaobin89 · 2026-09-03T05:52:32Z

Rebased again onto main to pick up #125 (passwordless hosts) and #103 (default SSH key path). The credential checks now follow the new passwordless semantics — blank passwords and empty key paths stay valid, while the stored-secret "keep on blank" behavior of this PR is unchanged. openConnectionSecrets runs before the shared connectionHasCredentials check so restore-time decisions see decrypted values.

Verification: 732 frontend tests + 177 Rust tests pass; tsc --noEmit and i18n key parity are clean.

Copilot

🟡 Changes recommended

Critical credential leakage, inheritance, and loss paths remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds keychain-backed AES-256-GCM encryption for stored connection credentials.

Changes:

Adds credential sealing, opening, and migration utilities.
Prevents plaintext exports and updates credential-editing behavior.
Adds localization, dependencies, and related tests.

File summaries

File	Review
`src/locales/zh-CN.json`	Adds Chinese credential UX strings.
`src/locales/en.json`	Adds English credential UX strings.
`src/lib/credential-crypto.ts`	Adds frontend credential sealing and opening helpers.
`src/lib/connection-storage.ts`	Critical: Legacy and failed-migration plaintext can be lost; prefix-only validation can persist malformed plaintext.
`src/lib/config-export-import.ts`	Critical: Profile credentials remain plaintext in profile storage despite export sanitization.
`src/components/connection-dialog.tsx`	Critical: New connections can inherit prior credentials. Moderate: retained credentials are omitted from connection requests.
`src/App.tsx`	Critical: Later writes can destroy credentials after sealing failures. Moderate: profile storage is not migrated.
`src/__tests__/connection-storage-sftp-ftp.property.test.ts`	Updates SFTP/FTP credential-storage tests.
`src/__tests__/connection-storage-proxy.test.ts`	Tests encrypted proxy storage.
`src/__tests__/connection-storage-advanced-fields.test.ts`	Tests encrypted advanced fields.
`src/__tests__/connection-duplicate-fields.test.tsx`	Updates secret-duplication expectations.
`src/__tests__/connection-dialog-tunnel.test.tsx`	Tests retained tunnel credentials.
`src/__tests__/connection-dialog-proxy.test.tsx`	Tests proxy credential behavior.
`src/__tests__/connection-dialog-edit-roundtrip.test.tsx`	Updates asynchronous edit/save tests.
`src/__tests__/connection-dialog-advanced-save.test.tsx`	Updates asynchronous persistence tests.
`src/__tests__/config-export-import.test.ts`	Verifies exported secrets are stripped.
`src-tauri/src/lib.rs`	Registers credential commands.
`src-tauri/src/commands.rs`	Moderate: malformed nonce lengths can panic instead of returning an error.
`src-tauri/Cargo.toml`	Adds cryptography and keychain dependencies.
`src-tauri/Cargo.lock`	Locks the updated dependency graph.

Review details

Suppressed comments (1)

src/components/connection-dialog.tsx:337

Do not trim secrets before encryption. Passwords and key passphrases may legitimately begin or end with whitespace, and this changes the credential so subsequent authentication fails; test emptiness without modifying the original string.

      const typed = (config[field] ?? '').trim();

Files reviewed: 19/20 changed files
Comments generated: 9
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+        } catch (error) {
+          // Keep the legacy plaintext on failure — never destroy the only copy.
+          console.error(`[Credential] Migration failed for ${conn.id}; keeping plaintext:`, error);
+        }


+        setPreviousSecrets({
+          password: editingConnection.password ?? '',
+          passphrase: editingConnection.passphrase ?? '',
+          proxyPassword: editingConnection.proxyPassword ?? '',
+          vncPassword: editingConnection.vncPassword ?? '',


+    // Connection profiles — strip secrets the same way.
    const profiles = ConnectionProfileManager.getProfiles();
    if (profiles.length) {
-      bundle.data.profiles = profiles;
+      bundle.data.profiles = profiles.map((p) => {


+  const clone = { ...connection };
+  for (const field of SECRET_FIELDS) {
+    const value = clone[field];
+    if (typeof value === 'string' && value.length > 0 && !value.startsWith('v1:')) {


+  for (const field of SECRET_FIELDS) {
+    const value = clone[field];
+    if (typeof value === 'string' && value.length > 0 && !value.startsWith('v1:')) {
+      delete clone[field];


            folder: session.folder?.replace(/All Sessions/g, 'All Connections')
          }));
-          localStorage.setItem(CONNECTIONS_STORAGE_KEY, JSON.stringify(connections));
+          persistConnections(connections);


+    let nonce_bytes = BASE64
+        .decode(parts[1])
+        .map_err(|e| format!("Corrupt nonce: {e}"))?;


+    legacyMigrationRef.current = (async () => {
+      const connections = ConnectionStorageManager.getConnections();
+      const withPlaintext = connections.filter((c) =>
+        SECRET_FIELDS.some((f) => isLegacyPlaintext(c[f])),


+    // Encrypt secrets for persistence (blank field = keep stored value).
+    // The connect request below uses the plaintext `config` as before — only
+    // the saved payload carries ciphertext.
+    let sealedSecrets: Pick<ConnectionConfig, 'password' | 'passphrase' | 'proxyPassword' | 'vncPassword' | 'tunnelPassword' | 'tunnelPassphrase'>;
+    try {


…ed master key Store connection secrets (password, passphrase, proxyPassword, vncPassword) encrypted in localStorage using AES-256-GCM with a single app-level master key kept in the OS keychain, instead of plaintext. - Rust: credential_seal/credential_open commands + master_key() that reads (or first-use creates) a random 32-byte key from the OS keychain via the keyring crate. Sealed format: v1:<b64 nonce>:<b64 ciphertext>. - Frontend: credential-crypto.ts (SECRET_FIELDS, seal/open, legacy migration); connection-storage.persistConnections() is the single write path and strips any non-v1: plaintext secret as a defensive layer. - Startup migration seals legacy plaintext (keeps plaintext on failure). - Edit dialog never echoes a stored secret; blank keeps stored, typed replaces. - Export strips all secret fields from shared config bundles. - Tests updated for sealed v1: values; 636 frontend + 152 Rust pass. Fixes GOODBOY008#100

- Add tunnelPassword and tunnelPassphrase to SECRET_FIELDS so the jump host credentials get the same AES-256-GCM treatment as other secrets - Seal tunnel secrets in every dialog persist path; blank fields keep the stored value - Never echo stored tunnel secrets back into the edit form (same hint as the password field) - Restore blank tunnel secrets from storage (decrypted) when a saved connection is used to connect Test: 718 frontend tests + 170 Rust tests pass; tsc and i18n parity clean

- Preserve legacy plaintext through schema migration until the startup seal migration encrypts it (was: stripped before it could be sealed, silently destroying passwords on upgrade) - Protect failed-seal records from strip-on-write (markSealFailed) so an unrelated persistence (updateLastConnected, folder ops) cannot destroy the only plaintext copy; lift protection once sealing succeeds - Reset the dialog's previousSecrets when opening a new connection and on close so stored secrets never leak across connections - Decrypt retained sealed secrets into the connect request when the dialog's secret fields are blank; persistence keeps the sealed form - Seal secrets as typed without trimming whitespace - Return an error for corrupt nonce lengths in credential_open instead of panicking - Seal legacy plaintext profile passwords on startup and strip secret fields from imported profile bundles - Use the 3-segment isSealed predicate for the persistence filter so a plaintext like "v1:foo" cannot bypass it Test: 753 frontend tests pass (10 new), 177 Rust tests pass, tsc/i18n clean

sunxiaobin89 · 2026-09-06T06:20:54Z

Thanks for the thorough review — verified each finding against the code and fixed them all. Rebased onto latest main (#127) in the same pass. Summary:

Fixed (all confirmed real):

Legacy migration destroyed plaintext (connection-storage.ts:152): migrateFromSessionStorage no longer routes through persistConnections; it writes migrated records verbatim so the startup seal migration can encrypt them. +tests
Seal-failure credential loss (App.tsx:330 / connection-storage.ts:113): added markSealFailed/clearSealFailed — persistence skips stripping for connections whose seal attempt failed, so no unrelated write (updateLastConnected, folder ops) can destroy the only plaintext copy; protection lifts once sealing succeeds. +tests
previousSecrets leak across connections (connection-dialog.tsx:229): now reset when the dialog opens for a new connection and when it closes. +tests
Connect used blank secrets (connection-dialog.tsx:389): handleConnect now builds a connect-only config that decrypts retained sealed secrets for blank fields (plaintext transient in memory only; persistence keeps the sealed form). Note: edit mode's Save→onSave→App.handleSaveConnection path already decrypted retained secrets — this closes the dialog-side handleConnect path symmetrically. +tests
Nonce length panic (commands.rs:3652): credential_open now returns a proper error when the decoded nonce is not 12 bytes.
Trimmed secrets (connection-dialog.tsx:337): secrets are sealed as typed; whitespace is preserved (the connect request already sent the untrimmed value — storage now matches).
Sealed-format validation parity (connection-storage.ts:112): the persistence filter now uses isSealed() (3-segment check) instead of a bare startsWith('v1:') prefix. +tests
Profile storage (config-export-import.ts:110 / App.tsx:318): startup migration now seals legacy plaintext profile passwords as well, and importProfiles strips secret fields from imported bundles (imports never persist outside secrets; storage secrets go through the migration). +tests

On severity for #8: saveProfile/updateProfile are only reachable from the unused _handle* dead code in connection-dialog.tsx, and current exports already strip all SECRET_FIELDS — so the live risk was limited to legacy stores / pre-sanitization bundles. Both paths are now covered regardless.

Verification: 753 frontend tests (10 new) + 177 Rust tests pass; tsc --noEmit and i18n parity clean; manual test of save / edit / connect / migration round-trip on the DMG build.

sunxiaobin89 force-pushed the feat/encrypted-credential-storage branch from 570b52a to 982d418 Compare September 1, 2026 07:13

sunxiaobin89 force-pushed the feat/encrypted-credential-storage branch from 982d418 to c5fbea6 Compare September 3, 2026 05:52

GOODBOY008 requested a balanced review from Copilot September 3, 2026 15:47

Copilot started reviewing on behalf of GOODBOY008 September 3, 2026 15:48 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

sunxiaobin89 added 3 commits September 6, 2026 13:37

sunxiaobin89 force-pushed the feat/encrypted-credential-storage branch from c5fbea6 to 1094439 Compare September 6, 2026 06:20

GOODBOY008 approved these changes Sep 6, 2026

View reviewed changes

GOODBOY008 merged commit f7e760a into GOODBOY008:main Sep 6, 2026
4 checks passed

GOODBOY008 mentioned this pull request Sep 6, 2026

feat(macos): red X hides the window; Dock reopens the same webview #129

Merged

sunxiaobin89 deleted the feat/encrypted-credential-storage branch September 7, 2026 06:13

Navigation Menu

feat(security): encrypt stored credentials at rest with keychain-backed master key - #114

feat(security): encrypt stored credentials at rest with keychain-backed master key#114
GOODBOY008 merged 3 commits into
GOODBOY008:mainfrom
sunxiaobin89:feat/encrypted-credential-storage

sunxiaobin89 commented Aug 26, 2026

Uh oh!

sunxiaobin89 commented Aug 31, 2026

Uh oh!

sunxiaobin89 commented Sep 1, 2026

Uh oh!

sunxiaobin89 commented Sep 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

sunxiaobin89 commented Sep 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

sunxiaobin89 commented Aug 26, 2026

Summary

Approach

Verification

Uh oh!

sunxiaobin89 commented Aug 31, 2026

Uh oh!

sunxiaobin89 commented Sep 1, 2026

Uh oh!

sunxiaobin89 commented Sep 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

sunxiaobin89 commented Sep 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants