Skip to content

backup --verify-integrity reports [OK] when it verified nothing: checkUser() returns ok:true with checked:0 whenever the check cannot run #124

Description

@perki

We deploy open-pryv.io at Health Data Safe. On our first restore test we saw
--verify-integrity print [OK] <user> — events=0 accesses=0 while the target
PostgreSQL held 9,118 events and 68 accesses. The restore itself was correct; the
verification was the problem.

We initially assumed this was specific to restoring a single-user archive. Reading the
code, it is not — it is general.

Where it comes from

components/business/src/integrity/IntegrityCheck.ts:

  • checkUser() only calls _checkUserEvents / _checkUserAccesses when
    integrity.isActive.events / .accesses are true (L67, L71). When integrity is
    inactive neither runs, and nothing records that it didn't.
  • _checkUserEvents returns early when the events store is absent or has no
    exportAll (L104) — and events is declared optional in the StorageLayer
    type (L18), so that is an anticipated state, not an anomaly.
  • Both methods return early on a null/undefined export.

ok is then derived only from error counts (L74):

report.ok = report.events.errors.length === 0 && report.accesses.errors.length === 0;

Nothing checked → no errors → ok: true.

bin/backup.js (L222) prints that as:

  [OK] <username> — events=0 accesses=0

with no separate branch for "nothing was verified".

Why we think it is worth fixing

--verify-integrity is documented as "Verify integrity hashes after restore; roll
back on failure"
(bin/backup.js L469), and it is the only automated post-restore
assurance. As written, a restore that verified nothing is indistinguishable from one
that verified everything — both print [OK] and both exit 0. That is the case where
an operator most needs to be told something is wrong.

This is a missing positive control, which is the same principle you applied in #116
("a positive control in each block, so the suite cannot pass by dropping everything").
That framing is what made us look here at all.

Suggested shape

Make ok require that something was actually checked, and give "not verified" its own
output line and exit status, distinct from [OK] — e.g. [NOT VERIFIED] <user> — integrity inactive vs [SKIPPED] when the store is unavailable. The distinction that
matters to an operator is verified-and-clean vs not-verified-at-all.

Happy to send a PR if you would like it in that shape, or a different one.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions