Drop vault: data minimisation (ADR-002)

[notedwin-dev]

Summary

Removes the encrypted-account vault and replaces it with a data-minimisation model. Per ADR-002, sensitive account fields (card number, CVV, expiry, holder name, account number) are dropped from the data model entirely. TOTP-derived encryption and WebAuthn-based biometric unlock for the vault are removed. The "Mask Mode" toggle stays as a UI-only flag (renamed from "Privacy Mode").

Why

The vault held a card number in the same trust boundary as the data it was supposed to protect. Even encrypted, the encryption key was derived from a TOTP secret stored in the user's Google Sheet, so the data was effectively plaintext to anyone with sheet access. ADR-002 captures the full reasoning and consequences.

What changes

Domain + types

• types.ts — drop AccountDetails; drop Account.details/isEncrypted; add Account.note?: string (unencrypted freeform); simplify UserCloudSettings to 4 fields
• src/lib/domain/migration.ts (new, temporary) — pure migrateSchemaV1toV2 + needsV1Migration predicate
• 33 new domain tests

Store rename

• usePrivacyStore → useMaskStore (file, types, barrel, tests)
• Vault fields dropped; maskMode boolean kept

Services + I/O

• services/sheets.services.ts — findUser / createUser no longer read/write totpSecret, isSecurityEnabled, isVaultLocked, vaultSalt, biometricCredId(s), devices, privacyMode; updateUser drops the array-merge branch
• services/storage.services.ts — KEYS.SECURITY removed; getStoredSecuritySettings / saveSecuritySettings deleted; profile defaults simplified
• services/auth.services.tsx — new profiles set schemaVersion: 2; useMaskStore wired; LegacyVaultProfile and vault branch removed
• services/security.services.ts, services/twofa.services.ts, services/biometric.services.ts — deleted (527 lines)

Application

• src/lib/application/commands/migration.ts (new, temporary) — runVaultSchemaMigration() orchestrator
• src/lib/application/commands/sync.ts — loadData no-decrypt; saveAccountSyncPost no-encrypt; _forceUnlock param
• src/lib/application/commands/accounts.ts — no encryptAccount call
• src/lib/application/commands/privacy.ts — deleted (479 lines)
• src/lib/application/commands.ts — privacy re-exports removed, runVaultSchemaMigration added

UI

• helpers/useAppInit.tsx — vault-lock effect removed; runVaultSchemaMigration wired into post-loadData .then()
• components/Profile.tsx (1441 → 603, −838) — 3 modals + Security & Biometrics section + vault handlers all removed
• pages/AccountPage.tsx (981 → 693, −288) — Identity Details block + Unlock Vault modal removed
• components/AccountForm.tsx (774 → 412, −362) — 5 sensitive inputs + Unlock Vault modal removed
• components/History.tsx, helpers/useMask.tsx, layouts/MainLayout.tsx, pages/ProfilePage.tsx — minor wiring updates

Migration strategy

1. First v2 load runs runVaultSchemaMigration() (idempotent, gated by profile.schemaVersion === 2)
2. Local accounts + profile are stripped of legacy fields and saved
3. A best-effort Sheets push writes the cleaned profile (skipped if offline / gapi not ready)
4. needsV1Migration predicate is checked on every sync to defend against a v1 client re-introducing legacy fields — no UI, no prompt, silent self-healing

The migration code carries a TEMPORARY header comment and is marked for removal in CONTEXT.md and docs/adrs/002-drop-vault-data-minimization.md (criterion: when no v1 clients are reachable and schemaVersion is no longer needed).

Validation

• npx tsc --noEmit — clean
• npx vitest run — 110/110 passing across 10 test files
• 33 new domain tests in migration.test.ts

Net diff

19 files changed, 525 insertions, 3471 deletions across 3 commits.

Refs: ADR-002, CONTEXT.md, issue #7

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
finance-tracker-app	Ready	Preview, Comment	Jun 5, 2026 8:40am

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: dd02799f-2d5f-4f8f-9416-2d0659844874

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/drop-vault

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

