fix(settings): blur recovery phrase by default#2658
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRecoveryPhrasePanel now requires an explicit reveal before showing a generated recovery phrase: the phrase area is blurred and non-interactive until the user clicks a "Reveal recovery phrase" overlay. The copy button is disabled until the phrase is revealed. Translation keys for the reveal label were added across i18n chunks. ChangesPrivacy-protected recovery phrase reveal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@app/src/components/settings/panels/RecoveryPhrasePanel.tsx`:
- Around line 279-283: In RecoveryPhrasePanel replace the hard-coded aria-label
on the reveal button (the element that calls setRevealed(true)) with a localized
string via the component's useT() hook (e.g.
t('settings.recovery.revealButton')), ensure useT is imported/used in the
component, and add the new key "settings.recovery.revealButton": "Reveal
recovery phrase" to the English translations file en.ts so the aria-label is
produced via t(...) instead of a literal string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2c936230-a670-4288-be84-511f589e5e7e
📒 Files selected for processing (1)
app/src/components/settings/panels/RecoveryPhrasePanel.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/components/settings/panels/RecoveryPhrasePanel.tsx (1)
47-54:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset
revealedstate when switching modes to prevent privacy bypass.The
switchModecallback resets several states but does not resetrevealed. This creates a privacy loophole:
- User reveals the recovery phrase in "generate" mode (
revealed = true)- User switches to "import" mode (phrase not shown in import mode, but
revealedstaystrue)- User switches back to "generate" mode
- Recovery phrase is immediately visible without requiring a new explicit reveal action
This defeats the one-way reveal protection introduced in this PR.
🔐 Proposed fix to reset revealed state
const switchMode = useCallback((nextMode: 'generate' | 'import') => { setMode(nextMode); setConfirmed(false); setError(null); setImportValid(null); setSelectedWordCount(IMPORT_SLOTS_INITIAL); setImportWords(Array(IMPORT_SLOTS_INITIAL).fill('')); + setRevealed(false); }, []);🤖 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 `@app/src/components/settings/panels/RecoveryPhrasePanel.tsx` around lines 47 - 54, The switchMode callback (function switchMode) resets several states but fails to reset the revealed flag, allowing a previously revealed recovery phrase to reappear when toggling modes; update switchMode to also set revealed to false (i.e., call setRevealed(false)) whenever switching modes so the phrase remains hidden until the user explicitly re-reveals it.
🧹 Nitpick comments (1)
app/src/components/settings/panels/RecoveryPhrasePanel.tsx (1)
78-93: ⚡ Quick winConsider adding a
revealedguard inhandleCopyfor defense in depth.While the copy button is correctly disabled when
!revealed(line 303), thehandleCopyfunction itself does not check the reveal state. This means programmatic calls (e.g., via developer tools or future code changes) could bypass the privacy protection.🛡️ Proposed defense-in-depth guard
const handleCopy = useCallback(async () => { + if (!revealed) return; try { await navigator.clipboard.writeText(mnemonic); setCopied(true);🤖 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 `@app/src/components/settings/panels/RecoveryPhrasePanel.tsx` around lines 78 - 93, Add a defense-in-depth guard in the handleCopy function to ensure it only copies when the recovery phrase is revealed: at the start of handleCopy check the revealed boolean (the same prop/state used to disable the button) and return early if not true; keep existing logic that uses mnemonic and setCopied unchanged so programmatic calls cannot bypass the UI guard.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@app/src/components/settings/panels/RecoveryPhrasePanel.tsx`:
- Around line 47-54: The switchMode callback (function switchMode) resets
several states but fails to reset the revealed flag, allowing a previously
revealed recovery phrase to reappear when toggling modes; update switchMode to
also set revealed to false (i.e., call setRevealed(false)) whenever switching
modes so the phrase remains hidden until the user explicitly re-reveals it.
---
Nitpick comments:
In `@app/src/components/settings/panels/RecoveryPhrasePanel.tsx`:
- Around line 78-93: Add a defense-in-depth guard in the handleCopy function to
ensure it only copies when the recovery phrase is revealed: at the start of
handleCopy check the revealed boolean (the same prop/state used to disable the
button) and return early if not true; keep existing logic that uses mnemonic and
setCopied unchanged so programmatic calls cannot bypass the UI guard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 135597d5-7f78-4011-bec0-4afdc1691ab9
📒 Files selected for processing (2)
app/src/components/settings/panels/RecoveryPhrasePanel.tsxapp/src/lib/i18n/en.ts
✅ Files skipped from review due to trivial changes (1)
- app/src/lib/i18n/en.ts
|
so fly!! |
2 participants
Fixes #2657
Added a default blur/reveal flow for the recovery phrase screen in RecoveryPhrasePanel.tsx.
This prevents the recovery phrase from being visible or copied immediately when opening the panel.
Screenshots
Summary by CodeRabbit
New Features
Chores
Tests