Plan: Quarantine applicant uploads until a real malware scan passes (#1006, epic &52)
On this page
- Status
- Context
- Ratified decisions (frozen)
- Key verified anchors
- Design
- State machine — bound to content identity
- Viewable predicate
- Gate law
- Skipped override (D6)
- Scanner backends
- Config (flat keys, serde defaults; cross-validated in a
from_configfrozen struct whose errors name the env var) - Guard (D3)
- Migration
20261106000000_document_scan_quarantine.sql - Promotion worker (
scan_worker.rs) - Rescan endpoint
- Audit pipeline (cross-service, same MR)
- Wire contract (pre-1.0, typed — no compat shims)
- UI rules
- Deployment (devstack contract + production guidance)
- AC → proof map (AC wording per the recorded reconciliation)
- Verification
- Risks
- Delivery mechanics
Architecture ratified on-issue 2026-08-10 (adapter shape, clamd default, fully-async lifecycle, fail-closed-default + accountable-override guard per the 2026-08-03 #1265 bundling ruling). Governing ADRs: ADR-008 (its scan-before-S3 + NoopScanner-devstack clauses are amended by this plan’s docs step), ADR-016 (expand-only), ADR-014 (audit chain), ADR-007 (CLI parity — rescan in-MR, override in #1417), ADR-004 (IDs-only event payloads). Pre-filed side issues: #1415 (orphan objects), #1416 (verification unresolve automation), #1417 (override CLI actor assertion). Non-overlapping with #546 (credential isolation; same epic).
Status
| Step | Description | Status |
|---|---|---|
0 |
Preflight (claim #1006, labels, reconciliation record, #1415/#1416/#1417 filed) + commit this plan, nav-linked under epic &52. |
Done (2026-08-10) — aaf01b34 (the AC-reconciliation record rides the issue closing comment; a tooling permission gate blocked the standalone note) |
1 |
|
Done (2026-08-10) — 5d1a37c1 |
2 |
|
Done (2026-08-10) — 7b7654ea |
3 |
|
Done (2026-08-10) — 57edfc5e |
4 |
|
Done (2026-08-10) — a80dc4b5 |
5 |
|
Done (2026-08-10) — 90a6cb83 |
6 |
|
Done (2026-08-10) — 7b32ddd3 |
7 |
Docs ripple: new short ADR + ADR-008 numbered amendment + api/data-models/services/security/security-operations/configuration-reference/testing/local-dev/shared-crates pages + |
Done (2026-08-10) — this commit (ADR-042; ADR-008 Amendment 4; runbook; api/data-models/services/security/config/testing/local-dev/shared-crates; CHANGELOG; OpenAPI regen). Deviation: the planned security-operations.adoc touch landed in security.adoc (the boundary section’s natural home); the backend-switch sweep is event-less by design (bulk + WARN count — the rescan endpoint is the audited path) |
8 |
Battery (pre-push, sole gate) + devstack probe + Draft MR → ready → force-merge → close #1006 with evidence + reconciliation record. |
Done (2026-08-10) — battery push + force-merge; SHAs + evidence in the #1006 closing comment |
Epic: &52
Issue: #1006 (priority::high, T1 — Correctness)
Branch: feature/1006-upload-quarantine
Context
canopy-applications always injects NoopScanner (main.rs:94) — every upload is marked clean without inspection — and get_document_content streams bytes to worker browsers with no scan check (api/documents.rs:311-340). Magic-byte/MIME validation is not malware detection. This MR adds the content-safety boundary: a real scanner, a quarantine lifecycle bound to content identity, and state-gated serving. Scanner outages, encrypted containers, and unsupported content become fail-closed quarantine states, never clean.
Ratified decisions (frozen)
| # | Decision |
|---|---|
D1 |
|
D2 |
clamd is the default backend: sidecar container + pure-Rust TCP INSTREAM client (musl-clean). |
D3 |
noop stays selectable, but a non-development environment with backend=noop and no explicit |
D4 |
Fully async lifecycle: every upload is accepted durable at |
D5 |
Serving gates on state ( |
D6 |
|
D7 |
Legacy rows are blocked from serving and batch-rescanned. |
Recorded reconciliations (from plan approval, 2026-08-10): the AC phrase "until marked clean" reads "until viewable — clean, or skipped carrying the audited accountable override" (D6 already authorizes this; the override is supervisor/admin-only and origin-enforced); the config key follows the service’s flat single- convention: CANOPY_APPLICATIONSSCANNER_BACKEND (tokens clamav | noop).
Key verified anchors
Pipeline. Scanner/ScanResult{Clean, Infected{signature}, Skipped{reason}}/ScanError{Backend, Timeout}: crates/canopy-store/src/scanner.rs:14-57. The scan runs inline as validate_upload step 5 (validation.rs:128-137) — its only await; removing it makes the fn sync. Callers: the applications upload handler (api/documents.rs:214) and Store::put_validated (store.rs:115). canopy-notices scans only via put_validated (generator.rs:261-268) — inline scanning is notices' correct semantic, so put_validated keeps its scanner param and notices is untouched. The upload handler collapses real-scanner outcomes at documents.rs:240-246 (a real Skipped is recorded clean — that defect dies here). Single byte egress: get_document_content (:311-340). BFF relay: canopy-web/src/api/actions.rs:730-789; proxy_error_status (:21-26) maps non-404 to 502 and the relay sets Cache-Control: no-transform, private (:782) — both change. Accept/reject SQL predicates only on ids (store/documents.rs:123,147) — TOCTOU until the predicate moves inside the UPDATE. Worker-side accept auto-resolves verifications (actions.rs:618-668). The portal attaches any own document to verification responses (portal/src/verifications.rs:109). Applicants have no download leg.
Identity & authz. s3_key = {program}/{application_id}/{sha_hex} is content-addressed but Store::put overwrites (store.rs:93) — a verdict is only safe if serving re-verifies content identity. Actor JWTs carry roles (canopy-auth claims via the configured roles path, lifted into claims.actor, middleware.rs:109-143) so the origin CAN enforce a role-gated override. canopy-web WorkerRole tier: Admin > StudioAdmin > Supervisor > … > Caseworker (session.rs:29-104); WritePermission includes caseworkers — insufficient for release-from-quarantine.
Pins & literals new surface breaks. Applications OpenAPI path-count pin 28 (api/mod.rs:1596-1601); web mutating-route canary EXPECTED_MUTATING_TOTAL = 65 (xtask/src/cmd/route_authz.rs:96, battery gate); web doc fixture (case_detail/sections/documents.rs:249-272); From<ApplicationDocumentRow> (domain.rs:262); four exhaustive ApplicationsConfig literals (config.rs:302, reconciler.rs:433, store/finalize_ops.rs:622, tests/common/mod.rs:451); store fixture new_doc (store/documents.rs:212); the pending-review badge counts quarantine-blind (sections/documents.rs:133).
Templates. Guard: canopy-verification/src/guard.rs (whole file + 4-test matrix; #[serde(default)] bool knob). Worker: canopy-appeals/src/assessment_worker.rs (5s poll, UPDATE-returning claim FOR UPDATE SKIP LOCKED, attempts-at-claim, backoff 60s×2^n cap 6h, RunOutcome/settle) + the notices work_items.rs fence, terminal at attempts >= max (:161 — same >= here). Publisher cloned into worker deps before the AppDeps move (appeals main.rs:49,228; applications main.rs:142). Audit: outbox event in the domain tx → canopy-security wildcard consumer → ADR-014 chain; the parser recognizes neither document_id (resource) nor overridden_by (actor) today (event_parsing.rs:41,172) — parser arms are part of this MR. Migration namespace max 20261105000000 ⇒ this MR uses 20261106000000; sqlx’s Migrator holds a per-DB advisory lock and DDL takes the table lock — no extra advisory lock. Errors: house 409 ApiError::Conflict (no 423/451 precedent). Test gating: infrastructure_available()/skip_or_panic; CANOPY_TEST_INFRA=required in the in-network lane; bare-schema partial-migration precedent canopy-api/tests/migration_phase_test.rs:27-58; fake-server shape canopy-test-lib/src/mock.rs MockHandle. e2e: exactly two document specs (applicant-portal.spec.ts :285-303/:331+, worker-determination-ele.spec.ts :173-232), both server-rendered; no seed inserts documents.
Crate. clamav-client (tokio feature) — the only maintained pure-Rust clamd client (clamd-client and clamav-tcp are unmaintained since 2022; a hand-rolled ~80-line INSTREAM client is the fallback). Exact version/features/dependency tree are re-verified against crates.io at implementation and recorded in the lockfile + MR.
Design
State machine — bound to content identity
scan_status ∈ {pending, clean, infected, skipped, error}; the 'noop' value dies entirely.
| Trigger | Transition |
|---|---|
Upload |
INSERT |
Worker finds size or sha256 mismatch vs the row |
→ |
|
→ |
|
→ |
|
→ |
|
stays |
Object missing in store |
→ |
Any non-clean settle on an accepted row |
the same UPDATE also revokes acceptance (clears |
Requeue (rescan endpoint / legacy backfill / noop-backend switch sweep) |
atomically: |
Content-identity binding. The worker fetches the object and checks len == size_bytes and sha256(bytes) == row.sha256 BEFORE scanning. get_document_content stops streaming blind: it reads the object fully (bounded by the existing 10 MiB upload cap), re-verifies sha256 == row.sha256, and only then serves — a replaced object is unservable regardless of scan state (500-class + ERROR log on mismatch). All content responses — success and error, origin and BFF relay — carry Cache-Control: no-store so a browser can never replay bytes past a later quarantine.
Claim protocol. Claim sets scan_claim_token = uuidv7() (+ claimed_at/claimed_by for observability), captures scan_generation, increments scan_attempts. EVERY subsequent mutation — settle, defer, terminal error, orphan error — is fenced WHERE id=$ AND scan_claim_token=$ AND scan_generation=$ AND scan_status='pending': a stale worker (lease-reclaimed or pre-requeue) can never mutate or emit events for a newer claim. Claim order ORDER BY scan_due_at, id; stale-lease eligibility scan_claimed_at < now() - lease. The whole attempt (fetch + hash + scan) runs under one deadline (scan_attempt_timeout_secs); config validation requires lease > attempt timeout. Attempts count claims; terminal at attempts >= scan_max_attempts evaluated at claim (notices >= semantics); tests at max−1 / max / max+1 / crash-after-claim.
Viewable predicate
Defined once in applications domain.rs, unit-tested, surfaced on the wire as derived scan_viewable: bool:
viewable = status == Clean || (status == Skipped && scan_override_at.is_some())
Gate law
get_document_content, accept_document, reject_document refuse 409 unless viewable. For accept/reject the predicate lives inside the UPDATE … WHERE (no check-then-update race); a typed outcome distinguishes Updated / Missing (404) / Quarantined (409). Verification auto-resolution therefore only ever fires on viewable documents. The web BFF’s proxy_error_status gains a Some(409) ⇒ CONFLICT arm + quarantine copy.
Accepted-row reconciliation policy. Acceptance is only ever valid for a viewable document — enforced by DB CHECK, by revocation-on-settle, and by revocation-on-requeue. Linked verification facts are NOT auto-unresolved in this MR: the acceptance_revoked event + the flagged document UI route it to human review; the cross-service automation is #1416. Covered-path test: accepted legacy row → backfilled pending → scans infected ⇒ acceptance revoked + event + never servable.
Skipped override (D6)
POST /v1/applications/{id}/documents/{document_id}/scan-override: service caller + verified actor whose roles include supervisor or admin — enforced at the origin from the actor JWT’s roles (403 otherwise); the BFF additionally gates the action and renders the affordance only for WorkerRole >= Supervisor. Body ScanOverrideRequest { reason: String } — trimmed, 8..=500 chars, 422 outside bounds. Guarded UPDATE … WHERE scan_status='skipped' AND scan_override_at IS NULL RETURNING; row-missing ⇒ 404, wrong-state/duplicate ⇒ 409 (duplicate POST is a clean 409). Response = the updated ApplicationDocument. The same tx stages application_document.scan_overridden {application_id, document_id, overridden_by, reason_sha256} — the digest tamper-binds the free-text reason (stored in scan_override_reason) without putting operator prose in the immutable chain.
Scanner backends
New crate crates/canopy-scanner-clamd (scanner.rs:3-8 keeps real impls out of canopy-store). Trait change (pre-1.0): scan() returns ScanReport { result: ScanResult, backend_version: Option<String> } — provenance is observed WITH the verdict. ClamdScanner: INSTREAM scan then VERSION on the same flow; VERSION failure after a verdict ⇒ ScanError::Backend (a clamav clean verdict without provenance never settles). The VERSION string carries the definition generation + date; parsed age > scanner_max_definition_age_days ⇒ ScanError::Backend("definitions stale…") — scanning fails closed, serving (D5) unaffected. Definition-age gauge + WARN.
| clamd response | Result |
|---|---|
|
|
|
|
|
|
any other |
|
|
|
other |
|
transport failure / deadline |
|
Strict framing: exactly one NUL/newline-terminated response line, ≤512 bytes, valid UTF-8 — anything else is Backend. The response-line parser carries a proptest (total, panic-free, grammar round-trip). The fake clamd validates real wire format (zINSTREAM\0, 4-byte big-endian chunk lengths, zero terminator) and scripts every row above plus oversized / multi-line / invalid-UTF-8 responses and connection drops.
ScannerBackend { #[default] Clamav, Noop } (verification AdapterSelection idiom). The Noop backend settles clean with scan_backend='noop' provenance (honest via provenance; production-blocked by the guard). Backend-switch requeue: on boot with backend ≠ Noop, a startup sweep requeues scan_backend='noop' rows in bounded batches (logged) — switching a database off noop re-scans everything noop ever touched.
Config (flat keys, serde defaults; cross-validated in a from_config frozen struct whose errors name the env var)
| Knob | Default | Validation |
|---|---|---|
|
|
— |
|
none |
required when clamav |
|
30 |
1..=300 |
|
120 |
> clamd_timeout; < lease |
|
5 |
1..=300 |
|
2 |
0..=8; 0 ⇒ ERROR log at boot + |
|
8 |
1..=32 |
|
600 |
> attempt timeout |
|
7 |
≥1 |
|
false |
— |
Guard (D3)
services/canopy-applications/src/guard.rs, #1265 clone: pure evaluate(env, allow), called only when backend == Noop, before scanner wiring; refusal names the hazard (unscanned citizen uploads served to workers), #1006, and the exact override var; the override warns every boot. Test matrix transplanted (dev-permissive ×2, all non-dev tiers refused, override boots, unset env ⇒ production).
Migration 20261106000000_document_scan_quarantine.sql
Single tx (Migrator’s per-DB lock + DDL table locks suffice; no extra advisory lock):
-
ALTER TABLE application_documents DROP CONSTRAINT application_documents_scan_status_check(plain — loud on name drift). -
ADD COLUMN: provenance (scan_backend TEXT,scan_backend_version TEXT,scanned_at TIMESTAMPTZ,scan_detail TEXT), worker (scan_attempts INT NOT NULL DEFAULT 0,scan_due_at TIMESTAMPTZ NOT NULL DEFAULT now(),scan_claimed_at TIMESTAMPTZ,scan_claimed_by TEXT,scan_claim_token UUID,scan_last_error TEXT,scan_generation INT NOT NULL DEFAULT 0), override (scan_override_by UUID,scan_override_at TIMESTAMPTZ,scan_override_reason TEXT). -
Backfill — every legacy row is unprovable:
UPDATE … SET scan_status='pending', accepted_at=NULL, accepted_by=NULL, scan_due_at = now() + (rn * interval '2 seconds')for ALL existing rows, staggered viarow_number()so the requeue cannot flood the queue or starve fresh uploads (fresh uploads getdue=now()). This subsumes'noop'and covers legacyclean|infected|skipped|errorrows with no provenance — including the collapse defect’s real-Skipped-recorded-clean class. -
ADD CHECK— the state machine as named constraints:ck_docs_scan_status(5 tokens);ck_docs_scan_attempts_nonneg;ck_docs_scan_claim_paired(claimed_at/claimed_by/claim_tokenall null or all set);ck_docs_scan_override_all_or_none;ck_docs_scan_override_only_skipped;ck_docs_scan_terminal_provenance(clean|infected|skipped ⇒ backend + scanned_at set; error ⇒ scanned_at set AND (detail OR last_error) set);ck_docs_accepted_viewable(accepted_at null OR clean OR skipped-with-override). -
ALTER COLUMN scan_status SET DEFAULT 'pending'. -
Partial index
(scan_due_at, id) WHERE scan_status='pending'.
Header cites #1006 + ADR-016 + the rolling-deploy note (an old binary’s 'noop' INSERT fails loudly; accepted pre-1.0, no shims). Proof test (bare schema + single-connection pool per the migration_phase_test.rs precedent; one schema reused across cells): partial-apply (temp dir minus this file) → seed noop + accepted-clean legacy rows → full apply → both pending, acceptance cleared, fresh 'noop' INSERT rejected, CHECKs active.
Promotion worker (scan_worker.rs)
WorkerDeps { db, store, scanner, publisher }, spawned per concurrency slot. Loop: claim → fetch + hash-verify + scan under the attempt deadline, outside any tx → settle in one short tx fenced on (token, generation, pending) → stage application_document.scan_completed {application_id, document_id, scan_status, scan_backend, scan_backend_version} (a PII-safe allowlist — enum-class tokens, no free text) in the settle tx; acceptance revocation folds into the same UPDATE when applicable. Telemetry: outcome counter, scan_pending_count, scan_oldest_pending_age_secs (WARN over threshold), definition-age gauge, scan_worker_disabled. Backlog policy: uploads stay admitted during outages (that IS D4/D5 — size-capped; count quotas explicitly out of scope), alerting via the pending-age gauge + runbook.
Rescan endpoint
POST /v1/applications/{id}/documents/{document_id}/rescan — service-caller only; the atomic requeue (generation bump); valid from any settled state (pending ⇒ 409 already-queued); stages application_document.scan_requeued {application_id, document_id, cause="manual"}. Supported recovery for terminal error rows — no direct SQL. CLI parity in this MR.
Audit pipeline (cross-service, same MR)
canopy-security event_parsing.rs gains recognized arms: document_id in the resource-id key list; overridden_by in the actor key list. Producer→parser integration test (stage each new event type, run the parser, assert resource/actor extraction). Events: scan_completed (incl. terminal error + identity-mismatch causes), scan_overridden (+reason_sha256), acceptance_revoked {ids, cause}, scan_requeued {ids, cause}. Backend-bypass (guard override) is boot-time WARN per D3.
Wire contract (pre-1.0, typed — no compat shims)
-
scan_status: ScanStatusenum (snake_case tokens,as_db()/from_db()+ token round-trip tests; row→DTO via TryFrom — CHECK-impossible tokens are internal errors, never fabricated states). Breaking wire change: both consumers updated in-MR; CHANGELOG=== Changed. -
New
#[serde(default)]fields (the contracts crate’s additive-field house style):scan_backend,scan_backend_version,scanned_at,scan_detail,scan_override_at, derivedscan_viewable: bool. Audience rule:scan_detail(signature/reason) is worker-facing; the portal projection does not read it. -
Doc-comment token list updated;
roundtrip.rsgains documents strategies from scratch + missing-field-defaults proptests.
UI rules
-
Web Documents section: scan-state pill; View/accept/reject rendered only when viewable; override affordance (reason input) on skipped rows for Supervisor+; the pending-review badge counts only viewable rows.
-
Web Verifications section: joins the application’s
LIST_DOCUMENTSonce per render, mapsdocument_id → scan state; non-viewable linked documents render the state instead of a dead link. -
Portal: attach-to-verification stays permitted for any own document (reference-only; attaching the fresh upload is the primary flow) — the worker side renders its quarantine state. The documents page covers all five states (pending "being checked" / clean / infected "flagged — upload a replacement" / skipped "couldn’t be scanned automatically — a worker will review" / error "couldn’t be processed — a worker will follow up"); the upload banner says the file "will be checked first".
Deployment (devstack contract + production guidance)
-
devstack/clamav/Dockerfile:FROM clamav/clamav@sha256:<digest>(exact digest — feature tags are mutable),USER clamav(container-ops non-root rule), bakedclamd.conf+test.ndb. -
Signature lifecycle, honestly: the image ships no definitions; a named volume
clamav-dbon/var/lib/clamavpersists them — first boot runs freshclam (network, minutes; healthcheckclamdcheck.sh+start_periodsized for it), later boots warm; in-container freshclam keeps them fresh. Production: the runbook covers mirror/egress, reload behavior, and the max-age policy (pairs with the service-side stale-definitions defer). -
clamd.conf: TCPSocket 3310;AlertEncrypted yes;AlertExceedsMax yes+ explicitMaxScanSize/MaxFileSize(≥16M, above our 10 MiB cap),MaxRecursion,MaxFiles,MaxScanTime;MaxThreads 4,MaxQueue 16(≥ replicas × concurrency = 1×2, with headroom). -
Compose: six profiles;
mem_limit: 4g(official guidance 3–4 GiB + reload peaks),cpus: 2,pids_limit: 256; loopback-only host publish, kept ONLY because host-lane integration tests need it (the in-network lane uses compose DNS; isolation guidance — unauthenticated protocol, never expose beyond the compose network — lands in security.adoc + the runbook). NOdepends_onfrom canopy-applications — the service boots degraded when clamd is down (the D4/D5 model); the battery’s full-stack health-wait covers the sidecar for e2e determinism. -
test.ndb: a custom signature matching a unique marker inside a magic-valid PDF fixture ⇒ the infected path is testable through the REAL upload endpoint (raw EICAR cannot pass magic-byte validation — stated honestly; the EICAR-over-INSTREAM test additionally proves the scanner leg with a seeded row + object). -
Plumbing:
PORT_MAPPINGS+push_derived_varsCANOPY_TEST__CLAMD_ADDR+validate_in_networkparity + test-libTestConfiggetter +canopy-integrationenv.
AC → proof map (AC wording per the recorded reconciliation)
| AC | Proof |
|---|---|
Production cannot silently select noop |
Guard matrix; the refusal message names the override var. |
Nothing downloadable/acceptable/rejectable until viewable |
6×3 integration matrix + in-UPDATE predicates + concurrency tests. |
Clean fixture readable after scan |
Real-clamd integration + both e2e flows. |
AV fixture marked infected, never served |
test.ndb upload-path integration + EICAR seeded-row + content 409 + |
Timeout/failure/Skipped never clean |
Fake-clamd fault matrix; stale-definitions defer. |
Provenance backend/version/result/timestamp |
|
Promotion idempotent, partial-failure safe |
Token+generation fencing on ALL mutations; crash-after-claim; lease theft single-commit. |
Legacy rows blocked + rescanned |
All-legacy staggered backfill + accepted-legacy revocation test + the D7 migration proof. |
Integration matrix incl. unavailable scanner |
Fake + real matrices; |
Deployment health/resources/isolation |
Digest-pinned non-root image, healthcheck, mem/cpu/pids limits, volume lifecycle, isolation guidance, runbook. |
Verification
-
Full pre-push battery (sole functional gate; push
-o ci.skip;PUSH-EXITecho +git ls-remoteare the landing truth). -
Devstack probe: upload → pending → clean ≲10 s → content 200 with
no-store; DB-flip to infected → 409 on content/accept/reject; replace object bytes → content refuses; skipped → supervisor override → viewable +scan_overriddenchain row withreason_sha256; rescan endpoint requeues anerrorrow; boot noop+uat → refusal; + override → WARN boot; clamd stopped → uploads still accepted, pending age climbs, service healthy. -
OpenAPI diff = the two endpoints + 409s +
ApplicationDocumentschema changes exactly;cargo deny checkgreen with the new crate.
Risks
-
First clamav boot downloads definitions (network, minutes) — volume-warm afterwards; battery
start_periodsized for cold boot; the runbook covers offline environments. -
The definition-freshness policy adds a deliberate fail-closed scanning path (stale ⇒ defer) — tunable per deployment.
-
The all-legacy requeue quarantines the installed base by design (unprovable verdicts); staggered due-times bound the drain; pre-UAT there is no production data.
-
Buffered-verify serving is bounded by the existing 10 MiB upload cap — no larger class can exist.
-
Two e2e flows gain a bounded reload-poll (the new moving part).
-
Step 4 is large by necessity (the security boundary lands atomically); its J-gate review is correspondingly heavier.
Delivery mechanics
Commits per the Status table (subjects ≤72, session-model trailer). PRECOMMIT token two-step per commit; J1–J8 + a fresh contextless review subagent per commit; battery via pre-push; Draft MR after the first push (iid for this plan’s Done flip) → ready after step 7; force-merge per house mechanics; close #1006 with implementation SHA + merge SHA + files + checked ACs (as reconciled) + links to #1415/#1416/#1417.