Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions docs/how-to/export-dataset-snapshot.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,16 @@ in the receipt is re-read and compared by size and sha256, and every finding
is returned in one report built from the shared verification types in
`hflow.verification` (a `VerificationReport` with status `ok`, `damaged`,
or `unverifiable`). Recorded paths are joined only onto the handed
directory, so a copied delivery verifies in place. Exit `0` clean,
`1` damaged, `3` unverifiable, `2` unreadable input. A pre-#401
`format.json` with no `integrity` key is reported as `no-receipt`,
unverifiable, not corrupt. Older HFlow overwrite checks only
`format` and `format_version`, and readers that only consume the string
`tables` map keep working; the `integrity` key is purely additive under
format version `1`.
directory, so a copied delivery verifies in place. A receipt path that
is absolute, contains `..`, or carries a Windows drive letter is refused
as unreadable input (exit `2`) before any file is read — the same family
storage keys refuse — so a tampered marker cannot hash bytes outside the
delivery. Exit `0` clean, `1` damaged, `3` unverifiable, `2` unreadable
input. A pre-#401 `format.json` with no `integrity` key is reported as
`no-receipt`, unverifiable, not corrupt. Older HFlow overwrite checks
only `format` and `format_version`, and readers that only consume the
string `tables` map keep working; the `integrity` key is purely additive
under format version `1`.

The receipt travels unsigned inside the `format.json` it describes, so it
catches corruption and accidental loss, not tampering: anyone who can edit a
Expand Down
31 changes: 23 additions & 8 deletions src/hflow/snapshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from hflow.app import ARTIFACT_MEASUREMENT_KEY_PREFIX, MEDIA_CONTACT_SHEET_STEP_NAME
from hflow.catalog import episode_status_case_sql
from hflow.curation import open_catalog_connection
from hflow.storage import StorageRoot, fetch_uri
from hflow.storage import StorageRoot, _validated_relative_key, fetch_uri

if TYPE_CHECKING:
from hflow.verification import VerificationReport
Expand Down Expand Up @@ -912,12 +912,16 @@ def verify_dataset_snapshot(
``integrity`` key. That snapshot is unverifiable, not corrupt.

Unreadable input (a missing directory, a missing or unparsable
``format.json``) raises instead: that is not a finding about a delivered
snapshot, it is the wrong input entirely. The same applies to a receipt
that is internally inconsistent with itself: a missing or malformed
``content_id``, or a recomputed inventory hash that disagrees with the
stored one, means the receipt no longer describes the delivered set
(tampering or a truncated write), which is exit 2, not damaged bytes.
``format.json``, or a receipt ``path`` that is absolute / contains ``..``
/ carries a Windows drive letter) raises instead: that is not a finding
about a delivered snapshot, it is the wrong input entirely. Path
containment reuses :func:`hflow.storage._validated_relative_key` so the
verifier and storage roots share one answer to "is this key inside the
root", and the check runs before any file read (#469). The same exit-2
class applies to a receipt that is internally inconsistent with itself:
a missing or malformed ``content_id``, or a recomputed inventory hash
that disagrees with the stored one, means the receipt no longer
describes the delivered set (tampering or a truncated write).

Extra files under ``assets/`` that the receipt does not name are ignored:
the receipt covers what was exported, not everything a recipient may add.
Expand Down Expand Up @@ -1021,7 +1025,18 @@ def verify_dataset_snapshot(
)

for record in receipt_records:
relative_path = record.path
# Containment (#469): refuse before any read. Absolute paths discard
# the handed root under pathlib; ``..`` climbs out of it. Both are
# malformed receipts (exit 2), not damaged bytes. Reuse the storage
# key validator so this family has one implementation.
try:
relative_path = _validated_relative_key(record.path)
except ValueError as error:
raise ValueError(
f"format.json integrity receipt path {record.path!r} must stay "
"under the handed snapshot directory (no leading '/', no '..', "
"no Windows drive letter); refused before any file read"
) from error
delivered_path = resolved_directory / relative_path
if not delivered_path.is_file():
findings.append(
Expand Down
86 changes: 86 additions & 0 deletions tests/test_snapshot_verify.py
Original file line number Diff line number Diff line change
Expand Up @@ -519,3 +519,89 @@ def test_receipt_entry_with_numeric_sha256_is_refused_at_the_boundary() -> None:

with pytest.raises(ValueError, match="sha256"):
_parse_file_integrity_record({"path": "samples.parquet", "size_bytes": 10, "sha256": 123})


def _hand_built_snapshot_with_receipt_path(
snap: Path, *, receipt_path: str, payload: bytes = b"secret-bytes"
) -> None:
"""Minimal identity+integrity marker whose single receipt uses ``receipt_path``.

``content_id`` matches that one-entry inventory so the deleted-member gate
is not the thing that fires; the path containment check is.
"""
from hflow.snapshot import (
DATASET_SNAPSHOT_FORMAT_NAME,
DATASET_SNAPSHOT_FORMAT_VERSION,
FileIntegrityRecord,
_inventory_content_id,
)

digest = hashlib.sha256(payload).hexdigest()
record = FileIntegrityRecord(path=receipt_path, size_bytes=len(payload), sha256=digest)
marker = {
"format": DATASET_SNAPSHOT_FORMAT_NAME,
"format_version": DATASET_SNAPSHOT_FORMAT_VERSION,
"media_mode": "references",
"media_uri_base": None,
"tables": ["samples.parquet"],
"integrity": {
"tables": {
"samples": {
"path": record.path,
"size_bytes": record.size_bytes,
"sha256": record.sha256,
}
},
"assets": [],
"content_id": _inventory_content_id([record]),
},
}
snap.mkdir(parents=True, exist_ok=True)
(snap / "format.json").write_text(json.dumps(marker, indent=2) + "\n")


def test_receipt_path_escaping_the_handed_directory_is_refused(tmp_path: Path) -> None:
"""#469: relative ``..`` and absolute paths hash outside the root today;
both must raise before any read (exit 2), not report ok."""
snap = tmp_path / "snap"
outside = tmp_path / "outside"
outside.mkdir()
secret = outside / "secret.bin"
secret.write_bytes(b"secret-bytes")

for escape_path in (f"../outside/{secret.name}", str(secret.resolve())):
_hand_built_snapshot_with_receipt_path(snap, receipt_path=escape_path)
with pytest.raises(ValueError, match="must stay under the handed snapshot directory"):
verify_dataset_snapshot(snap)
assert cli_main(["verify", "snapshot", str(snap)]) == 2


def test_normalized_parent_escape_through_an_existing_subdir_is_refused(
tmp_path: Path,
) -> None:
"""Kingston's third shape: ``tables/../../outside/...`` only looks like it
failed before because ``snap/tables/`` was missing. Create it and the bare
join escapes; containment must still refuse before the read."""
snap = tmp_path / "snap"
outside = tmp_path / "outside"
outside.mkdir()
(snap / "tables").mkdir(parents=True)
secret = outside / "secret.bin"
secret.write_bytes(b"secret-bytes")

_hand_built_snapshot_with_receipt_path(snap, receipt_path=f"tables/../../outside/{secret.name}")
with pytest.raises(ValueError, match="must stay under the handed snapshot directory"):
verify_dataset_snapshot(snap)
assert cli_main(["verify", "snapshot", str(snap)]) == 2


def test_honest_relative_receipt_path_still_verifies_after_containment_gate(
tmp_path: Path,
) -> None:
"""Containment must not break a clean export: relative keys under the root
still pass size and sha256 checks."""
output_directory, _ = _export_two_episode_snapshot(tmp_path, "references")
report = verify_dataset_snapshot(output_directory)
assert report.ok
assert report.findings == []
assert cli_main(["verify", "snapshot", str(output_directory)]) == 0