Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions packages/feature-shell/src/data-access/use-shell-unlock-gate.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
import { useAppContext } from '@workspace/context-react/use-app-context'
import type { WalletProtectionMode } from '@workspace/db/wallet/wallet'
import { useAccountActive } from '@workspace/db-react/use-account-active'
import { useWalletActive } from '@workspace/db-react/use-wallet-active'
import { toastError } from '@workspace/ui/lib/toast-error'
import { useVaultUnlockDialog } from '@workspace/vault-react/vault-unlock-provider'

export type ShellUnlockGateState = {
isChecking: boolean
isLocked: boolean
isUnlocking: boolean
walletName: string
walletProtectionMode: WalletProtectionMode
}

export type ShellUnlockGate = {
actions: {
unlock(): Promise<void>
}
state: ShellUnlockGateState
}

function shellUnlockStatusQueryKey(walletId: string) {
return ['shellUnlockStatus', walletId] as const
}

export function useShellUnlockGate(): ShellUnlockGate {
const account = useAccountActive()
const context = useAppContext()
const queryClient = useQueryClient()
const wallet = useWalletActive()
const { requestUnlock } = useVaultUnlockDialog()
const statusQuery = useQuery({
queryFn: async () => {
try {
await context.vault.requireWalletKey({ walletId: account.walletId })
return true
} catch {
if (wallet.protectionMode === 'unsecured') {
await context.vault.unlockWallet({ credential: '', walletId: account.walletId })
return true
}
return false
}
},
queryKey: shellUnlockStatusQueryKey(account.walletId),
retry: false,
})
const unlockMutation = useMutation({
mutationFn: async () =>
await requestUnlock({
mode: wallet.protectionMode,
reason: 'generic',
walletId: account.walletId,
}),
onError: (caught) => toastError(caught instanceof Error ? caught.message : `${caught}`),
onSuccess: async (unlocked) => {
if (unlocked) {
await queryClient.invalidateQueries({ queryKey: shellUnlockStatusQueryKey(account.walletId) })
}
},
})

return {
actions: {
unlock: async () => {
await unlockMutation.mutateAsync().catch(() => undefined)
},
},
state: {
isChecking: statusQuery.isLoading,
isLocked: statusQuery.data === false,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Fail closed while unlock status is unresolved

This gate currently treats every state except data === false as unlocked, while only showing a loader for isLoading. In this app, queries are persisted via PersistQueryClientProvider (packages/feature-shell/src/data-access/shell-providers.tsx), so a previously cached true can be hydrated on startup before requireWalletKey rechecks the new in-memory vault state. That means locked wallets can briefly render routed content (and run child effects) until refetch flips the value, which defeats the purpose of an app-wide lock gate.

Useful? React with 👍 / 👎.

Comment on lines +72 to +73

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Unlock gate fails open on query error instead of showing locked state

When statusQuery errors (e.g., unlockWallet throws for an unsecured wallet at use-shell-unlock-gate.tsx:41), statusQuery.data is undefined and statusQuery.isLoading is false. This means isChecking is false and isLocked is undefined === falsefalse. The gate then renders children (shell-ui-unlock-gate.tsx:16-17) even though the wallet wasn't actually unlocked. The gate should fail closed (show locked/error state) rather than fail open.

How the error propagates

In the queryFn, the catch block for requireWalletKey does not wrap the unlockWallet call in its own try-catch. If unlockWallet throws, the error propagates out of the queryFn, causing the query to enter an error state with data === undefined. Since the component only checks statusQuery.data === false for the locked state, the undefined value falls through to show content.

Suggested change
isChecking: statusQuery.isLoading,
isLocked: statusQuery.data === false,
isChecking: statusQuery.isLoading,
isLocked: statusQuery.data !== true,
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

isUnlocking: unlockMutation.isPending,
Comment on lines +72 to +74

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In TanStack React Query v5, when a query function throws (with retry disabled), what are the values of data, isError, and isLoading?

💡 Result:

If a TanStack React Query v5 query function throws (and retries are disabled with retry: false), then after the failed attempt settles: - data: undefined [1] - isError: true [1] - isLoading: false [1] Reasoning: In v5, isError is derived from status === 'error' [1], and data is only available in status === 'success' (otherwise it’s undefined) [1]. Also, isLoading is only true when the first fetch is in-flight, defined as isFetching && isPending [1]; once the query ends in status: 'error', it’s no longer pending/in-flight, so isLoading becomes false.

Citations:


🏁 Script executed:

# Check if file exists and view the relevant lines
if [ -f "packages/feature-shell/src/data-access/use-shell-unlock-gate.tsx" ]; then
  echo "=== File found ==="
  wc -l "packages/feature-shell/src/data-access/use-shell-unlock-gate.tsx"
  echo ""
  echo "=== Lines 60-85 context ==="
  sed -n '60,85p' "packages/feature-shell/src/data-access/use-shell-unlock-gate.tsx"
else
  echo "File not found at specified path"
  # Try to find it
  find . -name "use-shell-unlock-gate.tsx" -type f 2>/dev/null
fi

Repository: samui-build/samui-wallet

Length of output: 682


🏁 Script executed:

head -50 "packages/feature-shell/src/data-access/use-shell-unlock-gate.tsx"

Repository: samui-build/samui-wallet

Length of output: 1740


Fail closed when unlock status is not explicitly true.

isLocked: statusQuery.data === false treats undefined (after query error) as unlocked, exposing app content while lock status is unknown. With retry: false, query failures leave data: undefined, causing the gate to fail open.

💡 Proposed fix
     state: {
       isChecking: statusQuery.isLoading,
-      isLocked: statusQuery.data === false,
+      isLocked: statusQuery.data !== true,
       isUnlocking: unlockMutation.isPending,
       walletName: wallet.name,
       walletProtectionMode: wallet.protectionMode,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
isChecking: statusQuery.isLoading,
isLocked: statusQuery.data === false,
isUnlocking: unlockMutation.isPending,
isChecking: statusQuery.isLoading,
isLocked: statusQuery.data !== true,
isUnlocking: unlockMutation.isPending,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/feature-shell/src/data-access/use-shell-unlock-gate.tsx` around
lines 72 - 74, The gate currently treats undefined as unlocked via `isLocked:
statusQuery.data === false`; change this to fail-closed by setting `isLocked:
statusQuery.data !== true` so only an explicit `true` marks unlocked. While
editing use-shell-unlock-gate.tsx, update the `isLocked` assignment that
references `statusQuery.data`, and optionally adjust `isChecking` (which uses
`statusQuery.isLoading`) to include `statusQuery.isError` if you want the UI to
remain in a checking state on query failures; keep `isUnlocking` tied to
`unlockMutation.isPending`.

walletName: wallet.name,
walletProtectionMode: wallet.protectionMode,
},
}
}
5 changes: 4 additions & 1 deletion packages/feature-shell/src/ui/shell-ui-layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { NavLink, Outlet } from 'react-router'
import { ShellUiCommandMenu } from './shell-ui-command-menu.tsx'
import { ShellUiMenu } from './shell-ui-menu.tsx'
import { ShellUiMenuActions } from './shell-ui-menu-actions.tsx'
import { ShellUiUnlockGate } from './shell-ui-unlock-gate.tsx'
import { ShellUiWarningExperimental } from './shell-ui-warning-experimental.tsx'

export interface ShellLayoutLink {
Expand Down Expand Up @@ -43,7 +44,9 @@ export function ShellUiLayout() {
</div>
</header>
<main className="flex-1 overflow-y-auto p-1 md:p-2 lg:p-4">
<Outlet />
<ShellUiUnlockGate>
<Outlet />
</ShellUiUnlockGate>
Comment on lines +47 to +49

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Gate modal routes behind unlock check

ShellUiUnlockGate is applied only around this layout’s <Outlet />, so it protects child routes under ShellUiLayout but not sibling top-level routes like /modals/* (defined in packages/feature-shell/src/create-router.tsx as a sibling of the layout route). As a result, a locked wallet can still navigate directly to modal screens (e.g. receive/send flows), which breaks the new “app-wide unlock gate” behavior and exposes app content outside the lock gate.

Useful? React with 👍 / 👎.

</main>
<footer className="flex items-center justify-between bg-secondary/30 pb-[env(safe-area-inset-bottom)]">
{links.map(({ icon, label, to }) => (
Expand Down
34 changes: 34 additions & 0 deletions packages/feature-shell/src/ui/shell-ui-unlock-gate.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { useTranslation } from '@workspace/i18n'
import { Button } from '@workspace/ui/components/button'
import { UiCard } from '@workspace/ui/components/ui-card'
import { UiLoaderFull } from '@workspace/ui/components/ui-loader-full'
import type { ReactNode } from 'react'
import { useShellUnlockGate } from '../data-access/use-shell-unlock-gate.tsx'

export function ShellUiUnlockGate({ children }: { children: ReactNode }) {
const { t } = useTranslation('shell')
const { actions, state } = useShellUnlockGate()

if (state.isChecking) {
return <UiLoaderFull />
}

if (!state.isLocked) {
return children
}

return (
<div className="flex min-h-full items-center justify-center p-4">
<UiCard
className="w-full max-w-xs"
contentProps={{ className: 'space-y-4' }}
description={t(($) => $.unlockGateDescription, { walletName: state.walletName })}
title={t(($) => $.unlockGateTitle)}
>
<Button disabled={state.isUnlocking} onClick={actions.unlock} type="button">
{t(($) => $.unlockGateAction)}
</Button>
</UiCard>
</div>
)
}
3 changes: 3 additions & 0 deletions packages/i18n/locales/en/shell.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
"labelSettings": "Settings",
"labelTools": "Tools",
"networkSettings": "Network settings",
"unlockGateAction": "Unlock",
"unlockGateDescription": "Unlock {{walletName}} to continue.",
"unlockGateTitle": "Wallet locked",
"walletEdit": "Edit wallet",
"walletSettings": "Wallet settings"
}
3 changes: 3 additions & 0 deletions packages/i18n/locales/es/shell.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
"labelSettings": "Ajustes",
"labelTools": "Herramientas",
"networkSettings": "Ajustes de red",
"unlockGateAction": "Desbloquear",
"unlockGateDescription": "Desbloquea {{walletName}} para continuar.",
"unlockGateTitle": "Billetera bloqueada",
"walletEdit": "Editar billetera",
"walletSettings": "Ajustes de la billetera"
}
3 changes: 3 additions & 0 deletions packages/i18n/src/resources.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,9 @@ export default interface Resources {
labelSettings: 'Settings'
labelTools: 'Tools'
networkSettings: 'Network settings'
unlockGateAction: 'Unlock'
unlockGateDescription: 'Unlock {{walletName}} to continue.'
unlockGateTitle: 'Wallet locked'
walletEdit: 'Edit wallet'
walletSettings: 'Wallet settings'
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ export function useVaultUnlockProvider(): VaultUnlockProviderValue {
confirmPassword,
confirmPasswordLabel: copy.confirmPasswordLabel,
credential,
credentialInputType: pending?.mode === 'pin' ? 'text' : 'password',
credentialInputType: 'password',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 PIN input type changed to 'password', inconsistent with request unlock dialog

The credentialInputType was changed from pending?.mode === 'pin' ? 'text' : 'password' to a hardcoded 'password'. This means PIN-protected wallets now show a masked password input (dots) in the vault unlock dialog instead of visible digits. This is inconsistent with packages/feature-request/src/ui/request-ui-unlock-dialog.tsx:35 which still uses type={state.mode === 'pin' ? 'text' : 'password'} along with inputMode="numeric" and pattern="[0-9]*" for PIN mode. The previous code deliberately differentiated between PIN and password modes; this change removes that distinction.

Suggested change
credentialInputType: 'password',
credentialInputType: pending?.mode === 'pin' ? 'text' : 'password',
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

credentialLabel: pending?.mode === 'pin' ? copy.pinLabel : copy.passwordLabel,
description: setupMode ? copy.setupDescription : (pending?.description ?? copy.defaultDescription),
error,
Expand Down
Loading