# Read-only health reconciliation security review

Use this addendum when reviewing an offline reconciliation tool that reads private health databases/workbooks, emits private detail reports plus versioned aggregate evidence, and optionally performs an exact-link migration.

## Review sequence

1. Diff from the requested base and include untracked files. Re-run `git status --short` at the end because concurrent edits can appear during the audit. If the worktree is active, re-read every cited code/evidence line and regenerate the aggregate comparison immediately before reporting; do not preserve a finding that a concurrent remediation already resolved.
2. Separate three boundaries:
   - source audit: database opened with SQLite URI `mode=ro` and `PRAGMA query_only=ON`;
   - private artifacts: detailed values/IDs remain outside Git with enforced private filesystem semantics;
   - optional migration: explicitly gated and transactionally fail-closed.
3. Trace workbook helpers to ensure a function named “parse” or “best reference” does not save, recalculate, or rewrite the workbook.
4. Scan every changed/versioned Markdown and JSON file for raw values, parameter names, document names, paths, source IDs, and other PHI. Aggregate counts, fixed contract names, and approved digests may be allowed only when the release contract explicitly says so.
5. Verify server and browser query handling independently. Server parsers must reject unknown and duplicate scalar parameters. Browser URL restoration must reject duplicate controlled keys rather than silently taking the first value.
6. Trace document-original probing. A helper with a `reviewed` gate must receive the real review state; passing `reviewed=True` merely to compute coverage bypasses the boundary and may open/read unreviewed originals.
7. For link candidates, require existing exact immutable relations only. Do not infer links from filename, date, institution, category, OCR text, or similarity. Bind confirmation to the exact ordered candidate set and current database state.

## Deterministic private-artifact probes

Do not infer privacy from `os.open(..., 0o600)`. Test:

- new destination mode;
- existing `0644` destination (creation mode does not chmod it);
- symlink destination (`O_TRUNC` follows it without `O_NOFOLLOW`);
- destination inside the repository;
- concurrent/same-second filename collision;
- exception path before final chmod.

Safe pattern: require canonical containment beneath a dedicated private root outside Git; reject symlinked directories/targets; create a unique temporary inode with `O_NOFOLLOW | O_EXCL` and mode `0600`; write, flush, `fsync`, and atomically rename; verify final owner/type/mode.

SQLite backups contain full PHI. Pre-create the backup destination exclusively as `0600` before SQLite opens it, restrict the backup directory outside Git, and secure or remove incomplete backups on every exception path. A success-only `chmod(0600)` is insufficient.

## Transactional migration counterexample

A `with connection:` block commits when the block exits. Therefore this shape is unsafe:

1. update inside `with connection:`;
2. leave block (commit);
3. run integrity/digest checks;
4. raise on failed checks.

A trigger can mutate validation fields; the function then reports failure while both intended and unintended mutations remain committed. Reproduce with a synthetic database and an `AFTER UPDATE` trigger that changes a protected field.

Safe shape:

1. `BEGIN IMMEDIATE` **before** capturing the transaction's pre-state digests, so concurrent changes cannot land between digest capture and lock acquisition;
2. verify candidate/state digest and preconditions;
3. perform bounded updates;
4. check exact row counts, protected-field digest, intended-column-only changes, `integrity_check`, and `foreign_key_check` before commit;
5. rollback on any exception;
6. commit only after all checks pass.

Do not define “protected state” as only the edited table minus the intended column. An `AFTER UPDATE` trigger can mutate a document/review table, audit table, or any other writable table while the edited laboratory row still passes its digest. Build a deterministic counterexample trigger that changes a medically meaningful field in a second table; the migration must raise and the live database must retain its complete pre-state. Prefer a schema-aware whole-database logical digest with an explicit allowance for only the ordered target `(table, primary key, column, old, new)` changes, or equivalently verify every table and schema object that is not allowed to change.

Exercise the second idempotent run only after the first transaction succeeds. Distinguish a second SQL pass inside one transaction from a genuine second invocation when naming evidence.

## Canonical discrepancy and staging-plan coverage

When workbook and canonical/API rows are compared, map **both** through the same production allowlist identity helper before comparing parameter/unit/date/value. Add explicit probes for percent spellings (`%` versus `percent`) and both micro characters (`µ` U+00B5 and `μ` U+03BC); counting raw allowlist keys instead of unique public contracts can overcount aliases.

Report three separate counts when applicable: all canonical discrepancies, unique actionable staging candidates, and excluded/non-actionable discrepancies. If incomplete rows or duplicates are omitted from the executable plan, persist per-item exclusion records with stable IDs and reason codes in the private artifact; an aggregate difference such as `228 total / 204 actionable` is not item-level review coverage. Every emitted candidate must remain review-only (`verified_against_original=0`, non-`scanned_original` provenance) unless separately verified against an approved original.

For record-labs versus series parity, test above any legacy page boundary (commonly 100 rows) and assert observations, history metadata, and series points agree. Avoid fixing count drift by silently truncating one surface; either return the complete bounded set or fail both surfaces under the same explicit limit contract.

## Backup versus restore evidence

Opening a backup read-only and running `PRAGMA integrity_check` is a backup-integrity check, not a restore test. A real restore test restores into a separate temporary database, runs integrity/foreign-key checks there, and compares a pre-migration logical/schema digest. Label evidence accurately until that flow exists.

## Reporting

Rank concrete findings by blocker/high/medium/low and include `file:line`, violated property, consequence, and remediation. Keep successful evidence separate. Green tests do not override a design-level failure. For a read-only review, do not modify repository files; temporary synthetic databases outside the repository are acceptable for deterministic counterexamples.