fix(uploads): first publish wins ownership of a content-addressed key

pkg/proxy/handlers/uploads.go

@@ -192,18 +192,25 @@ func (h *UploadsHandler) handlePut(w http.ResponseWriter, r *http.Request) {
 
 	// Identical bytes map to the same key, so a re-publish keeps the existing
 	// owner — or lack of one: ownerless legacy objects must not be adoptable.
-	uploader := uploaderFrom(r.Context())
+	uploader, exists := uploaderFrom(r.Context()), false
 	if info, err := h.client.StatObject(r.Context(), h.cfg.Bucket, key, minio.StatObjectOptions{}); err == nil {
-		uploader = info.UserMetadata[uploaderMetaKey]
+		uploader, exists = info.UserMetadata[uploaderMetaKey], true
 	} else if minio.ToErrorResponse(err).Code != "NoSuchKey" {
 		h.fail(w, http.StatusBadGateway, "checking object", err)
 		return
 	}
 
-	opts := minio.PutObjectOptions{ContentType: contentType}
+	// Single-part so R2 evaluates If-None-Match atomically (it does not for
+	// multipart); objects are capped well below R2's 5 GiB single-PUT limit.
+	opts := minio.PutObjectOptions{ContentType: contentType, DisableMultipart: true}
 	if uploader != "" {
 		opts.UserMetadata = map[string]string{uploaderMetaKey: uploader}
 	}
+	// A first publish only creates: of two concurrent first publishes, the
+	// later one must not overwrite the earlier one's ownership.
+	if !exists {
+		opts.SetMatchETagExcept("*")
+	}
 	// Types a browser would execute in the bucket origin (HTML, SVG, JS) are
 	// forced to download — defuses phishing/XSS on a public bucket while charts,
 	// PDFs and text render inline. The one exception is text/html when RenderHTML
@@ -213,7 +220,10 @@ func (h *UploadsHandler) handlePut(w http.ResponseWriter, r *http.Request) {
 		opts.ContentDisposition = fmt.Sprintf("attachment; filename=%q", name)
 	}
 
-	if _, err := h.client.PutObject(r.Context(), h.cfg.Bucket, key, tmp, size, opts); err != nil {
+	// Losing the create race is success: the key is content-addressed, so the
+	// winner stored these exact bytes under this name.
+	if _, err := h.client.PutObject(r.Context(), h.cfg.Bucket, key, tmp, size, opts); err != nil &&
+		(exists || minio.ToErrorResponse(err).StatusCode != http.StatusPreconditionFailed) {
 		h.fail(w, http.StatusBadGateway, "putting object", err)
 		return
 	}

pkg/proxy/handlers/uploads_test.go

@@ -207,10 +207,12 @@ func TestDeletableKeyCoversBothPrefixes(t *testing.T) {
 }
 
 // fakeS3 is a minimal path-style S3 backend: enough of HEAD/PUT/DELETE for the
-// uploads handler, keeping only each object's x-amz-meta-* headers.
+// uploads handler, keeping only each object's x-amz-meta-* headers. beforePut,
+// when set, runs once ahead of the next PUT to stage a concurrent writer.
 type fakeS3 struct {
-	mu      sync.Mutex
-	objects map[string]http.Header
+	mu        sync.Mutex
+	objects   map[string]http.Header
+	beforePut func(objects map[string]http.Header, path string)
 }
 
 func (f *fakeS3) ServeHTTP(w http.ResponseWriter, r *http.Request) {
@@ -236,6 +238,15 @@ func (f *fakeS3) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("ETag", `"etag"`)
 	case http.MethodPut:
 		_, _ = io.Copy(io.Discard, r.Body)
+		if f.beforePut != nil {
+			f.beforePut(f.objects, r.URL.Path)
+			f.beforePut = nil
+		}
+		if _, exists := f.objects[r.URL.Path]; exists && r.Header.Get("If-None-Match") == "*" {
+			w.WriteHeader(http.StatusPreconditionFailed)
+			_, _ = io.WriteString(w, `<Error><Code>PreconditionFailed</Code><Message>At least one of the pre-conditions you specified did not hold</Message></Error>`)
+			return
+		}
 		meta := http.Header{}
 		for k, v := range r.Header {
 			if strings.HasPrefix(k, "X-Amz-Meta-") {
@@ -358,3 +369,21 @@ func TestUploadsHandlerRepublishDoesNotAdoptOwnerlessObject(t *testing.T) {
 		t.Fatalf("delete after re-publishing ownerless object: status = %d, want 403", got)
 	}
 }
+
+func TestUploadsHandlerConcurrentFirstPublishKeepsFirstOwner(t *testing.T) {
+	h, backend := fakeS3UploadsHandler(t)
+
+	// alice's identical publish lands between mallory's stat and put.
+	backend.beforePut = func(objects map[string]http.Header, path string) {
+		objects[path] = http.Header{"X-Amz-Meta-Uploader": {"alice"}}
+	}
+	key := uploadAs(t, h, "mallory", "<p>shared</p>")
+
+	if got := deleteAs(h, "mallory", key); got != http.StatusForbidden {
+		t.Fatalf("delete as race loser: status = %d, want 403", got)
+	}
+
+	if got := deleteAs(h, "alice", key); got != http.StatusNoContent {
+		t.Fatalf("delete as race winner: status = %d, want 204", got)
+	}
+}