carlos / platform Public

The anti-forgery property is still open on one route, and it is the route an app actually uses. Found against a real server tonight, on worktree-store-scan-on-demand, before any deployment has scanning switched on.

This is the third finding in the same property. #18 has the first two.

What is open

A POST-policy form upload — the browser-direct pattern, where an app signs a policy and the browser POSTs a multipart form straight to S3 without the bytes ever passing through the app — carries its tags in a form field, not a header. Against MinIO RELEASE.2025-09-07T16-13-09Z, with the current app policy including the s3:RequestObjectTagKeys Deny that closes every other route:

a POST-policy upload carrying carlos-scan SUCCEEDED (204)

The control in the same test passes — a POST-policy upload without our tag keys also succeeds — so the route is permitted and working, and the Deny simply does not fire. MinIO does not populate s3:RequestObjectTagKeys for this handler.

An app can therefore mark its own uploads clean at the moment it stores them. The sweep finds a version-matching clean tag and never scans the object; carlos store scan status reports it clean, permanently.

This is the worst of the three, because it is not an exotic API call an attacker has to reach for. It is the standard way to take a file from a user — which is exactly the workload object-storage scanning exists for.

Why IAM cannot close it

s3:PutObjectTagging, x-amz-tagging on PutObject, CreateMultipartUpload and CopyObject with a REPLACE directive are all closed, and verified closed by mutation (remove the Deny, all four succeed; restore it, all four refuse). Those work because the gateway populates the condition key.

For POST policy it does not, and there is no other condition key that describes "this request is a form upload". A bucket policy uses the same condition keys and has the same gap. The tool does not reach.

Whether real AWS populates it here is unverified and worth checking — Tito's app buckets are AWS, so Tito may be unaffected. But self-hosters are in scope by decision, and the property has to hold on the gateway rather than on the paperwork. That was the lesson of #18 and it applies again.

What would close it

Make the tag unforgeable rather than unwritable. Sign it: a fourth tag carrying HMAC(deployment key, bucket|key|version|verdict|at), verified in readTag. A tag that does not verify is treated as no tag at all, so a forged one causes the object to be scanned rather than skipped — which is the safe direction.

That is gateway-independent, and it closes every route at once including the ones nobody has thought of yet. The IAM Denys stay as defence in depth, which is the right role for them.

It needs a key the console does not currently have. By deliberate design the console holds only CARLOS_SECRETS_PUBKEY, a public key — apiconfig.go says so explicitly: "the console holds no such key". So this needs a new deployment secret, which is the same machinery §6.3 introduces for callback signing (HMAC(master, sqid + "/" + app)), and that is a design decision rather than a patch. It should be decided once and used for both.

Why the test is not committed

AGENTS.md requires go test ./... clean before a commit lands, so the failing assertion is not on the branch — it would be the known-failing test this repo has just paid for once. The reproduction is below and drops straight back into TestDeliveredContractMinio the moment the fix lands. Nothing is lost but the red.

Gate

This must be closed before the first member grant, not before the first deploy. Steps 1–4 of the rollout 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 that verdict can be false.

The reproduction

Two subtests plus the helper. There is no POST-policy presigner in the AWS SDK for Go, which is a fair part of why this route goes untested; the SigV4 for it is a four-step HMAC chain and needs no dependency.

		// POST POLICY: THE ROUTE THAT MATTERS MOST, and the only one with no
		// SDK helper — which is part of why it goes untested.
		//
		// A browser-direct upload is the standard way an app takes a file from
		// a user without proxying the bytes: the app signs a policy, the
		// browser POSTs a multipart form straight to S3. It is a completely
		// separate handler from PutObject, it carries its tags in a form
		// FIELD rather than a header, and it is exactly the shape an app
		// storing user uploads has. If `s3:RequestObjectTagKeys` is not
		// populated here, an app marks its own uploads clean at the moment of
		// upload and nothing else in this test file would notice.
		t.Run("not by a POST-policy form upload", func(t *testing.T) {
			status, body := postPolicyUpload(t, endpoint, key, secret, bucket,
				"contract-test/post-tagged.txt",
				"<Tagging><TagSet><Tag><Key>carlos-scan</Key><Value>clean</Value></Tag></TagSet></Tagging>")
			if status >= 200 && status < 300 {
				t.Fatalf("a POST-policy upload carrying carlos-scan SUCCEEDED (%d) — an app can mark its own uploads clean at the moment it stores them:\n%s", status, body)
			}
			if status != 403 {
				t.Fatalf("POST-policy upload = %d, want 403 AccessDenied:\n%s", status, body)
			}
		})

		// The control, for the same reason as the two above: if POST-policy
		// uploads were refused outright — MinIO not supporting them, a
		// malformed policy on our side — the assertion above would pass while
		// testing nothing at all.
		t.Run("but a POST-policy upload without our tags is allowed", func(t *testing.T) {
			const k = "contract-test/post-plain.txt"
			status, body := postPolicyUpload(t, endpoint, key, secret, bucket, k,
				"<Tagging><TagSet><Tag><Key>team</Key><Value>payments</Value></Tag></TagSet></Tagging>")
			if status < 200 || status >= 300 {
				t.Fatalf("POST-policy upload without our tags = %d — the refusal above was about the ROUTE, not the tag, and proves nothing:\n%s", status, body)
			}
			_, _ = client.DeleteObject(ctx, &awss3.DeleteObjectInput{
				Bucket: aws.String(bucket), Key: aws.String(k),
			})
		})

