From f86265aa7cfacf318dfa82f0c605b8e463540141 Mon Sep 17 00:00:00 2001 From: Den <2119348+dzianisv@users.noreply.github.com> Date: Sat, 18 Jul 2026 08:49:42 -0700 Subject: [PATCH] fix(security): re-lock biometric app-lock on background; persist edited password (#125) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues from a security review of the credential/auth path (the review also verified the fundamentals are solid — passwords in SecureStore, Sentry/analytics/ Chatwoot all scrub secrets). 1. HIGH: biometric app-lock never re-armed. authenticate() sets isAuthenticated =true once at cold start and lock() was never called (no AppState listener) — so 'Require Biometric to Open' was fully bypassable: after one unlock, anyone with brief physical access could reopen a backgrounded app straight into session history and connection details for the life of the JS process. Now an AppState 'background' listener calls lock() when the toggle is on. Fires on 'background' only, so the biometric prompt / app switcher (transient 'inactive') don't cause spurious re-locks. 2. Editing a connection's password did nothing: the edit screen's password field was never passed to updateConnection, which never wrote PASSWORDS_PREFIX — so a user rotating a server password silently kept using the old one. updateConnection now takes an optional password and writes it to SecureStore (blank = keep existing, since the field loads empty). typecheck clean, 187/187 tests. Claude-Session: https://claude.ai/code/session_01T12AhSnQVrSxNnvwfCx2z6 Co-authored-by: engineer Co-authored-by: Claude Fable 5 --- app/_layout.tsx | 20 +++++++++++++++++++- app/connection/[id].tsx | 20 +++++++++++++------- src/stores/connections.ts | 12 ++++++++++-- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/app/_layout.tsx b/app/_layout.tsx index b4ffe33..8e47b70 100644 --- a/app/_layout.tsx +++ b/app/_layout.tsx @@ -1,7 +1,7 @@ import { useEffect, useRef, useState } from "react" import { Stack, router } from "expo-router" import { StatusBar } from "expo-status-bar" -import { useColorScheme, View, ActivityIndicator } from "react-native" +import { useColorScheme, View, ActivityIndicator, AppState } from "react-native" import { QueryClient, QueryClientProvider } from "@tanstack/react-query" import { GestureHandlerRootView } from "react-native-gesture-handler" import { BottomSheetModalProvider } from "@gorhom/bottom-sheet" @@ -77,6 +77,24 @@ function RootLayout() { return unsubNotifications }, []) + // Re-arm the biometric app-lock when the app leaves the foreground. Without + // this, "Require Biometric to Open" is bypassable: authenticate() sets + // isAuthenticated=true once at cold start and nothing ever resets it, so the + // app stays unlocked for the whole JS-process lifetime — anyone with brief + // physical access can reopen a backgrounded app straight into session + // history and connection details. lock() flips isAuthenticated back to false + // so AuthGate shows the lock screen (and re-prompts) on next foreground. + // Fire on "background" only (not the transient "inactive" that the biometric + // prompt / app switcher / control center produce) to avoid spurious re-locks. + useEffect(() => { + const sub = AppState.addEventListener("change", (next) => { + if (next === "background" && useAuth.getState().settings.requireBiometric) { + useAuth.getState().lock() + } + }) + return () => sub.remove() + }, []) + // Connect/disconnect SSE and load catalog when client changes useEffect(() => { if (client && !sseStarted.current) { diff --git a/app/connection/[id].tsx b/app/connection/[id].tsx index a846d50..92f9268 100644 --- a/app/connection/[id].tsx +++ b/app/connection/[id].tsx @@ -136,13 +136,19 @@ export default function EditConnectionScreen() { } setIsSaving(true) - await updateConnection(connection.id, { - name: name.trim(), - type, - url: url.trim(), - directory: directory.trim() || undefined, - username: username.trim() || undefined, - }) + await updateConnection( + connection.id, + { + name: name.trim(), + type, + url: url.trim(), + directory: directory.trim() || undefined, + username: username.trim() || undefined, + }, + // Empty = keep existing password (the field loads blank); a typed value + // rotates it in SecureStore. + password || undefined, + ) // If this was the active connection, the SSE loop may have stopped // retrying after a prior 401 (see events.ts) — reconnect now with the // freshly saved credentials instead of leaving the user stuck until diff --git a/src/stores/connections.ts b/src/stores/connections.ts index 93a9ffa..b0a563b 100644 --- a/src/stores/connections.ts +++ b/src/stores/connections.ts @@ -47,7 +47,7 @@ interface ConnectionsState { source: ConnectionTestSource, password?: string, ) => Promise<{ ok: boolean; error?: string }> - updateConnection: (id: string, updates: Partial) => Promise + updateConnection: (id: string, updates: Partial, password?: string) => Promise refreshProject: () => Promise // Create a one-off client pointing at a specific directory (for cross-project operations). // Pass undefined to get a directory-less client that queries the server without project scope. @@ -275,11 +275,19 @@ export const useConnections = create((set, get) => ({ } }, - updateConnection: async (id, updates) => { + updateConnection: async (id, updates, password) => { const connections = get().connections.map((c) => (c.id === id ? { ...c, ...updates } : c)) await SecureStore.setItemAsync(CONNECTIONS_KEY, JSON.stringify(connections)) + // Persist a new password only when one was entered. The edit form loads the + // password field blank (passwords aren't read back for security), so an + // empty value means "keep the existing password", not "clear it". Written + // before the active-client rebuild below so the rebuilt client picks it up. + if (password) { + await SecureStore.setItemAsync(`${PASSWORDS_PREFIX}${id}`, password) + } + // If updating active connection, recreate client if (get().activeConnection?.id === id) { const active = connections.find((c) => c.id === id)!