amadan

carlos / platform Public

spec: scan on demand, draft 1

The shipped scanner answers "is this bucket clean?" hourly. It does not
answer "is this object clean, now?" — which is the question an app has
when someone has just uploaded a file it is about to serve. The docs
currently tell app authors to solve that themselves, which is the
platform declining work it is better placed to do.

The delay is scheduling, not scanning: clamd answers in well under a
second. So this is queueing and notification.

Decisions worth arguing with:

A new svcrole.Service rather than widening Instance. Instance is
described in its own doc comment as legacy and "defined by observation
of today's call sites" — not a role to hang a new API class on, and a
token minted to read logs must not silently gain the ability to spend
deployment CPU. Scoped to the class, so PDF rendering reuses it.

carlos system antivirus, because CARLOS_STORE_SCANNER conflates wiring
with policy. Wiring stays in env where the coordinates argument applies;
policy moves to system/antivirus.json, a typed schema-pinned record —
NOT features.json, which is per-app and which the platform deliberately
never interprets.

Pull before push. The status door long-polls and does the version
comparison console-side, because the tag records an internal
etag@last-modified token and an app comparing it wrongly gets the
laundering bug as a correctness bug. Webhooks accelerate that; they
never replace it, so a dropped delivery costs a poll rather than
stranding a file.

Callbacks take a PATH, never a host. Gotenberg takes a full URL, which
is safe when caller and operator are the same party and is an SSRF here,
where the fetcher is a console that can reach cloud metadata. And they
are signed: an unsigned callback lets anyone who learns the path POST
"clean" and defeat the whole control.

One object per queue item, not one queue object — distinct keys never
collide, which is the CAS contention the sidecar design already avoided.
Round-robin across apps, because FIFO lets one tenant's ten thousand
objects delay everyone else's single urgent upload.

Metered and unpriced, following the idiom usage.go already uses. Sweep
scans are not attributed to a tenant who did not ask for them.

And clamd moves off the console box. Putting it there made a control
plane shovel tenant bytes through its own memory; the cgroup limits in
the rollout plan treat the symptom.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev ae85ed7acb83f0771e1b5baa8423097e04bad201 parent 4fae369
1 file changed, +464 −0
  • docs/superpowers/specs/2026-09-02-scan-on-demand-design.md +464 −0
