fix(s3frontend): conformance-align multipart validation; abort in-flight uploads on DeleteBucket

Three upstream conformance cases return to the pass tables with
behavior fixes instead of xfail rows:
- CompleteMultipartUpload: a part entry missing PartNumber or ETag is
  MalformedXML, and a part number below 1 is InvalidArgument
  (PartNumber), mirroring versitygw's posix backend; the ETag match is
  now mandatory and the >10000 special case folds into the membership
  check (InvalidPart).
- ListMultipartUploads: upload-id-marker validation mirrors upstream's
  MultipartUploadLister — the marker must be a valid UUID naming an
  upload of the first key group at/after the key marker (else
  InvalidArgument, upload-id-marker), and listing resumes past it.

DeleteBucket now implicitly aborts the bucket's open multipart sessions
(upstream's teardown never aborts them and expects the delete to
succeed), releasing their parked part blobs before the hilt space
delete. CompleteMultipartUpload_conditional_writes stays xfail: its
conditional matrix passes, but the implicit abort's /blob/abort goes
out proofless — hilt's per-operation grants carry blob.Abort for the
S3 Abort operation only. Promote once hilt's s3perm map grants
blob.Abort on bucket delete.

Validated: full local TestForgeVersity, 219 pass / 0 fail.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: frrist

go.mod

