Plan: PII Coverage + Concurrency-Correctness Gap Closure

On this page

Status

Step Description Status

1

Plan filing + GitLab epic + 8 step issues. nav.adoc updated. CHANGELOG entry. ADR-025 (Identity Normalization) referenced from this plan; its acceptance landed in Plan A Step 2 (!231, archived 2026-05-10). No code changes.

Done (2026-05-08) — MR !230

2

F-002 implementation: HKDF the blind-index. Add hkdf workspace dep; refactor crates/craig-crypto/src/lib.rs:119-131::hmac to derive a purpose-specific subkey via HKDF-SHA256 (info = b"craig-crypto/blind-index/v1"). 3 callsites updated in services/craig-cases/src/api/persons.rs:71/146/252. Pre-1.0 destructive rebuild of cases.persons.ssn_hmac column via reseed.

Done (2026-05-13) — MR !276

3

F-001 implementation: extend FieldEncryptor coverage to cases.reports table. Encrypt reporter_phone, reporter_first_name, reporter_last_name, narrative, children JSONB content, adults JSONB content, raw_submission JSONB. Apply the same EncryptionMode { Required, Optional } posture from platform-stab-2 §D9. Depends on Plan A Step 2 (ADR-025 identity normalization — shipped !231, dependency now resolved).

Done (2026-05-14) — MR !277

4

F-003 implementation: salt the IP hash. Replace Sha256::digest(ip) at services/craig-intake/src/api/hashing.rs:6 with HMAC-SHA256(app_secret, ip). New env var CRAIG_INTAKE__IP_HASH_SECRET. Devstack reseed.

Done (2026-05-14) — MR !278

5

F-004 implementation: zeroize + TLS startup validators. (a) Zeroize + ZeroizeOnDrop derive on crates/craig-crypto/src/lib.rs::FieldEncryptor (struct-level per §D4 locked decision) + on services/craig-web/src/auth.rs::WebSession (struct-level; zeroizes access_token / refresh_token / id_token fields). (b) Startup TLS validators in crates/craig-db + crates/craig-mq requiring sslmode=verify-full (DB) and TLS-required AMQP URI; CRAIG_<CRATE>__ALLOW_INSECURE_TRANSPORT=true override for devstack.

Done (2026-05-14) — MR !279, partial — Zeroize derives shipped; TLS-URL validators dropped from scope (see Errata)

6

F-015 implementation: approve_adjustment race fix. Wrap the 5 store calls in services/craig-financial/src/api/adjustments.rs:140-180::approve_adjustment in pool.begin() …​ tx.commit(). New concurrency test using craig_test_lib::concurrent_fire_collect.

Done (2026-05-14) — MR !280 — tx wrap + SELECT FOR UPDATE on payment row (plain tx-wrap insufficient under READ COMMITTED; see Errata)

7

F-016 implementation: approve_case_plan race fix. Wrap the 2 UPDATE statements in services/craig-cases/src/store/case_plans.rs:90-118 in tx; add SELECT …​ FOR UPDATE on the case row at top; add DB-level partial UNIQUE INDEX UNIQUE(case_id) WHERE status = 'active' as defense-in-depth. New concurrency test.

Done (2026-05-14) — MR !281

8

Plan completion audit + archive.

Done (pre-ADR-030) — this MR

Epic: &25 (epic: PII coverage extension + concurrency-correctness gap closure (Plan B))
Issues: #335 (Step 1) · #336 (Step 2 F-002) · #337 (Step 3 F-001) · #338 (Step 4 F-003) · #339 (Step 5 F-004) · #340 (Step 6 F-015) · #341 (Step 7 F-016) · #342 (Step 8 archive)
Branch prefix: feat/pii-and-races- / fix/pii-and-races- / chore/pii-and-races-
*Milestone: TBD (no fixed milestone — security/hardening work; ship on readiness)

Context

Plan A closes the application-layer authorization gap. This plan closes the application-layer data-protection + concurrency-correctness gaps in the same hardening track.

PII coverage is currently scoped only to cases.persons.ssn_last_four (per platform-stab-2 §D9). The 2026-05-08 audit found that cases.reports — which holds the most sensitive class of CCWIS data per 45 CFR § 1355.52 — is plaintext at rest. F-001 closes this gap.

Concurrency-correctness was the subject of platform-stab-2, which closed 3 P0 races. The 2026-05-08 audit found 2 additional races platform-stab-2 didn’t catch (approve_adjustment 5-step non-transactional + approve_case_plan two-step supersede-then-promote without atomicity). F-015/F-016 close those.

F-002/F-003/F-004 are PII-related hygiene findings whose fixes are bounded and testable.

