carlos / platform Public

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

Plain git — no account needed to clone.

Download

Download this file

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 store usage assertion (3e0c990).
  • [x] CI green: carlos/platform @ 6f166b3 passed — 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 with gh pr merge, then push main to an amadan remote. This repo has one remote, origin = https://amadan.net/carlos/platform; there is no GitHub, no amadan remote, and gh cannot see it. Landing is amadan branch describe + amadan branch merge, gated by amadan 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=clean succeeds (204) with the current policy, because the gateway does not populate s3:RequestObjectTagKeys for 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): CreateMultipartUpload with x-amz-tagging and CopyObject with a REPLACE directive. Both were open in the sense that nothing tested them; the s3:RequestObjectTagKeys Deny 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:DeleteObjectVersionTagging added to the first Deny (0fcba4b) — unreachable today, which is when a shape is cheapest to close.
  • [ ] x-amz-tagging against real AWS — and for all the routes above, not just the header on PutObject. AWS documents that one as requiring s3:PutObjectTagging; it is far less explicit about multipart and POST policy.
  • [ ] The fixture must be checked against the policy, not commented at. d848c57 says 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 script cats, and add a unit test comparing it semantically to storePolicyJSON("arn:aws:s3:::contract-bucket", nil).
  • [ ] The enforce Deny 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 object carlos-scan=infected, the fixture gains the ExistingObjectTag Deny, and the scoped GetObject must be AccessDenied. Then the variant that matters: a scoped CopyObject from the infected source with no tags, and a GET of 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 a storePolicyJSON change, 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:

  • 6fba2b3 adds the contract subtest for the tagging routes, but the sweep branch's .amadan/ci.d/30-minio-contract fixture has no Deny statements — the upload succeeds and the MinIO gate fails. That needs d848c57 too.
  • The sweep's internal/console/apiscan_test.go asserts !Contains(policy, "Effect":"Deny") after grant, which the unconditional anti-forgery Denys break. That needs d6142ac too.

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) and TestEveryClosedDoorRefusesAServiceCredential (closedDoors). Both #385/#386 and the scan branches touch docs/site/cli.md and internal/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 e7236b7 and 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-demand onto the new main.

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_BYTES through INSTREAM. This sets the per-app budget and task 12's per-tick drain cap. Draft 1 claimed systemd-cgtop would set CARLOS_CLAMD_MAX_BYTES; it does not — memory sets MemoryMax, MAX_BYTES mirrors StreamMaxLength, 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 status saying 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). queuePermitted returns (*Record, QueueRefusal, error), and Permanent() 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_SCANNER still set.
  • [ ] Confirm carlos system antivirus status reports the record seeded by migration.
  • [ ] Then swap to CARLOS_CLAMD_ADDR — removing CARLOS_STORE_SCANNER and CARLOS_CLAMD_SOCKET in the same edit. cmd/console/main.go is fatal when CARLOS_CLAMD_ADDR and CARLOS_CLAMD_SOCKET are 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/rastrillo discussion #11 (not carlos/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:RequestObjectTagKeys for 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-tagging on 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 enforce Deny not denying. A storePolicyJSON change 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.