amadan

carlos / platform Public

Clone 
git clone https://amadan.net/carlos/platform

Plain git — no account needed to clone.

Download

Download this file

Scan on demand: design

Status: draft 7 — final, cleared to plan from (closes reviews 1-6). Builds on: 2026-09-01-object-storage-virus-scanning-design.md — the sweep, the Scanner seam, verdict tags, enforcement. Landed and reviewed.

It contradicts that spec in one place, deliberately. Prior spec §4.1 records: "No TCP option in v1: an unauthenticated scanning service reachable over a network is a worse problem than the one being solved. (decision)". §9 reverses it, and argues the reversal rather than assuming it.

---

1. What is being asked

The shipped scanner answers "is this bucket clean?" hourly. It does not answer "is this object clean, now?" — the question an app has when someone has just uploaded a file it is about to serve. The current docs 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 on a typical upload in well under a second. So this is queueing and notification.

1.1 What can honestly be claimed

> Object <key> was scanned at <T> against <engine>/<signature-db> and > found clean.

plus the sentence that makes the first one worth anything:

> N objects in this store carry no verdict.

On-demand scanning shortens T. It does not make scanning synchronous with upload: CARLOS never sees the PUT.

---

2. Scope

In: svcrole.Service; carlos system antivirus; a priority scan door and a long-polling status door; a queue; an opt-in signed webhook; metering, recorded and unpriced; clamd over TCP on a services box.

Out (§12): synchronous scan on PUT, GuardDuty, Gotenberg as a capability, pricing.

---

3. The service role

3.1 Why not Instance

svcrole.go:34-36 calls Instance "the LEGACY app-token authority, defined by observation of today's call sites rather than by its name." A role defined by what happens to call it is not one to hang a new API class on, and a token minted to read logs must not silently gain the ability to spend deployment CPU.

Enforced, not merely intended: TestInstanceRoleMatchesTodaysAppTokenReach (svcroutes_test.go:262-291) pins Instance to exactly nine doors and fails with "widening the legacy role".

3.2 svcrole.Service

An app, at runtime, asking a platform service to do work on its behalf — work the platform performs on the app's own data that costs the deployment resources. Scanning is the first; PDF rendering is next.

Service Role = "service"

Also update svcrole.go:41-44 ("the union of the three above") and the CLI's hardcoded role list in services.go:164 — "mintable with no further change" is true of the API, not of the CLI's usage string.

Allow unconditionally adds Admin (svcrole.go:109), so an Admin credential reaches the new doors by construction, and Admin can be account-scoped. That is within one account, not a tenant-boundary hole, but §11.5's property is scoped to role Service and must say so.

3.3 There is no migration

Draft 1 called this the document's biggest risk and proposed Allow(Service, Instance). Both were wrong.

| kind | minted by | roles | rotatable | |---|---|---|---| | person (kind="") | carlos auth login | none — never meets the role gate (authcli.go:425) | n/a | | legacy app token (kind="app") | a person, POST .../instance-token | Instance, hardcoded (svcprincipal.go:124) | no (svcprincipal.go:244-246) | | named service (kind="service") | account owner (apiservices.go:102-112,143) | any role svcrole.Parses (apiservices.go:180) | yes |

Adding Service to the catalogue makes it mintable: the mint door, the mint form (svcpage.go:176) and doorsFor all read svcrole.All().

Nothing migrates. A kind='app' token can never become Service, so an app that wants on-demand scanning is issued a new credential:

carlos services create -account <a> -app <app> -role service
carlos secrets set -app <app> CARLOS_SCAN_TOKEN=<...>

the same delivery step it already performed for its instance token. Allow(Service, Instance) is deleted: it cannot terminate (legacy tokens do not rotate into anything) and it would fail the pinned test above — the exact widening §3.1 argues against.

3.4 Scope must be per-app, enforced in one place

NewServicePrincipal permits app == "" (svcprincipal.go:140-141) and apiAccountAppBy binds only when p.App != "" (apicli.go:70), so an account-scoped credential reaches every app in its account.

There are two mint doors — the API (apiservices.go:211) and the browser form (svcpage.go:299, which explicitly offers "an account-wide credential"). So the refusal goes in NewServicePrincipal (svcprincipal.go:170-172, beside the existing role parse), not in a handler, or one door forgets it. It needs a named sentinel — ErrAccountScopedService — because the API door today matches only ErrServiceNameTaken (apiservices.go:211-215) and turns anything else into a generic failure. Both doors then surface the same 400.

---

4. carlos system antivirus

4.1 Wiring versus policy

CARLOS_STORE_SCANNER conflates there is a clamd at this address with scanning is on. Wiring belongs in the unit file; policy does not. Turning a compliance control on should not mean 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 the no-infra-access rule forbids.

| what | where | |---|---| | CARLOS_CLAMD_ADDR — host:port, or a path when it begins / | env, boot | | CARLOS_CLAMD_MAX_BYTES | env, boot | | CARLOS_SCAN_WEBHOOK_KEY (+ _PREVIOUS, §6.3) | env, boot | | enabled / disabled, engine, at, by | record |

There is no clamd timeout variable today (clamd.Timeouts{} defaults, clamav.go:71); this spec does not add one.

4.2 What replaces the env var

Every place that keys off CARLOS_STORE_SCANNER or a nil Scanner:

  1. storeScannerFor (main.go:755-758) builds the Scanner whenever CARLOS_CLAMD_ADDR, the retained CARLOS_CLAMD_SOCKET alias, or the deprecated CARLOS_STORE_SCANNER=clamav is set — the last of those retaining today's socket default (/run/clamav/clamd.ctl, main.go:774). Without that third case a deployment that set only the old variable gets a nil Scanner, no goroutine (scanpass.go:37-39) and nothing to seed a record from: scanning stops silently, which is what §4.2.1 exists to prevent. Safe because NewClamAVScanner does not ping (clamav.go:56-70). main.go:772's fatal when no store backend is configured stays.
  2. StartScanPass (scanpass.go:37-39) loops whenever Scanner != nil and reads system/antivirus.json at the top of each tick. Disabled means the tick does nothing — not that there is no goroutine. That is what makes enable restartless. A per-tick GET of one small object is negligible against the enumeration the tick already does.
  3. scan grant (apiscan.go:252) and scan sweep (scanpass.go:198) check the record, and 503 naming carlos system antivirus enable.
  4. RunSweep's refusal (sweep.go:163) checks the record.
  5. apiscan.go:71-81 — the engine_error branch — reports disabled as its own state, distinct from unreachable.
  6. apiscan.go:45's message names the command, not the variable.