26 application-layer hardening findings (F-001..F-026) were validated across 5 audit passes; this plan covers the PII + concurrency subset. Findings are durably anchored as the bodies of the 8 step-tracking GitLab issues filed by this plan’s Step 1; each issue carries the full F-NNN evidence (file:line, reproducer command, current state, desired state, validation steps).

Scope

In scope:

  • F-001 reports table PII encryption (extends platform-stab-2 §D9 pattern to a new table)

  • F-002 HKDF blind-index (purpose-key separation per RFC 5869)

  • F-003 IP hash salting (privacy)

  • F-004 FieldEncryptor + WebSession Zeroize, DB/MQ TLS startup validators

  • F-015 approve_adjustment transactional wrap

  • F-016 approve_case_plan transactional wrap + DB partial UNIQUE INDEX

Out of scope (covered elsewhere):

  • Authorization on reports / approvals — Plan A

  • Partner-edge concerns (rate-limit, JWS, signer-key expiry, etc.) — Plan C

  • Code-quality discipline (strum, pub(crate), silent-skip, cargo-deny) — Plan D

Design

D1. HKDF blind-index (F-002)

crates/craig-crypto/src/lib.rs:119-131::hmac currently uses self.key_bytes (the AES-GCM-SIV master key) directly as the HMAC key. RFC 5869 (HKDF) is the standard primitive for deriving purpose-specific subkeys from a master secret.

New implementation:

pub fn hmac(&self, plaintext: &str) -> String {
    use hkdf::Hkdf;
    use sha2::Sha256;
    let hk = Hkdf::<Sha256>::new(None, &self.key_bytes);
    let mut bi_key = [0u8; 32];
    hk.expand(b"craig-crypto/blind-index/v1", &mut bi_key)
        .expect("HKDF-SHA256 expand of 32 bytes must succeed");
    let mut mac = <HmacSha256 as Mac>::new_from_slice(&bi_key)
        .expect("HMAC-SHA256 accepts any key length");
    mac.update(plaintext.as_bytes());
    BASE64.encode(mac.finalize().into_bytes())
}

Migration: pre-1.0 destructive rebuild. Devstack reseed regenerates cases.persons.ssn_hmac with new HMAC values.

D2. Reports PII encryption (F-001)

cases.reports schema (per services/craig-cases/migrations/20260423110000_reports_and_screening.sql) carries:

  • reporter_first_name TEXT, reporter_last_name TEXT, reporter_phone TEXT (nullable, plaintext)

  • narrative TEXT NOT NULL (plaintext)

  • children JSONB NOT NULL DEFAULT '[]'::jsonb (plaintext content includes child names, DOBs)

  • adults JSONB NOT NULL DEFAULT '[]'::jsonb (plaintext content includes adult names, addresses)

  • raw_submission JSONB NOT NULL (verbatim partner payload)

All 7 fields encrypt at write; decrypt at read. Searchable metadata fields (admin_unit, received_at, received_request_id, received_ip_hash, id, partner_id, reporter_relation) stay plain.

Pattern mirrors services/craig-cases/src/api/persons.rs::create_person/get_person/update_person:

  • New helpers in services/craig-cases/src/api/encryption.rs: encrypt_report_pii(encryptor, mode, body) → Result<EncryptedReportFields, ApiError>, decrypt_report_pii(encryptor, mode, row) → Result<DecryptedReportFields, ApiError>.

  • For JSONB fields: serialize the entire field value to UTF-8 JSON bytes → encrypt → store as a single-key JSONB envelope {"v": "<base64-ciphertext>"} at the JSONB root. Locked decision (2026-05-13): envelope shape is {"v": "…​"} (one ciphertext per field). Searchable JSONB substructure is gone by design — any reads must decrypt the full envelope. If list-views need filterable surrogates later, derive them as separate _idx TEXT columns on cases.reports, not as JSONB substructure.

Migration is metadata-only (column types unchanged); behavior change is in the handler.

D3. IP hash salting (F-003)

services/craig-intake/src/api/hashing.rs:6::hash_ip currently:

pub fn hash_ip(ip: &str) -> String {
    let mut hasher = Sha256::new();
    hasher.update(ip.as_bytes());
    format!("{:x}", hasher.finalize())
}

New implementation (preserve hex output encoding for storage-format continuity; semantic change is the keyed primitive only):

pub fn hash_ip(secret: &SecretString, ip: &str) -> String {
    use hmac::{Hmac, Mac};
    use sha2::Sha256;
    let mut mac = <Hmac<Sha256> as Mac>::new_from_slice(secret.expose_secret().as_bytes())
        .expect("HMAC-SHA256 accepts any key length");
    mac.update(ip.as_bytes());
    format!("{:x}", mac.finalize().into_bytes())
}

crates/craig-common::settings::IntakeSettings adds ip_hash_secret: SecretString (required env CRAIG_INTAKE__IP_HASH_SECRET). Devstack .env.example sets a default value with a comment that production must override.

