[HIGH] fix(backup): back up salts of ledger approvers, not just creators

[elhoim]

BLUF

• Impact: a restored case can carry a verification ledger that nobody can verify — vhir review --findings --verify prints Skipped: No salt found for analyst <name> and the entire HMAC chain of custody for the case is silently unavailable.
• Root cause: the backup picks which examiner password-hash files to copy from each finding's created_by, but ledger entries are keyed by approved_by. Whenever the approver is not the stager, the approver's salt is never backed up.
• Blast radius: every case with a review workflow where one examiner stages and another approves — the normal senior-sign-off pattern. Also every case whose findings carry no created_by at all, which backs up zero hashes beside a full ledger. Timeline and IOC ledger entries were never covered by findings.json under any circumstance.
• Fix: 20 lines in one function — derive the examiner set from the distinct approved_by values in the ledger file already being copied, unioned with created_by so nothing that used to be backed up stops being backed up.
• Not security-sensitive: the missing salt makes verification skip, it does not let anything be forged or bypassed. Restore already installs every passwords/*.json it finds, so no restore-side change is needed.
• Evidence: new tests/test_backup.py (2 tests) fails on unmodified source with ['dana'] != ['dana', 'ravi'] and [] != ['ravi']; 430/430 green on Python 3.10 and 3.12, ruff clean.
• Re-checked on current main @ b2324ac (2026-10-05): the bug is still present — this PR's new tests, applied to main without the fix, still fail (2 failed). The branch now has main merged in.

Priority: HIGH

The defect

create_backup_data copies the HMAC verification ledger into the backup, then copies the examiner password-hash files needed to verify it. The two are selected from different fields:

# backup.py — the ledger that gets copied
ledger_path = VERIFICATION_DIR / f"{case_id}.jsonl"
...
# backup.py — but the salts that get copied
examiners_in_case = {
    f.get("created_by", "") for f in findings if f.get("created_by")
}

Ledger entries are written with approved_by, not created_by:

# approve.py:612
"approved_by": identity["examiner"],

and that is the only field verification ever looks at:

# verification.py:87
if entry.get("approved_by") != examiner:
    continue

So the backup copies a ledger signed by the approvers and the salts of the stagers.

Why it is wrong

Junior examiner dana stages findings (created_by = dana); senior examiner ravi approves them, so every entry in verification.jsonl has approved_by = ravi. The backup copies dana.json and the manifest advertises password_examiners: ["dana"]. On another machine, restore installs dana's hash and the ledger. Then:

$ vhir review --findings --verify
  Verifying entries for examiner 'ravi':
  Skipped: No salt found for analyst ravi

get_analyst_salt raises ValueError, review.py:407-408 swallows it, and the chain of custody for the whole case is unverifiable — from a backup that reported success and listed a password_examiners set. The same happens, more completely, whenever findings carry no created_by at all: zero hashes are backed up alongside a full ledger.

Consistency argument

approved_by is the field both consumers of the ledger filter on — verify_items (verification.py:87) and rehmac_entries (verification.py:127). It is also the only complete source: the ledger holds timeline and ioc entries as well as findings (approve.py derives item_type from the item id prefix), so findings.json could never account for every signer even in the single-examiner case. And main.py:108 documents --by as "Filter items by creator examiner", confirming created_by is the stager, not the approver.

The restore path needs no change: it globs backup_dir/passwords/*.json and installs each one without cross-checking names against findings, so once the salt is in the backup it is restored.

The fix

Derive the set from the ledger file that is actually being copied, and union it with created_by so the previous behavior is strictly preserved:

# Copy password hash files for every examiner who signed or staged items
password_examiners: list[str] = []
examiners_in_case: set[str] = set()
# Ledger signers first — approved_by is what verify_items keys on, so
# without their salt the restored ledger cannot be verified at all.
try:
    if ledger_path.is_file():
        for line in ledger_path.read_text().splitlines():
            ...
            if isinstance(entry, dict) and entry.get("approved_by"):
                examiners_in_case.add(entry["approved_by"])
except OSError:
    pass  # best-effort

followed by the unchanged created_by block, now calling .update() on the same set, and the unchanged copy loop. Reading ledger_path rather than verification.read_ledger() is deliberate: ledger_path is the exact file being copied, so the derived set is by construction the set of signers present in the backup. Each of the three stages keeps its own best-effort try/except, so a ledger read failure cannot abort the pre-existing created_by copies.

Testing

New tests/test_backup.py, two tests, monkeypatching backup._PASSWORDS_DIR and backup.VERIFICATION_DIR at temp paths so nothing touches /var/lib/vhir:

• test_backup_includes_approver_password_hash — findings staged by dana, ledger approved by ravi; asserts both salts land in the backup and in the manifest.
• test_backup_includes_approver_without_created_by — findings with no created_by; asserts the approver's salt is still backed up.

Against unmodified source (verified by reverting the source file and re-running only these tests):

E       AssertionError: assert ['dana'] == ['dana', 'ravi']
tests/test_backup.py:69: AssertionError
E       AssertionError: assert [] == ['ravi']
tests/test_backup.py:87: AssertionError
2 failed in 0.27s

With the fix: 430 passed on Python 3.10 and Python 3.12 (428 baseline + 2), ruff check and ruff format --check clean.

---

CI status

The checks on this PR will show action_required: GitHub gates workflow runs on pull requests from
first-time fork contributors until a maintainer approves them. They need an Approve and run click.

The CI job was reproduced locally after merging main @ b2324ac (761 tests on main), using the same steps as .github/workflows/ci.yml:

Step	Result
pip install -e ".[dev]" && pytest tests/ — Python 3.10	763 passed
pip install -e ".[dev]" && pytest tests/ — Python 3.11	763 passed
pip install -e ".[dev]" && pytest tests/ — Python 3.12	763 passed
ruff check . + ruff format --check . (latest ruff, unpinned as in CI)	check + format clean

🤖 Generated with Claude Code

[elhoim]

[pr-sweep] withdrawn: upstream b100718 (backups hold no credential, restores add keys as retired) made a case backup carry no password file at all, by design: the signed history carries the public keys that verify it. There is no salt selection left for this PR to fix, and adding approver salts back would go against that decision.