Still correctly branching on wiring, and unchanged: appsettings.go:838-840 and walk.go:466 (Teardown must release an engine that exists regardless of policy), apiscan.go:351 (revoke's best-effort EnsureDisabled), apiscan.go:419 (enforce-on needs an engine to name a tag).

Comments and message strings naming the variable, which are reworded rather than rewired: objstore.go:86, scanner.go:89, server.go:522-523, and main.go:772's message.

Two documents change with this: docs/site/cli.md:474-488, which lists the variables, and docs/superpowers/plans/2026-09-01-carloku-scanning-rollout.md, whose step 4 sets CARLOS_STORE_SCANNER=clamav and whose rollback is "unset it and restart". Step 4 becomes CARLOS_CLAMD_ADDR plus carlos system antivirus enable; the rollback becomes carlos system antivirus disable, which needs no restart — the feature's own selling point.

enable refuses with 503 when no Scanner is wired, or the record can claim enabled with nothing behind it.

4.2.1 Migration — an upgrade must not silently stop scanning

Today CARLOS_STORE_SCANNER=clamav is the only gate. After this change the pass reads a record that exists on no current deployment, so every tick would do nothing, and an operator who did not touch the unit file would get a nil Scanner as well. That is precisely the failure main.go:751-753,766-768 refuses to allow: "a typo that silently disabled scanning would leave an operator believing a compliance control is on when nothing is running."

Draft 3 answered this with a boot log.Fatal on the retired variable. That was worse than the problem. Console env is unit-file managed, so an operator who ships the build before editing env gets a console that will not start and has to edit a unit file on the box — the exact access §4.1 says this design should remove the need for. (Draft 4 justified this partly by claiming the box's adopt pass does not restart the unit on rollback. That is wrong — WithStartUnit (adopt.go:204-215) exists to start the unit after a rolled-back failed restart, and the host agent passes it in production (host/agent.go:602,1155,2960), so a fataling console is rolled back and started on the old build. The decision stands on the other leg alone.) The other order — edit env first — requires restarting the old binary, which then sees no engine and stops scanning anyway. One order costs a scanning outage; the other costs a trip to the box.

So the migration preserves behaviour instead of demanding attention:

  • CARLOS_STORE_SCANNER=clamav is accepted as a deprecated alias for wiring, logged loudly on every boot. Any other value stays fatal, which is today's behaviour for an unknown engine.
  • CARLOS_CLAMD_SOCKET is retained as an alias for CARLOS_CLAMD_ADDR; setting both to different values is fatal.
  • the record is seeded enabled, once, when the retired variable is set and system/antivirus.json is absent, stamped by: "migration from CARLOS_STORE_SCANNER". Draft 3 refused to seed on the grounds that auto-enabling a control that reads tenant bytes is the wrong default. That reasoning does not apply to a migration: the operator already enabled it, explicitly, and per-app grants still gate every byte. Refusing to seed does not withhold consent — it silently withdraws consent already given, which is the failure main.go:751-753 exists to prevent.
  • where the seed happens: at the top of the first StartScanPass tick — the same record read §4.2 item 2 already performs — not in storeScannerFor, which runs at main.go:262 before openStore() at :269 and has no release store to write to.
  • the write is create-only (Put(…, ""), release/store.go:38,516 — the IfNoneMatch:* that TrashApp already relies on, release/trash.go:51-54), so two consoles or a restart loop converge; ErrCASConflict means "someone else seeded it" and is success.
  • seeding is therefore idempotent: it happens only when no record exists, so an operator who later runs disable is not overridden on the next restart.
  • a read error at boot neither seeds nor fatals — log and retry on the next tick. Seeding on a failed read would enable scanning on a deployment that may have disabled it.
  • the seed fires only while the retired variable is still set, so the upgrade order matters and the rollout must say it: deploy the new build with CARLOS_STORE_SCANNER still in place (one tick seeds the record), then swap it for CARLOS_CLAMD_ADDR. Swapping in the same restart as the upgrade seeds nothing and stops scanning — warned every tick, but stopped. Running carlos system antivirus enable also settles it at any point.
  • main.go carries "the deprecated variable was set" to the first tick as a Config field (ScanSeedFromDeprecatedEnv bool); the pass does not read the environment itself.
  • the first tick warns when a Scanner is wired and no record exists — not boot, which has no store — and system antivirus status reports "engine wired, no record" as its own state.

4.3 The record

system/antivirus.json, deployment-scoped — sibling in spirit to infraview.InventoryKey (internal/infraview/infraview.go:25), with the helper beside release.FeaturesKey (internal/release/features.go:53):

// package release
func SystemKey(name string) string { return "system/" + name + ".json" }

type AntivirusSettings struct {
    Schema  int       `json:"schema"`
    Enabled bool      `json:"enabled"`
    Engine  string    `json:"engine"`
    At      time.Time `json:"at"`
    By      string    `json:"by"`
}

CAS read/write, schema pinned, unknown schema refuses — the rules objstore.Record and ScanSidecar follow, and for the same reason: this one decides whether tenant bytes get read.

A typed record per feature, not a generic map. A map is features.json by another name: uninterpreted and unpinnable. 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|enable|disable|rotate-webhook-key

A deployment-wide door has no {account} segment, so apiPrincipalAccount (apicli.go:176-187) cannot resolve it. Precedent exists: GET /api/cli/apps (authcli.go:471) is already a no-{account} door. These need a person-only resolver, registered svcrole.Set{} so no service principal reaches them, and added to closedDoors (svcroutes_test.go:83) or TestEveryCLIDoorNamesItsRoles (svcroutes_test.go:140-172) fails.

Authority is storeOperator(addr) — the CARLOS_STORE_OPERATORS list that already governs store grant (apistore.go:520) and every scan operator verb. Draft 1's "not Admin alone" confused a service role with person authority: operators are addresses (server.go:558-571), a service actor is service:/app: (apicli.go:89-100), and the two can never match.

status answers "is scanning healthy" without SSH: engine version, whether it answers, signature age, queue depth.

4.5 Two layers

Deployment enable scans nothing by itself. Per-app store scan request (member) and store scan grant (operator) are unchanged.

Per-app store scan status must surface the deployment state, or a record saying enabled: clamav reads as healthy while nothing runs — a new way for the status output to lie. Note apiscan_test.go:254 asserts engine == "clamav" on a fixture with a Scanner and no record; that fixture must write an enabled record.

---

5. The pull path

5.1 Request

POST /api/cli/apps/{account}/{app}/store/scan/object
{"key": "uploads/x.pdf", "callback_path": "/hooks/scan"}   // callback_path optional
→ 202 {"state":"pending"}
→ 409 {"state":"unavailable","reason":"not_granted"|"not_requested"|"encrypted"|"not_enabled"}
→ 503 {"state":"unavailable","reason":"disabled"|"no_engine"}
→ 429 on rate limit

Role Service. It enqueues; it does not scan inline. Inline scanning would hold a console goroutine for the duration and hand one tenant a trivial way to occupy them all.

5.1.1 The consent chain applies here too

This was the hole in draft 2. The landed sweep refuses to read tenant bytes without the member's attestation (sweep.go:151-160), skips stores attested encrypted (:147-149), and scan grant refuses without a request (apiscan.go:268-285) on the stated ground that an operator must not be able to start that on their own. Draft 2's enqueue checked only the deployment switch — so an app's own credential could make the console fetch and scan objects in a store that was never requested, never granted, or attested client-encrypted. A new door straight through the property the whole feature exists to protect.

Both the enqueue and the drain refuse unless all of:

  • rec != nil && rec.Provisioned != nil → else not_granted, the state apiscan.go:263-266 already names
  • the app is not in the trash — release.ReadAppTombstone, which the sweep checks at sweep.go:135-141 → else the drain deletes the items
  • rec.Scanning != nil && rec.Scanning.Requested != nil → else not_requested
  • !rec.Scanning.Requested.Encrypted → else encrypted
  • rec.Scanning.Enabled != nil → else not_enabled
  • the deployment record is enabled → else disabled

Draft 3 omitted the first two. The trash one matters specifically for the drain: HasAppIn filters deleted_at = 0 (console/store.go:1208) so a trashed app 404s at the door, but items enqueued before TrashApp sit under the prefix for the 30-day cooloff with store.json intact and Enabled non-nil. Without the tombstone check the drain would read bytes from a store the sweep deliberately will not touch.

The drain re-checks all of it because the record can change between enqueue and drain, and the drain is where bytes are actually read. It also applies RunSweep's engine-drift refusal (sweep.go:175-178).

On a read error — not an absence — the drain keeps the items and the index entry, logs, and retries next tick. objstore.Get returns (nil, "", nil) for absent and an error for failure; §7.5's "delete when store.json is gone" must never be implemented as "delete on any error", or a transient bucket failure discards a tenant's queue.

5.2 Read

GET .../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":"failed","detail":"..."}      // the engine read it and could not judge it
→ {"state":"denied"}                     // the console may not read it
→ {"state":"pending"}
→ {"state":"gone"}                       // object does not exist
→ {"state":"unavailable","reason":"..."} // as §5.1

The console does the version comparison, and the token never leaves it. The tag records etag@RFC3339 (sweep.go:49-51). An app must not 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.

Draft 2 said "the seam widens" for HeadObject. That understates it: scanS3API (clamav.go:25-29) is the scanner's private seam, and the door reaches the scanner only through Scanner (scanner.go), which cannot ask about one key. So Scanner gains:

// Status answers what is known about the CURRENT version of one object, and
// returns the object as a listing page would have described it so a caller
// can go on to Scan it without a HeadObject of its own.
//
// Verdict is the recorded one when a tag matches the current version,
// VerdictUnknown when the tag is absent or stale. ErrObjectGone when the
// object is not there.
Status(ctx context.Context, bucket, key string) (Verdict, ScanObject, error)

Status reads the tag as written; it does not apply Verdict's re-issue rule. The landed reader maps a stored denied or failed tag back to VerdictUnknown (clamav.go:160-163) so the sweep retries it — correct as sweep retry policy, and wrong as an answer to a question about an object. Left in place here, a failed scan reads as pending forever: the drain deletes the item as scanned, and nothing ever changes the answer until a sweep re-fails it. Verdict keeps the re-issue rule; Status reports what the tag says. Different questions, and the split is the point.

The ScanObject return is not decoration. Draft 3 had Status return a verdict alone, and §7.4 has the drain call Status before Scan — but Scan takes (bucket, key, version, size) (scanner.go:42) and the drain has no listing page. Without the object it would need its own HeadObject, which is exactly what keeping the version token inside the scanner forbids. Draft 2 had the same hole under a different name.

Status needs HeadObject, which scanS3API (clamav.go:25-29) and stubScanS3 (clamav_test.go:24) both gain.

When HeadObject itself is denied there is no version or size to hand Scan, so Status returns (VerdictDenied, ScanObject{}, nil) and the drain does not call Scan on a zero ScanObject. The SSE-KMS case the landed comment names behaves differently and correctly: HeadObject succeeds, GetObject is denied, so the drain gets a populated object, scans, and gets Denied from the fetch. memscanner.go and scanner_contract_test.go follow: MemScanner has no object model, so it gains a seeding call — PutObject(bucket, key, etag string, modified time.Time, size int64) — and the contract test asserts an absent object gives ErrObjectGone.

wait long-polls, capped at 30s, woken by in-process notification from the drain — a polling waiter costs one tag read per second per waiter. Cap concurrent waiters globally as well as per app.

skipped, gone and unavailable are not clean. Distinct states with reasons, because an app treating them as success reproduces the dishonesty the two-number rule exists to prevent.

5.2.1 When a verdict is reached but cannot be recorded

clamavScanner.tag returns success without writing in two landed cases: an object already carrying eight foreign tags (clamav.go:306-308, the ten-tag cap) and a pre-write tag read that is denied or gone (clamav.go:289-291). Both were deliberate — they stop one object stalling a whole sweep — and the sweep tolerates them because it re-lists every 24h.

A queue item does not. The drain would get a real verdict from Scan, send clean in the callback, delete the item — and Status would keep answering pending forever, because the tag is not there. The app's declared source of truth (§5.3) would never arrive, and the two answers it did get would disagree.

The Scanner interface changes, which draft 4 hid inside a sentence:

Scan(ctx, bucket, key, version string, size int64) (Verdict, bool, error)
//                                                          ^ recorded

The sweep's call site (sweep.go:235) takes _ for the flag and its counts are unaffected — Fresh and Reused count verdicts reached, which is what clamav.go:302-303 already says it is doing. MemScanner.Scan (memscanner.go:102) gains an Unrecordable(key) hook so the case is testable without a live S3 stub. The third return value touches roughly fifteen call sites in clamav_test.go and the contract test, not only the sweep. MemScanner.Verdict (memscanner.go:89-100) does not apply the re-issue rule today, so it gains it — otherwise the Verdict/Status split cannot be exercised against the fake — and scanner_contract_test.go pins both halves. The plan will extract a shared readTag helper: Verdict is called only from sweep.go:215 and no such helper exists yet.

Where the answer goes when the tag cannot hold it. Draft 4 said "keep the queue item, terminal" — but items are ULID-named and the door asks by key, so that would mean listing the whole queue prefix on every poll. Instead the drain writes a key-derived result object and deletes the queue item as normal:

apps/<acct>/<app>/scan-results/<sha256(key)>.json
{"schema":1, "key":"uploads/x.pdf", "verdict":"clean",
 "version":"<etag>@<RFC3339>", "engine":"clamav", "signature":"...",
 "at":"..."}

It carries the version, and that is not optional. Draft 5 keyed it by sha256(key) with the verdict alone — which reintroduces, through the new mechanism, exactly the bug §5.2 exists to prevent: v1 scanned, tag not written because the cap fired, result object says clean; the tenant overwrites with v2; the tag is still absent, so the door falls through to the result object and answers clean for bytes nobody has read. The door compares the recorded version against the ScanObject that Status already returns and answers pending on a mismatch. The token stays inside the console throughout, which is what §5.2 requires.

  • one GET for the door, on the pending path only
  • durable across restarts, so §11.10 holds without soft state
  • under the app prefix, so PurgeApp and the claim gate cover it (§7.1's argument for keeping the queue there applies unchanged)

Written read-then-CAS. release.Store.Put has exactly two modes (release/store.go:37-45): create-only with ifMatch "", or replace with an etag. The second write to the same key is a replace — v1's result object is present, the tenant uploads v2, v2 is scanned and again unrecordable — so a create-only write there returns ErrCASConflict and silently drops the v2 verdict, leaving the door on pending until a sweep tags it. Get, then Put with the etag if present or create-only if absent, retrying once on conflict: the writeSidecar posture at sweep.go:370-395.

It carries the engine and signature alongside the verdict, because the recorded:false answer is still an audit answer and needs its provenance.

Written with the same guard as a sidecar. The drain re-reads store.json immediately before writing, the way writeSidecar does (sweep.go:358-367), because the identical race applies: read the record, scan for seconds, a teardown and PurgeApp land, and a create-only write then succeeds under a purged prefix. A stranded queue item is drained on the next tick because its index entry survives; a stranded result object is found by nothing, and it makes the app name permanently unclaimable (server.go:2921).

Collected, three ways — draft 5 said "by TTL" and named no collector:

  • deleted by the drain as soon as a later scan persists a real verdict for that key
  • deleted with the app's queue items whenever §7.5 deletes them for absence, tombstone, or a record that no longer permits scanning
  • swept by the boot rebuild (§7.1), which lists scan-results/ as well as scan-queue/ and collects anything older than the TTL by ListMod age — the name carries no timestamp, so age has to come from the store
  • and swept the same way by the drain for each app it visits, so a console that never restarts does not accumulate them. Note a sweep that later tags the object does not delete the result object; only the on-demand drain does.

Age-based collection needs release.ModLister, which is optional and unimplemented by the test fakes. Type-assert it and fall back to skipping age collection with a single logged line, exactly as storagePass does for SizeLister (economicspass.go:141-145).

TTL for both kinds is 24h, one sweep cycle: after that the ordinary sweep has had its chance to write a real verdict, and a stale on-demand answer is worth less than no answer.

infected with recorded:false means enforcement is NOT in effect. The Deny statement matches s3:ExistingObjectTag/carlos-scan=infected (aws.go:159-165), and a verdict that could not be tagged does not trigger it. The app can still read the object and must act on the verdict itself. This goes in the docs sentence too, not only here.

5.3 Pull is the source of truth

The tag on the object is the record; the status door reads it. Webhooks accelerate that and never replace it (§6.4).

---

6. The push path

6.1 Why

Long-poll covers the app being inside a request. Presigned direct-to-S3 uploads break that: the browser PUTs straight to the bucket and the app is never in a request. Common enough that designing without it is a mistake.

6.2 Where a callback may go

Draft 1 claimed taking a path rather than a URL meant the vulnerability "does not exist". Overstated. The host is still tenant-influenced: the outbound fetcher picks the first ready non-static route (instances.go:88-97), which can be a tenant's custom domain whose DNS they control. And Go follows redirects by default — a tenant server answering 302 → http://10.x/ turns a signed POST into a GET at an internal address. The codebase already knows this (sesevents.go:143-158).

So, all of:

  • POST only to the platform-assigned host, <app>.<sqid>.<AppsDomain> (apiinstances.go:566,1250) — never a custom domain. Correction (review 7): an app with only its own domain DOES get callbacks. The domains loop mints the platform host for any app with a ready route, and firstReadyUpstream (domains/reconcile.go:268) accepts a custom-domain route as the source it copies an upstream from. The true condition is narrower and duller: an app with no ready route at all has no platform host yet, so it has nowhere to be called back and uses long-poll until it does. The docs say that, not the old sentence.
  • Why the host cannot be moved, with the evidence. app is tenant-chosen but ^[a-z][a-z0-9-]{0,31}$ (release.go:242) — no dots, so no escaping the label; sqid and AppsDomain are the platform's. A tenant cannot attach a custom domain under AppsDomain (apidomains.go:96). DNS for the name is written by the fleet's domains loop to edge IPs, not by a tenant. DNS rebinding is closed twice, and the second reason is the one that matters: TLS name verification against a public CA means a poisoned resolver still cannot present a valid certificate for that name. InsecureSkipVerify on the callback transport is therefore a tested absence, not a code-review item.
  • https only
  • CheckRedirect refuses every redirect
  • bounded timeout, LimitReader on the response

Path validation as a rule. Refuse: absolute URLs; protocol-relative //evil/x and /\evil/x; anything not beginning with exactly one /; raw or percent-encoded CR/LF; fragments; .. segments; over 1KB. Then:

u, err := url.Parse(p)   // must yield Scheme=="" Host=="" User==nil Opaque==""
target := url.URL{Scheme: "https", Host: appHost, Path: u.Path, RawQuery: u.RawQuery}

Built structurally — never string concatenation.

6.3 Signing

An unsigned callback is a 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.

What can and cannot be reused from store credentials. Draft 2 said the key "cannot be delivered the same way". That was wrong about delivery and right about retention. Delivery is exactly the store path — appconfig.Seal against CARLOS_SECRETS_PUBKEY then one appconfig.Apply, which bumps the serial and reconverges the fleet (walk.go:602-640). What cannot be reused is keeping nothing: the console must be able to sign later, and a sealed value it cannot open is no use to it.

  • one boot master, CARLOS_SCAN_WEBHOOK_KEY
  • per-app key derived at sign time: HMAC(master, "carlos-scan-webhook-v1" + 0x00 + sqid + "/" + app + "/" + gen). Nothing extra is stored and per-app isolation falls out — without it a single deployment secret lets app A forge callbacks to app B. The delimiter is injection-safe: a sqid is ^[a-z]{3,16}$ (qualified.go:56) and an app is ^[a-z][a-z0-9-]{0,31}$ (release.go:242), so neither can contain /, and gen is an integer. (Corrected: this bullet said HMAC(master, sqid + "/" + app) while the bullets below it added the domain label and the generation, so the section specified two different schemes. The implementer read both, followed the later one, and said so — which is the only reason it was caught before the wire format froze.)
  • written by store scan grant, gated by storeSealKey (apistore.go:425) — one config write on an operator verb, never per enqueue
  • header Carlos-Signature: t=<unix>,v1=<keyid>:<hex>[,v1=<keyid>:<hex>] over t + "." + body. Each signature carries its own key id; draft 3's single k= could not label two of them, and an app does not need the id anyway — it holds one derived key and tries it against each v1.
  • rotation carries two masters. CARLOS_SCAN_WEBHOOK_KEY_PREVIOUS is accepted; the console signs under each master and the app accepts if either verifies. Without that window every app is broken between the console's rotation and its own re-seal.

What rotation actually does, stated because it is not free. appconfig.Apply bumps the generation, and the box's Sync restarts the app unit on a bundle change (sync.go:33,42,331). So:

  • rotate-webhook-key restarts every scanning-enabled app on the deployment. It paces the re-seal rather than doing them at once, and says so before it starts.
  • store scan grant restarts the tenant's app as a side effect, because that is where the key is written. The docs sentence for grant says so.
  • "done" means sealed, not converged. The console cannot see when a host has picked up the new bundle — server.go:566-569 makes exactly this point for store rotation. So dropping _PREVIOUS is a separate operator step after convergence, not something rotate-webhook-key does. Dropping it early degrades to one lost callback per stale host, which is survivable because pull is the source of truth — but it should be a choice, not a surprise.
  • rotate-webhook-key is idempotent and re-runnable after a partial failure, and is also the backfill for apps granted before this ships. A re-run with an unchanged master re-seals the same plaintext and therefore does not restart anything: Sync restarts only when the rendered bundle changes (sync.go:340+).
  • "Paced" needs a mechanism, and it is scan sweep's. One budget per call: the door re-seals as many apps as it can inside scanSweepTimeout, answers with how many remain, and the CLI loops until none do. No background job and no pacing state — idempotence by key id is what makes a re-run safe, so a console that dies mid-rotation simply resumes.
  • Every console must be restarted with the new pair BEFORE the rotate runs. With more than one console, a console still holding only the old master will sign only under the old key, while apps another console has already re-sealed hold only the new derived key — and those callbacks fail verification. Order: restart all consoles with KEY=new, PREVIOUS=old, then run rotate-webhook-key, then drop _PREVIOUS once converged.
  • store scan revoke unsets the app's callback secret. A key outliving the consent it was issued under is the same mistake as a result object outliving it.
  • constant-time compare; skew window 5 minutes
  • the body carries recorded, so a callback saying clean and a door saying recorded:false cannot disagree — the disagreement §5.2.1 exists to prevent
  • the body carries account, app, key and the bare ETag — and not the opaque version. Draft 4 carried both, which contradicted §11.7 ("no door or app sees the token") in the same document; a review found the two sentences facing each other. §11.7 wins, because it is the property the version binding exists to protect: an app that holds the token is an app that can compare it wrongly, and that is the laundering bug arriving as a correctness bug. The app has the ETag from its own PUT response and matches on that.
  • Replay, stated plainly because there is no nonce. A callback replayed inside the 5-minute skew window verifies and is accepted. That is harmless and it is the design: the body is idempotent on (account, app, key, etag, verdict), which is the tuple an app uses as its idempotency key, and it is named here so two implementers do not choose differently. A nonce is deliberately NOT added — apps have no durable nonce store to check one against, so it would be assurance in name only.
  • Clock. t is the console's clock. Apps run on platform boxes and do have a clock, so the skew check is expected of them; skipping it only widens the (harmless) replay window above. The verifier must tolerate two v1= entries and unknown key ids.
  • Key id: HMAC("carlos-scan-webhook-key-id", master)[:16] — a different label from scantag.go's tag-key id, so the two schemes cannot produce a colliding id from the same bytes. PREVIOUS == current is refused, exactly as NewTagSigner refuses it: a rotation that rotated to nothing.
  • Derivation carries a domain label: HMAC(master, "carlos-scan-webhook-v1" + 0x00 + sqid + "/" + app). The label costs nothing and means no future HMAC(master, x) use can collide with a derived per-app key.
  • Why a delimiter here when scantag.go uses length prefixes. t + "." + body is injective because t is decimal digits — it cannot contain a . — so the first . splits it unambiguously. The tag scheme needs length prefixes because an object key can contain any delimiter you pick. Stated because the two schemes now look inconsistent, and the next person to notice should find the reason rather than "fix" it: length-prefixing here would make every non-Go verifier harder for no gain.
  • Per-app rotation. ScanBlock carries a non-secret webhook_gen, and the derivation is over sqid + "/" + app + "/" + gen. Without it a tenant that leaks its callback key has no remedy short of an operator rotating the deployment master and restarting every scanning app. The tag key needs no such thing because it never leaves the console; this one is handed to tenants, so it needs an owner-side rotation.
  • webhook_gen IS RANDOM, NOT A COUNTER FROM ZERO, and the reclaimed-name property depends entirely on that. Draft 5 said a generation stopped a reclaimed app name inheriting the deleted app's key. It did not. The implementer pointed out that PurgeApp lists the whole app prefix and deletes every key in it (trash.go:229-241), store.json included — so a counter starting at zero starts at zero again for the next app to claim that name, and derives the identical key. The claim was false as written. A fresh random value at grant makes it true, and makes it true because the record is destroyed rather than in spite of it: mint a random positive integer below 2^53 (JSON-number safe, since the console serves this to browsers) at the first grant, and mint a new one on every per-app rotation. Zero remains a legal derivation input, because records written before this ships have no field and their apps already hold a key derived with zero; rotate-webhook-key is the backfill that moves them onto a random one.
  • With no master configured, the enqueue door refuses a callback_path with 503 callbacks_not_configured rather than accepting it. A 202 would promise a delivery that cannot happen. Delivery refuses to build a request without a signer, which is a test (TestNeverPostsUnsigned) rather than a convention — §6.3 exists because an unsigned callback is the hole.

Considered and rejected: Gotenberg's replay-my-headers trick. Neat, avoids a signing scheme, but gives no body integrity, no replay protection, and puts a tenant secret in request logs.

6.4 Delivery is best effort

No dead-letter queue: the verdict is durably on the object and the status door always answers, so a dropped callback costs one poll rather than stranding a file. That is the dividend of building pull first.

The guarantee is AT-MOST-ONCE, not at-least-once. Draft 4 said the opposite and a review caught it: DrainApp deletes the queue items before delivery is attempted, so a console that dies between the delete and the POST loses that callback and nothing re-sends it — the sweep re-tags the object but never delivers. Duplicates are still possible (a retry after an ambiguous failure, or two consoles draining the same prefix), so apps must be idempotent and must not treat the callback as the only way an answer arrives. The docs sentence is: if no callback arrives within your own timeout, poll the status door.

The combination that actually hurts is infected + recorded:false + a lost callback: enforcement is not in effect (§5.2.1), the object is servable, and the app is never told unless it polls. That is the case the docs sentence is written for.

Delivery does not run in the drain's goroutine. Inline delivery would put a tenant's hanging endpoint directly in the path of every other tenant's verdict — 50 items times a bounded retry schedule, inside a tick that sweeps apps sequentially, which is the exact starvation §7.2 and §7.3 exist to prevent and a trivial tenant-triggered denial of service. Delivery is a bounded worker pool fed from done after the waiters are notified, with a bounded in-memory queue (overflow drops and logs — it is best effort by construction), a per-app in-flight cap, and retries scheduled by the pool rather than by the tick.

The policy, written down so it is not invented twice: success is 2xx only; 3xx is a refusal and is not retried, since redirects are refused by design; 4xx is not retried; 5xx, 429, timeouts and network errors are retried, three attempts at 1s/4s/16s; http.Client.Timeout around 10s so it covers connect, TLS, headers and body; the response body is drained through io.LimitReader(…, 4KiB) and discarded, because the console needs nothing from it; headers are Content-Type: application/json, a User-Agent and Carlos-Signature, and nothing else — no cookies, no credentials. Ordering is not guaranteed: retries reorder, and two verdicts for one key can arrive newest-first, which is why the body carries the ETag. One delivery per distinct callback path per key, which is already what Drained.CallbackPaths means.

---

7. The queue

7.1 Location and index

Items live in the app prefix:

apps/<acct>/<app>/scan-queue/<ulid>.json

One object per item, not one queue object: distinct keys never collide, so an enqueue needs no coordination. ULIDs sort by time, so a listing is the queue in order.

A top-level scan-queue/<acct>/<app>/ was rejected: PurgeApp lists only the app prefix (trash.go:229-238) and the name-claim gate (server.go:2875+) never looks outside it, so items would survive app deletion invisibly.

But S3 cannot list */scan-queue/*, so finding non-empty queues would be the full ListDirs("apps/") walk. Hence an index — and draft 2 left its removal rule unstated, which is a stranding race:

> drain lists (empty) → enqueue writes the item → enqueue marks the app → > drain removes the mark. The item is now in S3 with nothing pointing at it.

The rule, stated:

type queueIndex struct {
    mu  sync.Mutex
    gen map[string]uint64 // "acct/app" → generation
}
  • enqueue: write the object first, then gen[key]++ under the mutex, and only then answer 202 — a 202 sent between the two invites a client to stop retrying an item nothing points at
  • drain: read observed := gen[key], drain, then remove the entry only if gen[key] == observed and the listing returned zero items. Draft 4 said "fully drained — listing not truncated"; release.Store.List never truncates (release/store.go:555-569), so that clause was vacuous. Zero-items is the test that actually means "nothing is left", and it costs one empty listing per tick until the entry goes.
  • the drain needs a bounded, oldest-first listing, which does not exist today: List returns every key under a prefix (release/store.go:42-55), so a flooded app's whole queue is enumerated every 30s regardless of §7.3's cap. This is an optional interface, release.PageLister, type-asserted at the call site exactly as release.SizeLister (release/store.go:181-185) is at economicspass.go:141 — not a new method on release.Store, which has eleven implementations (DirStore, S3Store, prefixStore, fleetapi.store and seven test fakes). DirStore can serve it because ListMod is key-sorted (release/store.go:290), and S3 has StartAfter + MaxKeys, already used at sweep.go:71-78.
  • a crash between the object write and the increment strands that item until the next console start
  • boot rebuilds the index with one full enumeration, and in the same pass deletes items past their TTL — the only thing that collects a stranded item, so it is not optional

The removal condition is what draft 3 got wrong. §7.3 caps work per app per tick, so on a flooded app the drain deliberately leaves items behind; with generation-equality as the only condition it would then delete the entry pointing at them. The same happens when the drain aborts mid-app on an engine error — the RunSweep posture at sweep.go:222-240 that the drain inherits — or when Status errors on one item. Those items would sit until the next boot, and the callback and long-poll wake would be lost for every capped item, on precisely the app the cap exists for.

7.2 The drain is its own goroutine

Draft 1 put a queue phase inside the existing pass. That cannot deliver seconds of latency: scanPass sweeps apps sequentially, each for a full budget (scanpass.go:86-103), and the prior spec accepts ticks running back to back. A queue phase behind that starves for up to N × 60s.

Its own goroutine, its own ticker (30s). It never touches the sidecar; the only state shared with the sweep is object tags, idempotent per version.

7.3 Fairness

Round-robin across apps, 50 items per app per tick — at a 30s tick that is 100/minute, close to the door's own 120/minute limit, so a well-behaved app is never throttled by the drain. Not FIFO across the deployment. FIFO lets one tenant's ten thousand objects delay every other tenant's single urgent upload.

7.4 Amplification

A rate limit alone does not close this: at svcWriteBurst (120/min, authcli.go:140) one app can enqueue ~172k items a day, each up to the size ceiling.

  • the drain calls Status before Scan, as RunSweep calls Verdict first (sweep.go:215), so a key already judged clean, infected or skipped at its current version costs a tag read and nothing else
  • but the drain applies the re-issue rule itself: it scans when Status returns Unknown, Failed or Denied. Those two are transient by the landed code's own reasoning — "a permissions fix, a Glacier restore, an engine that was having a bad day" (clamav.go:151-163) — and the sweep re-issues them. Without this the on-demand path would be less able than the hourly one: an object the sweep last tagged failed would be answered from that tag forever and never rescanned on request. The cost is one failed GetObject on an object that is still denied.
  • items deduplicated by key per tick
  • per-app token bucket — the existing svcWrite limiter, keyed on principal name (svcprincipal.go:375-380), not a new one. Exceeding it is a 429, never a silent drop: an app being throttled needs to know, because its upload flow depends on the answer

7.5 Deletion and the purge race

This is the single deletion list; §5.1.1's "refuse" means "does not read bytes", not "does not delete".

Delete the item when it has been scanned (including §5.2.1, where a result object is written first); when its TTL has passed; when store.json is absent; when the app is in the trash; and when the record no longer permits scanning — Provisioned == nil, Requested == nil, the attestation is encrypted, or Scanning.Enabled == nil. None of those can change back in a way that makes the item useful.

Keep the item — up to the TTL when the deployment record is disabled, on engine drift, on an unreachable engine, and on any read error. All of those are conditions an operator can reverse. A deployment disabled for longer than a day drops its queue, which is right: both doors answered disabled at the time, and the sweep re-covers those objects once it is re-enabled.

Deletion is driven by absence, never by error — and absence at the release store layer is os.ErrNotExist (trash.go:64), not a nil value, so queue items and the system record must test for it explicitly.

"Inherits the purge machinery" inherits the hazard too: sweep.go:346-355 — any object written under a purged prefix makes the app name permanently unclaimable, and an enqueue racing PurgeApp leaves one behind. The orphan persists for up to one drain tick, during which claimApp refuses the name. This spec accepts that, the way writeSidecar accepts its one-round-trip race; §13's test asserts "not longer than one tick", not "never".

---

8. Metering

usage.go:26-38 carries MeterBackhaulGB recorded and explicitly unpriced. Scanning follows that idiom — but not into an existing object. EdgeDay is per-box (edge.go:41-46) and StorageDay is written once daily by a sweep rather than incremented (storage.go:13-18); neither takes increments from a door.

  • a new usage/scan/<date>.json, CAS-merged the way UpsertEdgeDay does (edge.go:85-116)
  • folded once per drain tick, not per object — each fold is a GET plus a conditional PUT of a deployment-wide object
  • new meter names in usage.go, frozen once merged (usage.go:4-5)
  • counts objects and bytes scanned on demand, per account per day
  • unpriced

Sweep-driven scanning is not attributed to a tenant: the operator asked for it, not them.

---

9. Where clamd lives

9.1 Draft 1's argument was wrong

It said the console "shovels every tenant object through its own memory". It does not: clamd.go:76-79,173-187 streams 64KB chunks and never buffers. The MemoryMax in the rollout plan is on clamd's cgroup, protecting the console from clamd's ~1.5GB signature RSS — not mitigation for a byte-shovelling smell, because there is not one.

9.2 The argument that holds

Moving clamd to a services box relocates the resident cost — ~1.5GB of signatures and the scanning CPU leave a control-plane box, permanently, for a feature idle by default. That is the whole benefit. It costs doubled network traffic, since the console still fetches from S3 and streams onward (§9.4), and it does not remove tenant bytes from the console; they still transit it.

A resource-placement decision, worth doing, worth stating at that size.

9.3 Reversing the prior decision

Prior spec §4.1: "No TCP option in v1: an unauthenticated scanning service reachable over a network is a worse problem than the one being solved."

That objection is right and is not answered by wanting a services box. It is answered by the network:

  • clamd has no authentication whatsoever
  • private interface only (Tailscale or equivalent), never a public port, and the services box firewalls the port to the console's address
  • a hard constraint on the infra ask, not a recommendation

internal/clamd has one "unix" literal (clamd.go:130); framing, deadlines and parsing are transport-independent. New gains a network parameter, and CARLOS_CLAMD_ADDR beginning with / means unix.

9.4 The console keeps fetching

Rather than giving the services box S3 read and tag permissions. Bytes move twice; tenant-data credentials stay on one box. Revisit with numbers.

---

10. Surface

carlos system antivirus status|enable|disable|rotate-webhook-key
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 queue depth and the deployment state (§4.5); system antivirus status gains queue depth deployment-wide.

---

11. Security properties

Each is a test.

  1. No callback reaches a host the app was not assigned — absolute URLs, //host, /\host, @-authority, traversal, CRLF, and encoded variants.
  2. No redirect is followed.
  3. A tampered body fails; app A's signature does not verify for app B; a stale timestamp is refused; both masters verify during rotation. These are properties of the APP's verifier, which is tenant code — so the console ships VerifyCallback(key, header, body, now) as the reference implementation, the docs reproduce it, and these tests run against it. Without that, the test tests nothing anybody runs. The signature is over the RAW BODY BYTES: an app that re-serialises the JSON before verifying will fail, and the docs say so.
  4. The app policy still has no s3:PutObjectTagging (aws.go:152-165).
  5. A role-Service credential cannot be minted account-scoped (§3.4) and cannot act on another app. (Admin reaches these doors by construction, §3.2 — this property is about Service.)
  6. A Service credential cannot reach system antivirus, scan grant, revoke, sweep or enforce.
  7. Version comparison is inside the scanner; no door or app sees the token.
  8. Enqueue and drain both enforce the consent chain (§5.1.1).
  9. Rate limits precede enqueue; a re-request of a key judged clean, infected or skipped costs a tag read and no fetch. (failed and denied are deliberately rescanned — §7.4.)
  10. A verdict that could not be persisted is reported as recorded:false, never as pending and never as a silent success (§5.2.1); and a failed or denied verdict is reported as itself, never as pending (§5.2).

---

12. Out of scope

Synchronous scan on PUT (CARLOS does not see the PUT); Gotenberg as a capability (the role is designed to be reused, the rendering service is its own brainstorm); GuardDuty (gated on the tagging measurement); pricing.

---

13. Testing

  • callback host and path validation as a table of hostile inputs; redirects refused
  • signature: tamper, cross-app, replay, skew, two-master rotation
  • pending for a stale tag, never the stale verdict; gone for a missing key
  • the consent chain: enqueue and drain both refuse not_requested, encrypted, not_enabled, disabled, and engine drift
  • one app flooding the queue does not delay another's item beyond one tick
  • a dropped callback leaves the verdict readable through the status door
  • system antivirus disable stops scanning without a restart and without touching per-app records — and per-app scan status then reports the deployment state rather than claiming health
  • request, revoke, enforce off still work while disabled
  • boot accepts CARLOS_STORE_SCANNER=clamav as a deprecated wiring alias, retaining the socket default; any other value stays fatal (§4.2.1)
  • the first pass tick — not boot — seeds the record and warns when a Scanner is wired with no record
  • a failed and a denied verdict each answer as themselves, not pending
  • unknown system/antivirus.json schema refuses
  • metering counts on-demand scans and not sweep scans
  • the index: no item strands while the process stays up (enqueue racing a drain's removal); an app flooded past the per-tick cap is fully drained across successive ticks — the test that would have caught review 3's blocker; an item written before a crash is drained after restart; TTL'd items are collected by the boot rebuild
  • an enqueue racing PurgeApp leaves no orphan for longer than one drain tick
  • a verdict reached but not persisted answers recorded:false, not pending (§5.2.1)
  • a result object written for v1 does not answer for v2 — the door compares versions and says pending
  • a result object is not written under a prefix purged mid-scan, and is collected by the boot rebuild once past TTL
  • the drain rescans an object whose tag says failed or denied, rather than answering from the tag (§7.4)
  • enable 503s when no Scanner is wired
  • the seed happens once from the retired variable and does not override an existing record (§4.2.1)
  • a second result object for the same key replaces the first (§5.2.1), and does not fail create-only
  • the drain keeps items and the index entry on a record read error, and deletes only on absence

---

14. Risks

  • The index is soft state, and losing an entry loses the callback. The fallback is the ordinary sweep, which re-sweeps a completed bucket only every ScanInterval — 24h by default (scanpass.go:31,171), not hourly. The object gets a verdict eventually and a polling app sees it; the webhook is simply not sent, and the queue item is collected at the next boot. Test by killing the console mid-queue.
  • Boot rebuild cost. One full enumeration at start on a deployment with many apps. Measure it.
  • Two new object kinds under the app prefix cost something. storagePass (economicspass.go:163) folds queue and result bytes into the tenant's BucketBytes, and the three full List("apps/") passes (provision/reconcile.go:586,1050, domains/reconcile.go:87, materialize.go:292) grow with a flooded queue until the TTL collects it. provision and domains skip unknown key shapes correctly, so for them this is cost, not breakage. materialize does not, by design: it counts any key under an app prefix (materialize.go:333-345, "convergence fails toward KEEPING the row") and restores a trashed row to live when it finds one (:377-382). So a result object stranded under a purged prefix does not only block the claim gate — it resurrects the app row on the next boot. That is the safe direction for materialize, and one more reason §5.2.1's purge guard is not optional.
  • Index size is bounded by the number of apps that have enqueued since boot and not yet been seen empty — at most the number of provisioned stores, a few dozen bytes each. Trashed and purged apps clear on their next empty listing, since PurgeApp deletes the items (trash.go:229-238).
  • Long-poll occupancy. Bounded wait, global and per-app waiter caps.