Add encrypted storage and biometric unlock foundation by PaulgSmith · Pull Request #128 · rubyforgood/alongwithyou

PaulgSmith · 2026-08-13T02:58:32Z

What this does

Encrypts the journal database with SQLCipher and puts a biometric gate in front of the app. This is the storage foundation the Phase 1 screens sit on — key management, schema migrations, a typed repository layer, and the unlock gate — plus four decision records for the choices building it forced.

0015 — where the SQLCipher key lives. A 256-bit CSPRNG key in Keychain/Keystore via expo-secure-store with WHEN_UNLOCKED_THIS_DEVICE_ONLY, deliberately without requireAuthentication, and the biometric prompt applied separately at the app shell. requireAuthentication binds the key to biometric enrollment, so adding a fingerprint or re-enrolling Face ID makes it unreadable — and per 0001 there is no server copy, so that is not an inconvenience but the permanent loss of someone's medical journal. (0007) accepts "lose the phone, lose the journal" and requires telling people about it; nobody has told anyone "add a fingerprint, lose the journal". The trade this makes — key protected by the device lock rather than bound in the secure element — is stated plainly in the record and is the part most worth a second pair of eyes.

0016 — development builds required. SQLCipher is a native fork of SQLite, so npx expo prebuild and a dev build are now the way to run this app; Expo Go is not. The alternative that preserved Expo Go was field-level encryption over plain SQLite, rejected because it is a hand-rolled scheme protecting medical data maintained by rotating volunteers, and because encrypted values cannot be indexed or sorted.

0017 — journal data is native-only. SQLCipher has no web build, so the journal refuses to open in the browser rather than silently falling back to unencrypted browser storage on whatever machine someone happened to use. Web stays the landing surface and the unlock gate passes it through.

0018 — deliberately left Open. A phone with neither biometric nor passcode has nothing to prompt with, and 0007 has no answer for it. The code explains the situation and lets the user continue; that is a placeholder chosen so the gap is visible, not a decision, and it needs a named owner.

Issue

Addresses #26 and #19 — 0015 resolves the key-storage half of the local-database-encryption question that both reference as living in architecture-plan.md.

Not closing either: all four records are Proposed or Open and need sign-off.

Follow-ups the records flag, now filed:

#129 — mobile/README.md still tells contributors to use Expo Go. 0016 asks for the docs to change with it and that has not happened yet, so on merge the onboarding instructions describe something that no longer works (mobile/README.md:13, :31, :53), for exactly the newcomers 0008 designed a gentle first ticket for. Worth landing with or close behind this PR.
#131 — 0018 needs a named owner, in the way 0009 has Decide who owns hosting, support, incident response, and long-term maintenance #23. The "explain and continue" behaviour shipping here is a placeholder, not the decision.
#130 — useSQLCipher is a build-wide flag, so catalog.db (0013) gets opened by a SQLCipher build with no PRAGMA key set. That is ordinary SQLCipher behaviour but is untested here, and cheap to check now versus expensive to discover late. 0016 names Manually test the app on a simulator or device #101 as the home for this; it is split out so it does not wait on a full manual QA pass, and Manually test the app on a simulator or device #101 is cross-referenced.
#115 — WHEN_UNLOCKED_THIS_DEVICE_ONLY keeps the key out of iCloud/Google backups; the database file itself still needs the equivalent.

Testing

Covered by tests

cd mobile && npm test — 69 tests pass across 8 suites. Of the new ones, that covers key generation and persistence, database open/close, the unlock flow, and the unlock gate component.

Two caveats worth stating rather than burying:

migrations.test.ts and repository.test.ts did not run locally. They use node:sqlite, which needs Node 22.5+; this machine is on Node 18. CI is on node-version: 22.x, so they run there — this PR is also the first real exercise of that (#99).

Related, and a genuine rough edge: jest/in-memory-sqlite.ts exports a HAS_NODE_SQLITE guard, but the static import { DatabaseSync } from 'node:sqlite' throws before the guard is ever evaluated. So on any Node below 22.5 those two suites error out instead of skipping — including Node 20, which engines currently permits (>=20.19.4). Filed as #133 rather than fixed here, to keep this PR to the storage foundation — happy to fold the lazy-require fix in instead if reviewers prefer.
Nothing here tests SQLCipher. node:sqlite is stock SQLite, so the tests verify schema, queries, ordering, constraints and transaction rollback against a real engine — but encryption has no Node build and needs hardware. That is Manually test the app on a simulator or device #101, and the helper says so at the top so nobody mistakes a green suite for verified encryption.

Not run locally: npm run lint and npm run typecheck also want a newer Node than this machine has (#100 covers the lint half of that), so CI is the check for both.

expo-sqlite, expo-secure-store, expo-local-authentication and expo-crypto, all on the SDK 57 line, plus the config plugins they need. useSQLCipher is what makes the journal database encrypted at rest. It is a build-level flag rather than a runtime one, so it requires `npx expo prebuild` and a development build, and it ends Expo Go for local development. That cost is real and is recorded in 0016 rather than left for the next person to discover. enableFTS is set explicitly even though it already defaults to true. 0013's drug search is SQLite FTS5, so an edit that switched it off would break the Medicine Diary while looking like a storage-layer change — not somewhere anyone would think to look. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The encrypted local database and the unlock gate that the rest of Phase 1 sits on. Neither existed: the decision for both lives in architecture-plan.md and was referenced by 0003 and 0007, but was never turned into implementation work, so every Phase 1 list screen was blocked on something with no ticket. src/lib/db holds the storage layer. key.ts generates a 256-bit key and keeps it in Keychain/Keystore; database.ts opens the file, keys it before anything else touches the connection, and migrates; migrations.ts runs each step in its own exclusive transaction with the user_version bump, so an interrupted upgrade leaves a version that matches the schema; and repository.ts is the repeatable-entry data layer that contacts, providers, allergies and the rest are all variants of, per 0008. The key is deliberately stored without SecureStore's requireAuthentication. That option invalidates the stored value when biometric enrollment changes, which on a device holding the only copy of the data (0001) turns adding a fingerprint into silent, permanent loss of someone's journal. The biometric gate is applied separately at the app shell instead, which is what 0007 actually asks for. The trade is written up in 0015 and there is a test asserting the option stays off. The database refuses to open on web rather than falling back to plain SQLite, since a quietly unencrypted medical journal is worse than a loud failure. No domain tables ship here — the first one belongs to the first feature that needs it, #118. 95 tests. The repository and migration suites run real SQL through node:sqlite rather than asserting on generated strings, so ordering, constraints and rollback are genuinely exercised. What none of it proves is that the file is actually encrypted; that needs hardware and is the gap #101 tracks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Four choices surfaced while building the storage foundation. None were settled anywhere, and three of them are the kind that look like implementation detail in a diff and like a policy decision six months later, so they are recorded rather than left in the code. 0015 — Where the database key lives, and why it is stored without SecureStore's requireAuthentication. Expo's docs are explicit that a value stored that way becomes inaccessible when biometric settings change, and with no server copy (0001) that makes adding a fingerprint destroy the journal. 0007 accepts "lose the phone, lose the journal" and requires telling users so; nobody has told anyone about the fingerprint case. Needs technical-lead sign-off, and states plainly what the trade costs. 0016 — Development builds are now required and Expo Go no longer runs the app. Records the alternative that would have preserved it (app-level field encryption) and why it loses. Also notes that useSQLCipher is build-wide rather than per-database, so 0013's public catalog.db gets opened by a SQLCipher build with no key set — ordinary SQLCipher behaviour, but undocumented by Expo and untested here, so it is flagged for on-device verification before the Medicine Diary depends on it. 0017 — Journal data is native-only; the database fails loudly on web instead of falling back to an unencrypted one. Scoped to patient data: 0013's public drug catalog is explicitly unaffected. 0018 — Unlock behaviour on a device with no lock screen, left Open. 0007 was written about lockout and assumes there is a device lock to fall back to. Four options with their tradeoffs, and the current "explain and continue" behaviour marked as a placeholder rather than a decision. Needs a named owner, like 0009. The README's "Related, not duplicated here" note claimed local database encryption lived only in architecture-plan.md. The direction still does; what that document left unspecified is now in-repo, so the note points at 0015-0017. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* update brakeman version * switch to double qoutes * bump sqlite

Jberma23 · 2026-08-16T19:29:34Z

Reviewed this properly rather than skimming — the ADRs made that unusually easy.
On the two things you asked for:

Verification. I ran the full suite on Node 22.20.0, which closes both of your "not run locally" caveats: npx jest → 10 suites, 95 tests, all pass; npx tsc --noEmit clean; npx expo lint clean. The arithmetic lines up with your run — the two suites you couldn't execute hold 9 + 17 = 26 tests, and 69 + 26 = 95. So migrations.test.ts and repository.test.ts do pass on 22.x.

The 0015 trade. I'd ratify it. Expo's types confirm your mechanism exactly — requireAuthentication maps to biometryCurrentSet on iOS, which invalidates on any enrollment change. For a local-only journal with no recovery path, trading that confidentiality property against a silent, permanent, user-triggered data-loss bomb is the right way round, and I wouldn't relitigate it.

Where I'd push is that the ADR reasons carefully about choosing the trade and then doesn't follow through on what it implies. Two of those are line comments above (the restore-to-new-phone path, and the key living in the heap for process lifetime). Two more are ADR text:

Add a threat model. "Needs technical-lead sign-off" isn't answerable as written, because there's nothing to sign off against. Three rows would do it, and one matters: someone holding the unlocked phone gets the key, and nothing here stops them. Given 0004's caregiver role and 0006's excluded-field boundary, that's a person this team already designs around rather than an abstract attacker — and it's the case requireAuthentication would partly have covered. Naming it plainly makes the sign-off a real decision.
Record what would change the answer. Availability has to win only because 0001 says no server copy and nothing yet gives users a copy of their own. Once export / [FEATURE] Exclude app data directory from OS auto-backup (iOS + Android) #115 puts a passphrase-protected backup in the user's hands, key loss stops being terminal and stronger binding becomes affordable. That turns 0015 from "please approve this" into "correct until X exists" — much easier to sign.

On #133: I'd fold the lazy-require fix in here. It's four lines, and leaving it means anyone on Node 20, which engines permits and sees two red suites on the PR that introduces them.
Already filed from the rest of the review: #134 (repository layer — update() breaks when destructured, create() returns data the DB doesn't hold), #135 (destroyJournalDatabase can report success when the file wasn't deleted), #136 (iOS app-switcher snapshot). I also expanded #130 to cover the inverse of its original question — a build with no SQLCipher silently writing a plaintext journal.

#134 is the one I'd consider pulling into this PR rather than leaving as a follow-up: it's ~20 lines, and #118 builds directly on that layer, so fixing it here means the broken update() never exists on main.

Two gaps between what 0015 decided and what the code does. The first is the case 0015 designs for. It keeps the key WHEN_UNLOCKED_THIS_DEVICE_ONLY precisely so it does not travel in an iCloud backup - but journal.db lives in Documents and does. Restoring a backup onto a new phone, which is what people do when they replace a handset, brings the file back without the key. Nothing handled that. SecureStore returns null both for "nothing stored yet" and for "the entry was invalidated", key.ts read null as a first run and minted a fresh key, and database.ts asked for the key before opening the file so it could not know better. The result was a generic "Could not open the journal database" on that launch and every launch after, with no explanation and nowhere to go. getOrCreateDatabaseKey now reports whether it minted the key, and database.ts uses that: a key we just created cannot fail to read a file we just created, so if the decrypt probe fails the file was already there and belonged to a key that is gone. That is now UnrecoverableJournalError rather than a generic failure, and the useless key is deleted so the next launch reaches the same branch instead of degrading to something undiagnosable. The journal really is unrecoverable - 0007 commits to saying so during onboarding - but "made on a different phone" is a true thing to say and leaves the user somewhere to go. The UI for starting over is not built; this is the mechanism it needs. The second gap is that the unlock gate locked the view and nothing else. database.ts held a decrypted handle and the key stayed in memory for the life of the process, which made WHEN_UNLOCKED_THIS_DEVICE_ONLY a property of the first launch and nothing after it, because the keychain was never asked again. The gate now closes the database on the same background event that re-locks. Also corrects two comments in key.ts that were wrong rather than merely thin. keychainAccessible is iOS-only, so THIS_DEVICE_ONLY keeps the key out of iCloud but does nothing about Google backup - that comes from the expo-secure-store config plugin's backup rules, which makes the bare plugin entry in app.json load-bearing. And WHEN_PASSCODE_SET_THIS_DEVICE_ONLY is recorded as considered and rejected: it looks like a free upgrade, but it cannot store a key at all on the devices 0018 is about, and it turns removing a passcode into a data-loss event. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Two defects in the repeatable-entry layer that #118 and #49 are both built on. Neither was caught by the tests, and neither is caught by TypeScript. update() reached its sibling method through `this`, which is only bound when it is called as repo.update(...). `const { update } = repo` and `onPress={repo.update}` are both ordinary React and both threw "TypeError: find is not a function" at runtime. find is now a plain function above the returned object, so nothing in the repository depends on how it was called. create() built its return value from the caller's input rather than from what it wrote. A field the caller omitted was stored as NULL and returned as absent, so create() and find() disagreed about the same row; and keys that were never columns were dropped on insert but echoed back, making them look saved. It now returns the declared fields and only those, with omissions turned into the NULL that actually goes to the database. Nothing consumes createRepository yet, so this is not a regression - but the cost of fixing it rises with every screen written against the old behaviour, and #118 is next. Fixes #134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

HAS_NODE_SQLITE was meant to let the SQLite-backed suites skip on a Node without node:sqlite. It could never do that: the static import is resolved before any module code runs, so on Node below 22.5 the module threw at import time and the try/catch around the guard was never reached. The two suites errored out instead of skipping, on a version engines still permits (>=20.19.4). CI runs 22.x, which is exactly why it could sit unnoticed. The require is now lazy, so the guard evaluates, and createInMemoryDatabase fails with a message naming the Node version it needs rather than a resolution error from inside node_modules. Adds a test for the helper itself. Making the require throw is the only way to exercise the old-Node path from a new Node, and without it nothing in CI would notice this regressing back - which is how it got here. Fixes #133 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Jberma23 · 2026-08-16T19:53:00Z

Added some fixes for the things I commented on here #138

The README got someone as far as a simulator and stopped. For this app that is not far enough: the unlock path in 0015 cannot be checked in a simulator at all, because simulators do not enforce biometric authentication when retrieving a stored secret - expo-secure-store's own documentation says so - and nothing in the test suite shows the database is encrypted either, since node:sqlite is stock SQLite. A green simulator run and a green test run together still leave the two things this foundation exists for unverified. So: the device path, and the fact that a free Apple ID is enough for it. There is nothing here needing a paid account - no push, no app groups, no associated domains - and that is worth saying, because assuming otherwise is the kind of thing that quietly stops a volunteer testing on the phone in their pocket. Signing, trusting the certificate, going wireless after the first cable build, the seven-day expiry, and expo-dev-client for when the Mac's address on the network keeps moving. Also warns that prebuild stops and asks for a bundle identifier, because app.json does not set one. Whatever gets typed at that prompt is written to app.json and becomes the app's permanent identity, so the README now says not to invent one. Better still would be setting it deliberately in app.json, which belongs with #128 rather than here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ng note Two follow-ups to #138. The fix for #133 makes the SQLite-backed suites skip on a Node without node:sqlite, but the test added to prove it asserts HAS_NODE_SQLITE is true with no guard. On Node 20.19.4 - permitted by `engines`, and the exact range #133 is about - the fix therefore fails its own regression test while the suites it protects skip cleanly. Verified on Node 18: before, 1 failure; after, 109 tests with 2 suites skipped. The absent direction is not fakeable, so it takes the same `describeSql` guard migrations.test.ts and repository.test.ts already use. The file-before-key comment on destroyJournalDatabase described neither interrupted state: file first leaves a key with nothing to open, not "an unreadable database". #138 supplies the reason it was missing - key first would strand a file no key can open, which is UnrecoverableJournalError, reached by the very path destroy is meant to be the escape from. Reworded here rather than deferred to #135, which is about destroy reporting success without deleting and does not touch the ordering rationale. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

PaulgSmith and others added 3 commits August 13, 2026 02:32

PaulgSmith marked this pull request as draft August 13, 2026 03:01

update brakeman version (#137)

5f13bad

* update brakeman version * switch to double qoutes * bump sqlite