diff --git a/docs/superpowers/specs/2026-09-02-scan-on-demand-design.md b/docs/superpowers/specs/2026-09-02-scan-on-demand-design.md
new file mode 100644
index 000000000..59798a01e
--- /dev/null
+++ b/docs/superpowers/specs/2026-09-02-scan-on-demand-design.md
@@ -0,0 +1,464 @@
+# Scan on demand: design
+
+**Status:** draft 1, for review.
+**Builds on:** `2026-09-01-object-storage-virus-scanning-design.md` (the sweep,
+the `Scanner` seam, verdict tags, enforcement). That work is landed on
+`worktree-store-virus-scanning` and reviewed; nothing here contradicts it.
+
+---
+
+## 1. What is being asked
+
+The shipped scanner answers "is this bucket clean?" on an hourly cycle. It does
+not answer "is *this object* clean, now?" — and that is the question an app
+actually has when someone has just uploaded a file it is about to serve.
+
+The current documentation tells app authors to build that guarantee
+themselves. That is the platform declining work it is far better placed to do.
+
+**The delay is scheduling, not scanning.** `clamd` answers on a typical upload
+in well under a second. An hour of latency is entirely "the pass has not come
+round yet". So this is a queueing and notification problem.
+
+### 1.1 What can honestly be claimed afterwards
+
+> Object `<key>` was scanned at `<T>` against `<engine>/<signature-db>` and
+> found clean.
+
+and, unchanged from the sweep design, the second sentence that makes the first
+one worth anything:
+
+> `N` objects in this store carry no verdict.
+
+On-demand scanning shortens T. It does not change what a verdict means, and it
+must not be allowed to imply that scanning is now synchronous with upload. It
+is not: **CARLOS never sees the PUT**, and no design here changes that.
+
+---
+
+## 2. Scope
+
+In:
+
+- a **new service role** for apps calling platform services at runtime
+- `carlos system antivirus` — deployment-level operator control, replacing the
+ on/off half of `CARLOS_STORE_SCANNER`
+- a **priority scan door** and a **status door** with long-poll
+- a **queue** that survives restarts and cannot be starved by one tenant
+- an opt-in **signed webhook**
+- **metering** of on-demand scans, recorded and unpriced
+- **clamd over TCP** on a services box, rather than a unix socket on the
+ console box
+
+Out (§12 has the reasoning): synchronous scan-on-PUT, GuardDuty, Gotenberg as
+a capability, pricing.
+
+---
+
+## 3. The service role
+
+### 3.1 Why not `Instance`
+
+`svcrole.Instance` is the app-token authority today and already covers runtime
+reads — `apifeatures.go` has the app's own instance credential reading
+`features.json`, and never writing. So an app calling the console at runtime is
+not new.
+
+But `svcrole.go:35` describes `Instance` as *"the LEGACY app-token authority,
+defined by observation of today's call sites rather than by its name — it
+includes domains and logs."* A role defined by what happens to call it is not a
+role to hang a new class of API on. Widening it further makes it less
+describable, not more useful, and a token issued for log-reading would silently
+gain the ability to spend deployment CPU.
+
+### 3.2 `svcrole.Service`
+
+A fifth role: **an app, at runtime, asking a platform service to do work on its
+behalf.**
+
+```go
+// Service is an app calling a platform SERVICE at runtime — work the
+// platform performs for the app, on the app's own data, that costs the
+// deployment resources. Scanning an object is the first; PDF rendering is
+// the next. It is deliberately NOT Instance: that role is defined by
+// observation of its call sites, and a token minted to read logs must not
+// silently gain the ability to spend the deployment's CPU.
+Service Role = "service"
+```
+
+`Admin` continues to be the union of everything, so existing CLI tokens keep
+working unchanged.
+
+**What it is not.** Not a tenant-data role — it grants no read of objects, no
+config, no credentials. Every `Service` door acts on the app named in its own
+path and nothing else.
+
+### 3.3 What still needs checking during planning
+
+How tokens are minted and which roles an app's delivered credential actually
+carries. `Instance` tokens exist and reach `apifeatures.go`; whether a token
+can be minted carrying `Service`, and where that happens, is a code question
+this spec does not pretend to have answered. **If it turns out apps cannot be
+issued a new role without a credential migration, that changes the plan** — the
+fallback is `Allow(Service, Instance)` on the new doors for one release, with
+`Instance` removed once credentials have rotated.
+
+---
+
+## 4. `carlos system antivirus`
+
+### 4.1 The problem with the env var
+
+`CARLOS_STORE_SCANNER` conflates two things: **wiring** (there is a clamd, it
+is at this address) and **policy** (scanning is on for this deployment).
+
+Wiring belongs in the unit file. Policy does not: turning a compliance control
+on should not require editing a systemd unit and restarting the control plane,
+and today the only way to know whether scanning is healthy is to log into the
+box — which AGENTS.md's no-infra-access rule says it must not be.
+
+### 4.2 The split
+
+| what | where | why |
+|---|---|---|
+| `CARLOS_CLAMD_ADDR` | env, boot | coordinates. Same argument as `CARLOS_STORE_REGION`: a console that guessed would scan against the wrong engine |
+| `CARLOS_CLAMD_MAX_BYTES`, timeouts | env, boot | must match the engine's own config |
+| enabled / disabled | **record** | policy, changeable without a restart |
+| engine name, enabled at, by whom | **record** | audit |
+
+`CARLOS_STORE_SCANNER` is **retired**. Its on/off meaning moves to the record;
+its "which engine" meaning is implied by the address.
+
+### 4.3 The record, and the reusable pattern
+
+`system/antivirus.json`, a sibling of `infra/inventory.json`
+(`infraview.go:25`). Deployment-scoped, not per-account.
+
+```go
+// package release
+// SystemKey is a deployment-wide settings record: system/<name>.json.
+// Distinct from features.json, which is per-app and which the platform
+// stores WITHOUT interpreting (apifeatures.go). These the platform reads
+// and acts on, so each has a typed shape and a pinned schema.
+func SystemKey(name string) string { return "system/" + name + ".json" }
+```
+
+```go
+type AntivirusSettings struct {
+ Schema int `json:"schema"` // pinned; unknown refuses
+ Enabled bool `json:"enabled"`
+ Engine string `json:"engine"` // "clamav"
+ At time.Time `json:"at"`
+ By string `json:"by"`
+}
+```
+
+Read with CAS, written with CAS, schema pinned and an unknown schema refused —
+the rules `objstore.Record` and `ScanSidecar` already follow, for the same
+reason: a future shape could mean anything, and this one drives whether tenant
+bytes get read.
+
+**Why a typed record per feature rather than one generic settings map.** A map
+is a features.json by another name — untyped, uninterpreted, and impossible to
+schema-pin meaningfully. Each `system/<name>.json` gets a struct, so adding
+`system/pdf.json` later is a new type and a new door, not a new magic string.
+
+### 4.4 The doors
+
+```
+carlos system antivirus status # engine, reachable?, signature age, queue depth
+carlos system antivirus enable
+carlos system antivirus disable
+```
+
+Operator authority — `CARLOS_STORE_OPERATORS`, the list that already governs
+`store grant`. Not `Admin` alone: enabling this starts reading tenant bytes
+across the deployment, which is a deployment-owner decision, not any admin's.
+
+`status` is the answer to "is scanning healthy" without SSH. It reports the
+engine version, whether it answers at all, how old the signature database is,
+and how deep the queue is.
+
+### 4.5 Two layers, deliberately
+
+Deployment-level enable does **not** start scanning anything. Per-app
+`store scan request` (member) and `store scan grant` (operator) remain exactly
+as shipped. The layers answer different questions:
+
+- `system antivirus` — does this deployment have scanning at all?
+- `store scan grant` — is it on for this store?
+
+Collapsing them would either scan every bucket the moment an operator enabled
+the engine, or make the deployment switch meaningless.
+
+---
+
+## 5. The pull path
+
+### 5.1 Request a scan
+
+```
+POST /api/cli/apps/{account}/{app}/store/scan/object
+{"key": "uploads/x.pdf"}
+→ 202 {"state": "pending"}
+```
+
+Role: `Service`. Enqueues; does not scan inline. A door that scanned inline
+would hold a console goroutine for the duration and give one tenant a trivial
+way to occupy them all.
+
+### 5.2 Read the verdict
+
+```
+GET /api/cli/apps/{account}/{app}/store/scan/object?key=uploads/x.pdf&wait=15s
+→ {"state":"clean", "at":"...", "engine":"clamav", "signature":"..."}
+→ {"state":"infected", "signature":"Eicar-Test"}
+→ {"state":"skipped", "reason":"too_large"}
+→ {"state":"pending"}
+→ {"state":"unavailable", "reason":"no_engine"}
+```
+
+**The console does the version comparison.** The verdict tag records
+`etag@last-modified` (§3.1 of the sweep spec). An app must not be asked to
+reconstruct that: it is an internal format, and an app comparing it wrongly
+gets the *laundering bug as a correctness bug* — upload v2 over a key whose v1
+was tagged clean, read the stale tag, serve the payload. So the door
+HEADs the object, compares, and answers `pending` when the tag is absent **or**
+stale.
+
+**`wait` long-polls**, bounded (say 30s max). For the upload-then-serve flow
+the app is already inside a request, so this delivers push latency with none of
+the webhook tax. It is the primary mechanism; §6 is for when nobody is waiting.
+
+**`skipped` and `unavailable` are not `clean`.** An app that treats them as
+success reproduces exactly the dishonesty the two-number rule exists to
+prevent, so they are distinct states with reasons attached rather than an
+error or an empty verdict.
+
+### 5.3 Pull is the source of truth
+
+The tag on the object is the record. The status door reads it. Webhooks are an
+accelerator over this, never a replacement — see §6.4.
+
+---
+
+## 6. The push path
+
+### 6.1 Why it is needed at all
+
+Long-poll covers the case where the app is in a request. **Presigned
+direct-to-S3 uploads break that assumption**: the browser PUTs straight to the
+bucket and the app is never in a request at all. That is a common S3 pattern
+and designing without it would be a mistake.
+
+### 6.2 No caller-supplied host
+
+Gotenberg takes a full webhook URL. That is safe in its deployment model —
+self-hosted, caller and operator the same party. **It is not safe here**: the
+caller is a tenant and the fetcher is the console, a box that can reach the
+deployment's internal network and cloud metadata endpoint. A tenant-supplied
+URL is a straightforward SSRF.
+
+The platform already knows where the app lives. So the request carries a
+**path**, never a host:
+
+```json
+{"key": "uploads/x.pdf", "callback_path": "/hooks/scan"}
+```
+
+and the console POSTs to the app's own host at that path. The parameter that
+creates the vulnerability does not exist.
+
+### 6.3 Signed, or it is worse than nothing
+
+An unsigned callback is a security hole wearing a convenience's clothes:
+anyone who learns the path POSTs `{"verdict":"clean"}` and the app serves a
+payload believing the platform cleared it.
+
+HMAC-SHA256 over `timestamp + "." + body`, header
+`Carlos-Signature: t=<unix>,v1=<hex>`, key delivered to the app as config
+(`CARLOS_SCAN_WEBHOOK_SECRET`) the same way store credentials are. Apps reject
+a stale timestamp, which gives replay protection body-signing alone does not.
+
+Considered and rejected: Gotenberg's trick of replaying caller-supplied
+headers. It is neat and it avoids a signing scheme, but it gives no body
+integrity and no replay protection, and it puts a tenant secret in a request
+log.
+
+### 6.4 Delivery is best effort, and that is fine
+
+Retries with exponential backoff, a small bounded number, then give up. **No
+dead-letter queue is needed**, because the verdict is durably on the object and
+the status door always answers — a dropped callback costs an app one poll, not
+a stuck file. That is the whole benefit of building pull first: retries can be
+lazy rather than heroic.
+
+Apps must treat callbacks as at-least-once and idempotent.
+
+---
+
+## 7. The queue
+
+### 7.1 Shape
+
+One object per item, under the app's own prefix:
+
+```
+apps/<acct>/<app>/scan-queue/<ulid>.json
+```
+
+**Not one queue object.** Many writers doing read-modify-write on a shared
+object is CAS contention — the exact problem the sidecar design avoided by
+having a single writer. Distinct keys never collide, so an enqueue needs no
+coordination at all. ULID keys sort by time, so a prefix listing is the queue
+in order.
+
+It also inherits the purge machinery: the prefix goes when the app does, which
+is the failure the sweep's pre-write re-read exists to prevent.
+
+### 7.2 Draining
+
+The scan pass gains a queue phase that runs **before** the sweep on each tick,
+and a much shorter tick (30s, say) so on-demand latency is seconds. Sweep
+budget is unchanged and still runs on the hourly cycle.
+
+### 7.3 Fairness
+
+Round-robin across apps with a per-tick cap per app, not FIFO across the whole
+deployment. FIFO lets one tenant enqueuing ten thousand objects delay every
+other tenant's single urgent upload behind them.
+
+### 7.4 Rate limiting
+
+Per-app token bucket on the request door. Exceeding it is a 429, not a silent
+drop — an app that is being throttled needs to know, because its own
+upload flow depends on the answer.
+
+---
+
+## 8. Metering
+
+`usage.go` already carries units recorded as data and explicitly unpriced
+pending a price sheet. On-demand scans follow that idiom exactly:
+
+- count **objects scanned on demand** and **bytes scanned on demand**, per
+ account per day, in the existing daily usage object
+- ship unpriced
+
+Sweep-driven scanning is *not* counted against a tenant: they did not ask for
+it, the deployment operator did. Only work an app requested is attributed to
+it.
+
+Counting from day one is the point. Retrofitting attribution after apps depend
+on the endpoint means either guessing at history or starting the meter at zero
+with usage already in flight.
+
+---
+
+## 9. Where clamd lives
+
+The sweep spec put `clamd` on the console box over a unix socket. That was
+wrong in a way worth naming: **the console is a control plane, and this made it
+shovel every tenant object through its own memory.** The cgroup limits in the
+rollout plan mitigate the symptom.
+
+`clamd` moves to a **services box**, reached over TCP. `internal/clamd` dials
+`unix` today; the INSTREAM framing, deadlines and reply parsing are already
+transport-agnostic, so this is a dialer change and a config change.
+
+**clamd has no authentication whatsoever.** Anything that reaches the port can
+have it scan arbitrary bytes. So: private interface only (Tailscale or
+equivalent), never a public port, and the services box firewalls the port to
+the console. This is a hard constraint, not a recommendation — it belongs in
+the infra ask and in the docs.
+
+**The console keeps fetching from S3** and streaming to the engine, rather than
+the services box fetching for itself. That moves bytes twice and is less
+efficient, but it keeps tenant-data credentials on exactly one box. Giving the
+services box its own S3 read is a trade to make later with numbers.
+
+---
+
+## 10. Surface
+
+```
+carlos system antivirus status|enable|disable operator, deployment-wide
+carlos store scan request|status member, per app (shipped)
+carlos store scan grant|revoke|sweep|enforce operator, per app (shipped)
+```
+
+App-facing HTTP (role `Service`):
+
+```
+POST .../store/scan/object request a scan
+GET .../store/scan/object?key=&wait= read the verdict
+```
+
+`store scan status` gains the queue depth for that app, and
+`system antivirus status` gains it deployment-wide, because "why is my scan
+slow" should be answerable without a log.
+
+---
+
+## 11. Security properties
+
+Each of these is a test, not a hope.
+
+1. **No caller-supplied host reaches an outbound request.** Callbacks go to the
+ app's own host; the path is validated and cannot escape it.
+2. **Every callback is signed**, and a body altered in flight fails
+ verification.
+3. **The app policy still has no `s3:PutObjectTagging`.** Unchanged from the
+ sweep design and still the thing that stops an app marking its own object
+ clean.
+4. **A `Service` token cannot act on another app.** The path names the app and
+ the token is scoped to it.
+5. **A `Service` token cannot reach operator doors** — `system antivirus`,
+ `scan grant`, `enforce`.
+6. **Version comparison happens console-side**, so a stale tag can never be
+ served to an app as a fresh verdict.
+7. **Rate limits are enforced before work is enqueued**, so the queue cannot be
+ used as an amplifier.
+
+---
+
+## 12. Out of scope
+
+- **Synchronous scan on PUT.** CARLOS does not see the PUT and giving it that
+ would mean proxying tenant data through the control plane — the opposite of
+ the direction this spec moves in.
+- **Gotenberg as a capability.** The `Service` role is designed so it can reuse
+ it, but PDF rendering is its own brainstorm.
+- **GuardDuty.** Still gated on the tagging-cost measurement.
+- **Pricing.** Units are recorded; a price sheet is a separate decision.
+
+---
+
+## 13. Testing
+
+Beyond the per-task tests, the properties worth naming:
+
+- a callback POST is never issued to a host the app did not come from, for any
+ `callback_path` including absolute URLs, `//host`, and traversal
+- a tampered body fails signature verification
+- `pending` is returned for a stale tag, not the stale verdict
+- one app flooding the queue does not delay another app's item beyond one tick
+- a dropped callback still leaves the verdict readable through the status door
+- disabling via `system antivirus disable` stops the pass without a restart and
+ without touching per-app records
+- an unknown `system/antivirus.json` schema refuses rather than guessing
+- metering counts on-demand scans and does **not** count sweep scans
+
+---
+
+## 14. Risks
+
+- **The role migration is the unknown.** §3.3. If existing app credentials
+ cannot carry a new role without rotation, the transitional
+ `Allow(Service, Instance)` costs a release cycle.
+- **A shorter tick multiplies the enumeration cost.** The scan pass lists every
+ account and app each tick; at 30s that is 120× the hourly cost. The queue
+ phase should list the queue prefixes only, and fall back to full enumeration
+ on the sweep cycle. Named here so it is measured rather than discovered.
+- **Long-poll holds console goroutines.** Bounded wait and a cap on concurrent
+ waiters per app; otherwise it is a cheap way to occupy the console.