rubyforgood / rubyforgood/alongwithyou

SQLCipher build/key mismatch: verify catalog.db opens unkeyed, and fail loudly when the build has no SQLCipher

Open
#130 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug needs-verification severity:significant
Dominant language
TypeScript
Stars
9
Forks
4
Avg merge
9h 34m
Merged PRs (30d)
16

Description

Description

ADR 0016 (lands with #128) flags a gap that nothing has tested:

useSQLCipher is a build-wide flag, not a per-database one. 0013 puts two databases on the device: journal.db encrypted, and catalog.db — the public drug catalog — deliberately not, because it is public data and treating it otherwise buys nothing. Turning SQLCipher on links the whole app against it, so catalog.db gets opened by a SQLCipher build with no PRAGMA key set. Reading a plaintext file that way is ordinary SQLCipher behaviour, but Expo's documentation does not cover it and nothing here has tested it.

mobile/app.json now sets "useSQLCipher": true, so this applies as soon as #128 merges.

This cannot be covered by the jest suite: jest/in-memory-sqlite.ts runs node:sqlite, which is stock SQLite with no SQLCipher build, and says so at the top of the file. It needs a device or simulator.

0016's words: "a cheap check that would be expensive to discover late." If a SQLCipher-linked build cannot read an unencrypted catalog.db, the on-device drug search in 0013 needs rearchitecting, and that is much better known now than after the Medicine Diary is built on top of it.


Added scope: the inverse case — a build with no SQLCipher at all

Added from the code review of #128. Kept here rather than split out, because both halves are the same question in opposite directions and one device session covers both.

The check above asks what a SQLCipher build does with no key. The mirror image is untested and strictly more dangerous: what a non-SQLCipher build does with a key.

Build PRAGMA key Question
Original scope SQLCipher not set Can it read a plaintext catalog.db?
Added here no SQLCipher set Does it silently write a plaintext journal.db?

mobile/src/lib/db/database.ts:53-58 keys the connection and then probes the schema:

// Touch the schema to force SQLCipher to actually decrypt a page. Without
// this, a wrong key is not discovered until the first real query...
await db.getFirstAsync('SELECT count(*) FROM sqlite_master');

That probe does not do what the comment claims. SQLite ignores unrecognised pragmas rather than erroring, so on a build without SQLCipher PRAGMA key is a silent no-op and the probe succeeds regardless. Verified against stock SQLite:

PRAGMA key on plain SQLite: ACCEPTED SILENTLY (no error)
sqlite_master probe still works: {"c":0}
PRAGMA cipher_version -> null

The result is an unencrypted medical journal, written with no error, no warning, and nothing observable to distinguish it from the encrypted case. Ways to be in that build: Expo Go (config plugins do not apply there, and mobile/README.md still tells contributors to use it until #129 lands), a stale ios/ or android/ directory from a prebuild predating #128, or a cached EAS build.

This is the same silent downgrade database.ts:42 already refuses to allow on web, and it is exactly what ADR 0017 means by "Failing loudly is better than a fallback that is wrong." Detection is one statement — PRAGMA cipher_version returns a row on a SQLCipher build and nothing on stock SQLite:

const cipher = await db.getFirstAsync<{ cipher_version: string }>('PRAGMA cipher_version');
if (!cipher) {
  throw new DatabaseUnavailableError(
    'This build has no SQLCipher, so the journal would be stored unencrypted. Run `npx expo prebuild` and use a development build — Expo Go cannot run this app.'
  );
}

Placed before PRAGMA key, that message doubles as the onboarding hint #129 is about.

Sequencing note: unlike the catalog.db verification, this half is a code change that should ship whatever the device check finds, and it matters now rather than at Medicine Diary time. See the amended note under Additional Info.

Acceptance Criteria
  • On a development build with useSQLCipher: true, open a plaintext (unencrypted) SQLite file with no PRAGMA key set and confirm reads succeed
  • Confirm FTS5 queries work against that plaintext database (0013's search is FTS5, and enableFTS is set in app.json)
  • Verified on both iOS and Android — the SQLCipher build differs per platform
  • Result recorded on this issue; if it fails, open a follow-up against 0013's on-device search design before Medicine Diary work starts
  • The feature/s being implemented are covered by unit tests - If not, create tests for them on this ticket (not unit-testable — needs hardware; see above)

Guard against a non-SQLCipher build (added scope)

  • getJournalDatabase() refuses to open when the running build has no SQLCipher — PRAGMA cipher_version returns no row
  • The check runs before PRAGMA key, and its error names the remedy (prebuild + development build; Expo Go cannot run this app)
  • The comment at database.ts:55-57 is corrected: the sqlite_master probe detects a wrong key, not a missing SQLCipher
  • Unit test covers the guard — unlike the catalog.db check, this half is testable in jest by stubbing the pragma result
Additional Info and Resources
  • Related: #101 (general on-device verification) — this is a specific, separable check, split out so it does not wait on a full manual QA pass
  • SQLCipher's documented behaviour is that an unkeyed connection reads plaintext databases normally, but this is unverified for Expo's expo-sqlite SQLCipher build specifically
  • The catalog.db verification blocks nothing today; it should be resolved before #101 / Medicine Diary implementation depends on it. The non-SQLCipher guard added above is not deferrable in the same way — it is a code change that should land with or shortly after #128, because until it exists there is no way to tell an encrypted journal from an unencrypted one
QA
  • Run a dev build on an iOS simulator/device, open a plaintext test .db, read and FTS-query it
  • Repeat on Android
  • Confirm journal.db still opens correctly with its key in the same build
  • In the same dev build, confirm PRAGMA cipher_version returns a version string and journal.db opens normally
  • Temporarily force the guard to fire and confirm the error message is actionable for a contributor who landed in Expo Go

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in mobile/src/lib/db/database.ts around lines 42 and 53-58, then inspect mobile/app.json and the SQLite limitation noted in jest/in-memory-sqlite.ts. Add coverage for the SQLCipher availability guard and run the relevant Jest tests. Done means the guard rejects non-SQLCipher builds, while iOS and Android development builds can read and FTS5-query plaintext catalog.db and still open keyed journal.db.

Written by the indexing model from the issue text.

Assessment

Tech stack
sqlite, typescript
Domain
database, mobile, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.