diff --git a/apps/frontend/electron.main.js b/apps/frontend/electron.main.js index 544a3151..c1a4d8ba 100644 --- a/apps/frontend/electron.main.js +++ b/apps/frontend/electron.main.js @@ -126,10 +126,16 @@ powerMonitor.on("resume", () => { }) }) +ipcMain.handle("secure-storage:is-encryption-available", async () => safeStorage.isEncryptionAvailable()) + ipcMain.handle("secure-storage:encrypt", async (event, password) => { - if (!password || !safeStorage.isEncryptionAvailable()) return password - const encrypted = safeStorage.encryptString(password) - return encrypted.toString("base64") + if (!password) return { ok: true, value: "" } + // Fail closed: never return the plaintext password when the OS has no secure + // store, so the renderer can't persist an unprotected secret while implying it + // is encrypted. The caller keeps the plaintext for the live connection and marks + // it do-not-persist. + if (!safeStorage.isEncryptionAvailable()) return { ok: false } + return { ok: true, value: safeStorage.encryptString(password).toString("base64") } }) ipcMain.handle("secure-storage:decrypt", async (event, encryptedBase64) => { diff --git a/apps/frontend/preload.js b/apps/frontend/preload.js index c081778d..82b504f3 100644 --- a/apps/frontend/preload.js +++ b/apps/frontend/preload.js @@ -4,6 +4,7 @@ const { contextBridge, ipcRenderer } = require("electron") contextBridge.exposeInMainWorld("secureStorage", { encrypt: (password) => ipcRenderer.invoke("secure-storage:encrypt", password), decrypt: (encrypted) => ipcRenderer.invoke("secure-storage:decrypt", encrypted), + isEncryptionAvailable: () => ipcRenderer.invoke("secure-storage:is-encryption-available"), }) contextBridge.exposeInMainWorld("electronNavigation", { diff --git a/apps/frontend/src/components/ValkeyReconnect.tsx b/apps/frontend/src/components/ValkeyReconnect.tsx index 242bfd24..3ddea1c3 100644 --- a/apps/frontend/src/components/ValkeyReconnect.tsx +++ b/apps/frontend/src/components/ValkeyReconnect.tsx @@ -4,11 +4,12 @@ import { useNavigate, useParams } from "react-router" import { CONNECTED, CONNECTING, ERROR } from "@common/src/constants" import { Loader2, Database, AlertCircle } from "lucide-react" import * as R from "ramda" +import { toast } from "sonner" import RetryProgress from "./ui/retry-progress" import { PasswordPromptModal } from "./ui/password-prompt-modal" import type { RootState } from "@/store" import { connectPending } from "@/state/valkey-features/connection/connectionSlice" -import { secureStorage } from "@/utils/secureStorage" +import { secureStorage, PASSWORD_NOT_STORED_WARNING } from "@/utils/secureStorage" export function ValkeyReconnect() { const dispatch = useDispatch() @@ -51,10 +52,12 @@ export function ValkeyReconnect() { const handlePasswordSubmit = async (password: string) => { if (!connection) return - const encryptedPassword = await secureStorage.encryptIfAvailable(password) + const result = await secureStorage.encryptForStorage(password) + if (!result.ok) toast.warning(PASSWORD_NOT_STORED_WARNING) dispatch(connectPending({ connectionId: id!, - connectionDetails: { ...connection.connectionDetails, password: encryptedPassword }, + connectionDetails: { ...connection.connectionDetails, password: result.ok ? result.value : password }, + isPasswordEncrypted: result.ok, })) } diff --git a/apps/frontend/src/components/cluster-topology/cluster-node-row.tsx b/apps/frontend/src/components/cluster-topology/cluster-node-row.tsx index 77516533..cad71dd8 100644 --- a/apps/frontend/src/components/cluster-topology/cluster-node-row.tsx +++ b/apps/frontend/src/components/cluster-topology/cluster-node-row.tsx @@ -8,6 +8,7 @@ import { buildConnectionId } from "@common/src/connection-id.ts" import { calculateHitRatio } from "@common/src/cache-hit-ratio.ts" import { formatBytes } from "@common/src/bytes-conversion.ts" import { TooltipProvider } from "@radix-ui/react-tooltip" +import { toast } from "sonner" import { Badge } from "../ui/badge" import { CustomTooltip } from "../ui/tooltip" import { Button } from "../ui/button" @@ -21,9 +22,9 @@ import { getUtilizationLevel, type UtilizationLevel } from "@/state/valkey-featu import { connectPending, type ConnectionDetails } from "@/state/valkey-features/connection/connectionSlice.ts" import { useAppDispatch } from "@/hooks/hooks" import { - selectIsAtConnectionLimit, selectEncryptedPassword, selectClusterDb + selectIsAtConnectionLimit, selectClusterPassword, selectClusterDb } from "@/state/valkey-features/connection/connectionSelectors" -import { secureStorage } from "@/utils/secureStorage.ts" +import { secureStorage, PASSWORD_NOT_STORED_WARNING } from "@/utils/secureStorage.ts" import { cn } from "@/lib/utils" const UTILIZATION_BADGE: Record = { @@ -79,9 +80,9 @@ export function ClusterNodeRow({ const isDisabled = useSelector(selectIsAtConnectionLimit) - // Look up encrypted password from an existing connection in the same cluster. - // Available when secureStorage was active during the original connection. - const encryptedPassword = useSelector(selectEncryptedPassword(clusterId)) + // Look up a stored password from an existing connection in the same cluster, + // together with its isPasswordEncrypted marking. + const clusterPassword = useSelector(selectClusterPassword(clusterId)) const [showPasswordModal, setShowPasswordModal] = useState(false) @@ -109,15 +110,17 @@ export function ClusterNodeRow({ awsReplicationGroupId: primaryConfig.awsReplicationGroupId, }, })) - } else if (R.isNotNil(encryptedPassword)) { - // Password already encrypted from existing cluster connection — do NOT re-encrypt + } else if (R.isNotNil(clusterPassword)) { + // Reuse the sibling connection's stored password, carrying its + // isPasswordEncrypted marking so an unencrypted one is still never persisted. dispatch(connectPending({ connectionId, connectionDetails: { ...baseDetails, username: primaryConfig.username ?? "", - password: encryptedPassword, + password: clusterPassword.password, }, + isPasswordEncrypted: clusterPassword.isPasswordEncrypted, })) } else { // No stored password — prompt for password @@ -126,14 +129,16 @@ export function ClusterNodeRow({ } const handlePasswordSubmit = async (password: string) => { - const encryptedPw = await secureStorage.encryptIfAvailable(password) + const result = await secureStorage.encryptForStorage(password) + if (!result.ok) toast.warning(PASSWORD_NOT_STORED_WARNING) dispatch(connectPending({ connectionId, connectionDetails: { ...baseDetails, username: primaryConfig.username ?? "", - password: encryptedPw, + password: result.ok ? result.value : password, }, + isPasswordEncrypted: result.ok, })) } diff --git a/apps/frontend/src/components/connection/Connection.tsx b/apps/frontend/src/components/connection/Connection.tsx index c5bc9eb1..01247fba 100644 --- a/apps/frontend/src/components/connection/Connection.tsx +++ b/apps/frontend/src/components/connection/Connection.tsx @@ -2,6 +2,7 @@ import { useState } from "react" import { useSelector } from "react-redux" import { HousePlug } from "lucide-react" import { CONNECTED, CONNECTING, MAX_CONNECTIONS, RECONNECTING } from "@common/src/constants.ts" +import { toast } from "sonner" import ConnectionForm from "../ui/connection-form.tsx" import EditForm from "../ui/edit-form.tsx" import { PasswordPromptModal } from "../ui/password-prompt-modal.tsx" @@ -17,7 +18,7 @@ import { selectConnections } from "@/state/valkey-features/connection/connection import { ConnectionEntry } from "@/components/connection/ConnectionEntry.tsx" import { ClusterConnectionGroup } from "@/components/connection/ClusterConnectionGroup.tsx" import { useAppDispatch } from "@/hooks/hooks.ts" -import { secureStorage } from "@/utils/secureStorage.ts" +import { secureStorage, PASSWORD_NOT_STORED_WARNING } from "@/utils/secureStorage.ts" const matchesSearch = (q: string, connection: ConnectionState) => connection.searchableText.includes(q) @@ -49,10 +50,12 @@ export function Connection() { if (!passwordPromptConnectionId) return const connection = connections[passwordPromptConnectionId] if (!connection) return - const encryptedPassword = await secureStorage.encryptIfAvailable(password) + const result = await secureStorage.encryptForStorage(password) + if (!result.ok) toast.warning(PASSWORD_NOT_STORED_WARNING) dispatch(connectPending({ connectionId: passwordPromptConnectionId, - connectionDetails: { ...connection.connectionDetails, password: encryptedPassword }, + connectionDetails: { ...connection.connectionDetails, password: result.ok ? result.value : password }, + isPasswordEncrypted: result.ok, preservedHistory: connection.connectionHistory, })) } diff --git a/apps/frontend/src/components/ui/connection-form.tsx b/apps/frontend/src/components/ui/connection-form.tsx index c6e19524..fc479c7d 100644 --- a/apps/frontend/src/components/ui/connection-form.tsx +++ b/apps/frontend/src/components/ui/connection-form.tsx @@ -2,6 +2,7 @@ import { type FormEvent, useEffect, useState } from "react" import { useSelector } from "react-redux" import { buildConnectionId, isValidDatabaseIndex } from "@common/src/connection-id.ts" import { CONNECTED, CONNECTING, ERROR } from "@common/src/constants.ts" +import { toast } from "sonner" import { ConnectionModal } from "./connection-modal.tsx" import { useAppDispatch, useAppSelector } from "@/hooks/hooks" import { connectPending, type ConnectionDetails } from "@/state/valkey-features/connection/connectionSlice.ts" @@ -10,7 +11,7 @@ import { discoveryEndpointPending, clearEndpointDiscovery } from "@/state/valkey-features/topology/topologySlice.ts" -import { secureStorage } from "@/utils/secureStorage.ts" +import { secureStorage, PASSWORD_NOT_STORED_WARNING } from "@/utils/secureStorage.ts" interface ConnectionFormProps { onClose: () => void @@ -87,22 +88,29 @@ function ConnectionForm({ onClose }: ConnectionFormProps) { awsReplicationGroupId: connectionDetails.awsReplicationGroupId?.trim(), } - const detailsToDispatch = connectionDetails.password - ? { ...trimmed, password: await secureStorage.encryptIfAvailable(connectionDetails.password) } - : trimmed + let isPasswordEncrypted: boolean | undefined + let detailsToDispatch = trimmed + if (connectionDetails.password) { + const result = await secureStorage.encryptForStorage(connectionDetails.password) + isPasswordEncrypted = result.ok + // On failure keep the plaintext so this session can connect; it will not be + // persisted (see the persistence layer), and the user is warned. + detailsToDispatch = { ...trimmed, password: result.ok ? result.value : connectionDetails.password } + if (!result.ok) toast.warning(PASSWORD_NOT_STORED_WARNING) + } if (trimmed.endpointType === "cluster-endpoint") { const newDiscoveryId = `discovery-${buildConnectionId(trimmed.host, trimmed.port, 0)}` setDiscoveryId(newDiscoveryId) setConnectionId(null) - dispatch(discoveryEndpointPending({ discoveryId: newDiscoveryId, connectionDetails: detailsToDispatch })) + dispatch(discoveryEndpointPending({ discoveryId: newDiscoveryId, connectionDetails: detailsToDispatch, isPasswordEncrypted })) return } const newConnectionId = buildConnectionId(trimmed.host, trimmed.port, trimmed.db) setConnectionId(newConnectionId) setDiscoveryId(null) - dispatch(connectPending({ connectionId: newConnectionId, connectionDetails: detailsToDispatch })) + dispatch(connectPending({ connectionId: newConnectionId, connectionDetails: detailsToDispatch, isPasswordEncrypted })) } return ( diff --git a/apps/frontend/src/components/ui/edit-form.tsx b/apps/frontend/src/components/ui/edit-form.tsx index 6957b225..ac392af6 100644 --- a/apps/frontend/src/components/ui/edit-form.tsx +++ b/apps/frontend/src/components/ui/edit-form.tsx @@ -2,6 +2,7 @@ import { type FormEvent, useState, useEffect, useCallback } from "react" import { useSelector } from "react-redux" import { buildConnectionId, isValidDatabaseIndex } from "@common/src/connection-id.ts" import { CONNECTED } from "@common/src/constants" +import { toast } from "sonner" import { ConnectionModal } from "./connection-modal.tsx" import { updateConnectionDetails, @@ -16,7 +17,7 @@ import { selectIsAtConnectionLimit } from "@/state/valkey-features/connection/connectionSelectors" import { useAppDispatch } from "@/hooks/hooks" -import { secureStorage } from "@/utils/secureStorage.ts" +import { secureStorage, PASSWORD_NOT_STORED_WARNING } from "@/utils/secureStorage.ts" interface EditFormProps { onClose: () => void @@ -146,15 +147,23 @@ function EditForm({ onClose, connectionId }: EditFormProps) { dispatch(deleteConnection({ connectionId, silent: true })) // Encrypt password only if user typed a new one; otherwise it's already encrypted from Redux - const detailsToDispatch = passwordChanged && connectionDetails.password - ? { ...trimmed, password: await secureStorage.encryptIfAvailable(connectionDetails.password) } - : trimmed + // and carries the source connection's marking (the connect below targets a new + // connectionId, so the reducer can't inherit it). + let isPasswordEncrypted = passwordChanged ? undefined : fullConnection?.isPasswordEncrypted + let detailsToDispatch = trimmed + if (passwordChanged && connectionDetails.password) { + const result = await secureStorage.encryptForStorage(connectionDetails.password) + isPasswordEncrypted = result.ok + detailsToDispatch = { ...trimmed, password: result.ok ? result.value : connectionDetails.password } + if (!result.ok) toast.warning(PASSWORD_NOT_STORED_WARNING) + } dispatch( connectPending({ connectionId: newConnectionId, connectionDetails: detailsToDispatch, isEdit: true, + isPasswordEncrypted, preservedHistory: connectionHistory, }), ) diff --git a/apps/frontend/src/state/epics/valkeyEpics.persist.test.ts b/apps/frontend/src/state/epics/valkeyEpics.persist.test.ts new file mode 100644 index 00000000..4b73d50f --- /dev/null +++ b/apps/frontend/src/state/epics/valkeyEpics.persist.test.ts @@ -0,0 +1,58 @@ +import { LOCAL_STORAGE } from "@common/src/constants" +import { persistConnections } from "./valkeyEpics" +import type { ConnectionState } from "@/state/valkey-features/connection/connectionSlice" + +// persistConnections is the single write path to localStorage. Its security job: +// never write a password that could not be encrypted (isPasswordEncrypted === false). +describe("persistConnections", () => { + beforeEach(() => localStorage.clear()) + + const read = () => JSON.parse(localStorage.getItem(LOCAL_STORAGE.VALKEY_CONNECTIONS) ?? "{}") + + const conn = (overrides: Partial): ConnectionState => ({ + status: "NOT_CONNECTED" as ConnectionState["status"], + errorMessage: null, + searchableText: "", + connectionDetails: { + host: "h", port: "6379", tls: false, verifyTlsCertificate: false, endpointType: "node", db: 0, + password: "secret", + }, + ...overrides, + }) + + it("strips the password and drops the flag when the password is unencrypted", () => { + persistConnections({ a: conn({ isPasswordEncrypted: false }) }) + + const stored = read().a + expect(stored.connectionDetails.password).toBeUndefined() + expect("isPasswordEncrypted" in stored).toBe(false) + }) + + it("preserves an encrypted password (flag true)", () => { + persistConnections({ a: conn({ isPasswordEncrypted: true, connectionDetails: { + host: "h", port: "6379", tls: false, verifyTlsCertificate: false, endpointType: "node", db: 0, + password: "ciphertext", + } }) }) + + expect(read().a.connectionDetails.password).toBe("ciphertext") + }) + + it("preserves a password when the flag is absent (existing/normal connections)", () => { + persistConnections({ a: conn({}) }) + expect(read().a.connectionDetails.password).toBe("secret") + }) + + it("only strips the flagged connection, leaving others intact", () => { + persistConnections({ + bad: conn({ isPasswordEncrypted: false }), + good: conn({ isPasswordEncrypted: true, connectionDetails: { + host: "g", port: "6379", tls: false, verifyTlsCertificate: false, endpointType: "node", db: 0, + password: "keepme", + } }), + }) + + const stored = read() + expect(stored.bad.connectionDetails.password).toBeUndefined() + expect(stored.good.connectionDetails.password).toBe("keepme") + }) +}) diff --git a/apps/frontend/src/state/epics/valkeyEpics.ts b/apps/frontend/src/state/epics/valkeyEpics.ts index 427f111e..81d34536 100644 --- a/apps/frontend/src/state/epics/valkeyEpics.ts +++ b/apps/frontend/src/state/epics/valkeyEpics.ts @@ -51,6 +51,24 @@ const getCurrentConnections = () => R.pipe( (s) => (s === null ? {} : JSON.parse(s)), )(LOCAL_STORAGE.VALKEY_CONNECTIONS) +// Single write path to localStorage. A password that could not be encrypted +// (isPasswordEncrypted === false) is never written to disk: it is dropped to +// undefined so the connection re-prompts on next use, and the transient flag is +// not persisted either. All persistence writes must go through this. +export const persistConnections = (connections: Record) => { + const safe = Object.fromEntries( + Object.entries(connections).map(([id, conn]) => { + if (conn?.isPasswordEncrypted === false) { + const stripped = { ...conn, connectionDetails: { ...conn.connectionDetails, password: undefined } } + delete stripped.isPasswordEncrypted + return [id, stripped] + } + return [id, conn] + }), + ) + localStorage.setItem(LOCAL_STORAGE.VALKEY_CONNECTIONS, JSON.stringify(safe)) +} + export const connectionEpic = (store: Store) => merge( action$.pipe( @@ -97,10 +115,11 @@ export const connectionEpic = (store: Store) => status: NOT_CONNECTED, connectionHistory: connection?.connectionHistory ?? [], searchableText: connection?.searchableText ?? "", + isPasswordEncrypted: connection?.isPasswordEncrypted, } currentConnections[payload.connectionId] = connectionToSave - localStorage.setItem(LOCAL_STORAGE.VALKEY_CONNECTIONS, JSON.stringify(currentConnections)) + persistConnections(currentConnections) if (baseConnectionDetails?.host?.includes(".serverless.")) { toast.warning( @@ -156,6 +175,7 @@ export const connectionEpic = (store: Store) => port: String(firstNode.port), endpointType: "node", }, + isPasswordEncrypted: discovery.isPasswordEncrypted, })) // store the connectionId in the discovery state so we can show the correct connection status store.dispatch(discoveryNodeConnecting({ discoveryId, connectionId })) @@ -372,7 +392,7 @@ export const deleteConnectionEpic = () => const currentConnections = getCurrentConnections() if (currentConnections[connectionId]) { currentConnections[connectionId].userDisconnected = true - localStorage.setItem(LOCAL_STORAGE.VALKEY_CONNECTIONS, JSON.stringify(currentConnections)) + persistConnections(currentConnections) } } catch (e) { console.error(e) @@ -415,7 +435,8 @@ export const updateConnectionDetailsEpic = (store: Store) => currentConnections[connectionId].connectionDetails = connection.connectionDetails currentConnections[connectionId].connectionHistory = connection.connectionHistory || [] currentConnections[connectionId].searchableText = connection.searchableText ?? "" - localStorage.setItem(LOCAL_STORAGE.VALKEY_CONNECTIONS, JSON.stringify(currentConnections)) + currentConnections[connectionId].isPasswordEncrypted = connection.isPasswordEncrypted + persistConnections(currentConnections) } } catch (e) { console.error(e) diff --git a/apps/frontend/src/state/valkey-features/connection/connectionSelectors.ts b/apps/frontend/src/state/valkey-features/connection/connectionSelectors.ts index 78b9c924..3d153278 100644 --- a/apps/frontend/src/state/valkey-features/connection/connectionSelectors.ts +++ b/apps/frontend/src/state/valkey-features/connection/connectionSelectors.ts @@ -18,10 +18,15 @@ export const selectIsAnyConnecting = (state: RootState) => Object.values(selectConnections(state)).some((c) => c.status === CONNECTING) export const selectJsonModuleAvailable = (id: string) => (state: RootState) => atId(id, state)?.connectionDetails?.jsonModuleAvailable ?? false -export const selectEncryptedPassword = (clusterId: string) => (state: RootState) => - Object.values(state.valkeyConnection?.connections ?? {}).find( +export const selectClusterPassword = (clusterId: string) => (state: RootState) => { + const source = Object.values(state.valkeyConnection?.connections ?? {}).find( (c) => c.connectionDetails?.clusterId === clusterId && R.isNotNil(c.connectionDetails?.password), - )?.connectionDetails?.password + ) + if (!source) return undefined + // Conservative default: an absent marker (e.g. a connection restored from + // localStorage) is treated as unencrypted so reuse can never persist plaintext. + return { password: source.connectionDetails.password, isPasswordEncrypted: source.isPasswordEncrypted ?? false } +} export const selectClusterDb = (clusterId: string) => (state: RootState) => Object.values(state.valkeyConnection?.connections ?? {}).find( diff --git a/apps/frontend/src/state/valkey-features/connection/connectionSlice.test.ts b/apps/frontend/src/state/valkey-features/connection/connectionSlice.test.ts index c19bc061..d4a1f6b6 100644 --- a/apps/frontend/src/state/valkey-features/connection/connectionSlice.test.ts +++ b/apps/frontend/src/state/valkey-features/connection/connectionSlice.test.ts @@ -22,6 +22,43 @@ describe("connectionSlice", () => { }) describe("connectPending", () => { + it("preserves isPasswordEncrypted=false across re-dispatches that omit the flag (retry/resume)", () => { + const details = { + host: "localhost", port: "6379", username: "admin", password: "plaintext", + tls: false, verifyTlsCertificate: false, alias: "Test", + } + // Initial connect marks the password as unencrypted (no OS keystore). + let state = connectionReducer( + initialState, + connectPending({ connectionId: "conn-1", connectionDetails: details, isPasswordEncrypted: false }), + ) + expect(state.connections["conn-1"].isPasswordEncrypted).toBe(false) + + // A retry re-dispatch carries the same details but no flag — the marking must survive, + // otherwise the persistence strip would be bypassed and plaintext could reach disk. + state = connectionReducer( + state, + connectPending({ connectionId: "conn-1", connectionDetails: details, isRetry: true }), + ) + expect(state.connections["conn-1"].isPasswordEncrypted).toBe(false) + }) + + it("lets a new explicit flag override the preserved one (password re-entered with keystore available)", () => { + const details = { + host: "localhost", port: "6379", username: "admin", password: "ciphertext", + tls: false, verifyTlsCertificate: false, alias: "Test", + } + let state = connectionReducer( + initialState, + connectPending({ connectionId: "conn-1", connectionDetails: details, isPasswordEncrypted: false }), + ) + state = connectionReducer( + state, + connectPending({ connectionId: "conn-1", connectionDetails: details, isPasswordEncrypted: true }), + ) + expect(state.connections["conn-1"].isPasswordEncrypted).toBe(true) + }) + it("should create connection with CONNECTING status and store details with no password", () => { const state = connectionReducer( initialState, diff --git a/apps/frontend/src/state/valkey-features/connection/connectionSlice.ts b/apps/frontend/src/state/valkey-features/connection/connectionSlice.ts index 9db74e15..bd87cb3d 100644 --- a/apps/frontend/src/state/valkey-features/connection/connectionSlice.ts +++ b/apps/frontend/src/state/valkey-features/connection/connectionSlice.ts @@ -72,6 +72,7 @@ export interface ConnectionState { connectionHistory?: ConnectionHistoryEntry[]; wasEdit?: boolean; userDisconnected?: boolean; + isPasswordEncrypted?: boolean; // Set when a connect is automatic (refresh resume / socket-drop reconnect) autoConnect?: boolean; } @@ -138,6 +139,7 @@ const connectionSlice = createSlice({ isRetry?: boolean; isResume?: boolean; isEdit?: boolean; + isPasswordEncrypted?: boolean; autoConnect?: boolean; preservedHistory?: ConnectionHistoryEntry[]; }>, @@ -147,6 +149,7 @@ const connectionSlice = createSlice({ connectionDetails, isRetry = false, isEdit = false, + isPasswordEncrypted, autoConnect = false, preservedHistory, } = action.payload @@ -166,6 +169,9 @@ const connectionSlice = createSlice({ }, searchableText: buildSearchableText(connectionId, connectionDetails), wasEdit: isEdit, + // Re-dispatches (retry/resume/auto-reconnect) omit the flag but carry the + // same in-memory password, so keep the existing marking. + isPasswordEncrypted: isPasswordEncrypted ?? existingConnection?.isPasswordEncrypted, autoConnect, ...(isRetry && existingConnection?.reconnect && { reconnect: existingConnection.reconnect, diff --git a/apps/frontend/src/state/valkey-features/topology/topologySlice.ts b/apps/frontend/src/state/valkey-features/topology/topologySlice.ts index 7cc75648..22bf9554 100644 --- a/apps/frontend/src/state/valkey-features/topology/topologySlice.ts +++ b/apps/frontend/src/state/valkey-features/topology/topologySlice.ts @@ -12,6 +12,7 @@ export type DiscoveryStatus = "pending" | "fulfilled" | "node_connecting" | "rej export interface DiscoveryState { status: DiscoveryStatus connectionDetails: ConnectionDetails + isPasswordEncrypted?: boolean clusterNodes?: Record errorMessage?: string nodeConnectionId?: string @@ -31,12 +32,13 @@ const topologySlice = createSlice({ reducers: { discoveryEndpointPending: ( state, - action: PayloadAction<{ discoveryId: string; connectionDetails: ConnectionDetails }>, + action: PayloadAction<{ discoveryId: string; connectionDetails: ConnectionDetails; isPasswordEncrypted?: boolean }>, ) => { - const { discoveryId, connectionDetails } = action.payload + const { discoveryId, connectionDetails, isPasswordEncrypted } = action.payload state.discoveries[discoveryId] = { status: "pending", connectionDetails, + isPasswordEncrypted, } }, discoveryEndpointFulfilled: ( diff --git a/apps/frontend/src/utils/secureStorage.test.ts b/apps/frontend/src/utils/secureStorage.test.ts index b5078495..8d7a47af 100644 --- a/apps/frontend/src/utils/secureStorage.test.ts +++ b/apps/frontend/src/utils/secureStorage.test.ts @@ -3,58 +3,68 @@ import { secureStorage } from "./secureStorage" // Tests for the renderer-side secureStorage wrapper (secureStorage.ts). // This wrapper delegates to window.secureStorage (exposed via contextBridge) -// and gracefully falls back when running outside Electron. +// and gracefully fails closed when running outside Electron. describe("secureStorage wrapper", () => { beforeEach(() => { delete (window as Record).secureStorage }) // Outside Electron (e.g. in tests or a browser), window.secureStorage is - // undefined. The wrapper should return empty string to prevent unencrypted persistence. + // undefined. Persistence encryption must fail closed so no cleartext is stored. describe("when window.secureStorage is not available", () => { - it("encrypt returns empty string", async () => { - expect(await secureStorage.encrypt("password")).toBe("") + it("encryptForStorage fails closed for a real password", async () => { + expect(await secureStorage.encryptForStorage("password")).toEqual({ ok: false }) + }) + + it("encryptForStorage reports ok for empty input (nothing to protect)", async () => { + expect(await secureStorage.encryptForStorage("")).toEqual({ ok: true, value: "" }) }) it("decrypt returns empty string", async () => { expect(await secureStorage.decrypt("encrypted")).toBe("") }) - it("encrypt returns empty string for empty input", async () => { - expect(await secureStorage.encrypt("")).toBe("") + it("isEncryptionAvailable is false", async () => { + expect(await secureStorage.isEncryptionAvailable()).toBe(false) }) - it("decrypt returns empty string for empty input", async () => { - expect(await secureStorage.decrypt("")).toBe("") + it("isAvailable is false", () => { + expect(secureStorage.isAvailable()).toBe(false) }) }) describe("when window.secureStorage is available", () => { let mockEncrypt: ReturnType let mockDecrypt: ReturnType + let mockIsEncryptionAvailable: ReturnType beforeEach(() => { mockEncrypt = vi.fn() mockDecrypt = vi.fn() + mockIsEncryptionAvailable = vi.fn() Object.defineProperty(window, "secureStorage", { - value: { encrypt: mockEncrypt, decrypt: mockDecrypt }, + value: { encrypt: mockEncrypt, decrypt: mockDecrypt, isEncryptionAvailable: mockIsEncryptionAvailable }, writable: true, configurable: true, }) }) - it("encrypt delegates to window.secureStorage.encrypt", async () => { - mockEncrypt.mockResolvedValue("base64encrypted") - const result = await secureStorage.encrypt("mypassword") + it("encryptForStorage returns the structured success result from the bridge", async () => { + mockEncrypt.mockResolvedValue({ ok: true, value: "base64encrypted" }) + const result = await secureStorage.encryptForStorage("mypassword") expect(mockEncrypt).toHaveBeenCalledWith("mypassword") - expect(result).toBe("base64encrypted") + expect(result).toEqual({ ok: true, value: "base64encrypted" }) + }) + + it("encryptForStorage surfaces a failure (encryption unavailable) from the bridge", async () => { + mockEncrypt.mockResolvedValue({ ok: false }) + expect(await secureStorage.encryptForStorage("mypassword")).toEqual({ ok: false }) }) - // Empty passwords should short-circuit without invoking the IPC channel - it("encrypt returns empty string for empty input", async () => { - const result = await secureStorage.encrypt("") + it("encryptForStorage short-circuits empty input without invoking the bridge", async () => { + const result = await secureStorage.encryptForStorage("") expect(mockEncrypt).not.toHaveBeenCalled() - expect(result).toBe("") + expect(result).toEqual({ ok: true, value: "" }) }) it("decrypt delegates to window.secureStorage.decrypt", async () => { @@ -69,6 +79,13 @@ describe("secureStorage wrapper", () => { expect(mockDecrypt).not.toHaveBeenCalled() expect(result).toBe("") }) + + it("isEncryptionAvailable reflects the real OS backend", async () => { + mockIsEncryptionAvailable.mockResolvedValue(true) + expect(await secureStorage.isEncryptionAvailable()).toBe(true) + mockIsEncryptionAvailable.mockResolvedValue(false) + expect(await secureStorage.isEncryptionAvailable()).toBe(false) + }) }) }) diff --git a/apps/frontend/src/utils/secureStorage.ts b/apps/frontend/src/utils/secureStorage.ts index 457db5e4..6698216e 100644 --- a/apps/frontend/src/utils/secureStorage.ts +++ b/apps/frontend/src/utils/secureStorage.ts @@ -1,7 +1,17 @@ +type EncryptResult = { ok: true; value: string } | { ok: false } + +export const PASSWORD_NOT_STORED_WARNING = + "This system has no secure credential store, so the password can't be saved and will be requested " + + "on the next connection. Install or unlock your system keyring (e.g. gnome-keyring or KWallet) to " + + "enable saved passwords." + export const secureStorage = { - encrypt: async (unencrypted: string): Promise => { - if (!unencrypted || !window.secureStorage) return "" - return await window.secureStorage.encrypt(unencrypted) + // Encrypt for PERSISTENCE. Reports whether real encryption happened so the + // caller can refuse to persist an unprotected secret. + encryptForStorage: async (password: string): Promise => { + if (!password) return { ok: true, value: "" } + if (!window.secureStorage) return { ok: false } + return await window.secureStorage.encrypt(password) }, decrypt: async (encrypted: string): Promise => { @@ -9,11 +19,11 @@ export const secureStorage = { return await window.secureStorage.decrypt(encrypted) }, - encryptIfAvailable: async (password: string): Promise => { - if (password.length > 0 && secureStorage.isAvailable()) { - return await secureStorage.encrypt(password) - } - return password + // True only when the OS actually has a secure store (not merely that we are in + // Electron). Reflects safeStorage.isEncryptionAvailable() in the main process. + isEncryptionAvailable: async (): Promise => { + if (!window.secureStorage?.isEncryptionAvailable) return false + return await window.secureStorage.isEncryptionAvailable() }, isAvailable: (): boolean => { @@ -24,8 +34,9 @@ export const secureStorage = { declare global { interface Window { secureStorage?: { - encrypt: (password: string) => Promise + encrypt: (password: string) => Promise decrypt: (encrypted: string) => Promise + isEncryptionAvailable: () => Promise } } }