D4. Zeroize + TLS validators (F-004)

Zeroize + ZeroizeOnDrop derives on:

  • crates/craig-crypto/src/lib.rs::FieldEncryptor — Locked decision (2026-05-13): derive on the struct (not the individual key_bytes field). Struct-level derive composes correctly with the existing Clone derive and keeps the secret-material site obvious. zeroize crate handles per-field zeroization automatically.

  • services/craig-web/src/auth.rs::WebSession — derive on the struct; per-field Zeroize applies to access_token, refresh_token, id_token. (Note: WebSession lives in auth.rs, not session.rs. There is no session.rs in craig-web.)

TLS startup validators:

  • crates/craig-db/src/lib.rs::PgPool::new (or wherever the pool is built): parse DATABASE_URL; require sslmode=verify-full unless CRAIG_DB__ALLOW_INSECURE_TRANSPORT=true. Bail with clear error message at boot.

  • crates/craig-mq/src/lib.rs::Connection::new: parse RABBITMQ_URL; require amqps:// scheme unless override set.

D5. Concurrency races (F-015 + F-016)

F-015 approve_adjustment race fix. Wrap the 5 store calls inside services/craig-financial/src/api/adjustments.rs:140-180::approve_adjustment (the approve_adjustment handler body; lines 76-128 are an earlier handler’s authz scaffolding, not the race site) inside a single tx:

let mut tx = app.db.inner().begin().await?;
let current = store::adjustments::get_adjustment(&mut *tx, id).await?...
transitions::validate_adjustment_transition(&current.status, "approved")?;
let adjustment = store::adjustments::update_adjustment_status(&mut *tx, id, "approved", ...).await?;
let total_adjustments = store::adjustments::sum_approved_adjustments(&mut *tx, current.payment_id).await?;
let payment = store::payments::get_payment(&mut *tx, current.payment_id).await?...
let net_amount = payment.gross_amount + total_adjustments;
store::payments::update_payment_adjustments(&mut *tx, current.payment_id, &total_adjustments, &net_amount).await?;
tx.commit().await?;
Ok(Json(adjustment))

Store fns already accept PgExecutor per platform-stab-2 conventions (verify; refactor minor if needed).

F-016 approve_case_plan race fix. Wrap the 2 UPDATEs in services/craig-cases/src/store/case_plans.rs:90-118 inside a tx; add SELECT …​ FOR UPDATE on the parent case row at the top of the function:

pub async fn approve_case_plan(pool: &PgPool, id: Uuid, ...) -> Result<...> {
    let mut tx = pool.begin().await?;
    // Lock the parent case to serialize concurrent approvals on the same case.
    sqlx::query("SELECT id FROM cases WHERE id = (SELECT case_id FROM case_plans WHERE id = $1) FOR UPDATE")
        .bind(id).execute(&mut *tx).await?;
    sqlx::query("UPDATE case_plans SET status = 'superseded' ...").bind(id).execute(&mut *tx).await?;
    let row = sqlx::query_as::<_, CasePlan>("UPDATE case_plans SET status = 'active' ...")
        .bind(id).bind(approved_by).bind(worker_name)
        .fetch_optional(&mut *tx).await?;
    tx.commit().await?;
    Ok(row)
}

Plus DB-level partial UNIQUE INDEX:

CREATE UNIQUE INDEX idx_one_active_case_plan_per_case
    ON case_plans (case_id) WHERE status = 'active';

This is defense-in-depth: even if a future code change re-introduces the race, the DB rejects the second active row.

Cross-cutting invariants

