Skip to content

Make backup keygen cancellable and clear retained plaintext passwords - #564

Merged
bradleymackey merged 1 commit into
mainfrom
fix/backup-keygen-cancel
Sep 15, 2026
Merged

bradleymackey merged 1 commit into
mainfrom
fix/backup-keygen-cancel

Conversation

@bradleymackey

Copy link
Copy Markdown
Member

Problem

The backup key-change screen advertised "up to 3 minutes" of key derivation but was uncancellable in practice:

  • The Cancel toolbar button was .disabled(isLoading) — disabled exactly while the keygen ran, with interactiveDismissDisabled also active, so the .keygenCancelled state was unreachable from the UI.
  • onDisappear did not cancel keyGenerationTask, so a dismissed view could still complete store(backupPassword:) in the background and silently replace the user's backup password.
  • Even a cancelled task would complete the store: the KDF body is synchronous (cancellation can't interrupt it) and there was no cancellation check between keygen and store.
  • The plaintext password fields were cleared only on the success path — retained in the view model on keygen error, cancellation, and after the view disappeared.

Fix

  • Cancel stays enabled during .creating (comment documents why); swipe-dismiss remains blocked.
  • onDisappear cancels the in-flight keygen task before resetting state.
  • try Task.checkCancellation() after the KDF returns and before the derived key replaces the stored password — cancellation is now authoritative; worst case the CPU work completes in the detached task and is discarded, leaving the old backup password intact.
  • Entered passwords cleared in didDisappear() and on the keygen-error/cancelled paths. Deliberately retained on confirm-mismatch (user is mid-correction, view still frontmost — documented inline).

Tests

  • saveEnteredPassword_cancelledBeforeStore_setsKeygenCancelledAndDoesNotStore (store mock set never called)
  • saveEnteredPassword_cancelled_clearsEnteredPasswords, _keygenError_clearsEnteredPasswords, _passwordConfirmError_retainsEnteredPasswords, didDisappear_clearsEnteredPasswords
  • New snapshot layoutCreatingState (light/dark): view pinned in .creating via a blocking test deriver, wrapped in a NavigationStack so the toolbar renders — the enabled Cancel button is the point of the image.

Local verification: VaultFeedTests + VaultiOSTests schemes passed on iPhone 18 Pro Max / iOS 27.0 (snapshot recorded, then clean pass).

⚠️ Automatic CI is still disabled (#548), so this is local verification only.

🤖 Generated with Claude Code

Cancel was disabled exactly while the up-to-3-minute key derivation
ran, with interactive dismissal also blocked, so the keygenCancelled
state was unreachable. A dismissed view could still complete the key
change because onDisappear never cancelled the task and the sync KDF
cannot observe cancellation — check it explicitly before storing the
derived key. Clear the entered plaintext passwords on every exit path
except mid-correction confirm-mismatch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bradleymackey
bradleymackey merged commit 5cf4f64 into main Sep 15, 2026
@bradleymackey
bradleymackey deleted the fix/backup-keygen-cancel branch September 15, 2026 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant