carlos / platform Public
Getting scanning onto real infrastructure
Status: draft 3 — closes a second Fable review, which found four blockers (two of them security) and seventeen findings. Sequencing, not tasks. Covers: the branch stack from where it stands to a scanner running against real objects, twice. Plans it sequences: 2026-09-01-object-storage-virus-scanning.md (the sweep, 14 tasks, all landed), 2026-09-01-carloku-scanning-rollout.md (the operational sequence), 2026-09-02-scan-on-demand.md (17 tasks, 10 landed).
This is not a third implementation plan. It answers one question — what is the shortest honest path to something we can deploy and test — and the answer changes the order of work already planned.
Draft 1's central argument was wrong and is gone. It claimed three facts could only be learned by deploying, and made that the reason to deploy the sweep first. The version question is now answered and pinned on every push. The tagging question is answered for MinIO and still open for AWS — the deployment actually being planned — which is a weaker sentence than draft 2's "two of the three have been answered" and the true one. The third never needed a deployment either. What survives is a better reason, and one draft 1 did not have at all.
---
The state, stated correctly
Draft 1 measured against a stale local main. Corrected:
| | | |---|---| | origin/main | 6a59497 (#386, path-based routing) | | local main | 97eb53f — 24 commits behind | | worktree-store-scan-on-demand | 57 commits above origin/main | | worktree-store-virus-scanning | 24 of those; a strict ancestor | | the sweep branch's pushed tip | 37d955d — local tip e7236b7 is not pushed |
The sweep layer is complete: 14 tasks, inert until three separate things are true. The on-demand layer has tasks 11–17 outstanding.
CI is green now, and was red for a week
TestStoreVerbDispatchAndRefusals asserted the carlos store usage block; scan was added at df947ec and the assertion was not. Fixed at 3e0c990; amadan ci status carlos/platform passes.
It went unread because of a note — repeated into a second plan's global constraints — saying five root-package tests fail on this machine for an environmental reason. True, and exactly the camouflage a real failure needs.
The diagnosis is worse than the note. Locally the test binary exits after 0.2 seconds — a log.Fatal on the same root cause takes the process down, against 147 seconds in CI — so most of that package has never been run here at all. One cause: walkConfig climbs from cwd to /, passes through /home/paulca, and adopts the developer's own global config as an authoritative project pin. Any checkout under a home directory gets it. Filed as discussion #17.
A known-failing package cannot be a known-failing package. The count was what got read instead of the names.
---
What has already been answered, without deploying anything
Draft 1 named three facts and scheduled them for a deployment. Here is where they actually stand.
The version question: answered, and pinned forever
ScanObject.Version() is etag@RFC3339-lastmodified. The sweep binds its tag to the listing's version (RunSweep → o.Version(); it never calls HeadObject, by design — the listing carries size, ETag and last-modified). The status door does the opposite: Status heads the object to find its own version, then compares the tag against it character for character.
So a gateway reporting those differently would put the two paths into a rescan loop — not, as draft 1 claimed, invalidate every downstream seam. The version travels as an opaque string; the queue, result object, status door and callback body compare it and never parse it. The blast radius is one or two functions, and draft 1's "the seam count that has to be revisited is every one of them" was a sentence that sounded like a dependency analysis and was not one.
It is now a subtest of TestDeliveredContractMinio, which CI runs against a real S3 server on every push (.amadan/ci.d/30-minio-contract). They agree. Answered once by hand would have been worth less than answered on every push.
The tagging question: answered, and it was a real hole
This one did not survive contact with a real server, and it is the reason draft 1 exists at all.
The anti-forgery property is that the console can write the carlos-scan tag and the app cannot. storePolicyJSON said so in capitals, and a purpose-built helper enforced it. Both true, and neither sufficient — because there are two ways to tag an object, and leaving s3:PutObjectTagging out of the Allow closes only the explicit call. The x-amz-tagging header on PutObject names no action at all, so no search for an action name could ever have found it.
(Draft 2 also said "AGENTS.md carries it as a standing rule", and so did 6fba2b3's commit message. Both were false — AGENTS.md had no mention of it. It does now, in the two-route form, and it says the absence of the permission is not the property.)
Against MinIO, given the app policy verbatim, the pre-tagged upload succeeded. An app could have marked its own malware clean, and the sweep — finding a version-matching clean tag — would never have scanned it.
And the fix had a second hole behind it, which the same review found: closing "an app can mark itself clean" left "an app can make itself unmarkable". S3 caps an object at ten tags; tag() kept every foreign tag, appended our three, and on overflow returned (false, nil) silently. Upload malware with eight tags of your own and carlos-scan=infected can never be written — so the enforce Deny, which keys on that tag being present, never engages, and the object stays readable through the app's own credential. Every surface reports a system working normally: the sweep counts it reached, the result object says recorded:false, the status door answers unknown. Fixed at 63e80cc — an infected verdict now evicts foreign tags to fit, clean still yields.
The obvious fix does not work: an explicit Deny on s3:PutObjectTagging left the upload route wide open. Only denying s3:PutObject conditioned on s3:RequestObjectTagKeys closed it. Fixed at 6fba2b3, verified with the policy the code generates against a live server, written up as discussion #18.
Tito's app buckets are real AWS (NewAWSProvisioner builds IAM with no endpoint override, deliberately — "IAM has no S3-compatible twin"), and AWS documents the header as requiring s3:PutObjectTagging, so Tito was probably never exposed. Self-hosters are in scope by decision, and the property has to hold on the gateway rather than on the paperwork.
The clamd question: never needed a deployment either
Resident-signature memory and the reload spike are properties of the ClamAV database, measurable on any box with clamav-daemon installed. Throughput — seconds per object at 1 KB, 1 MB and MAX_BYTES through INSTREAM — is the only genuinely box-specific number, and it is the one that matters, because spec §1's "well under a second" is scan-on-demand's entire premise and is currently a guess. Draft 1 asked for systemd-cgtop and did not ask for throughput at all.
---
The decision: split the stack, deploy twice
Still split. On different reasons.
1. One branch carrying two features is not a reviewable diff
AGENTS.md's stated purpose for the gate is that the PR is "the one place the branch's diff is read as a single piece". A branch spanning a sweep and an on-demand queue is not that, whatever the number — and the honest form of this reason is two features, two PRs, which needs no count. (Draft 2 argued from "50 commits"; that cuts both ways, since the on-demand half will pass 30 by task 17.)
2. Tito's half has a lead time we do not control
tito/carlos-infrastructure #6 — clamd unit, freshclam timer, cgroup limits, the console's S3 permissions — is filed, detailed and unanswered. Nothing past rollout step 2 moves without it.
Precisely: the split does not start that clock — filing #6 did. What it buys is that if #6 is answered before tasks 11–17 finish, the sweep can deploy without waiting for them. And the second ask of Tito (the console unit's env change and restart, Phase 2 step 4) is not filed at all, so half the wait is not yet on anyone's list. File it in Phase 0 or this reason is about half of what it claims.
3. ~~The sweep is what ships the policy fix~~ — withdrawn
Draft 2 called this the strongest of the three. It is not a reason at all, and the review was right to say so: if the stack is not split, the policy fixes ship by construction, on the branch that merges. "The sweep is what ships the fix" is only true after deciding the sweep ships alone, so it has zero weight on the split question. It is a constraint on Phase 1, not an argument for Phase 1, and it has moved there.
Draft 2 also had it both ways — a must here and "a tidy-up" in the closing section. The decision stands on reasons 1 and 2, honestly, or not at all.
What the split actually costs
Not "one rebase now versus the same rebase later" — both are conflict-free today; that sentence was rhetoric. The real cost is the migration seam, and draft 1 did not mention it. See Phase 4.
---
Phase 0 — green, and honest about what red means
- [x] Fix the
carlos storeusage assertion (3e0c990). - [x] CI green:
carlos/platform @ 6f166b3passed — first pass on either scan branch in a week. - [x] Diagnose the local failures properly; discussion #17.
- [ ] Reconcile AGENTS.md with the actual topology. AGENTS.md says PRs live on GitHub
origin, land withgh pr merge, then pushmainto anamadanremote. This repo has one remote,origin = https://amadan.net/carlos/platform; there is no GitHub, noamadanremote, andghcannot see it. Landing isamadan branch describe+amadan branch merge, gated byamadan ci status. Phase 1 cannot follow a document that describes a different repository. - [x] AGENTS.md gains the anti-forgery rule, in the two-route form. Draft 2 said "AGENTS.md carries it as a standing rule" and
6fba2b3's message said the same; both were false — AGENTS.md had no mention of it. Now it does, and it says that the absence of the permission is not the property. - [ ] Zero known-failing tests, and a line in AGENTS.md that a known-failing test is a defect in its own right — for the reason two sections up: the count was read instead of the names, and a week of red went unexamined.
- [ ] File the second Tito ask (see reason 2).
Phase 0.5 — the checks that never needed a deployment
Runs now, in parallel with everything. Draft 1 had these inside Phase 2, which delayed them behind another team.
- [x] Head/List version agreement — a contract subtest, on every push.
- [x] The app cannot tag its own object, by either route — same test.
- [ ] THE POST-POLICY ROUTE IS OPEN, AND IAM CANNOT CLOSE IT — discussion #19. Verified against MinIO: a browser-direct form upload carrying
carlos-scan=cleansucceeds (204) with the current policy, because the gateway does not populates3:RequestObjectTagKeysfor that handler. The control passes, so the route works and the Deny simply does not fire. It is the worst of the three findings in this property, because it is not an exotic call — it is the standard way to take a file from a user, which is the workload scanning exists for.
The fix is to make the tag unforgeable rather than unwritable: a signed fourth tag, HMAC(deployment key, bucket|key|version|verdict|at), verified in readTag, so a tag that does not verify is treated as no tag and the object is scanned. Gateway-independent, and it closes every route including the ones nobody has thought of. It needs a key the console does not have — by design it holds only CARLOS_SECRETS_PUBKEY — so it is the same new deployment secret §6.3 introduces for callback signing, and should be decided once for both.
This gates the first member grant, not the first deploy. Rollout steps 1–4 are inert. The moment a member's store is granted and swept, a clean verdict is being reported, and this is the route by which it can be false.
- [x] Two more tagging routes, closed and now pinned (
0fcba4b):CreateMultipartUploadwithx-amz-taggingandCopyObjectwith a REPLACE directive. Both were open in the sense that nothing tested them; thes3:RequestObjectTagKeysDeny already covered them, verified by mutation — remove the Deny and all of them succeed. Each carries a control asserting the verb itself still works, without which the refusal proves nothing. - [ ] ~~THREE MORE TAGGING ROUTES, untested and each authorised as plain
s3:PutObject.~~ Done, above — and the third one is a hole, not a check. "By either route" is two routes; the property has more: - [x]
s3:PutObjectVersionTagging/s3:DeleteObjectVersionTaggingadded to the first Deny (0fcba4b) — unreachable today, which is when a shape is cheapest to close. - [ ]
x-amz-taggingagainst real AWS — and for all the routes above, not just the header onPutObject. AWS documents that one as requirings3:PutObjectTagging; it is far less explicit about multipart and POST policy. - [ ] The fixture must be checked against the policy, not commented at.
d848c57says the CI fixture "has to be the policy" and nothing enforces it; "verified against a live server" was true once, by hand. Move the policy out of the heredoc into a file the scriptcats, and add a unit test comparing it semantically tostorePolicyJSON("arn:aws:s3:::contract-bucket", nil). - [ ] The
enforceDeny actually denies — and this needs no deployment either. The contract fixture's scoped user is an app-shaped credential against a real IAM: root tags an objectcarlos-scan=infected, the fixture gains theExistingObjectTagDeny, and the scopedGetObjectmust beAccessDenied. Then the variant that matters: a scopedCopyObjectfrom the infected source with no tags, and aGETof the copy — on a gateway that does not evaluate the source's tags, that is a clean enforcement bypass. If any of this fails the repair is astorePolicyJSONchange, which rewrites every existing store's policy on the next re-walk — itself a rollout event, and it must precede any member grant. - [ ] clamd resident memory and reload spike, on any box with the package.
Draft 2 said obtaining an app credential was "legitimate platform work; pick one before writing the step". That was draft 1's own mistake wearing a later date: none of these need a deployed app at all. They need a scoped credential against a real IAM, which the contract fixture already mints on every push.
Phase 1 — land the sweep half
Half a day. Order matters and draft 1 had it wrong.
Do not cherry-pick. Reorder, then advance the pointer. Draft 2 listed two commits to pick and would have left the sweep branch red twice:
6fba2b3adds the contract subtest for the tagging routes, but the sweep branch's.amadan/ci.d/30-minio-contractfixture has no Deny statements — the upload succeeds and the MinIO gate fails. That needsd848c57too.- The sweep's
internal/console/apiscan_test.goasserts!Contains(policy, "Effect":"Deny")after grant, which the unconditional anti-forgery Denys break. That needsd6142actoo.
And 63e80cc (the untaggable fix) is now a third. Picking a list that has to be kept complete by hand is the wrong shape: rebase the on-demand commits so the fix commits precede them, then advance worktree-store-virus-scanning over them. The on-demand branch stays a strict descendant, no patch is duplicated, and the last Phase 1 step stops depending on patch-id dedupe surviving an amend.
- [ ] Rebase onto
origin/main. Draft 1 rebased only the on-demand branch, after the merge, which would have landed code CI never ran against #386. Trial merges are conflict-free today — cheap, not free, and it was missing. - [ ] Post-rebase, run the two places a textually clean merge can still be semantically short:
internal/docsite(the heading gate) andTestEveryClosedDoorRefusesAServiceCredential(closedDoors). Both #385/#386 and the scan branches touchdocs/site/cli.mdandinternal/console/authcli.go. - [ ]
amadan branch describe— the description is the PR body, and carries what fourteen tasks and four review rounds decided, written for someone who has read none of it. - [ ] Green on that branch's pushed tip, then merge. (Draft 2 opened Phase 1 by pushing
e7236b7and then immediately rewrote it; any green that produced would have been exactly the misleading reading the bullet warns about. Push once, after the reorder.) - [ ] Rebase
worktree-store-scan-on-demandonto the newmain.
Phase 2 — deploy the sweep to Tito, and test it
Executes 2026-09-01-carloku-scanning-rollout.md steps 1–7 against Tito only, with the rollout plan's original step 4 and Rollback section.
Draft 1's "Amendment A" is deleted. It said step 4 becomes CARLOS_CLAMD_ADDR + carlos system antivirus enable, with disable as a no-restart rollback. None of that exists on the sweep branch — the record, the system verb, apisystem.go, the TCP dial and CARLOS_CLAMD_ADDR are all on-demand commits. For this deployment, wiring is CARLOS_STORE_SCANNER=clamav plus CARLOS_CLAMD_SOCKET, and rollback is unset-and-restart. Describing a binary you are not deploying is how a rollback gets attempted with a verb that is not there.
Step 5a, reduced to what genuinely needs this deployment:
- [ ] clamd throughput on the real box: seconds per object at 1 KB, 1 MB and
MAX_BYTESthrough INSTREAM. This sets the per-app budget and task 12's per-tick drain cap. Draft 1 claimedsystemd-cgtopwould setCARLOS_CLAMD_MAX_BYTES; it does not — memory setsMemoryMax,MAX_BYTESmirrorsStreamMaxLength, and throughput sets the budgets. Nothing in draft 1 measured throughput. - [ ] Enumeration cost per tick against the real endpoint, on an app with many objects. Spec §11 lists it as a risk to verify and nothing did.
- [ ] EICAR end to end: uploaded, swept, tagged infected,
carlos store scan statussaying so. The first time clamd runs outside a fake. - [ ] Overwrite a tagged-clean object and confirm the fresh count rises.
Step 6 gains the rollback that does not roll back. Teardown refuses when a record has scanning enabled and the scanner is nil (internal/objstore/walk.go:465 — named, because internal/console/appsettings.go:838 is a second, UI-level copy of the same refusal that step 6 also exercises), so after unset-and-restart no scanning-enabled app can be destroyed until scanning is re-enabled or revoked. The rollout plan's Rollback names only the enforce Deny. Destroy an enabled app during rollback, observe the refusal, confirm scan revoke clears it with no engine.
Blocked on: discussion #6 for step 2, plus a second, unnamed ask of Tito — the console unit's env change and restart for step 4 are also tito/carlos-infrastructure work, and #6 covers only clamd.
Phase 3 — scan-on-demand, tasks 11–17
Sequence 11 → 12 → 13 → (14 → 15) → 16 → 17, with two changes:
- [x] The deferred refactor is done (
4e2bfb8).queuePermittedreturns(*Record, QueueRefusal, error), andPermanent()is §7.5's delete-vs-keep split in the one place both callers read it from. §5.1.1's concern is closed. - [x] The callback path validator is done (
4846e42), ahead of the door that accepts the field rather than four commits behind it. - [ ] The enqueue door itself: the route, the six refusals, the attestation hole, the Service-role gate, 429.
- Task 12 delivers
QueueIndex.Depth(), deferred out of task 10.
Phase 2's throughput numbers land in task 12's budget.
Phase 4 — deploy on-demand, migration seam first
This is what splitting actually costs, and draft 1 omitted it.
Deploying the sweep first means Tito runs on CARLOS_STORE_SCANNER. On-demand then upgrades under the §4.2.1 migration, whose seed fires only on the first tick while the deprecated variable is still set. Swap the variable in the same restart as the binary and scanning stops silently — warned per tick, but stopped. This code path has only ever run in tests. So:
- [ ] Deploy on-demand with
CARLOS_STORE_SCANNERstill set. - [ ] Confirm
carlos system antivirus statusreports the record seeded by migration. - [ ] Then swap to
CARLOS_CLAMD_ADDR— removingCARLOS_STORE_SCANNERandCARLOS_CLAMD_SOCKETin the same edit.cmd/console/main.gois fatal whenCARLOS_CLAMD_ADDRandCARLOS_CLAMD_SOCKETare both set and disagree, and rollout step 4 sets the socket explicitly. Leave both and the console fatals, and the adopt pass rolls back to the old binary.
Followed by: mint a Service credential, enqueue one object, watch it drain, receive one signed callback, verify the signature app-side. Then the failures only a deployment shows — a callback to a dead app, a redirect, a queue flooded past the per-tick limit, a disable with items in flight. Then Carloku, per rollout step 8. Then the skills repo, held to task 17 so the carlos store scan section is not shipped twice.
---
Not in this plan
- The services box and Gotenberg. Agreed in principle, unspecced. clamd goes on the Tito console box under #6's limits; moving it later is a change of address, which the client already supports deliberately (
f889f20). rastrillo/rastrillodiscussion #11 (notcarlos/platform#11) is theirs; scanning does not wait on it.- Anything on Carloku before Tito.
What would change this plan now
Not the timestamps — that is settled and pinned. The live ones:
- Real AWS not populating
s3:RequestObjectTagKeysfor POST policy either (#19). Then the signed-tag fix is not a hardening for self-hosters, it is the only thing standing between an app and a forged clean verdict on Tito, and it moves ahead of tasks 11–17 rather than beside them. x-amz-taggingon real AWS behaving like MinIO, by any of the other routes in Phase 0.5. Then the policy fixes are not belt-and-braces on Tito but the only thing between an app and a forged clean verdict — which changes the urgency of Phase 0.5, not the shape of Phase 1: the fixes ride the first half either way.- The
enforceDeny not denying. AstorePolicyJSONchange rewrites every existing store's policy on the next re-walk — a rollout event that must land before the first member grant, not after it. - clamd throughput far off "well under a second". Scan-on-demand's premise is a callback that arrives while the user is still there. If a 1 MB object takes ten seconds, task 12's budget is not a tuning parameter, it is a redesign — and that is worth knowing before task 11, which is the one argument draft 1 was reaching for and made about the wrong fact.