phase 1.6: lock Gate 3b = C, reconcile docs to shipped state, fix double-upload file leak
Four threads landing together — all driven by the audit J asked for before
production cutover.
(1) Gate 3b DECIDED: Option C (defer classifications). `BiometricCollection.classifications`
stays `Option<JSON> = None` in v1. `docs/specs/GATE_3B_DEEPFACE_DESIGN.md` status
flipped from "draft / awaits product" to DECIDED. Consent template + retention
schedule revised to remove all "automated facial-classification" / "deepface"
language so disclosed scope matches implemented scope.
(2) Endpoint-path drift reconciled across 3 docs. `PHASE_1_6_BIPA_GATES.md`,
`BIPA_DESTRUCTION_RUNBOOK.md`, and `biometric_retention_schedule_v1.md` had
references to legacy `/v1/identity/subjects/*` paths (proposed under a separate
identityd daemon, never shipped) — corrected to actual shipped routes
`/biometric/subject/*` (catalogd-local). Schema block in PHASE_1_6_BIPA_GATES
rewritten to reflect JSON `SubjectManifest.biometric_collection` substrate
(not the proposed Postgres `subjects` table).
(3) New operational artifacts:
- `scripts/staffing/verify_biometric_erasure.sh` — checks 4 things post-erasure
(manifest cleared, uploads dir empty, audit row matches, chain verified).
Smoke-tested live against WORKER-2.
- `scripts/staffing/biometric_destruction_report.sh` — monthly anonymized
destruction-event aggregation. Smoke-tested clean.
- `scripts/staffing/bundle_counsel_packet.sh` — tarballs the counsel-review
packet with per-file SHA-256 manifest.
- `docs/runbooks/LEGAL_AUDIT_KEY_ROTATION.md` — formal rotation procedure
operationalized after the 2026-05-05 /tmp wipe incident.
- `docs/counsel/COUNSEL_REVIEW_PACKET_2026-05-05.md` — cover note bundling
all eng-staged BIPA docs for counsel review with per-doc questions, sign-off
checklist, recommended review sequence.
(4) Double-upload file leak fixed in `crates/catalogd/src/biometric_endpoint.rs`.
`verify_biometric_erasure.sh` smoked WORKER-2 and surfaced a stranded photo
file. Investigation showed the file was 13-byte test-fixture bytes (zero PII,
no biometric content); audit timeline showed two consecutive uploads followed
by one erasure — the second upload had silently overwritten manifest.data_path,
orphaning the first file. Patched `process_upload` to refuse a second upload
with HTTP 409 + `error: "biometric_already_collected"` when
`biometric_collection.is_some()` on the manifest. Operator must explicitly
POST `/biometric/subject/{id}/erase` first.
Tests: new `second_upload_without_erase_returns_409` (asserts 409 + manifest
pointer unchanged + first file untouched on disk). Replaced
`repeated_uploads_grow_the_chain` with `upload_erase_upload_grows_the_chain_cleanly`
(covers the legitimate re-collection cycle: chain grows to 3 rows). Updated
`content_type_with_parameters_accepted` to use 2 distinct subjects (was
using 1 subject with 2 uploads to test ct parsing — would now 409).
22/22 biometric_endpoint tests + 59/59 catalogd lib tests green post-patch.
Production posture: gateway needs `cargo build --release -p gateway` +
`systemctl restart lakehouse.service` to pick up the new 409 in live traffic.
Counsel calendar is now the only remaining blocker for first real-photo intake.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -307,6 +307,21 @@ pub async fn process_upload(
|
||||
consent_status: None,
|
||||
}));
|
||||
}
|
||||
// Refuse double-upload. If a BiometricCollection already exists on
|
||||
// the manifest, the operator must explicitly erase before re-uploading.
|
||||
// Without this gate, a second POST silently overwrites manifest.data_path
|
||||
// and orphans the previous photo file on disk — creating a forever-leak
|
||||
// pattern and a BIPA defensibility hole ("we said we erased the photo,
|
||||
// but the previous version of it is still under the same subject dir").
|
||||
// Caught 2026-05-05 by verify_biometric_erasure.sh against WORKER-2.
|
||||
if manifest.biometric_collection.is_some() {
|
||||
return Err((StatusCode::CONFLICT, ErrorResponse {
|
||||
error: "biometric_already_collected",
|
||||
detail: "subject already has a BiometricCollection on the manifest; \
|
||||
POST /biometric/subject/{id}/erase first if you intend to replace the photo".into(),
|
||||
consent_status: None,
|
||||
}));
|
||||
}
|
||||
|
||||
let template_hash = {
|
||||
let mut h = Sha256::new();
|
||||
@@ -947,15 +962,92 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn repeated_uploads_grow_the_chain() {
|
||||
let state = fixture_state("repeated").await;
|
||||
let _ = state.registry.put_subject(fixture_manifest("WORKER-5", BiometricConsentStatus::Given, SubjectStatus::Active)).await;
|
||||
async fn second_upload_without_erase_returns_409() {
|
||||
// BIPA defensibility: a second upload to a subject that already
|
||||
// has a BiometricCollection must fail-closed. Without this gate,
|
||||
// the second upload silently overwrites manifest.data_path and
|
||||
// orphans the first photo on disk forever (caught 2026-05-05 on
|
||||
// WORKER-2 by verify_biometric_erasure.sh).
|
||||
let state = fixture_state("second_upload_409").await;
|
||||
let _ = state.registry.put_subject(fixture_manifest("WORKER-DUP", BiometricConsentStatus::Given, SubjectStatus::Active)).await;
|
||||
let storage_root = state.storage_root.clone();
|
||||
let registry = state.registry.clone();
|
||||
|
||||
// First upload succeeds.
|
||||
let resp1 = process_upload(&state, "WORKER-DUP", Some(TEST_TOKEN), Some("image/jpeg"), "v1", "", &jpeg_bytes())
|
||||
.await.unwrap();
|
||||
let first_path = storage_root.join(&resp1.data_path);
|
||||
assert!(first_path.exists(), "first upload should produce a file");
|
||||
|
||||
// Second upload refused with 409.
|
||||
let err = process_upload(&state, "WORKER-DUP", Some(TEST_TOKEN), Some("image/jpeg"), "v1", "", &jpeg_bytes())
|
||||
.await.unwrap_err();
|
||||
assert_eq!(err.0, StatusCode::CONFLICT);
|
||||
assert_eq!(err.1.error, "biometric_already_collected");
|
||||
|
||||
// Manifest still points at the first upload — pointer was NOT overwritten.
|
||||
let m = registry.get_subject("WORKER-DUP").await.unwrap();
|
||||
let bc = m.biometric_collection.as_ref().expect("collection should still be set");
|
||||
assert_eq!(bc.data_path, resp1.data_path,
|
||||
"manifest data_path must be unchanged after refused second upload");
|
||||
|
||||
// First file remains on disk untouched (refusal must not unlink it).
|
||||
assert!(first_path.exists(), "first upload's file must remain after refused second upload");
|
||||
let still_on_disk = tokio::fs::read(&first_path).await.unwrap();
|
||||
assert_eq!(still_on_disk, jpeg_bytes(),
|
||||
"first upload's bytes must not have been overwritten");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn upload_erase_upload_grows_the_chain_cleanly() {
|
||||
// Prior version of this test allowed repeated uploads to chain;
|
||||
// that conflated chain growth with allowed re-upload. Under the
|
||||
// double-upload guard (409 above), the only legitimate way to
|
||||
// re-collect is upload → erase → upload. Chain grows to 3 rows
|
||||
// (collection, erasure, collection); on-disk file count returns
|
||||
// to one after the second upload.
|
||||
let state = fixture_state("upload_erase_upload").await;
|
||||
let _ = state.registry.put_subject(fixture_manifest("WORKER-CYCLE", BiometricConsentStatus::Given, SubjectStatus::Active)).await;
|
||||
let writer = state.writer.clone();
|
||||
for _ in 0..2 {
|
||||
let _ = process_upload(&state, "WORKER-5", Some(TEST_TOKEN), Some("image/jpeg"), "", "", &jpeg_bytes())
|
||||
.await.unwrap();
|
||||
}
|
||||
assert_eq!(writer.verify_chain("WORKER-5").await.unwrap(), 2);
|
||||
let storage_root = state.storage_root.clone();
|
||||
|
||||
// First upload.
|
||||
let resp1 = process_upload(&state, "WORKER-CYCLE", Some(TEST_TOKEN), Some("image/jpeg"), "", "", &jpeg_bytes())
|
||||
.await.unwrap();
|
||||
let first_path = storage_root.join(&resp1.data_path);
|
||||
assert!(first_path.exists());
|
||||
|
||||
// Erase. Uses process_erase test helper (the production path
|
||||
// parses the EraseRequest from request body; tests inject it
|
||||
// directly). Note: the erase flow flips biometric.status to
|
||||
// Withdrawn, so the post-erase second upload must reset consent
|
||||
// first (production flow would require new consent collection).
|
||||
let _ = process_erase(&state, "WORKER-CYCLE", Some(TEST_TOKEN), "trace-cycle", fixture_erase_request("biometric_only"))
|
||||
.await.unwrap();
|
||||
assert!(!first_path.exists(), "first photo file must be unlinked by erase");
|
||||
|
||||
// Reset consent + status on the post-erase manifest so the second
|
||||
// upload can proceed (production flow would require new consent
|
||||
// collection here; for this test we directly flip the manifest).
|
||||
let mut post_erase = state.registry.get_subject("WORKER-CYCLE").await.unwrap();
|
||||
post_erase.consent.biometric.status = BiometricConsentStatus::Given;
|
||||
post_erase.status = SubjectStatus::Active;
|
||||
post_erase.biometric_collection = None;
|
||||
let _ = state.registry.put_subject(post_erase).await;
|
||||
|
||||
// Second upload (legitimate, after erase).
|
||||
let resp2 = process_upload(&state, "WORKER-CYCLE", Some(TEST_TOKEN), Some("image/jpeg"), "", "", &jpeg_bytes())
|
||||
.await.unwrap();
|
||||
let second_path = storage_root.join(&resp2.data_path);
|
||||
assert!(second_path.exists(), "second upload should produce a file");
|
||||
assert_ne!(resp1.data_path, resp2.data_path, "second upload should land at a new path");
|
||||
|
||||
// Chain has 3 rows: collection, erasure, collection.
|
||||
assert_eq!(writer.verify_chain("WORKER-CYCLE").await.unwrap(), 3);
|
||||
let rows = writer.read_rows_in_range("WORKER-CYCLE", None, None).await.unwrap();
|
||||
assert_eq!(rows[0].accessor.kind, "biometric_collection");
|
||||
assert_eq!(rows[1].accessor.kind, "biometric_erasure");
|
||||
assert_eq!(rows[2].accessor.kind, "biometric_collection");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -985,18 +1077,23 @@ mod tests {
|
||||
// Caught 2026-05-03 opus scrum WARN; regression test ensures
|
||||
// the bare media type is matched after stripping parameters.
|
||||
let state = fixture_state("ct_with_params").await;
|
||||
let _ = state.registry.put_subject(fixture_manifest("WORKER-CT", BiometricConsentStatus::Given, SubjectStatus::Active)).await;
|
||||
// Two distinct subjects so each upload exercises the "first upload"
|
||||
// path. Prior version used one subject and two uploads — now blocked
|
||||
// by the double-upload guard (409). The test's actual intent is
|
||||
// content-type parsing, not re-upload tolerance.
|
||||
let _ = state.registry.put_subject(fixture_manifest("WORKER-CT-A", BiometricConsentStatus::Given, SubjectStatus::Active)).await;
|
||||
let _ = state.registry.put_subject(fixture_manifest("WORKER-CT-B", BiometricConsentStatus::Given, SubjectStatus::Active)).await;
|
||||
let resp = process_upload(
|
||||
&state, "WORKER-CT", Some(TEST_TOKEN),
|
||||
&state, "WORKER-CT-A", Some(TEST_TOKEN),
|
||||
Some("image/jpeg; charset=binary"), "", "", &jpeg_bytes(),
|
||||
).await.unwrap();
|
||||
assert_eq!(resp.candidate_id, "WORKER-CT");
|
||||
assert_eq!(resp.candidate_id, "WORKER-CT-A");
|
||||
// Also case-insensitive matching: "Image/JPEG" should work too.
|
||||
let resp2 = process_upload(
|
||||
&state, "WORKER-CT", Some(TEST_TOKEN),
|
||||
&state, "WORKER-CT-B", Some(TEST_TOKEN),
|
||||
Some("Image/JPEG"), "", "", &jpeg_bytes(),
|
||||
).await.unwrap();
|
||||
assert_eq!(resp2.candidate_id, "WORKER-CT");
|
||||
assert_eq!(resp2.candidate_id, "WORKER-CT-B");
|
||||
}
|
||||
|
||||
// ─── Erasure tests (Gate 5) ──────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user