// postPolicyUpload performs a browser-style direct upload: a base64 policy
// document signed with SigV4, POSTed as a multipart form. There is no helper
// for this in the AWS SDK for Go, which is a good part of why this route goes
// untested — and it is the route an app actually uses to take a file from a
// user without proxying the bytes.
//
// Hand-rolled rather than pulled in: this repo adds no third-party
// dependencies, and the whole of SigV4 for a POST policy is a four-step HMAC
// chain over a date, a region, a service and a terminator.
func postPolicyUpload(t *testing.T, endpoint, akid, secret, bucket, key, taggingXML string) (int, string) {
	t.Helper()

	const region = "us-east-1"
	now := time.Now().UTC()
	amzDate := now.Format("20060102T150405Z")
	scopeDate := now.Format("20060102")
	credential := akid + "/" + scopeDate + "/" + region + "/s3/aws4_request"

	// The policy the browser would be handed. `starts-with` on $tagging is
	// what an app would write if it wanted the browser to choose tags — which
	// is precisely the permissive shape worth testing.
	policy := fmt.Sprintf(`{"expiration":%q,"conditions":[
		{"bucket":%q},
		{"key":%q},
		{"x-amz-algorithm":"AWS4-HMAC-SHA256"},
		{"x-amz-credential":%q},
		{"x-amz-date":%q},
		["starts-with","$tagging",""]
	]}`, now.Add(15*time.Minute).Format(time.RFC3339), bucket, key, credential, amzDate)
	encoded := base64.StdEncoding.EncodeToString([]byte(policy))

	mac := func(k []byte, s string) []byte {
		h := hmac.New(sha256.New, k)
		h.Write([]byte(s))
		return h.Sum(nil)
	}
	signing := mac(mac(mac(mac([]byte("AWS4"+secret), scopeDate), region), "s3"), "aws4_request")
	sig := hex.EncodeToString(mac(signing, encoded))

	var buf bytes.Buffer
	w := multipart.NewWriter(&buf)
	for _, f := range [][2]string{
		{"key", key},
		{"x-amz-algorithm", "AWS4-HMAC-SHA256"},
		{"x-amz-credential", credential},
		{"x-amz-date", amzDate},
		{"policy", encoded},
		{"x-amz-signature", sig},
		{"tagging", taggingXML},
	} {
		if err := w.WriteField(f[0], f[1]); err != nil {
			t.Fatal(err)
		}
	}
	// The file part must come last: S3 ignores anything after it.
	part, err := w.CreateFormFile("file", "upload.txt")
	if err != nil {
		t.Fatal(err)
	}
	if _, err := part.Write([]byte("payload")); err != nil {
		t.Fatal(err)
	}
	if err := w.Close(); err != nil {
		t.Fatal(err)
	}

	// Path-style, matching the client the rest of this test builds.
	req, err := http.NewRequest(http.MethodPost, strings.TrimSuffix(endpoint, "/")+"/"+bucket, &buf)
	if err != nil {
		t.Fatal(err)
	}
	req.Header.Set("Content-Type", w.FormDataContentType())

	resp, err := http.DefaultClient.Do(req)
	if err != nil {
		t.Fatal(err)
	}
	defer resp.Body.Close()
	body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<16))
	if err != nil {
		t.Fatal(err)
	}
	return resp.StatusCode, string(body)
}

Imports it needs: crypto/hmac, crypto/sha256, encoding/base64, encoding/hex, fmt, mime/multipart, net/http, strings, time.