carlos / platform Public

Download this file

1	# Getting scanning onto real infrastructure
2
3	**Status:** draft 3 — closes a second Fable review, which found four blockers
4	(two of them security) and seventeen findings. Sequencing, not tasks.
5	**Covers:** the branch stack from where it stands to a scanner running against
6	real objects, twice.
7	**Plans it sequences:** `2026-09-01-object-storage-virus-scanning.md` (the
8	sweep, 14 tasks, all landed), `2026-09-01-carloku-scanning-rollout.md` (the
9	operational sequence), `2026-09-02-scan-on-demand.md` (17 tasks, 10 landed).
10
11	This is not a third implementation plan. It answers one question — *what is the
12	shortest honest path to something we can deploy and test* — and the answer
13	changes the order of work already planned.
14
15	**Draft 1's central argument was wrong and is gone.** It claimed three facts
16	could only be learned by deploying, and made that the reason to deploy the
17	sweep first. The version question is now answered and pinned on every push. The
18	tagging question is **answered for MinIO and still open for AWS** — the
19	deployment actually being planned — which is a weaker sentence than draft 2's
20	"two of the three have been answered" and the true one. The third never needed
21	a deployment either. What survives is a better reason, and one draft 1 did not
22	have at all.
23
24	---
25
26	## The state, stated correctly
27
28	Draft 1 measured against a **stale local `main`**. Corrected:
29
30	| | |
31	|---|---|
32	| `origin/main` | `6a59497` (#386, path-based routing) |
33	| local `main` | `97eb53f` — **24 commits behind** |
34	| `worktree-store-scan-on-demand` | **57 commits above `origin/main`** |
35	| `worktree-store-virus-scanning` | 24 of those; a strict ancestor |
36	| the sweep branch's pushed tip | `37d955d` — **local tip `e7236b7` is not pushed** |
37
38	The sweep layer is complete: 14 tasks, inert until three separate things are
39	true. The on-demand layer has tasks 11–17 outstanding.
40
41	### CI is green now, and was red for a week
42
43	`TestStoreVerbDispatchAndRefusals` asserted the `carlos store` usage block;
44	`scan` was added at `df947ec` and the assertion was not. Fixed at `3e0c990`;
45	`amadan ci status carlos/platform` passes.
46
47	It went unread because of a note — repeated into a second plan's global
48	constraints — saying *five root-package tests fail on this machine for an
49	environmental reason*. True, and exactly the camouflage a real failure needs.
50
51	The diagnosis is worse than the note. Locally the test binary **exits after 0.2
52	seconds** — a `log.Fatal` on the same root cause takes the process down, against
53	147 seconds in CI — so most of that package has never been run here at all. One
54	cause: `walkConfig` climbs from cwd to `/`, passes through `/home/paulca`, and
55	adopts the developer's own global config as an authoritative *project* pin. Any
56	checkout under a home directory gets it. Filed as **discussion #17**.
57
58	**A known-failing package cannot be a known-failing package.** The count was
59	what got read instead of the names.
60
61	---
62
63	## What has already been answered, without deploying anything
64
65	Draft 1 named three facts and scheduled them for a deployment. Here is where
66	they actually stand.
67
68	### The version question: answered, and pinned forever
69
70	`ScanObject.Version()` is `etag@RFC3339-lastmodified`. The sweep binds its tag
71	to the **listing's** version (`RunSweep` → `o.Version()`; it never calls
72	`HeadObject`, by design — the listing carries size, ETag and last-modified). The
73	status door does the opposite: `Status` heads the object to find its own
74	version, then compares the tag against it character for character.
75
76	So a gateway reporting those differently would put the two paths into a rescan
77	loop — not, as draft 1 claimed, invalidate every downstream seam. **The version
78	travels as an opaque string**; the queue, result object, status door and
79	callback body compare it and never parse it. The blast radius is one or two
80	functions, and draft 1's "the seam count that has to be revisited is every one
81	of them" was a sentence that sounded like a dependency analysis and was not one.
82
83	It is now a subtest of `TestDeliveredContractMinio`, which CI runs against a
84	real S3 server on every push (`.amadan/ci.d/30-minio-contract`). **They agree.**
85	Answered once by hand would have been worth less than answered on every push.
86
87	### The tagging question: answered, and it was a real hole
88
89	This one did not survive contact with a real server, and it is the reason draft
90	1 exists at all.
91
92	The anti-forgery property is that the console can write the `carlos-scan` tag
93	and the app cannot. `storePolicyJSON` said so in capitals, and a purpose-built
94	helper enforced it. **Both true, and neither sufficient** — because there are
95	two ways to tag an object, and leaving `s3:PutObjectTagging` out of the Allow
96	closes only the explicit call. The `x-amz-tagging` header on `PutObject` names
97	no action at all, so no search for an action name could ever have found it.
98
99	(Draft 2 also said "AGENTS.md carries it as a standing rule", and so did
100	`6fba2b3`'s commit message. **Both were false** — AGENTS.md had no mention of
101	it. It does now, in the two-route form, and it says the absence of the
102	permission is not the property.)
103
104	Against MinIO, given the app policy verbatim, the pre-tagged upload **succeeded**.
105	An app could have marked its own malware clean, and the sweep — finding a
106	version-matching clean tag — would never have scanned it.
107
108	**And the fix had a second hole behind it**, which the same review found:
109	closing "an app can mark itself clean" left "an app can make itself
110	*unmarkable*". S3 caps an object at ten tags; `tag()` kept every foreign tag,
111	appended our three, and on overflow returned `(false, nil)` silently. Upload
112	malware with eight tags of your own and `carlos-scan=infected` can never be
113	written — so the `enforce` Deny, which keys on that tag being *present*, never
114	engages, and the object stays readable through the app's own credential. Every
115	surface reports a system working normally: the sweep counts it reached, the
116	result object says `recorded:false`, the status door answers unknown. Fixed at
117	`63e80cc` — an infected verdict now evicts foreign tags to fit, clean still
118	yields.
119
120	The obvious fix does not work: an explicit `Deny` on `s3:PutObjectTagging` left
121	the upload route wide open. Only denying `s3:PutObject` conditioned on
122	`s3:RequestObjectTagKeys` closed it. Fixed at `6fba2b3`, verified with the
123	policy the code generates against a live server, written up as **discussion
124	#18**.
125
126	Tito's app buckets are real AWS (`NewAWSProvisioner` builds IAM with no endpoint
127	override, deliberately — "IAM has no S3-compatible twin"), and AWS documents the
128	header as requiring `s3:PutObjectTagging`, so Tito was probably never exposed.
129	Self-hosters are in scope by decision, and the property has to hold on the
130	gateway rather than on the paperwork.
131
132	### The clamd question: never needed a deployment either
133
134	Resident-signature memory and the reload spike are properties of the ClamAV
135	database, measurable on any box with `clamav-daemon` installed. **Throughput** —
136	seconds per object at 1 KB, 1 MB and `MAX_BYTES` through INSTREAM — is the only
137	genuinely box-specific number, and it is the one that matters, because spec §1's
138	"well under a second" is scan-on-demand's entire premise and is currently a
139	guess. Draft 1 asked for `systemd-cgtop` and did not ask for throughput at all.
140
141	---
142
143	## The decision: split the stack, deploy twice
144
145	Still split. **On different reasons.**
146
147	### 1. One branch carrying two features is not a reviewable diff
148
149	AGENTS.md's stated purpose for the gate is that the PR is "the one place the
150	branch's diff is read as a single piece". A branch spanning a sweep and an
151	on-demand queue is not that, whatever the number — and the honest form of this
152	reason is **two features, two PRs**, which needs no count. (Draft 2 argued from
153	"50 commits"; that cuts both ways, since the on-demand half will pass 30 by
154	task 17.)
155
156	### 2. Tito's half has a lead time we do not control
157
158	`tito/carlos-infrastructure` **#6** — clamd unit, freshclam timer, cgroup limits,
159	the console's S3 permissions — is filed, detailed and **unanswered**. Nothing
160	past rollout step 2 moves without it.
161
162	Precisely: the split does not *start* that clock — filing #6 did. What it buys
163	is that **if #6 is answered before tasks 11–17 finish, the sweep can deploy
164	without waiting for them.** And the second ask of Tito (the console unit's env
165	change and restart, Phase 2 step 4) is **not filed at all**, so half the wait is
166	not yet on anyone's list. File it in Phase 0 or this reason is about half of
167	what it claims.
168
169	### 3. ~~The sweep is what ships the policy fix~~ — withdrawn
170
171	Draft 2 called this the strongest of the three. It is not a reason at all, and
172	the review was right to say so: **if the stack is not split, the policy fixes
173	ship by construction**, on the branch that merges. "The sweep is what ships the
174	fix" is only true *after* deciding the sweep ships alone, so it has zero weight
175	on the split question. It is a **constraint on Phase 1**, not an argument for
176	Phase 1, and it has moved there.
177
178	Draft 2 also had it both ways — a *must* here and "a tidy-up" in the closing
179	section. The decision stands on reasons 1 and 2, honestly, or not at all.
180
181	### What the split actually costs
182
183	Not "one rebase now versus the same rebase later" — both are conflict-free
184	today; that sentence was rhetoric. The real cost is **the migration seam**, and
185	draft 1 did not mention it. See Phase 4.
186
187	---
188
189	## Phase 0 — green, and honest about what red means
190
191	- [x] Fix the `carlos store` usage assertion (`3e0c990`).
192	- [x] CI green: `carlos/platform @ 6f166b3` passed — first pass on either scan
193	branch in a week.
194	- [x] Diagnose the local failures properly; **discussion #17**.
195	- [ ] **Reconcile AGENTS.md with the actual topology.** AGENTS.md says PRs live
196	on GitHub `origin`, land with `gh pr merge`, then push `main` to an `amadan`
197	remote. This repo has **one** remote, `origin = https://amadan.net/carlos/platform`;
198	there is no GitHub, no `amadan` remote, and `gh` cannot see it. Landing is
199	`amadan branch describe` + `amadan branch merge`, gated by `amadan ci status`.
200	Phase 1 cannot follow a document that describes a different repository.
201	- [x] AGENTS.md gains the anti-forgery rule, in the two-route form. Draft 2 said
202	"AGENTS.md carries it as a standing rule" and `6fba2b3`'s message said the
203	same; **both were false** — AGENTS.md had no mention of it. Now it does, and
204	it says that the absence of the permission is not the property.
205	- [ ] Zero known-failing tests, and a line in AGENTS.md that a known-failing
206	test is a defect in its own right — for the reason two sections up: the count
207	was read instead of the names, and a week of red went unexamined.
208	- [ ] File the second Tito ask (see reason 2).
209
210	## Phase 0.5 — the checks that never needed a deployment
211
212	**Runs now, in parallel with everything.** Draft 1 had these inside Phase 2,
213	which delayed them behind another team.
214
215	- [x] Head/List version agreement — a contract subtest, on every push.
216	- [x] The app cannot tag its own object, **by either route** — same test.
217	- [ ] **THE POST-POLICY ROUTE IS OPEN, AND IAM CANNOT CLOSE IT — discussion
218	#19.** Verified against MinIO: a browser-direct form upload carrying
219	`carlos-scan=clean` **succeeds** (204) with the current policy, because the
220	gateway does not populate `s3:RequestObjectTagKeys` for that handler. The
221	control passes, so the route works and the Deny simply does not fire. It is
222	the worst of the three findings in this property, because it is not an exotic
223	call — it is the standard way to take a file from a user, which is the
224	workload scanning exists for.
225
226	The fix is to make the tag **unforgeable rather than unwritable**: a signed
227	fourth tag, `HMAC(deployment key, bucket|key|version|verdict|at)`, verified in
228	`readTag`, so a tag that does not verify is treated as no tag and the object
229	is scanned. Gateway-independent, and it closes every route including the ones
230	nobody has thought of. It needs a key the console does not have — by design it
231	holds only `CARLOS_SECRETS_PUBKEY` — so it is the same new deployment secret
232	§6.3 introduces for callback signing, and should be decided once for both.
233
234	**This gates the first member grant, not the first deploy.** Rollout steps 1–4
235	are inert. The moment a member's store is granted and swept, a clean verdict
236	is being reported, and this is the route by which it can be false.
237	- [x] Two more tagging routes, closed and now pinned (`0fcba4b`):
238	`CreateMultipartUpload` with `x-amz-tagging` and `CopyObject` with a REPLACE
239	directive. Both **were** open in the sense that nothing tested them; the
240	`s3:RequestObjectTagKeys` Deny already covered them, verified by mutation —
241	remove the Deny and all of them succeed. Each carries a control asserting the
242	verb itself still works, without which the refusal proves nothing.
243	- [ ] ~~THREE MORE TAGGING ROUTES, untested and each authorised as plain
244	`s3:PutObject`.~~ Done, above — and the third one is a hole, not a check. "By either route" is two routes; the property has more:
245	- [x] `s3:PutObjectVersionTagging`/`s3:DeleteObjectVersionTagging` added to the
246	first Deny (`0fcba4b`) — unreachable today, which is when a shape is cheapest
247	to close.
248	- [ ] `x-amz-tagging` against **real AWS** — and for all the routes above, not
249	just the header on `PutObject`. AWS documents that one as requiring
250	`s3:PutObjectTagging`; it is far less explicit about multipart and POST
251	policy.
252	- [ ] **The fixture must be checked against the policy, not commented at.**
253	`d848c57` says the CI fixture "has to be the policy" and nothing enforces it;
254	"verified against a live server" was true once, by hand. Move the policy out
255	of the heredoc into a file the script `cat`s, and add a unit test comparing it
256	semantically to `storePolicyJSON("arn:aws:s3:::contract-bucket", nil)`.
257	- [ ] The `enforce` Deny actually denies — **and this needs no deployment
258	either.** The contract fixture's scoped user *is* an app-shaped credential
259	against a real IAM: root tags an object `carlos-scan=infected`, the fixture
260	gains the `ExistingObjectTag` Deny, and the scoped `GetObject` must be
261	`AccessDenied`. Then the variant that matters: a scoped `CopyObject` from the
262	infected source with no tags, and a `GET` of the copy — on a gateway that does
263	not evaluate the source's tags, that is a clean enforcement bypass. If any of
264	this fails the repair is a `storePolicyJSON` change, which rewrites every
265	existing store's policy on the next re-walk — itself a rollout event, and it
266	must precede any member grant.
267	- [ ] clamd resident memory and reload spike, on any box with the package.
268
269	Draft 2 said obtaining an app credential was "legitimate platform work; pick one
270	before writing the step". That was draft 1's own mistake wearing a later date:
271	none of these need a deployed app at all. They need a scoped credential against
272	a real IAM, which the contract fixture already mints on every push.
273
274	## Phase 1 — land the sweep half
275
276	**Half a day. Order matters and draft 1 had it wrong.**
277
278	**Do not cherry-pick. Reorder, then advance the pointer.** Draft 2 listed two
279	commits to pick and would have left the sweep branch **red twice**:
280
281	- `6fba2b3` adds the contract subtest for the tagging routes, but the sweep
282	branch's `.amadan/ci.d/30-minio-contract` fixture has no Deny statements — the
283	upload succeeds and the MinIO gate fails. That needs `d848c57` too.
284	- The sweep's `internal/console/apiscan_test.go` asserts `!Contains(policy,
285	"Effect":"Deny")` after grant, which the unconditional anti-forgery Denys
286	break. That needs `d6142ac` too.
287
288	And `63e80cc` (the untaggable fix) is now a third. Picking a list that has to be
289	kept complete by hand is the wrong shape: **rebase the on-demand commits so the
290	fix commits precede them, then advance `worktree-store-virus-scanning` over
291	them.** The on-demand branch stays a strict descendant, no patch is duplicated,
292	and the last Phase 1 step stops depending on patch-id dedupe surviving an
293	amend.
294
295	- [ ] **Rebase onto `origin/main`.** Draft 1 rebased only the on-demand branch,
296	*after* the merge, which would have landed code CI never ran against #386.
297	Trial merges are conflict-free today — cheap, not free, and it was missing.
298	- [ ] Post-rebase, run the two places a textually clean merge can still be
299	semantically short: `internal/docsite` (the heading gate) and
300	`TestEveryClosedDoorRefusesAServiceCredential` (`closedDoors`). Both #385/#386
301	and the scan branches touch `docs/site/cli.md` and
302	`internal/console/authcli.go`.
303	- [ ] `amadan branch describe` — the description is the PR body, and carries
304	what fourteen tasks and four review rounds decided, written for someone who
305	has read none of it.
306	- [ ] Green on **that branch's pushed tip**, then merge. (Draft 2 opened Phase 1
307	by pushing `e7236b7` and then immediately rewrote it; any green that produced
308	would have been exactly the misleading reading the bullet warns about. Push
309	once, after the reorder.)
310	- [ ] Rebase `worktree-store-scan-on-demand` onto the new `main`.
311
312	## Phase 2 — deploy the sweep to Tito, and test it
313
314	Executes `2026-09-01-carloku-scanning-rollout.md` steps 1–7 against **Tito
315	only**, with the rollout plan's **original** step 4 and Rollback section.
316
317	**Draft 1's "Amendment A" is deleted.** It said step 4 becomes
318	`CARLOS_CLAMD_ADDR` + `carlos system antivirus enable`, with `disable` as a
319	no-restart rollback. **None of that exists on the sweep branch** — the record,
320	the `system` verb, `apisystem.go`, the TCP dial and `CARLOS_CLAMD_ADDR` are all
321	on-demand commits. For this deployment, wiring is `CARLOS_STORE_SCANNER=clamav`
322	plus `CARLOS_CLAMD_SOCKET`, and rollback is unset-and-restart. Describing a
323	binary you are not deploying is how a rollback gets attempted with a verb that
324	is not there.
325
326	**Step 5a**, reduced to what genuinely needs this deployment:
327
328	- [ ] **clamd throughput** on the real box: seconds per object at 1 KB, 1 MB and
329	`MAX_BYTES` through INSTREAM. This sets the per-app budget and task 12's
330	per-tick drain cap. Draft 1 claimed `systemd-cgtop` would set
331	`CARLOS_CLAMD_MAX_BYTES`; it does not — memory sets `MemoryMax`, `MAX_BYTES`
332	mirrors `StreamMaxLength`, and throughput sets the budgets. Nothing in draft
333	1 measured throughput.
334	- [ ] **Enumeration cost per tick** against the real endpoint, on an app with
335	many objects. Spec §11 lists it as a risk to verify and nothing did.
336	- [ ] **EICAR end to end**: uploaded, swept, tagged infected, `carlos store scan
337	status` saying so. The first time clamd runs outside a fake.
338	- [ ] **Overwrite a tagged-clean object** and confirm the fresh count rises.
339
340	**Step 6 gains the rollback that does not roll back.** Teardown refuses when a
341	record has scanning enabled and the scanner is nil
342	(`internal/objstore/walk.go:465` — named, because
343	`internal/console/appsettings.go:838` is a second, UI-level copy of the same
344	refusal that step 6 also exercises), so after
345	unset-and-restart **no scanning-enabled app can be destroyed** until scanning is
346	re-enabled or revoked. The rollout plan's Rollback names only the enforce Deny.
347	Destroy an enabled app during rollback, observe the refusal, confirm `scan
348	revoke` clears it with no engine.
349
350	**Blocked on:** discussion #6 for step 2, plus a **second, unnamed ask of Tito** —
351	the console unit's env change and restart for step 4 are also
352	`tito/carlos-infrastructure` work, and #6 covers only clamd.
353
354	## Phase 3 — scan-on-demand, tasks 11–17
355
356	Sequence 11 → 12 → 13 → (14 → 15) → 16 → 17, with two changes:
357
358	- [x] **The deferred refactor is done** (`4e2bfb8`). `queuePermitted` returns
359	`(*Record, QueueRefusal, error)`, and `Permanent()` is §7.5's delete-vs-keep
360	split in the one place both callers read it from. §5.1.1's concern is closed.
361	- [x] **The callback path validator is done** (`4846e42`), ahead of the door
362	that accepts the field rather than four commits behind it.
363	- [ ] The enqueue door itself: the route, the six refusals, the attestation
364	hole, the Service-role gate, 429.
365	- **Task 12 delivers `QueueIndex.Depth()`**, deferred out of task 10.
366
367	Phase 2's throughput numbers land in task 12's budget.
368
369	## Phase 4 — deploy on-demand, migration seam first
370
371	**This is what splitting actually costs, and draft 1 omitted it.**
372
373	Deploying the sweep first means Tito runs on `CARLOS_STORE_SCANNER`. On-demand
374	then upgrades under the §4.2.1 migration, whose seed fires **only on the first
375	tick while the deprecated variable is still set**. Swap the variable in the same
376	restart as the binary and scanning stops silently — warned per tick, but stopped.
377	This code path has only ever run in tests. So:
378
379	- [ ] Deploy on-demand with `CARLOS_STORE_SCANNER` **still set**.
380	- [ ] Confirm `carlos system antivirus status` reports the record seeded by
381	migration.
382	- [ ] *Then* swap to `CARLOS_CLAMD_ADDR` — **removing `CARLOS_STORE_SCANNER`
383	and `CARLOS_CLAMD_SOCKET` in the same edit.** `cmd/console/main.go` is fatal
384	when `CARLOS_CLAMD_ADDR` and `CARLOS_CLAMD_SOCKET` are both set and disagree,
385	and rollout step 4 sets the socket explicitly. Leave both and the console
386	fatals, and the adopt pass rolls back to the old binary.
387
388	Followed by: mint a Service credential, enqueue one object, watch it drain,
389	receive one signed callback, verify the signature app-side. Then the failures
390	only a deployment shows — a callback to a dead app, a redirect, a queue flooded
391	past the per-tick limit, a `disable` with items in flight. Then Carloku, per
392	rollout step 8. Then the skills repo, held to task 17 so the `carlos store scan`
393	section is not shipped twice.
394
395	---
396
397	## Not in this plan
398
399	- **The services box and Gotenberg.** Agreed in principle, unspecced. clamd goes
400	on the Tito console box under #6's limits; moving it later is a change of
401	address, which the client already supports deliberately (`f889f20`).
402	- **`rastrillo/rastrillo` discussion #11** (not `carlos/platform` #11) is theirs;
403	scanning does not wait on it.
404	- **Anything on Carloku before Tito.**
405
406	## What would change this plan now
407
408	Not the timestamps — that is settled and pinned. The live ones:
409
410	- **Real AWS not populating `s3:RequestObjectTagKeys` for POST policy either**
411	(#19). Then the signed-tag fix is not a hardening for self-hosters, it is the
412	only thing standing between an app and a forged clean verdict on Tito, and it
413	moves ahead of tasks 11–17 rather than beside them.
414	- **`x-amz-tagging` on real AWS behaving like MinIO**, by any of the other
415	routes in Phase 0.5. Then the policy fixes are not belt-and-braces on Tito but the only
416	thing between an app and a forged clean verdict — which changes the urgency of
417	Phase 0.5, not the shape of Phase 1: the fixes ride the first half either way.
418	- **The `enforce` Deny not denying.** A `storePolicyJSON` change rewrites every
419	existing store's policy on the next re-walk — a rollout event that must land
420	before the first member grant, not after it.
421	- **clamd throughput far off "well under a second".** Scan-on-demand's premise
422	is a callback that arrives while the user is still there. If a 1 MB object
423	takes ten seconds, task 12's budget is not a tuning parameter, it is a
424	redesign — and that is worth knowing before task 11, which is the one argument
425	draft 1 was reaching for and made about the wrong fact.
426