These invariants must hold regardless of which step is in flight. Codify in inline tests where practical.

  1. HKDF info-string versioning. b"craig-crypto/blind-index/v1" is the only info string that produces today’s blind-index. If a future change rotates the derivation, bump to /v2 and reseed; never reuse a previous version label.

  2. Encryption-mode-parity with platform-stab-2 §D9. cases.reports’s `EncryptionMode { Required, Optional } posture must propagate via the same CRAIG_CASES__ENCRYPTION_MODE env var as cases.persons. No service-local mode toggle; deployments switch all encrypted tables together.

  3. Storage-format continuity for IP hashes. Pre-F-003 hash_ip produces hex; post-F-003 still produces hex (HMAC-SHA256 output formatted via format!("{:x}", …​)). Encoding stays hex so column type (TEXT) and downstream consumers don’t change.

  4. Reports searchable-surface preservation. admin_unit, received_at, received_request_id, received_ip_hash, partner_id, reporter_relation, id remain plaintext columns. Any list-view filter currently using these fields must continue to work after F-001.

  5. Zeroize derive site. Always derive on the struct (not individual fields). Composes with Clone; makes the secret-material site obvious to readers.

  6. Concurrency-test pattern. F-015 and F-016 tests both use craig_test_lib::concurrent_fire_collect(N) for deterministic completion via barrier; no tokio::time::sleep for synchronization.

  7. Pre-1.0 destructive-rebuild posture. Per the pre-1.0 destructive-rebuild posture — Steps 2, 3, 4, 7 all involve destructive schema changes followed by cargo xtask dev reseed. No expand-contract dance; no backfill for legacy data. This invariant applies until first-production deploy.

Steps

Step 1: Plan filing + GitLab issue tree

Files:

  • docs/modules/ROOT/pages/plans/pii-and-races.adoc (filed by prep MR; this step is bookkeeping)

  • docs/modules/ROOT/nav.adoc (filed by prep MR)

  • CHANGELOG.adoc (entry filed by prep MR)

GitLab artifacts: 1 epic + 8 step issues (filed by prep MR).

Branch: (within prep MR)

MR title: (within prep MR)

Verification:

  1. cargo xtask check-docs — Tier 1 docs untouched

  2. After prep MR merge: glab epic view <N> shows PII+races epic with 8 issues linked

CHANGELOG draft: (rolled into prep MR’s combined entry)

Step 2: HKDF blind-index (F-002)

Files:

  • crates/craig-crypto/Cargo.toml (add hkdf = "0.12")

  • crates/craig-crypto/src/lib.rs:119-131 (rewrite hmac per §D1)

  • services/craig-cases/migrations/<TS>_rebuild_ssn_hmac.sql (new — destructive rebuild: UPDATE cases.persons SET ssn_hmac = NULL + reseed; or DROP COLUMN + ADD COLUMN if cleaner)

  • services/craig-cases/src/api/persons.rs:71/146/252 (no code change — these call encryptor.hmac() which now derives the subkey internally)

  • crates/craig-crypto/tests/hkdf_blind_index.rs (NEW — 3 unit tests: same plaintext = same hmac across calls; HKDF-derived key bytes ≠ encryption key bytes; tampering with key bytes changes output)

Branch: feat/pii-and-races-step2-hkdf-blind-index

MR title: feat(craig-crypto): HKDF the blind-index key per RFC 5869 [Step 2 of pii-and-races]

Verification:

  1. cargo build -p craig-crypto — clean

  2. cargo nextest run -p craig-crypto — 3 new tests pass + existing pass

  3. cargo xtask dev reseed — devstack regenerates ssn_hmac column with new HKDF-derived values

  4. cargo nextest run -p craig-cases --test api persons — existing person-search-by-ssn tests pass

  5. cargo xtask validate --skip-docker — pre-push green

CHANGELOG draft:

=== HKDF blind-index key derivation [Step 2 of pii-and-races] (DATE)

`craig_crypto::FieldEncryptor::hmac` now derives a purpose-specific
blind-index subkey via HKDF-SHA256 (info = b"craig-crypto/blind-index/v1"),
separating it from the AES-GCM-SIV encryption key per RFC 5869 best practice.
Pre-1.0 destructive rebuild of cases.persons.ssn_hmac via devstack reseed.
3 new unit tests in craig-crypto. F-002 closed.

Step 3: Reports table PII encryption (F-001)

Files:

  • services/craig-cases/migrations/<TS>_encrypt_reports.sql (new — column comments noting application-layer encryption applied to listed fields; data preserved as TEXT/JSONB with encrypted bytes inside)

  • services/craig-cases/src/api/encryption.rs (extend with encrypt_report_pii / decrypt_report_pii helpers)

  • services/craig-cases/src/api/reports.rs (encrypt at create_report; decrypt at get_report + list_reports)

  • services/craig-cases/src/store/reports.rs (no changes if encryption happens at handler boundary)

  • services/craig-cases/tests/api/reports_encryption.rs (NEW — 4 tests: encrypt-decrypt round-trip; tampered-ciphertext fails closed; required-mode-no-key returns 500; encryption-mode optional + plaintext rows pass through)

Branch: feat/pii-and-races-step3-reports-encryption

MR title: feat(craig-cases): extend FieldEncryptor coverage to reports table PII fields [Step 3 of pii-and-races]

Dependencies: required Plan A Step 2 (ADR-025 identity normalization — cases.persons.assigned_worker UUID shape). Resolved: Plan A Step 2 shipped in !231 (2026-05-10).

Verification:

  1. cargo xtask dev reseed — schema migration applies cleanly

  2. cargo nextest run -p craig-cases --test api reports_encryption — 4 new tests pass

  3. cargo nextest run -p craig-cases — full suite passes (existing report tests adapted)

  4. Manual: submit a report via partner-intake; query DB directly (SELECT narrative FROM cases.reports LIMIT 1); confirm value is base64 ciphertext, not plaintext narrative

  5. Manual: GET /v1/cases/reports/{id} as authorized caller; confirm narrative decrypts to plaintext

CHANGELOG draft:

=== Reports table PII encryption [Step 3 of pii-and-races] (DATE)

cases.reports now encrypts reporter_first_name, reporter_last_name, reporter_phone,
narrative, children JSONB content, adults JSONB content, and raw_submission JSONB
at the application layer per platform-stab-2 §D9 pattern. Searchable metadata
fields (admin_unit, received_at, partner_id, etc.) remain plain. Closes federal-
compliance gap surfaced by 2026-05-08 audit (45 CFR § 1355.52). F-001 closed.

Step 4: IP hash salting (F-003)

Files:

  • services/craig-intake/src/api/hashing.rs:6 (rewrite hash_ip per §D3)

  • crates/craig-common/src/settings.rs::IntakeSettings (add ip_hash_secret: SecretString required field)

  • services/craig-intake/.env.example (document new env var with a non-trivial example value)

  • docker-compose.yml (set CRAIG_INTAKE__IP_HASH_SECRET for the intake container)

  • services/craig-intake/tests/api/ip_hashing.rs (NEW — 3 tests: same IP + same salt = same hash; different salts = different hashes; missing secret fails at boot)

Branch: feat/pii-and-races-step4-ip-hash-salt

MR title: feat(craig-intake): salt the IP hash via HMAC-SHA256(secret, ip) [Step 4 of pii-and-races]

Verification:

  1. cargo nextest run -p craig-intake --test api ip_hashing — 3 tests pass

  2. cargo xtask dev reseed — devstack reseeds with new salt

  3. Manual: query cases.reports.received_ip_hash from a freshly-seeded DB; confirm hashes look different from pre-migration (proves salt is applied)

CHANGELOG draft:

=== IP hash salted [Step 4 of pii-and-races] (DATE)

received_ip_hash now uses HMAC-SHA256(secret, ip) instead of bare SHA256(ip).
Closes the brute-forceable-IPv4 privacy gap from F-003. New required env var
CRAIG_INTAKE__IP_HASH_SECRET. Devstack reseeded with new default secret.

Step 5: Zeroize + TLS startup validators (F-004)

Files:

  • crates/craig-crypto/Cargo.toml (add zeroize = { version = "1.7", features = ["derive"] })

  • crates/craig-crypto/src/lib.rs::FieldEncryptor (add #[derive(Zeroize, ZeroizeOnDrop)] on the struct per §D4 locked decision)

  • services/craig-web/Cargo.toml (add zeroize dep)

  • services/craig-web/src/auth.rs::WebSession (Zeroize+ZeroizeOnDrop derive on the struct; auto-zeroizes access_token / refresh_token / id_token fields)

  • crates/craig-db/src/lib.rs (add validate_database_url_tls(url) → Result<()> called at pool-construction; bails if sslmode != verify-full unless CRAIG_DB__ALLOW_INSECURE_TRANSPORT=true)

  • crates/craig-mq/src/lib.rs (analogous validator for AMQP URI scheme: amqps:// required unless override)

  • crates/craig-common/src/settings.rs (add allow_insecure_transport: bool field, default false, env-overridable)

  • docker-compose.yml + .env.example (set CRAIG_DBALLOW_INSECURE_TRANSPORT=true and CRAIG_MQALLOW_INSECURE_TRANSPORT=true for devstack)

  • crates/craig-db/tests/tls_validator.rs (NEW — 3 tests: valid sslmode passes; missing sslmode + override=false fails; missing sslmode + override=true passes)

  • crates/craig-mq/tests/tls_validator.rs (NEW — 3 analogous tests)

Branch: feat/pii-and-races-step5-zeroize-and-tls

MR title: feat(craig-crypto, craig-db, craig-mq, craig-web): zeroize secrets + require TLS at boot [Step 5 of pii-and-races]

Verification:

  1. cargo nextest run -p craig-crypto -p craig-db -p craig-mq -p craig-web — new tests pass

  2. Manual: in a test container, unset CRAIG_DB__ALLOW_INSECURE_TRANSPORT, set DATABASE_URL=postgres://…​ (no sslmode); start service; confirm boot fails with clear error message

  3. cargo xtask dev reseed — devstack with overrides set still boots

  4. cargo xtask validate --skip-docker — pre-push green

CHANGELOG draft:

=== Zeroize secrets + require TLS at boot [Step 5 of pii-and-races] (DATE)

FieldEncryptor key + WebSession token fields now ZeroizeOnDrop; memory dumps
no longer reveal long-lived secrets after they leave scope. craig-db + craig-mq
validate TLS configuration at pool/connection construction; service bails at
boot with a clear error if `sslmode=verify-full` (DB) or `amqps://` (MQ) is
absent unless CRAIG_<CRATE>__ALLOW_INSECURE_TRANSPORT=true override is set
(devstack default). F-004 closed.

Step 6: approve_adjustment race fix (F-015)

Files:

  • services/craig-financial/src/api/adjustments.rs:140-180::approve_adjustment (wrap in pool.begin() …​ tx.commit() per §D5)

  • services/craig-financial/src/store/{adjustments,payments}.rs (verify PgExecutor shape; refactor if any fns still take &PgPool instead of impl PgExecutor)

  • services/craig-financial/tests/api/approve_adjustment_concurrent.rs (NEW — concurrency test using craig_test_lib::concurrent_fire_collect: 10 parallel approve_adjustment calls on 10 different adjustments of the same payment; assert final payments.net_amount = gross + sum(approved.amount))

Branch: fix/pii-and-races-step6-approve-adjustment-tx

Verification of the race site: lines 76-128 (an unrelated authz scaffolding block) appear earlier in the file; do not edit those. The race is in the approve_adjustment handler body starting at line 140.

MR title: fix(craig-financial): wrap approve_adjustment in transaction [Step 6 of pii-and-races]

Verification:

  1. cargo nextest run -p craig-financial --test api approve_adjustment_concurrent — passes consistently (run 50× in CI to confirm not flaky)

  2. cargo nextest run -p craig-financial — full suite passes

  3. cargo xtask validate --skip-docker — pre-push green

CHANGELOG draft:

=== approve_adjustment now transactional [Step 6 of pii-and-races] (DATE)

services/craig-financial/src/api/adjustments.rs::approve_adjustment wrapped in
pool.begin()...tx.commit() to close the lost-update race surfaced by 2026-05-08
audit. New concurrency test in approve_adjustment_concurrent.rs uses
concurrent_fire_collect(10) to spawn parallel approvals on the same payment;
asserts payments.net_amount converges. F-015 closed.

Step 7: approve_case_plan race fix (F-016)

Files:

  • services/craig-cases/src/store/case_plans.rs:90-118 (wrap in tx; add SELECT …​ FOR UPDATE on parent case at top per §D5)

  • services/craig-cases/migrations/<TS>_one_active_case_plan.sql (NEW — CREATE UNIQUE INDEX idx_one_active_case_plan_per_case ON case_plans (case_id) WHERE status = 'active')

  • services/craig-cases/tests/api/approve_case_plan_concurrent.rs (NEW — concurrency test: 2 draft case plans on same case; spawn 2 parallel approve_case_plan calls; assert exactly one ends active, the other is either superseded or remains draft)

Branch: fix/pii-and-races-step7-approve-case-plan-tx

MR title: fix(craig-cases): approve_case_plan transactional + DB partial unique index [Step 7 of pii-and-races]

Verification:

  1. cargo xtask dev reseed — partial unique index applies cleanly to seeded data

  2. cargo nextest run -p craig-cases --test api approve_case_plan_concurrent — passes consistently (50× in CI)

  3. Pre-migration sanity check: query for any case with > 1 active case plan; if found, manually triage before migration applies (pre-1.0 + reseed makes this unlikely)

CHANGELOG draft:

=== approve_case_plan transactional + DB-level invariant enforcement [Step 7 of pii-and-races] (DATE)

services/craig-cases/src/store/case_plans.rs::approve_case_plan wrapped in tx
with SELECT ... FOR UPDATE on parent case row to serialize concurrent approvals.
Defense-in-depth: new partial UNIQUE INDEX on (case_id) WHERE status='active'
makes the "exactly one active case plan per case" invariant DB-enforced. New
concurrency test in approve_case_plan_concurrent.rs. F-016 closed.

Step 8: Plan completion audit + archive

Files:

  • docs/modules/ROOT/pages/plans/pii-and-races.adoc (Status table → all Complete)

  • docs/modules/ROOT/nav.adoc (move from Active to Archive)

  • docs/modules/ROOT/pages/plans/archive.adoc (NEW row under Security & Compliance with all step MRs)

  • .claude/CLAUDE.md (Phase Status row added)

  • CHANGELOG.adoc (wrap-up entry)

Branch: chore/pii-and-races-step8-archive

MR title: chore: pii-and-races plan completion + archive [Step 8 of pii-and-races]

Verification: mirror platform-stab-2 Step 13 verification pattern.

Files Touched

File Step Change

crates/craig-crypto/{Cargo.toml,src/lib.rs}

2,5

HKDF + Zeroize

services/craig-cases/migrations/<TS>_rebuild_ssn_hmac.sql

2

NEW

crates/craig-crypto/tests/hkdf_blind_index.rs

2

NEW

services/craig-cases/migrations/<TS>_encrypt_reports.sql

3

NEW

services/craig-cases/src/api/{encryption,reports}.rs

3

EDIT