@@ -16,6 +16,7 @@ require (
 	github.com/fxamacker/cbor/v2 v2.9.2
 	github.com/go-jose/go-jose/v4 v4.1.4
 	github.com/gofiber/fiber/v3 v3.3.0
+	github.com/google/uuid v1.6.0
 	github.com/ipfs/go-block-format v0.2.3
 	github.com/ipfs/go-cid v0.6.1
 	github.com/ipfs/go-ipld-cbor v0.2.1
@@ -111,7 +112,6 @@ require (
 	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/google/go-cmp v0.7.0 // indirect
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
-	github.com/google/uuid v1.6.0 // indirect
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.29.0 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect

itest/versity_multipart_test.go

@@ -9,9 +9,7 @@ import (
 // Multipart groups of the S3 conformance partition, partitioned empirically
 // against the forge-mode stack (see the curation note in README.md). The
 // remaining xfail surface: part-level checksums (FIL-620), tagging/object-lock
-// /ACL on create (FIL-534/FIL-525), the UploadPartCopy group (FIL-586),
-// upstream error-code alignment (InvalidArgument where ingot returns a more
-// specific code), and conditional writes on Complete.
+// /ACL on create (FIL-534/FIL-525), and the UploadPartCopy group (FIL-586).
 
 var createMultipartPass = []forgeCase{
 	{name: "non_existing_bucket", fn: integration.CreateMultipartUpload_non_existing_bucket},
@@ -106,6 +104,7 @@ var listMultipartUploadsPass = []forgeCase{
 	{name: "max_uploads", fn: integration.ListMultipartUploads_max_uploads},
 	{name: "exceeding_max_uploads", fn: integration.ListMultipartUploads_exceeding_max_uploads},
 	{name: "ignore_upload_id_marker", fn: integration.ListMultipartUploads_ignore_upload_id_marker},
+	{name: "invalid_uploadId_marker", fn: integration.ListMultipartUploads_invalid_uploadId_marker},
 	{name: "keyMarker_not_from_list", fn: integration.ListMultipartUploads_keyMarker_not_from_list},
 	{name: "delimiter_truncated", fn: integration.ListMultipartUploads_delimiter_truncated},
 	{name: "prefix", fn: integration.ListMultipartUploads_prefix},
@@ -114,11 +113,7 @@ var listMultipartUploadsPass = []forgeCase{
 	{name: "with_checksums", fn: integration.ListMultipartUploads_with_checksums},
 }
 
-var listMultipartUploadsXFail = []forgeCase{
-	// Upstream expects InvalidArgument for a malformed upload-id-marker;
-	// ingot returns InvalidRequest.
-	{name: "invalid_uploadId_marker", fn: integration.ListMultipartUploads_invalid_uploadId_marker},
-}
+var listMultipartUploadsXFail = []forgeCase{}
 
 var abortMultipartPass = []forgeCase{
 	{name: "non_existing_bucket", fn: integration.AbortMultipartUpload_non_existing_bucket},
@@ -135,6 +130,8 @@ var completeMultipartPass = []forgeCase{
 	// upstream function name carries a typo (CompletedMultipartUpload_...).
 	{name: "non_existing_bucket", fn: integration.CompletedMultipartUpload_non_existing_bucket},
 	{name: "incorrect_part_number", fn: integration.CompleteMultipartUpload_incorrect_part_number},
+	{name: "missing_part_fields", fn: integration.CompleteMultipartUpload_missing_part_fields},
+	{name: "invalid_part_number", fn: integration.CompleteMultipartUpload_invalid_part_number},
 	{name: "default_content_type", fn: integration.CompleteMultipartUpload_default_content_type},
 	{name: "invalid_ETag", fn: integration.CompleteMultipartUpload_invalid_ETag},
 	{name: "small_upload_size", fn: integration.CompleteMultipartUpload_small_upload_size},
@@ -160,14 +157,13 @@ var completeMultipartPass = []forgeCase{
 // Part-level / composite-checksum verification is FIL-620;
 // racey_data_integrity additionally leans on atomic concurrent overwrites.
 var completeMultipartXFail = []forgeCase{
-	// A part entry missing its ETag is accepted (200 with a result body)
-	// where upstream expects an Error response.
-	{name: "missing_part_fields", fn: integration.CompleteMultipartUpload_missing_part_fields},
-	// Upstream expects InvalidArgument; ingot returns InvalidPartNumber.
-	{name: "invalid_part_number", fn: integration.CompleteMultipartUpload_invalid_part_number},
-	// Conditional headers on Complete are unenforced; the object written
-	// past the precondition then fails the case's bucket teardown (409
-	// BucketNotEmpty).
+	// The conditional matrix itself passes; the case then fails its bucket
+	// teardown: upstream's teardown never aborts in-flight uploads, so
+	// DeleteBucket implicitly aborts them (s3frontend/bucket.go), but the
+	// /blob/abort goes out proofless — hilt's per-operation proof grants
+	// carry blob.Abort for the S3 Abort operation only, not for
+	// DeleteBucket/UploadPart. Promote once hilt's s3perm map grants
+	// blob.Abort on bucket delete.
 	{name: "conditional_writes", fn: integration.CompleteMultipartUpload_conditional_writes},
 	{name: "invalid_checksum_part", fn: integration.CompleteMultipartUpload_invalid_checksum_part},
 	{name: "multiple_checksum_part", fn: integration.CompleteMultipartUpload_multiple_checksum_part},

s3frontend/bucket.go

@@ -222,6 +222,27 @@ func (b *Backend) DeleteBucket(ctx context.Context, name string) error {
 			}
 		}
 
+		// In-flight multipart uploads do not block deletion (the upstream
+		// conformance contract's teardown deletes buckets without aborting
+		// them): abort any open sessions, releasing their parked part blobs
+		// from the space, before asking hilt to delete the space — which
+		// refuses while the space still holds blob registrations.
+		sessions, err := b.multipart.ListSessions(ctx, name)
+		if err != nil {
+			return fmt.Errorf("s3frontend: delete bucket: list mp sessions: %w", err)
+		}
+		aborted := 0
+		for _, s := range sessions {
+			if s.State == registry.SessionOpen {
+				b.abortOpenSession(ctx, st.Space, s)
+				aborted++
+			}
+		}
+		if aborted > 0 {
+			b.logger.Info("delete bucket: aborted in-flight multipart sessions",
+				zap.String("bucket", name), zap.Int("aborted", aborted), zap.Int("total", len(sessions)))
+		}
+
 		req, ok := reqscope.Request(ctx)
 		if !ok {
 			return errors.New("s3frontend: delete bucket: no request in context")

s3frontend/multipart.go

@@ -17,8 +17,10 @@ import (
 	"github.com/fil-forge/versitygw/backend"
 	"github.com/fil-forge/versitygw/s3err"
 	"github.com/fil-forge/versitygw/s3response"
+	"github.com/google/uuid"
 	"github.com/ipfs/go-cid"
 	mh "github.com/multiformats/go-multihash"
+	"go.uber.org/zap"
 
 	msbucket "github.com/fil-forge/ingot/bucket"
 	"github.com/fil-forge/ingot/bucketop"
@@ -267,13 +269,17 @@ func (b *Backend) CompleteMultipartUpload(ctx context.Context, input *s3.Complet
 	etagHasher := md5.New()
 	prev := 0
 	for _, rp := range input.MultipartUpload.Parts {
-		if rp.PartNumber == nil {
-			return s3response.CompleteMultipartUploadResult{}, "", s3err.GetAPIError(s3err.ErrInvalidPart)
+		// A part entry missing either field is malformed XML; a part number
+		// below 1 is an InvalidArgument (both per the upstream posix
+		// backend). Out-of-range numbers fall through to the membership
+		// check (no stored part can match) and report InvalidPart.
+		if rp.PartNumber == nil || rp.ETag == nil {
+			return s3response.CompleteMultipartUploadResult{}, "", s3err.GetAPIError(s3err.ErrMalformedXML)
 		}
 		num := int(*rp.PartNumber)
-		if num < 1 || num > 10000 {
+		if num < 1 {
 			return s3response.CompleteMultipartUploadResult{}, "",
-				s3err.GetAPIError(s3err.ErrInvalidPartNumberRange)
+				s3err.GetInvalidArgumentErr(s3err.InvalidArgCompleteMpPartNumber, strconv.Itoa(num))
 		}
 		if num <= prev {
 			return s3response.CompleteMultipartUploadResult{}, "", s3err.GetAPIError(s3err.ErrInvalidPartOrder)
@@ -283,7 +289,7 @@ func (b *Backend) CompleteMultipartUpload(ctx context.Context, input *s3.Complet
 		if !ok {
 			return s3response.CompleteMultipartUploadResult{}, "", s3err.GetAPIError(s3err.ErrInvalidPart)
 		}
-		if rp.ETag != nil && !etagsEqual(*rp.ETag, hex.EncodeToString(sp.ETagMD5)) {
+		if !etagsEqual(*rp.ETag, hex.EncodeToString(sp.ETagMD5)) {
 			return s3response.CompleteMultipartUploadResult{}, "", s3err.GetAPIError(s3err.ErrInvalidPart)
 		}
 		requested = append(requested, sp)
@@ -438,6 +444,27 @@ func (b *Backend) AbortMultipartUpload(ctx context.Context, input *s3.AbortMulti
 	return nil
 }
 
+// abortOpenSession force-aborts an open multipart session exactly like a
+// client Abort: latch (losing gracefully to a concurrent Complete/Abort),
+// drop the session, release its parts' now-unreferenced blobs. Used by
+// DeleteBucket's implicit abort of in-flight uploads.
+func (b *Backend) abortOpenSession(ctx context.Context, space did.DID, sess registry.MultipartSession) {
+	won, err := b.multipart.LatchSession(ctx, sess.UploadID, registry.SessionOpen, registry.SessionAborting)
+	if err != nil || !won {
+		return
+	}
+	var digests [][]byte
+	if parts, err := b.multipart.ListParts(ctx, sess.UploadID); err == nil {
+		for _, p := range parts {
+			digests = append(digests, p.BlobDigests...)
+		}
+	}
+	if err := b.multipart.DeleteSession(ctx, sess.UploadID); err != nil {
+		return
+	}
+	b.cleanupPartBlobs(ctx, space, sess.UploadID, digests)
+}
+
 // cleanupPartBlobs removes spooled blobs that belonged to aborted, expired, or
 // superseded parts of uploadID — unless the blob is still referenced: by a
 // part of another in-flight session (content-addressed dedup), by a part still
@@ -490,7 +517,10 @@ func (b *Backend) cleanupPartBlobs(ctx context.Context, space did.DID, uploadID
 		if state == registry.IntentParked {
 			if park, err := b.parks.GetPark(ctx, d); err == nil {
 				if cause, err := cid.Cast(park.AddTask); err == nil {
-					_ = b.deferred.AbortBlob(ctx, space, mh.Multihash(d), cause)
+					if aerr := b.deferred.AbortBlob(ctx, space, mh.Multihash(d), cause); aerr != nil {
+						b.logger.Warn("abort parked blob failed; provider-side release deferred",
+							zap.String("digest", hex.EncodeToString(d)), zap.Error(aerr))
+					}
 				}
 				_ = b.parks.DeletePark(ctx, d)
 			}
@@ -728,43 +758,37 @@ func (b *Backend) ListMultipartUploads(ctx context.Context, input *s3.ListMultip
 		}
 	}
 
-	// Marker positioning. An upload-id marker is meaningful only alongside a
-	// key marker: when the key marker names a live key, the id must belong to
-	// one of that key's uploads (else InvalidArgument); when it doesn't (the
-	// marker key was completed/aborted meanwhile), the id positions within the
-	// keys ordered after it.
+	// Marker positioning, mirroring upstream's MultipartUploadLister: an
+	// upload-id marker is meaningful only alongside a key marker; it must be
+	// a valid UUID and must name an upload of the FIRST key group at or
+	// after the key marker (else InvalidArgument), and the listing resumes
+	// just past it.
 	start := 0
 	if keyMarker != "" {
 		if uploadIDMarker != "" {
-			keyLive := false
-			pos := -1
-			for i, s := range inflight {
-				if s.ObjectKey == keyMarker {
-					keyLive = true
-					if s.UploadID == uploadIDMarker {
-						pos = i
-					}
-				}
-			}
-			if keyLive && pos < 0 {
+			if _, err := uuid.Parse(uploadIDMarker); err != nil {
 				return s3response.ListMultipartUploadsResult{},
-					s3err.GetAPIError(s3err.ErrInvalidRequest)
+					s3err.GetInvalidArgumentErr(s3err.InvalidArgUploadIdMarker, uploadIDMarker)
 			}
-			if pos < 0 {
-				for i, s := range inflight {
-					if s.ObjectKey >= keyMarker && s.UploadID == uploadIDMarker {
-						pos = i
+			i := 0
+			for i < len(inflight) && inflight[i].ObjectKey < keyMarker {
+				i++
+			}
+			pos := -1
+			if i < len(inflight) {
+				firstKey := inflight[i].ObjectKey
+				for j := i; j < len(inflight) && inflight[j].ObjectKey == firstKey; j++ {
+					if inflight[j].UploadID == uploadIDMarker {
+						pos = j
 						break
 					}
 				}
 			}
-			if pos >= 0 {
-				start = pos + 1
-			} else {
-				for start < len(inflight) && inflight[start].ObjectKey <= keyMarker {
-					start++
-				}
+			if pos < 0 {
+				return s3response.ListMultipartUploadsResult{},
+					s3err.GetInvalidArgumentErr(s3err.InvalidArgUploadIdMarker, uploadIDMarker)
 			}
+			start = pos + 1
 		} else {
 			for start < len(inflight) && inflight[start].ObjectKey <= keyMarker {
 				start++