services/sheets.services.ts (2)

661-680: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Strip legacy sensitive account keys before regenerating sheet headers.

This still derives headers from combinedData verbatim. If any legacy Accounts row in Sheets carries removed fields like cardNumber, cvv, expiry, holderName, accountNumber, or details, those columns will be written straight back on every save and the minimisation migration never actually removes them.

🧹 Proposed fix

+		const LEGACY_ACCOUNT_FIELDS = new Set([
+			"cardNumber",
+			"cvv",
+			"expiry",
+			"holderName",
+			"accountNumber",
+			"details",
+		]);
+
+		const sanitizeRow = (item: any) => {
+			if (sheetName !== "Accounts") return item;
+			return Object.fromEntries(
+				Object.entries(item).filter(
+					([key]) => !LEGACY_ACCOUNT_FIELDS.has(key),
+				),
+			);
+		};
+
-		const combinedData = Array.from(mergedMap.values());
+		const combinedData = Array.from(mergedMap.values()).map(sanitizeRow);
@@
-		combinedData.forEach((item) => {
+		combinedData.forEach((item) => {
 			Object.keys(item).forEach((key) => {
 				headerSet.add(key);
 			});
 		});

🤖 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 `@services/sheets.services.ts` around lines 661 - 680, The current header/row
generation uses combinedData verbatim and will resurrect legacy sensitive
Account fields; before building headerSet and mapping rows (symbols:
combinedData, headerSet, headers, rowsToUpdate) filter out any legacy sensitive
keys (cardNumber, cvv, expiry, holderName, accountNumber, details — and any
other configured legacy sensitive names) from each item: create a sanitizedItems
= combinedData.map(item => sanitized copy without those keys), then compute
headerSet and rowsToUpdate from sanitizedItems (keeping the existing hasId logic
to force 'id' first); this ensures removed sensitive columns are not
reintroduced when regenerating the sheet.

---

388-405: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Initialize lastSyncAt when creating the Profile sheet.

findUser() now treats lastSyncAt as required, but Lines 391-405 still create a 9-column header without it. The first lookup on a fresh spreadsheet will immediately rewrite the header row and recurse.

🛠 Proposed fix

 			await window.gapi.client.sheets.spreadsheets.values.update({
 				spreadsheetId: fileId,
-				range: `'${sheetName}'!A1:I1`,
+				range: `'${sheetName}'!A1:J1`,
 				valueInputOption: "RAW",
 				resource: {
 					values: [
 						[
 							"email",
 							"password",
 							"name",
 							"createdAt",
 							"maskMode",
 							"schemaVersion",
 							"showAIAssistant",
 							"syncChatToSheets",
 							"lastUpdatedAt",
+							"lastSyncAt",
 						],
 					],
 				},
 			});
@@
 			if (h === "schemaVersion") return userData.schemaVersion ?? 2;
 			if (h === "showAIAssistant") return userData.showAIAssistant !== false;
 			if (h === "syncChatToSheets") return userData.syncChatToSheets !== false;
 			if (h === "lastUpdatedAt") return new Date().toISOString();
+			if (h === "lastSyncAt") return "";
 			return "";
 		});

Also applies to: 417-426

🤖 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 `@services/sheets.services.ts` around lines 388 - 405, The header creation for
the Profile sheet omits the required "lastSyncAt" column so findUser() treats it
as missing and rewrites the header; update the headers passed to
window.gapi.client.sheets.spreadsheets.values.update (the resource.values array
used when creating the Profile sheet) to include "lastSyncAt" and expand the
range (e.g., A1:J1) accordingly, and make the same change in the other
header-creation call later in the file (the second spreadsheets.values.update
block that builds profile/cloud headers) so both initial header rows include the
"lastSyncAt" column.

services/auth.services.tsx (1)

