Normalize killphrase queries so trailing whitespace still fires - #561
Merged
Merged
Conversation
Digests were built from trimmed phrases but matched against the raw search-bar text, so a trailing space silently stopped a killphrase from firing. Trim in the digester on both sides (no case/canonical fold — existing digests were computed without one), and pass the sanitized query to the deleter so match input equals the search predicate input. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bradleymackey
added a commit
that referenced
this pull request
Sep 15, 2026
## Changes (test-only) — final PR of the pre-release audit series (#561–#572) Snapshot coverage for security-relevant screens rebuilt in #551–#560 that had no view tests: - **`BackupKeyDecryptorViewSnapshotTests`** — initial state (light/dark × 3 type sizes, in a NavigationStack so the Cancel toolbar renders) plus a deterministic decrypt-failure state: the failure is produced by actually running `attemptDecryption()` with the fast testing deriver and an erroring decoder mock, not by faking view state. - **`BackupImportFlowViewSnapshotTests`** — all three `BackupImportContext` variants (empty vault / merge / override), light/dark. - **`VaultDetailEncryptionEditViewSnapshotTests`** — encryption-disabled (full grid) and encryption-enabled variants. - `SettingsDangerView` was covered in #565; `AutoBackupSettingsView` is deliberately not given a standalone suite — it is already snapshotted transitively through `BackupCreateView` (`BackupViewSnapshotTests`), and its enabled/error states are only reachable through an async `.task` handoff that would flake under synchronous snapshot rendering. Driving those states needs a small initial-state injection refactor — left as follow-up. Release gate: full `CI_iOS` scheme (all 13 test targets, `iOSAllTests` plan including the TSAN configuration) run locally on iPhone 18 Pro Max / iOS 27.0.⚠️ Automatic CI is still disabled (#548), so this is local verification only. --- ## Release findings — report-only (no code in this series) The pre-release audit surfaced the following items that need **design decisions**, not patches. Recorded here so they are not lost: **MANIFESTO C7 gaps (protective defaults):** - Clipboard paste TTL defaults to never-expire (`PasteTTL.default = nil`) — copied OTPs/passwords sit on the pasteboard indefinitely unless the user opts in to a TTL. - No screenshot / app-switcher privacy protection anywhere (no `privacySensitive()`, no capture detection, no cover view). - Danger Zone full wipe has no confirmation dialog — one tap + biometric. **Design-level:** - Backups export killphrase/search-passphrase salts+digests; anyone holding the backup password can enumerate which items are duress-protected (C5 tension). - The killphrase/search-passphrase HMAC keys are device-local and not exported, so a restore onto a new device silently disarms every killphrase and permanently hides `.onlyPassphrase` items (rows exist, digests unverifiable). - No app-level lock / auto-lock; background purge clears only the backup password from memory. - Killphrase-triggered auto-backup + widget reload is an out-of-band success signal for a hidden item's deletion (C2 tension). - `payloadHash` and `lastBackupHash` live in plaintext UserDefaults — mutation-time evidence (C6 tension). - `deleteVault()` does not refresh the auto-backup hash, so the newest auto-backup still describes the wiped vault (recovery safety net vs C6 — decide). - `DerivedEncryptionKey.debugDescription` prints raw key material as hex; keychain replace (remove→store) is non-atomic; killphrase/passphrase edit fields are plain `TextField` not `SecureField`; the `vault://` HOTP-increment deep link is unauthenticated; `Data.random` relies on `SystemRandomNumberGenerator` (CSPRNG on Apple platforms, but unannotated as the app's sole randomness source); no `protectedDataWillBecomeUnavailable` handling. **Hygiene (non-blocking):** CI triggers commented out; CHANGELOG ~9 versions stale vs MARKETING_VERSION 2.0; hardcoded strings in rebuilt screens bypass the string catalogs (app is currently English-only, so cosmetic); stale scheme/test-plan references (`CI_iOS` scheme, orphan `VaultUITests` scheme) and a stale snapshot directory; `VaultBackup.xcstrings` not declared as a target resource; the keygen speedtest CLI prints a derived key in hex; feed search reload has no debounce/cancellation; `ForEach` identity built from `Hasher().finalize()`; reorder persist failures are swallowed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Killphrase digests are created from trimmed phrases (
VaultDataModelEditorAdapter), but matching used the rawitemsSearchQuerywhile the search predicate used the trimmeditemsSanitizedQuery. A trailing space from the iOS keyboard meant the search matched visually but the killphrase silently never fired.Fix
KillphraseDigesternow normalizes (trims whitespace/newlines) in bothmakeDigestandmatches. Backward compatible: all persisted digests were already computed from trimmed phrases, and trim is idempotent. No case/canonical fold — existing digests were computed without one and killphrases stay exact-match otherwise.VaultDataModel.reloadItems()passes the sanitized query to the deleter, so match input equals the search-predicate input.Tests
VaultDataModel: untrimmed search query reaches the deleter sanitized; new fail-safe negatives — deleter is never invoked when the digester was never loaded or the key store fails (previously untested lock-state behaviour).deleteItems(matchingKillphrase: "phrase ")deletes an item whose phrase is"phrase".Local verification: VaultFeedTests scheme, 852 tests in 72 suites, all passed on iPhone 18 Pro Max / iOS 27.0.
🤖 Generated with Claude Code