Annie Voigt commited on
Commit ·
b05aecf
1
Parent(s): b83539c
feat(integrity): wire ADR-0010 on-load verify + re-pin 0.1.9 (deploy slice)
Browse filesAdds the ADR-0010 runtime on top of the ADR-0011 upload gate for a clean deploy
off origin/main (NO ADR-0007 sandbox stack): resolve_to_local_path verifies a
materialized file against the manifest SHA-256 baseline (biodata-registry 0.1.9)
before parse/cache, refusing a tampered file and deleting a bad temp download.
Re-applied surgically onto origin/main so the cached-huggingface-cli-token
fallback (data_io) and the tzlocal>=5.4.3 limma fix (requirements) are preserved
— not regressed by the older feat/sandbox-executor base. Re-pin 0.1.8 -> 0.1.9.
test_integrity.py 9 + test_upload_gate.py 27 green; verified end-to-end.
- docs/adr/ADR-0010-dataset-integrity-verification.md +25 -4
- requirements.in +1 -1
- requirements.txt +1 -1
- src/core/data_io.py +33 -3
- tests/test_integrity.py +143 -0
docs/adr/ADR-0010-dataset-integrity-verification.md
CHANGED
|
@@ -1,6 +1,6 @@
|
|
| 1 |
# ADR-0010 — Dataset Integrity / Tamper Verification on Load
|
| 2 |
|
| 3 |
-
**Status:**
|
| 4 |
**Date:** 2026-07-01
|
| 5 |
**Deciders:** Annie Voigt (project lead)
|
| 6 |
**Driver:** OHSU security review Q7 — "detect unavailable/modified/compromised datasets." Today
|
|
@@ -48,14 +48,35 @@ Add **content-hash verification on load**, keyed off the manifest.
|
|
| 48 |
SHA-256 verify in `resolve_to_local_path`; refusal path + test (good hash loads, altered file
|
| 49 |
refused, absent hash = load with a "no integrity baseline" note); backfill hashes for the
|
| 50 |
current registered datasets.
|
| 51 |
-
*(
|
| 52 |
-
|
| 53 |
-
|
| 54 |
2. **At AWS (ADR-0007 Phase 4):** the pre-stage sync job writes checksums into the read-only OHSU
|
| 55 |
source bucket, so integrity is anchored to an OHSU-controlled copy rather than trust-on-first-use
|
| 56 |
against the public source. This ADR's on-load check is the same code; only the hash's provenance
|
| 57 |
improves.
|
| 58 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 59 |
## Consequences
|
| 60 |
|
| 61 |
- **Positive:** closes the Q7 tamper gap with a deterministic control, independent of AWS; dovetails
|
|
|
|
| 1 |
# ADR-0010 — Dataset Integrity / Tamper Verification on Load
|
| 2 |
|
| 3 |
+
**Status:** Accepted — step 1 (AWS-independent) implemented 2026-07-02
|
| 4 |
**Date:** 2026-07-01
|
| 5 |
**Deciders:** Annie Voigt (project lead)
|
| 6 |
**Driver:** OHSU security review Q7 — "detect unavailable/modified/compromised datasets." Today
|
|
|
|
| 48 |
SHA-256 verify in `resolve_to_local_path`; refusal path + test (good hash loads, altered file
|
| 49 |
refused, absent hash = load with a "no integrity baseline" note); backfill hashes for the
|
| 50 |
current registered datasets.
|
| 51 |
+
*(The shared hashing helper `src/core/integrity.py` — `compute_sha256` / `verify_sha256`,
|
| 52 |
+
streamed — landed with ADR-0011 and is the exact low-level code this step's on-load layer
|
| 53 |
+
builds on.)*
|
| 54 |
2. **At AWS (ADR-0007 Phase 4):** the pre-stage sync job writes checksums into the read-only OHSU
|
| 55 |
source bucket, so integrity is anchored to an OHSU-controlled copy rather than trust-on-first-use
|
| 56 |
against the public source. This ADR's on-load check is the same code; only the hash's provenance
|
| 57 |
improves.
|
| 58 |
|
| 59 |
+
### Step 1 — as implemented (2026-07-02)
|
| 60 |
+
|
| 61 |
+
- **Manifest field** (`biodata-registry` 0.1.9): optional `integrity:` block on `DatasetManifest`
|
| 62 |
+
— `sha256` (primary/expression file) + optional `files:` map (per-file, for separate metadata)
|
| 63 |
+
+ `recorded`/`recorded_from` provenance. Validated (64-hex) in `manifest.validate()`; absent =
|
| 64 |
+
valid + a "no integrity baseline" warning. Reverse-lookup helper `expected_sha256_for_url()`.
|
| 65 |
+
- **Recorder** `scripts/record_integrity.py` streams SHA-256 of each source and writes the block
|
| 66 |
+
into the YAML (comment-preserving text edit). **Backfilled 21/22** registered datasets;
|
| 67 |
+
`cptac_pda_counts` skipped — its hosted `cptac_pda_counts.h5ad` is **404 on HF** (pre-existing
|
| 68 |
+
missing-file issue, tracked separately).
|
| 69 |
+
- **On-load verify** (`DecoupleRpy_Agent` `src/core/integrity.py`): `verify_file()` is called
|
| 70 |
+
centrally in `resolve_to_local_path` after materialize / before parse+cache. Keyed on the source
|
| 71 |
+
URL via the registry reverse-lookup, so **no tool signature changes**. Mismatch → `IntegrityError`
|
| 72 |
+
refusal naming the dataset; a tampered temp download is unlinked before raising; missing/absent
|
| 73 |
+
baseline degrades to "load unverified". Verified end-to-end (real load passes, tampered copy
|
| 74 |
+
refused).
|
| 75 |
+
- **Not covered by step 1:** `src/workflows/geo.py` direct GEO-series-matrix loads bypass
|
| 76 |
+
`resolve_to_local_path` (dataset-prep path, not runtime) — add the same hook there if a manifest
|
| 77 |
+
ever points `expression_source.url` at a GEO matrix. Trust-on-first-use caveat below still applies
|
| 78 |
+
until step 2.
|
| 79 |
+
|
| 80 |
## Consequences
|
| 81 |
|
| 82 |
- **Positive:** closes the Q7 tamper gap with a deterministic control, independent of AWS; dovetails
|
requirements.in
CHANGED
|
@@ -25,7 +25,7 @@
|
|
| 25 |
# so git+https://huggingface.co/... fails the Space build. To release new
|
| 26 |
# manifests: bump version in biodata-registry/pyproject.toml, run its
|
| 27 |
# scripts/release.sh, then update this URL + commit.
|
| 28 |
-
biodata-registry @ https://huggingface.co/anne-voigt/biodata-registry/resolve/
|
| 29 |
|
| 30 |
# --- GEO platform (GPL) SOFT parsing (scripts/assemble_*.py precompute) ---
|
| 31 |
GEOparse==2.0.4
|
|
|
|
| 25 |
# so git+https://huggingface.co/... fails the Space build. To release new
|
| 26 |
# manifests: bump version in biodata-registry/pyproject.toml, run its
|
| 27 |
# scripts/release.sh, then update this URL + commit.
|
| 28 |
+
biodata-registry @ https://huggingface.co/anne-voigt/biodata-registry/resolve/3a86474d4971f3590fa44b042ab46196bdd9247e/biodata_registry-0.1.9-py3-none-any.whl
|
| 29 |
|
| 30 |
# --- GEO platform (GPL) SOFT parsing (scripts/assemble_*.py precompute) ---
|
| 31 |
GEOparse==2.0.4
|
requirements.txt
CHANGED
|
@@ -44,7 +44,7 @@ backports-tarfile==1.2.0
|
|
| 44 |
# via jaraco-context
|
| 45 |
beartype==0.22.9
|
| 46 |
# via py-key-value-aio
|
| 47 |
-
biodata-registry @ https://huggingface.co/anne-voigt/biodata-registry/resolve/
|
| 48 |
# via -r requirements.in
|
| 49 |
brotli==1.2.0
|
| 50 |
# via gradio
|
|
|
|
| 44 |
# via jaraco-context
|
| 45 |
beartype==0.22.9
|
| 46 |
# via py-key-value-aio
|
| 47 |
+
biodata-registry @ https://huggingface.co/anne-voigt/biodata-registry/resolve/3a86474d4971f3590fa44b042ab46196bdd9247e/biodata_registry-0.1.9-py3-none-any.whl
|
| 48 |
# via -r requirements.in
|
| 49 |
brotli==1.2.0
|
| 50 |
# via gradio
|
src/core/data_io.py
CHANGED
|
@@ -29,6 +29,9 @@ def resolve_to_local_path(path_or_url: str) -> tuple[str, bool]:
|
|
| 29 |
"""
|
| 30 |
s = str(path_or_url)
|
| 31 |
if not s.startswith(("http://", "https://", "ftp://")):
|
|
|
|
|
|
|
|
|
|
| 32 |
return s, False
|
| 33 |
|
| 34 |
# Authenticated path for private HF repos.
|
|
@@ -43,6 +46,7 @@ def resolve_to_local_path(path_or_url: str) -> tuple[str, bool]:
|
|
| 43 |
except Exception:
|
| 44 |
token = None
|
| 45 |
if token:
|
|
|
|
| 46 |
try:
|
| 47 |
from huggingface_hub import hf_hub_download
|
| 48 |
|
|
@@ -61,9 +65,14 @@ def resolve_to_local_path(path_or_url: str) -> tuple[str, bool]:
|
|
| 61 |
revision=revision,
|
| 62 |
token=token,
|
| 63 |
)
|
| 64 |
-
return local, False # HF cache file — caller must not delete it
|
| 65 |
except Exception:
|
| 66 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 67 |
|
| 68 |
suffix = Path(s.split("?", 1)[0]).suffix or ".h5ad"
|
| 69 |
with tempfile.NamedTemporaryFile(suffix=suffix, delete=False) as tmp:
|
|
@@ -73,4 +82,25 @@ def resolve_to_local_path(path_or_url: str) -> tuple[str, bool]:
|
|
| 73 |
if not chunk:
|
| 74 |
break
|
| 75 |
tmp.write(chunk)
|
| 76 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 29 |
"""
|
| 30 |
s = str(path_or_url)
|
| 31 |
if not s.startswith(("http://", "https://", "ftp://")):
|
| 32 |
+
# Local paths are still integrity-verified when the manifest baselines
|
| 33 |
+
# this exact path (rare, but keeps the check uniform); no-op otherwise.
|
| 34 |
+
_verify_or_raise(s, s, is_temp=False)
|
| 35 |
return s, False
|
| 36 |
|
| 37 |
# Authenticated path for private HF repos.
|
|
|
|
| 46 |
except Exception:
|
| 47 |
token = None
|
| 48 |
if token:
|
| 49 |
+
local = None
|
| 50 |
try:
|
| 51 |
from huggingface_hub import hf_hub_download
|
| 52 |
|
|
|
|
| 65 |
revision=revision,
|
| 66 |
token=token,
|
| 67 |
)
|
|
|
|
| 68 |
except Exception:
|
| 69 |
+
local = None # fall through to unauthenticated urllib (e.g. public file)
|
| 70 |
+
|
| 71 |
+
# Verify OUTSIDE the try above so an IntegrityError is never swallowed
|
| 72 |
+
# into the urllib fallback (which fetches the same tampered content).
|
| 73 |
+
if local is not None:
|
| 74 |
+
_verify_or_raise(local, s, is_temp=False)
|
| 75 |
+
return local, False # HF cache file — caller must not delete it
|
| 76 |
|
| 77 |
suffix = Path(s.split("?", 1)[0]).suffix or ".h5ad"
|
| 78 |
with tempfile.NamedTemporaryFile(suffix=suffix, delete=False) as tmp:
|
|
|
|
| 82 |
if not chunk:
|
| 83 |
break
|
| 84 |
tmp.write(chunk)
|
| 85 |
+
tmp_path = tmp.name
|
| 86 |
+
# Verify before returning so a tampered download is never handed to a loader
|
| 87 |
+
# or admitted to the cache; a bad temp file is deleted here, not leaked.
|
| 88 |
+
_verify_or_raise(tmp_path, s, is_temp=True)
|
| 89 |
+
return tmp_path, True
|
| 90 |
+
|
| 91 |
+
|
| 92 |
+
def _verify_or_raise(local_path: str, url: str, *, is_temp: bool) -> None:
|
| 93 |
+
"""Run the ADR-0010 integrity check; delete a bad temp file before re-raising.
|
| 94 |
+
|
| 95 |
+
No-op when the URL has no recorded baseline. On a hash mismatch the load is
|
| 96 |
+
refused (:class:`IntegrityError`); for a temp file we unlink first so the
|
| 97 |
+
tampered bytes are never left on disk or admitted to the cache.
|
| 98 |
+
"""
|
| 99 |
+
from src.core.integrity import IntegrityError, verify_file
|
| 100 |
+
|
| 101 |
+
try:
|
| 102 |
+
verify_file(local_path, url)
|
| 103 |
+
except IntegrityError:
|
| 104 |
+
if is_temp:
|
| 105 |
+
Path(local_path).unlink(missing_ok=True)
|
| 106 |
+
raise
|
tests/test_integrity.py
ADDED
|
@@ -0,0 +1,143 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
"""Tests for on-load content-integrity verification (ADR-0010).
|
| 2 |
+
|
| 3 |
+
Covers the ``src.core.integrity`` primitive (streamed SHA-256, mismatch refusal,
|
| 4 |
+
no-baseline pass-through) and its wiring into ``resolve_to_local_path`` — that a
|
| 5 |
+
good hash loads, a tampered file is refused, and the tampered temp file is deleted
|
| 6 |
+
(never handed to a loader or cache). The registry reverse-lookup is monkeypatched
|
| 7 |
+
so no network or installed manifests are required.
|
| 8 |
+
"""
|
| 9 |
+
from __future__ import annotations
|
| 10 |
+
|
| 11 |
+
import hashlib
|
| 12 |
+
import sys
|
| 13 |
+
from pathlib import Path
|
| 14 |
+
|
| 15 |
+
import pytest
|
| 16 |
+
|
| 17 |
+
sys.path.insert(0, str(Path(__file__).parent.parent))
|
| 18 |
+
|
| 19 |
+
import src.core.integrity as integ # noqa: E402
|
| 20 |
+
import src.core.data_io as data_io # noqa: E402
|
| 21 |
+
from src.core.integrity import ( # noqa: E402
|
| 22 |
+
IntegrityError,
|
| 23 |
+
compute_sha256,
|
| 24 |
+
verify_file,
|
| 25 |
+
verify_sha256,
|
| 26 |
+
)
|
| 27 |
+
|
| 28 |
+
|
| 29 |
+
# ---------------------------------------------------------------------------
|
| 30 |
+
# compute_sha256 / verify_sha256 (shared low-level primitives, ADR-0011 + 0010)
|
| 31 |
+
# ---------------------------------------------------------------------------
|
| 32 |
+
|
| 33 |
+
def test_compute_sha256_matches_hashlib(tmp_path):
|
| 34 |
+
p = tmp_path / "f.bin"
|
| 35 |
+
payload = b"decoupler-py integrity test\n" * 1000
|
| 36 |
+
p.write_bytes(payload)
|
| 37 |
+
assert compute_sha256(p) == hashlib.sha256(payload).hexdigest()
|
| 38 |
+
|
| 39 |
+
|
| 40 |
+
def test_verify_sha256_match_and_mismatch(tmp_path):
|
| 41 |
+
p = tmp_path / "f.bin"
|
| 42 |
+
p.write_bytes(b"payload")
|
| 43 |
+
good = compute_sha256(p)
|
| 44 |
+
assert verify_sha256(p, good) is True
|
| 45 |
+
assert verify_sha256(p, good.upper()) is True # case-insensitive
|
| 46 |
+
assert verify_sha256(p, "d" * 64) is False
|
| 47 |
+
assert verify_sha256(p, "") is False # absent baseline is not a pass
|
| 48 |
+
|
| 49 |
+
|
| 50 |
+
# ---------------------------------------------------------------------------
|
| 51 |
+
# verify_file (primitive)
|
| 52 |
+
# ---------------------------------------------------------------------------
|
| 53 |
+
|
| 54 |
+
def test_verify_file_passes_on_match(tmp_path, monkeypatch):
|
| 55 |
+
p = tmp_path / "f.h5ad"
|
| 56 |
+
p.write_bytes(b"good bytes")
|
| 57 |
+
good = compute_sha256(p)
|
| 58 |
+
monkeypatch.setattr(integ, "_lookup_from_registry", lambda url: (good, "ds1"))
|
| 59 |
+
verify_file(p, "https://host/f.h5ad") # no raise
|
| 60 |
+
|
| 61 |
+
|
| 62 |
+
def test_verify_file_raises_on_mismatch(tmp_path, monkeypatch):
|
| 63 |
+
p = tmp_path / "f.h5ad"
|
| 64 |
+
p.write_bytes(b"tampered bytes")
|
| 65 |
+
monkeypatch.setattr(integ, "_lookup_from_registry", lambda url: ("a" * 64, "ds1"))
|
| 66 |
+
with pytest.raises(IntegrityError) as exc:
|
| 67 |
+
verify_file(p, "https://host/f.h5ad")
|
| 68 |
+
assert "ds1" in str(exc.value)
|
| 69 |
+
assert exc.value.actual == compute_sha256(p)
|
| 70 |
+
|
| 71 |
+
|
| 72 |
+
def test_verify_file_noop_without_baseline(tmp_path, monkeypatch):
|
| 73 |
+
p = tmp_path / "f.h5ad"
|
| 74 |
+
p.write_bytes(b"whatever")
|
| 75 |
+
monkeypatch.setattr(integ, "_lookup_from_registry", lambda url: (None, None))
|
| 76 |
+
verify_file(p, "https://host/f.h5ad") # no raise — unbaselined loads
|
| 77 |
+
|
| 78 |
+
|
| 79 |
+
# ---------------------------------------------------------------------------
|
| 80 |
+
# resolve_to_local_path wiring (local-path branch — no network)
|
| 81 |
+
# ---------------------------------------------------------------------------
|
| 82 |
+
|
| 83 |
+
def test_resolve_local_path_good_hash_loads(tmp_path, monkeypatch):
|
| 84 |
+
p = tmp_path / "data.h5ad"
|
| 85 |
+
p.write_bytes(b"real dataset")
|
| 86 |
+
good = compute_sha256(p)
|
| 87 |
+
monkeypatch.setattr(integ, "_lookup_from_registry", lambda url: (good, "ds1"))
|
| 88 |
+
local, is_temp = data_io.resolve_to_local_path(str(p))
|
| 89 |
+
assert local == str(p)
|
| 90 |
+
assert is_temp is False
|
| 91 |
+
|
| 92 |
+
|
| 93 |
+
def test_resolve_local_path_bad_hash_refused(tmp_path, monkeypatch):
|
| 94 |
+
p = tmp_path / "data.h5ad"
|
| 95 |
+
p.write_bytes(b"tampered dataset")
|
| 96 |
+
monkeypatch.setattr(integ, "_lookup_from_registry", lambda url: ("b" * 64, "ds1"))
|
| 97 |
+
with pytest.raises(IntegrityError):
|
| 98 |
+
data_io.resolve_to_local_path(str(p))
|
| 99 |
+
|
| 100 |
+
|
| 101 |
+
def test_resolve_absent_baseline_loads(tmp_path, monkeypatch):
|
| 102 |
+
p = tmp_path / "data.h5ad"
|
| 103 |
+
p.write_bytes(b"unbaselined dataset")
|
| 104 |
+
monkeypatch.setattr(integ, "_lookup_from_registry", lambda url: (None, None))
|
| 105 |
+
local, is_temp = data_io.resolve_to_local_path(str(p))
|
| 106 |
+
assert local == str(p)
|
| 107 |
+
|
| 108 |
+
|
| 109 |
+
# ---------------------------------------------------------------------------
|
| 110 |
+
# resolve_to_local_path wiring (temp-download branch) — tampered temp is deleted
|
| 111 |
+
# ---------------------------------------------------------------------------
|
| 112 |
+
|
| 113 |
+
def test_resolve_tampered_download_deletes_temp(tmp_path, monkeypatch):
|
| 114 |
+
"""A tampered http(s) download is refused AND the temp file is unlinked."""
|
| 115 |
+
payload = b"tampered downloaded bytes"
|
| 116 |
+
created = {}
|
| 117 |
+
|
| 118 |
+
class _FakeResp:
|
| 119 |
+
def __init__(self, data):
|
| 120 |
+
self._data = data
|
| 121 |
+
def read(self, n):
|
| 122 |
+
d, self._data = self._data[:n], self._data[n:]
|
| 123 |
+
return d
|
| 124 |
+
def __enter__(self):
|
| 125 |
+
return self
|
| 126 |
+
def __exit__(self, *a):
|
| 127 |
+
return False
|
| 128 |
+
|
| 129 |
+
monkeypatch.setattr(data_io.urllib.request, "urlopen", lambda url: _FakeResp(payload))
|
| 130 |
+
# Capture the temp path the resolver creates.
|
| 131 |
+
real_ntf = data_io.tempfile.NamedTemporaryFile
|
| 132 |
+
def _spy_ntf(*a, **kw):
|
| 133 |
+
f = real_ntf(*a, **kw)
|
| 134 |
+
created["path"] = f.name
|
| 135 |
+
return f
|
| 136 |
+
monkeypatch.setattr(data_io.tempfile, "NamedTemporaryFile", _spy_ntf)
|
| 137 |
+
monkeypatch.setattr(integ, "_lookup_from_registry", lambda url: ("c" * 64, "ds1"))
|
| 138 |
+
|
| 139 |
+
with pytest.raises(IntegrityError):
|
| 140 |
+
data_io.resolve_to_local_path("https://host/remote.h5ad")
|
| 141 |
+
|
| 142 |
+
assert "path" in created
|
| 143 |
+
assert not Path(created["path"]).exists(), "tampered temp file must be deleted"
|