Backup check reads each archive once, not twice

CHANGELOG.md

@@ -6,6 +6,12 @@ replacing a module has to be a decision with visible consequences.
 
 ## Unreleased
 
+- **Backup verification reads each archive once instead of twice.** `GET /backups` hashed every
+  snapshot file and then read every archive member's payload to the end, doubling the disk reads
+  of large model archives on each request. The hash already proves the bytes match the manifest,
+  and the tar header walk rejects the same truncated archives, so members are now checked by
+  their headers, plus the first 16 bytes of each required SQLite store.
+
 - **`/processes` no longer returns command lines, and wrong keys no longer cost PBKDF2.**
   `/processes` listed each host process's command line, which can carry a password or
   a key; it now returns the same metadata as `/runtime` (pid, name, user, CPU, memory).

docs/backups.md

@@ -15,9 +15,10 @@ never extracted. A snapshot changing during verification is rejected.
 `unavailable`, an empty root reports `empty`, and rejected candidates make the response `degraded`.
 An incomplete directory does not become a recovery point merely because it has a recent name.
 
-Verification reads the snapshot files in full on each request; large model archives can make
-this endpoint slow. Poll it deliberately rather than at dashboard animation frequency. Responses
-include `checked_at` and per-snapshot `verified_at`; there is no stale success cache. Hashes establish
+Verification hashes each snapshot file in full once per request; archive members are checked by
+their headers, plus the first 16 bytes of each required SQLite store. Large model archives can
+still make this endpoint slow. Poll it deliberately rather than at dashboard animation frequency.
+Responses include `checked_at` and per-snapshot `verified_at`; there is no stale success cache. Hashes establish
 consistency with the manifest, not authenticity against an attacker who can rewrite both.
 `restore_status: not_tested` is explicit: this telemetry does not test decryption, restore a stack,
 replay later deletions or reconcile actions. Manifest deletion-ledger and execution-journal states

systemgate/backups.py

@@ -76,12 +76,13 @@ def _archive(path: Path, required: str | None) -> None:
             seen.add(member.name)
             if member.name == required and not member.isfile():
                 _invalid("Required SQLite store is not a file; obtain an intact snapshot.")
-            if member.isfile():
+            # The manifest hash already proved these bytes, and walking to the
+            # next header rejects archives cut inside a member's data, so only
+            # the required store's SQLite magic needs a payload read.
+            if member.isfile() and member.name == required:
                 with archive.extractfile(member) as stream:
-                    if member.name == required and stream.read(16) != b"SQLite format 3\x00":
+                    if stream.read(16) != b"SQLite format 3\x00":
                         _invalid("Required SQLite store is invalid; obtain an intact snapshot.")
-                    while stream.read(1024 * 1024):
-                        pass
     if required and required not in seen:
         _invalid("Required SQLite store is missing; obtain an intact snapshot.")
 

tests/test_backups.py

@@ -131,6 +131,54 @@ def test_even_hash_matching_unsafe_archives_fail(tmp_path, member_name, kind):
     assert backups.backup_status(tmp_path)["latest"] is None
 
 
+def test_a_truncated_archive_is_never_a_recovery_point(tmp_path):
+    directory = snapshot(tmp_path)
+    buffer = io.BytesIO()
+    with tarfile.open(fileobj=buffer, mode="w") as archive:
+        payload = b"SQLite format 3\x00" + b"x" * 5000
+        member = tarfile.TarInfo("pi.db")
+        member.size = len(payload)
+        archive.addfile(member, io.BytesIO(payload))
+        tail = tarfile.TarInfo("data")
+        tail.size = 1024
+        archive.addfile(tail, io.BytesIO(b"y" * 1024))
+    data = buffer.getvalue()
+    # pi.db's 5016 bytes occupy 5120 padded, so the cut lands 100 bytes into the
+    # second member's payload.
+    (directory / "pi.tar").write_bytes(data[:512 + 5120 + 512 + 100])
+    save_manifest(directory)
+    result = backups.backup_status(tmp_path)
+    assert result["latest"] is None
+    assert [row["name"] for row in result["rejected"]] == ["snapshot-a"]
+
+
+def test_a_required_store_that_is_not_sqlite_is_never_a_recovery_point(tmp_path):
+    directory = snapshot(tmp_path)
+    with tarfile.open(directory / "pi.tar", "w") as archive:
+        member = tarfile.TarInfo("pi.db")
+        member.size = 16
+        archive.addfile(member, io.BytesIO(b"not a database!!"))
+    save_manifest(directory)
+    result = backups.backup_status(tmp_path)
+    assert result["latest"] is None
+    assert [row["name"] for row in result["rejected"]] == ["snapshot-a"]
+
+
+def test_only_the_required_store_payload_is_opened(tmp_path, monkeypatch):
+    snapshot(tmp_path)
+    opened = []
+    original = tarfile.TarFile.extractfile
+
+    def record(archive, member):
+        opened.append(member.name)
+        return original(archive, member)
+
+    monkeypatch.setattr(tarfile.TarFile, "extractfile", record)
+    result = backups.backup_status(tmp_path)
+    assert result["latest"] is not None and result["latest"]["status"] == "verified"
+    assert sorted(opened) == ["pi.db", "toolgate.db"]
+
+
 def test_read_errors_are_unverifiable_without_leaking_payload(tmp_path, monkeypatch):
     snapshot(tmp_path)
     monkeypatch.setattr(backups, "_hash", lambda path: (_ for _ in ()).throw(