fix: refuse to export captures with no bytes and no custody binding (#158) - #164
Merged
Merged
Conversation
…158) A capture whose media type had no packet export mapping (.heic, the iPhone default photo format) shipped with shared_name="", nothing in media/, and no copied_for_sharing custody entry -- and habitable verify still reported the packet READY. This closes the gap with the issue's three separable decisions: 1. Fail closed at export. packet.build_packet now refuses to publish any item that would carry neither a shared copy nor an embedded original, raising a PacketError naming the capture id and media type. 2. Close the map gap. capture.py and packet.py now read one canonical registry (habitable.media_types) instead of two independently hand-maintained maps, so this class of gap cannot recur unnoticed. .heic is registered as export_kind="unsupported" (this project's only image-processing dependency, Pillow, cannot decode HEIC without a new native codec dependency that needs its own supply-chain/licensing review) rather than given a broken sanitizer -- --include-originals remains a real, tested, disclosed way to export it byte-exact. 3. Tightened the verifier as defense-in-depth. ItemVerdict gained evidence_present, folded into structurally_intact: an item with no shared media and no embedded original can never be evidence_ready, even with an otherwise-valid, authority-trusted timestamp. packet.html's per-item figure and evidence appendix now visibly say when an item has no shared preview or no evidence bytes at all, instead of rendering an empty figure indistinguishable from an intact one. Incidentally discovered and fixed: exif.py's non-JPEG raster stripping path called a Pillow accessor this project's Pillow floor (12.3.0) deprecated, uncaught until the new registry-driven regression test exercised PNG/WEBP/ TIFF export end to end for the first time. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nn4wq6C7WeV7zbYyUwcHu5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug (issue #158)
.heic(the iPhone default photo format) is a first-class capture type —capture._MEDIA_TYPESmapped it toimage/heic— but was absent from the packet exporter's_EXT_BY_TYPE/_DATA_EXT_BY_TYPE._build_item'sif ext: ... elif data_ext: ...had noelse, so a.heiccapture exported as an item withshared_name="",shared_hash="", nothing written intomedia/, and nocopied_for_sharingcustody entry. The verifier's structural checks (shared_media_ok,custody_binding_ok) are both gated onshared_namebeing non-empty, so an empty name read as "nothing to check, therefore intact" rather than "nothing was exported, therefore broken." Result:habitable verifyprintedevidence readiness: READY, exit 0, for a packet containing zero photographs.Reproduced exactly as the issue describes: synthetic vault, one
.heiccapture,build_packet(..., make_pdf=False), thenverify_packetwith the issuer's cert as trust anchor — before this fix,shared_name='', 0 files inmedia/, 0copied_for_sharingcustody entries, andevidence_ready=True.tests/test_media_types.py::test_every_registered_media_type_has_a_working_export_path[image/heic]now pins the fixed behavior; revertingpacket.py's_require_shareable_bytescall reproduces the original bug against that test.The three decisions
Decision 1 — fail closed at export (the issue's own recommendation, taken as-is).
_require_shareable_bytes(packet.py) runs after every capture/artifact item is built and raisesPacketErrornaming the capture id and media type if the item would carry neither a shared copy nor an embedded original. This is a whole-packet refusal, not a per-item skip — packet v4 has no scoped/partial export mode, so silently dropping just the bad item would itself produce a packet whose appendix count no longer matched what the operator expected, with no record of what went missing.--include-originalsremains available as an explicit escape hatch (see decision 2).Decision 2 — close the map gap. I took the issue's preferred shape (one source of truth) rather than the narrower "just add
image/heicto the map" fix, for a reason discovered mid-investigation: Pillow (this project's only image-processing dependency) cannot decode HEIC at all —PIL.features.check('heic')isFalseon the pinned floor (12.3.0), and there's nopillow-heif/libheifdependency in this repo. Naively mapping.heic→.heicand routing it through the existing Pillow-based sanitizer (exif.py::_strip_with_pillow) would make every real.heiccapture's default export raise (Pillow can't open it), which is honest but total — it would strand the iPhone tenant's primary photo format with no escape hatch, since the code path that raises happens before the code ever reaches the--include-originalsfallback.Adding real HEIC decode/strip support means a new native-codec dependency (
pillow-heif/libheif), which I confirmed resolves viauvand round-trips EXIF/GPS correctly in a scratch test — butlibheif's bundled encoder path carries non-uniform licensing (the PyPI package lists a GPLv2 classifier alongside its own BSD-3-Clause one), and evaluating that against this AGPL/Apache dual-licensed, supply-chain-audited project is its own review, not a rider on a bug fix. So:src/habitable/media_types.pyis now the single registrycapture.pyandpacket.pyboth read (extension → MIME type, and MIME type → export handling in one place);image/heicis registered withexport_kind="unsupported"— recognized, hashed, sealed, and timestamped exactly like any other capture, but honestly marked as having no default sanitizer.--include-originals— already-existing, already-tested code, unconditional in_build_item/_build_artifact_item— is HEIC's real, working, disclosed export path, proven end to end (capture → export → independentverify_packet) in the new regression suite.tests/test_media_types.py::test_every_registered_media_type_has_a_working_export_pathis parametrized over every entry in the registry and proves, with a real capture→export→verify round trip (not just data-shape assertions), that no media type is ever a dead end: a supported type exports real hash-verified shared media by default;image/heicrefuses the default export and exports real hash-verified bytes via--include-originals. A second test proves the backstop is independent of the registry too:test_a_genuinely_unmapped_media_type_is_refused_at_exportcaptures with an explicit, made-upmedia_typethat appears nowhere inmedia_types.pyand confirmsbuild_packetstill refuses it by name.Decision 3 — tightened the verifier (my judgment). Given decision 1 makes the byteless state unreachable through normal export, I still chose to close it in
verify.pyfor defense-in-depth:ItemVerdictgainedevidence_present(Trueunless the item has neither ashared_namenor an embedded original), folded intostructurally_intact. A hash-and-timestamp-only item can now never beevidence_ready, full stop — even with an otherwise-valid, authority-trusted timestamp (seetests/test_packet_verify.py::test_byteless_item_is_never_structurally_intact_or_evidence_ready, which hand-crafts exactly that item). I did not treat "hash+timestamp only, no bytes" as a legitimate disclosure state on its own: this tool's whole value proposition is that READY means something specific to a non-technical reader (tenant, inspector, legal-aid worker, court), and "READY" for an item with literally nothing to look at is exactly the failure mode the issue reports. The--include-originals-without-a-sanitized-preview state (real bytes, just not a browser-viewable shared copy) is different and legitimate — it readsevidence_present=Trueand can reachevidence_ready, but I made it visible, not just a machine note:packet.html's per-item figure now renders "No shared preview copy was made for this item... download the original (...it may retain full metadata, including location)" with a real link, instead of a blank figure, and the evidence appendix table gained a "Media" column (included/original only (no shared preview)/NONE — no evidence bytes) so both states are visible in the human-readable view, not buried innotes.docs/verifier-decision-table.md§0 and new §4.2b state this precisely;docs/embedding-the-verifier.mddocuments the new field for embedders.Incidental fix
The new registry-driven test was the first thing to ever export a PNG/WEBP/TIFF capture end to end, which surfaced a real, pre-existing, unrelated bug:
exif.py::_strip_with_pillowcalledImage.getdata(), deprecated in the pinned Pillow floor (12.3.0), which this project'sfilterwarnings = ["error"]pytest policy turns into a hard failure. Swapped for the documented replacement (get_flattened_data(), byte-identical semantics, confirmed via aputdataround-trip); addedtests/test_evidence_exif.py::test_strip_non_jpeg_raster_removes_embedded_gpsas direct regression coverage (GPS-embedded PNG → stripped copy has no location, pixels survive).What did not change
Per scope: RFC 3161 timestamp trust logic, chain verification, and every already-correctly-exporting capture type (jpeg/png/webp/tiff/mp4/mov/m4a/mp3/wav/csv) are untouched in behavior —
_EXT_BY_TYPE/_DATA_EXT_BY_TYPEare now derived from the registry but produce byte-identical dicts to the hand-written originals for every previously-supported type.campaign.UnitHealth.export_readyis unchanged in behavior (it's a vault-level "nothing known to block starting an export" signal, computed without callingbuild_packet, so it never made a false claim about packet contents) — I added a docstring clarifying its distinct scope fromverify_packet'sevidence_ready, since a unit can now beexport_ready=Trueand still havehabitable exportrefuse with a namedPacketError, which is the correct, honest outcome (loud refusal beats a silent roll-up lie).packet.pdf's analogous per-item rendering path (pdf.py::_render_evidence_item) has the same historical blank-space gap for a byteless item aspacket.htmldid — I left it untouched: the README statespacket.htmlis "the designated accessible rendering" and the PDF is explicitly not tagged/accessible, and decision 1 makes this state unreachable via real export regardless, so I judged expanding into a third rendering module's ReportLab-specific code out of scope for this fix. Noting it here rather than silently leaving it.Verification
Full gate, from a fresh worktree off
origin/main:uv run habitable demoalso re-run manually end to end (unaffected capture types) — still exports and independently verifiesevidence readiness: READYfor its two synthetic JPEGs.New/changed test coverage:
tests/test_media_types.py(new) — the registry↔exporter consistency guard, the genuinely-unmapped-type backstop test, and the parametrized every-registered-type round trip (11 media types × capture→export→verify).tests/test_packet_verify.py— decision 3's byteless-item unit test, plus two pre-existing hand-crafted-item tests updated (they relied on the now-forbidden byteless state to isolate unrelated timestamp-authority logic; fixed by giving them real embedded-original bytes instead of deleting their assertions).tests/test_htmlpacket.py— both new visible-rendering states (nothing at all / original-only), simulated via a real built packet's bundle mutated to the byteless/original-only shape (sincebuild_packetitself can no longer produce it).tests/test_evidence_exif.py— regression coverage for the incidental Pillow-deprecation fix.This is safety-relevant software for people documenting housing conditions under retaliation risk; I did not touch anything about what READY is allowed to mean beyond making it stricter, and I'd rather under-claim than over-claim what this fix guarantees — in particular, HEIC photos still have no default, metadata-stripped, browser-viewable export path;
--include-originalsis real and tested but is a deliberate, disclosed, higher-fidelity/higher-disclosure choice an operator has to make on purpose, not a drop-in equivalent to what jpeg/png/webp/tiff already get by default.