246-258: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clear the refresh token during logout.

Lines 247-248 only clear the active access-token path. google_refresh_token is still left in localStorage, and initAuth() will use it on the next app start to mint a fresh Sheets token even after logout.

🔒 Proposed fix

 const logout = () => {
   googleLogout();
   SheetService.clearGapiAccessToken();
+  localStorage.removeItem("google_refresh_token");
   const emptyProfile: UserProfile = {
     name: "",
     email: "",
     isLoggedIn: 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 `@services/auth.services.tsx` around lines 246 - 258, The logout function
currently clears the access token but not the persisted refresh token; update
the logout implementation (function logout) to also remove the Google refresh
token (key "google_refresh_token") from storage so initAuth() cannot reuse it
after logout—either call an existing StorageService.remove or add
StorageService.clearRefreshToken(), or call
localStorage.removeItem("google_refresh_token") before setting the empty profile
and resetting stores (after SheetService.clearGapiAccessToken() and before
StorageService.saveProfile()).

layouts/MainLayout.tsx (1)

303-315: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Give the mobile Mask Mode toggle an accessible name and state.

This is an icon-only custom toggle, so assistive tech currently gets an unnamed button with no on/off state. Add an explicit label plus aria-pressed or switch semantics.

♿ Suggested fix

             <button
               onClick={() => setMaskMode(!maskMode)}
+              aria-label={maskMode ? "Disable mask mode" : "Enable mask mode"}
+              aria-pressed={maskMode}
               className={`p-2 rounded-xl transition-all ${
                 maskMode
                   ? "bg-indigo-600 text-white shadow-[0_0_15px_rgba(79,70,229,0.4)]"

🤖 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 `@layouts/MainLayout.tsx` around lines 303 - 315, The mobile Mask Mode
icon-only toggle in MainLayout.tsx lacks an accessible name and state; update
the button (the element using setMaskMode and maskMode) to include an explicit
accessible label (e.g., aria-label or a visually hidden span text) and expose
state via aria-pressed="true|false" or role="switch" with
aria-checked={maskMode}; ensure the toggle still calls setMaskMode(!maskMode)
and that the label updates or is descriptive (e.g., "Mask mode" / "Toggle mask
mode") so screen readers receive both name and on/off state.

🤖 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 `@components/AccountForm.tsx`:
- Around line 366-372: The placeholder in the Notes textarea currently
encourages storing account identifiers ("last 4 digits") which is disallowed;
update the placeholder text in the textarea (the element using value={note} and
onChange={(e) => setNote(e.target.value)}) to a non-identifying example (e.g.,
"Optional notes (e.g. payment reference, internal memo)") and add a simple
client-side guard in the onChange handler or a validator used by setNote to
strip or reject obvious account identifiers (sequences of 3+ consecutive digits
or patterns like last XXXX) before saving to state so the field no longer
accepts or suggests storing sensitive account identifiers.

In `@components/Profile.tsx`:
- Around line 289-304: The toggle in Profile.tsx is mutating profile.maskMode
directly, causing divergence from the global mask state used by
helpers/useMask.tsx and layouts/MainLayout.tsx; switch the toggle to use the
centralized store instead: import the useMaskStore hook used elsewhere, read
maskMode from useMaskStore (instead of profile.maskMode) and onClick call the
store's updater (e.g., a provided toggle/setMaskMode function or
useMaskStore.setState({ maskMode: !maskMode })) so both controls share the same
source of truth; you may still call onUpdate to persist the profile if needed,
but the UI toggle must drive useMaskStore.maskMode as the primary updater.

In `@helpers/useAppInit.tsx`:
- Around line 29-32: The init chain currently calls loadData(profile).then(() =>
{ runVaultSchemaMigration(); }) which fires runVaultSchemaMigration() without
returning its promise; change this so the migration promise is returned/awaited
(e.g., return runVaultSchemaMigration() from the .then or use async/await) so
that any rejection flows into the same .catch() and preserves ordering between
loadData and runVaultSchemaMigration; update the call site in useAppInit.tsx
where loadData(profile).then(...) is used to return the migration promise.

In `@pages/AccountPage.tsx`:
- Around line 617-620: The account note is rendered raw (account.note) and
therefore bypasses Mask Mode; update the rendering to pass account.note through
the masking utility (e.g., maskText(account.note) or a note-specific masker that
preserves whitespace/pre-wrap), or use a masked renderer component when Mask
Mode is active so wrapped formatting is preserved; locate this in the JSX that
conditionally renders account.note and replace the raw insertion with the masked
output while keeping the existing className and whitespace-pre-wrap behavior.

In `@pages/ProfilePage.tsx`:
- Around line 41-43: The exported `data` object currently includes `accounts`
which may still carry legacy runtime keys (`details`, `isEncrypted`) even if TS
types were updated; before building `data` (or before JSON.stringify/export),
map over `accounts` and create a sanitized array that removes those legacy keys
(e.g., strip `details` and `isEncrypted` from each account) and use that
sanitized array in place of `accounts` so the export will not contain legacy
fields; update the code that constructs `data` (the `data` const) to use this
sanitized accounts array.

In `@src/lib/application/commands/sync.ts`:
- Around line 448-455: The mergedAccounts array must be sanitized for legacy v1
fields before persisting or re-uploading: after computing mergedAccounts
(variable mergedAccounts) run needsV1Migration on each account and, if needed,
pass them through migrateSchemaV1toV2 to produce a cleanedAccounts array, then
call store.setAccounts(cleanedAccounts) and await
StorageService.saveAccounts(cleanedAccounts) (and use cleanedAccounts for any
subsequent syncWithGoogleSheets upload). Apply the same sanitize-then-save/sync
change to the other merge/save block referenced (around the 521-526 region) so
no raw v1 fields are reintroduced.

In `@src/lib/domain/__tests__/migration.test.ts`:
- Line 44: Replace the PAN-like test fixture used when creating the account in
migration.test.ts: locate the asAccount call that assigns acc (asAccount({
details: { cardNumber: "4111111111111111", cvv: "123" } })) and change the
cardNumber value to a clearly non-card placeholder (for example
"NON_PAN_PLACEHOLDER" or "placeholder-card-number"); leave the asAccount call
and acc variable unchanged otherwise.

In `@src/lib/domain/migration.ts`:
- Around line 47-50: The migration currently drops the v1 preference
`privacyMode` and forces `maskMode` to false; update the logic around `cleaned`
(the result of stripFields(profile, VAULT_PROFILE_FIELDS)) so that when
`cleaned.maskMode` is undefined you fall back to the original
`profile.privacyMode` before defaulting to false. In other words, use
`profile.privacyMode` as a fallback to preserve existing user settings
(referencing `profile`, `cleaned`, `maskMode`, and `privacyMode`) and only set
`maskMode` to false if neither value exists.

---

Outside diff comments:
In `@layouts/MainLayout.tsx`:
- Around line 303-315: The mobile Mask Mode icon-only toggle in MainLayout.tsx
lacks an accessible name and state; update the button (the element using
setMaskMode and maskMode) to include an explicit accessible label (e.g.,
aria-label or a visually hidden span text) and expose state via
aria-pressed="true|false" or role="switch" with aria-checked={maskMode}; ensure
the toggle still calls setMaskMode(!maskMode) and that the label updates or is
descriptive (e.g., "Mask mode" / "Toggle mask mode") so screen readers receive
both name and on/off state.

In `@services/auth.services.tsx`:
- Around line 246-258: The logout function currently clears the access token but
not the persisted refresh token; update the logout implementation (function
logout) to also remove the Google refresh token (key "google_refresh_token")
from storage so initAuth() cannot reuse it after logout—either call an existing
StorageService.remove or add StorageService.clearRefreshToken(), or call
localStorage.removeItem("google_refresh_token") before setting the empty profile
and resetting stores (after SheetService.clearGapiAccessToken() and before
StorageService.saveProfile()).

In `@services/sheets.services.ts`:
- Around line 661-680: The current header/row generation uses combinedData
verbatim and will resurrect legacy sensitive Account fields; before building
headerSet and mapping rows (symbols: combinedData, headerSet, headers,
rowsToUpdate) filter out any legacy sensitive keys (cardNumber, cvv, expiry,
holderName, accountNumber, details — and any other configured legacy sensitive
names) from each item: create a sanitizedItems = combinedData.map(item =>
sanitized copy without those keys), then compute headerSet and rowsToUpdate from
sanitizedItems (keeping the existing hasId logic to force 'id' first); this
ensures removed sensitive columns are not reintroduced when regenerating the
sheet.
- Around line 388-405: The header creation for the Profile sheet omits the
required "lastSyncAt" column so findUser() treats it as missing and rewrites the
header; update the headers passed to
window.gapi.client.sheets.spreadsheets.values.update (the resource.values array
used when creating the Profile sheet) to include "lastSyncAt" and expand the
range (e.g., A1:J1) accordingly, and make the same change in the other
header-creation call later in the file (the second spreadsheets.values.update
block that builds profile/cloud headers) so both initial header rows include the
"lastSyncAt" column.

🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: 98aa2e37-7909-4d8b-9bec-3c3f9a4db4c8

📥 Commits

Reviewing files that changed from the base of the PR and between f07dd10 and a32ea14.

📒 Files selected for processing (28)

• components/AccountForm.tsx
• components/History.tsx
• components/Profile.tsx
• helpers/useAppInit.tsx
• helpers/useMask.tsx
• layouts/MainLayout.tsx
• pages/AccountPage.tsx
• pages/ProfilePage.tsx
• services/auth.services.tsx
• services/biometric.services.ts
• services/security.services.ts
• services/sheets.services.ts
• services/storage.services.ts
• services/twofa.services.ts
• src/lib/application/commands.ts
• src/lib/application/commands/__tests__/migration.test.ts
• src/lib/application/commands/accounts.ts
• src/lib/application/commands/migration.ts
• src/lib/application/commands/privacy.ts
• src/lib/application/commands/sync.ts
• src/lib/domain/__tests__/migration.test.ts
• src/lib/domain/migration.ts
• src/stores/__tests__/mask.store.test.ts
• src/stores/__tests__/privacy.store.test.ts
• src/stores/index.ts
• src/stores/mask.store.ts
• src/stores/privacy.store.ts
• types.ts

💤 Files with no reviewable changes (7)

• services/twofa.services.ts
• src/stores/privacy.store.ts
• src/stores/tests/privacy.store.test.ts
• services/biometric.services.ts
• components/History.tsx
• services/security.services.ts
• src/lib/application/commands/privacy.ts

📜 Review details

🧰 Additional context used

🪛 OpenGrep (1.22.0)

src/lib/domain/__tests__/migration.test.ts

[ERROR] 44-44: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.

(coderabbit.pii.credit-card-number)

🔇 Additional comments (9)

types.ts (1)

35-35: LGTM!

Also applies to: 86-87

src/stores/mask.store.ts (1)

3-21: LGTM!

src/stores/__tests__/mask.store.test.ts (1)

4-20: LGTM!

src/stores/index.ts (1)

4-5: LGTM!

src/lib/application/commands.ts (1)

12-12: LGTM!

src/lib/application/commands/migration.ts (1)

1-43: LGTM!

src/lib/application/commands/__tests__/migration.test.ts (1)

1-247: LGTM!

src/lib/application/commands/accounts.ts (1)

11-11: LGTM!

Also applies to: 27-27, 47-47

src/lib/application/commands/sync.ts (1)

104-105: LGTM!

Also applies to: 121-121, 368-372