services/craig-cases/tests/api/reports_encryption.rs

3

NEW

services/craig-intake/src/api/hashing.rs

4

EDIT

crates/craig-common/src/settings.rs

4,5

EDIT

services/craig-intake/tests/api/ip_hashing.rs

4

NEW

services/craig-web/{Cargo.toml,src/auth.rs}

5

EDIT

crates/craig-db/src/lib.rs

5

EDIT

crates/craig-mq/src/lib.rs

5

EDIT

crates/craig-{db,mq}/tests/tls_validator.rs

5

NEW

services/craig-financial/src/api/adjustments.rs

6

EDIT

services/craig-financial/tests/api/approve_adjustment_concurrent.rs

6

NEW

services/craig-cases/src/store/case_plans.rs

7

EDIT

services/craig-cases/migrations/<TS>_one_active_case_plan.sql

7

NEW

services/craig-cases/tests/api/approve_case_plan_concurrent.rs

7

NEW

Verification

After every step:

  1. cargo xtask validate --skip-docker

  2. cargo xtask dev reseed (Steps 2, 3, 7)

  3. cargo nextest run --workspace

  4. cargo xtask check-docs

Plan-wide:

  1. End-to-end encryption round-trip on a real report (Step 3)

  2. Concurrency soak test on Steps 6/7 (50 iterations of concurrent_fire_collect; zero flakes)

  3. Manual: stop service with insecure DB URL (Step 5); confirm boot bails clearly

Expected test-count delta

Workspace test count today is ~2005 (post Plan F !274). Plan B should add ~18 tests:

Step New tests Count

2

HKDF: same input ⇒ same hmac; HKDF-derived key ≠ master key; master-key tamper changes output

3

3

Reports encrypt-decrypt round-trip; tampered-ciphertext fails closed; required-mode-no-key returns 500; optional-mode plaintext passthrough

4

4

Same IP + same salt = same hash; different salts = different hashes; missing secret fails at boot

3

5

DB TLS validator: valid sslmode passes / missing+override-false fails / missing+override-true passes; MQ equivalents

6

6

approve_adjustment concurrent invariant: payments.net_amount = gross + Σ approved.amount over 10 parallel calls

1

7

approve_case_plan concurrent: exactly one ends active; loser rejected by DB partial UNIQUE (not just by app code)

1

Target post-Plan-B: ~2023 tests. Step 8 archive reconciles actual delta per Reconcile test-count deltas.

Migration / rollout / rollback

Pre-1.0 + destructive-reseed posture (per the pre-1.0 destructive-rebuild posture):

  • Steps 2, 3, 4, 7 introduce schema or hash-format changes. Devstack rollout: cargo xtask dev reseed after each merge regenerates all derived columns. No production data exists yet, so no expand-contract dance.

  • Step 5 TLS validators are operator-facing. Devstack continues to work because CRAIG_DBALLOW_INSECURE_TRANSPORT=true + CRAIG_MQALLOW_INSECURE_TRANSPORT=true are set in docker-compose.yml. Production deployments must set these to false (or omit; default is false) and provision proper TLS at the DB + MQ layers.

  • Rollback path per step:

  • Step 2: revert craig-crypto/src/lib.rs + drop the rebuild migration; reseed.

  • Step 3: revert reports.rs + encryption.rs handler changes; reseed (plaintext re-enters DB).

  • Step 4: revert hashing.rs + remove IP_HASH_SECRET env var; reseed.

  • Step 5: revert validators; remove ALLOW_INSECURE_TRANSPORT settings (the env vars become no-ops).

  • Step 6, 7: revert tx wrappers + the case_plans partial UNIQUE INDEX migration. The Step 7 DB constraint may reject existing data if multiple active case plans existed at the time the constraint applied — pre-1.0 + reseed makes this unlikely, but document the manual triage path (find rows; demote excess to superseded).

Documentation Updates

  • Per-step CHANGELOG entries

  • .claude/docs/security.md — encryption coverage extended to reports table; HKDF blind-index posture; TLS-required-at-boot

  • .claude/docs/services.md — encryption notes on cases.reports

  • docs/modules/ROOT/pages/architecture.md — concurrency-correctness section addendum (F-015/F-016)

  • docs/modules/ROOT/pages/deployment-guide.adoc — new required env vars (CRAIG_INTAKE__IP_HASH_SECRET); TLS posture for production

  • Step 8 archive entry

Risks

Risk Mitigation

Step 3 reports encryption changes searchable surfaces in BFF list views

Searchable fields stay plain (admin_unit, status, dates); narrative + reporter PII opaque; documented in security.md

Step 7 partial UNIQUE INDEX fails on existing data with > 1 active case plan per case

Pre-migration sanity query; manual triage; pre-1.0 + reseed makes it unlikely

Step 5 TLS-required gate breaks devstack onboarding

Override env var documented in .env.example + local-dev.md

Concurrency tests Steps 6/7 flaky on slow CI

Use concurrent_fire_collect (deterministic completion via barrier); generous timeouts

Errata

  • Step 3 (2026-05-13): 4 new helper tests landed inline in services/craig-cases/src/api/encryption.rs rather than the planned services/craig-cases/tests/api/reports_encryption.rs integration test file. The new tests are pure helper unit tests that exercise the (encryptor, mode, is_encrypted) matrix — they belong next to the existing encrypt_field / decrypt_field tests (also inline) for review-surface consistency. Functional coverage is identical to the planned spec (round-trip, tampered-fails-closed, required-no-encryptor-500s, optional-plaintext-passthrough).

  • Step 5 (2026-05-14) — TLS-URL validators dropped from F-004 scope: The §D4 design called for validate_database_url_tls (requiring sslmode=verify-full) + validate_amqp_url_tls (requiring amqps://) startup gates with CRAIG_<CRATE>__ALLOW_INSECURE_TRANSPORT=true override for devstack. Dropped after analyzing actual deployment topologies. A URL-level gate only fits one of four common production patterns (self-hosted with TLS-direct); breaks or papers-over the other three:

    • Sidecar mTLS (K8s service mesh — Istio/Linkerd): services connect to localhost in cleartext; sidecar handles mTLS. URL-level gate forces operators to disable the validator entirely.

    • Managed DB/MQ inside private VPC (AWS RDS / AWS MQ): TLS available but operator-cert-chain-dependent; verify-full requires a CA bundle path the validator can’t synthesize. Often operators run TLS-off inside the VPC perimeter.

    • Devstack / docker-compose: no TLS provisioning today; would require an rcgen-based cert generator + postgres/rabbitmq config rewrites + per-container CA mounting.

      Net: the validator only catches the operator who runs production CRAIG with cleartext DB on the public internet — the very-low-frequency case the threat model already considers a self-inflicted ops failure. Forcing the gate creates more support friction than it prevents loss.

      What Step 5 still ships: the F-004 Zeroize derives. Process-memory protection for secrets-in-flight (FieldEncryptor master key, WebSession bearer tokens) is independent of transport posture and benefits every deployment pattern equally.

      Recommended posture for production deployers (documented in docs/modules/ROOT/pages/deployment-guide.adoc): operate TLS at whichever layer the deployment architecture is designed around — direct-TLS in URL, sidecar mTLS, VPC + private subnet, etc. CRAIG is transport-neutral by design.

  • Step 5 (2026-05-14) — Files-Touched table: the planned crates/craig-{db,mq}/tests/tls_validator.rs test files do not land (validators dropped per above). crates/craig-common/src/settings.rs::allow_insecure_transport field does not land (no validator means no override flag).

  • Step 6 (2026-05-14) — SELECT FOR UPDATE added beyond plan §D5: the plan body called for pool.begin() …​ tx.commit() to wrap the 5 store calls. Empirically discovered during the Step 6 concurrency test write that pure tx-wrap is insufficient under Postgres READ COMMITTED isolation — each tx sees only its own pre-commit writes to the adjustment rows, so two concurrent approvers each compute SUM from a narrow view + the later committer’s UPDATE payments SET adjustments = its_narrow_sum silently overwrites the earlier. Final fix adds SELECT id FROM payments WHERE id = $1 FOR UPDATE at the top of the tx to serialize approvers on the payment row. The test was the discovery vector: it failed deterministically with pure-tx-wrap and passed 20/20 with the FOR UPDATE lock. Plan body §D5 didn’t anticipate this — the §D7 fix for F-016 explicitly does include FOR UPDATE; the design parallels weren’t fully consistent. Documented in commit + CHANGELOG; no separate ADR filed.

  • Test count delta — actual vs planned (2026-05-14): Plan body Expected test-count delta projected ~+18 (3+4+3+6+1+1). Actual: +12 (3+4+3+0+1+1). The 6-test gap is entirely Step 5: planned 6 TLS-validator tests didn’t land because the validators didn’t land. Workspace count tracked 2005 → 2017 across the 6 implementation steps. Reconciliation per Reconcile test-count deltas.

After this plan lands

  • All known PII fields in cases.reports encrypted at application layer

  • Blind-index uses HKDF; encryption + HMAC have separate purpose-derived keys

  • IP hash is non-reversible without the deployment secret

  • FieldEncryptor + WebSession secrets zeroize on drop

  • DB + MQ require TLS at boot (production); devstack override path documented

  • 2 additional concurrency races (post platform-stab-2) closed with tx wrappers + DB-level invariant

  • Plan A’s authz layer + Plan B’s data-protection layer together close the federally-relevant application-layer gaps

Edit this page · latest