From 53c33669566b08ee98af0a6618efd842e348bdad Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Thu, 5 Nov 2026 10:59:39 -0600 Subject: [PATCH 1/8] feat(storage): retention policies, bucket lock, object holds and retention, soft delete and Requester Pays --- README.md | 19 +- gcp/storage/buckets.go | 216 ++++++++- gcp/storage/http_test.go | 40 ++ gcp/storage/objects.go | 355 +++++++++++++-- gcp/storage/permissions.go | 71 ++- gcp/storage/protection.go | 280 ++++++++++++ gcp/storage/server.go | 162 +++++-- gcp/storage/softdelete.go | 241 +++++++++++ sdktest/gcp/storage/permissions_test.go | 23 + sdktest/gcp/storage/protection_test.go | 553 ++++++++++++++++++++++++ 10 files changed, 1869 insertions(+), 91 deletions(-) create mode 100644 gcp/storage/protection.go create mode 100644 gcp/storage/softdelete.go create mode 100644 sdktest/gcp/storage/protection_test.go diff --git a/README.md b/README.md index 5121b22..2cd0b9e 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ failure replays from its seed. | [`gcp/resourcemanager`](https://pkg.go.dev/github.com/parallelworks/squall/gcp/resourcemanager) | Cloud Resource Manager: projects (get by ID or number, getIamPolicy, setIamPolicy with etag checks, testIamPermissions answered from the server's access policy), seeded by the test. | | [`gcp/servicenetworking`](https://pkg.go.dev/github.com/parallelworks/squall/gcp/servicenetworking) | Service Networking: private service connections (create, list by network, patch with force, deleteConnection), checked against a `gcp/compute` server's networks and allocated ranges and peering the network there, and their long-running operations (get). | | [`gcp/serviceusage`](https://pkg.go.dev/github.com/parallelworks/squall/gcp/serviceusage) | Service Usage: services (enable, disable, get, list by state) in projects seeded by the test and named by number or ID, and their long-running operations (get). | -| [`gcp/storage`](https://pkg.go.dev/github.com/parallelworks/squall/gcp/storage) | Cloud Storage: buckets (insert, get, list, patch labels, versioning and CORS, delete) and objects (multipart and media uploads, list with versions, delete by generation). | +| [`gcp/storage`](https://pkg.go.dev/github.com/parallelworks/squall/gcp/storage) | Cloud Storage: buckets (insert, get, list, patch labels, versioning, CORS, retention policies, soft delete policies and Requester Pays, lock a retention policy, delete with soft delete, list and restore soft-deleted ones) and objects (multipart and media uploads, get, patch and update metadata, temporary, event-based and default holds, object retention, list with versions or soft-deleted generations, delete by generation, restore). | Coverage grows with use: each stand-in implements the methods someone needed, answers anything else with 404, and documents how faithful it is. Contributions @@ -392,14 +392,23 @@ For Cloud Storage: the caller's own bucket fails with 409, as the client's retry of a create that took effect does. - Bucket names and labels follow the API's rules; locations are upper-cased - but not checked. Fields the server does not model, such as lifecycle rules - or retention, are rejected with 400. A patch with a field set to null - clears it, and a label set to null is deleted. + but not checked. Fields the server does not model, such as lifecycle rules, + are rejected with 400. A patch with a field set to null clears it, and a + label set to null is deleted. +- Retention policies, holds and object retention refuse deletes and + overwrites with Cloud Storage's 403s; a locked policy can only grow. Soft + delete is on by default for 7 days: deleted and replaced generations are + listed with `softDeleted=true` and restored until their `hardDeleteTime` on + the server's clock (`WithClock`), and deleted buckets are kept the same + way. Requester Pays is enforced only under `WithPolicy`. `SetObjectHolds`, + `SetObjectRetention`, `SetRetentionPolicy` and `PutSoftDeletedObject` set + up protected or soft-deleted state directly. - Objects keep their size, MD5 and CRC32C, which an upload's checksums must match. With versioning on, overwrites and deletes keep the old generation as noncurrent, and a bucket with any generation left cannot be deleted. Page tokens are positions, so a listing survives deletes between pages. - Resumable uploads, reads, IAM policies and HMAC keys are not supported. + Resumable uploads, media reads, IAM policies and HMAC keys are not + supported. - A fault rule's `Resource` is the path below `/storage/v1/`, such as `b/my-bucket/o/my-object`, and for an insert the bucket or object it makes. diff --git a/gcp/storage/buckets.go b/gcp/storage/buckets.go index a050065..ad3ff4d 100644 --- a/gcp/storage/buckets.go +++ b/gcp/storage/buckets.go @@ -12,6 +12,7 @@ import ( "net/http" "net/netip" "slices" + "strconv" "strings" "time" "unicode" @@ -21,6 +22,10 @@ import ( const ( maxLabels = 64 maxLabelLength = 63 + + maxRetentionPeriod = 3_155_760_000 + minSoftDelete = 7 * 24 * 60 * 60 + maxSoftDelete = 90 * 24 * 60 * 60 ) // Bucket is a bucket, in the API's JSON shape. @@ -43,6 +48,14 @@ type Bucket struct { CustomPlacementConfig *CustomPlacementConfig `json:"customPlacementConfig,omitzero"` Encryption *Encryption `json:"encryption,omitzero"` RPO string `json:"rpo,omitzero"` + Generation int64 `json:"generation,string"` + RetentionPolicy *RetentionPolicy `json:"retentionPolicy,omitzero"` + DefaultEventBasedHold bool `json:"defaultEventBasedHold,omitzero"` + ObjectRetention *BucketObjectRetention `json:"objectRetention,omitzero"` + SoftDeletePolicy *SoftDeletePolicy `json:"softDeletePolicy,omitzero"` + Billing *Billing `json:"billing,omitzero"` + SoftDeleteTime string `json:"softDeleteTime,omitzero"` + HardDeleteTime string `json:"hardDeleteTime,omitzero"` // Project is the ID of the project the bucket is in; it is not in the JSON. Project string `json:"-"` @@ -63,10 +76,39 @@ type Encryption struct { DefaultKmsKeyName string `json:"defaultKmsKeyName,omitzero"` } +// RetentionPolicy is the minimum age an object in a bucket must reach before it can be deleted or replaced. +type RetentionPolicy struct { + RetentionPeriod int64 `json:"retentionPeriod,string"` + EffectiveTime string `json:"effectiveTime"` + IsLocked bool `json:"isLocked,omitzero"` +} + +// SoftDeletePolicy is how long a bucket keeps what is deleted from it; zero keeps nothing. +type SoftDeletePolicy struct { + RetentionDurationSeconds int64 `json:"retentionDurationSeconds,string"` + EffectiveTime string `json:"effectiveTime,omitzero"` +} + +// BucketObjectRetention says whether objects in a bucket take their own retention, with Mode "Enabled". +type BucketObjectRetention struct { + Mode string `json:"mode"` +} + +// Billing is a bucket's billing setting. +type Billing struct { + RequesterPays bool `json:"requesterPays"` +} + type bucket struct { Bucket // objects holds each name's generations, oldest first; the last is live unless it has TimeDeleted. objects map[string][]*Object + // trash holds soft-deleted generations, in the order they were deleted. + trash []*Object + // nextPurge is when the first of trash goes for good. + nextPurge time.Time + // hardDelete is when a soft-deleted bucket goes for good. + hardDelete time.Time } func (b *bucket) clone() Bucket { @@ -75,6 +117,18 @@ func (b *bucket) clone() Bucket { if b.Versioning != nil { c.Versioning = new(*b.Versioning) } + if b.RetentionPolicy != nil { + c.RetentionPolicy = new(*b.RetentionPolicy) + } + if b.ObjectRetention != nil { + c.ObjectRetention = new(*b.ObjectRetention) + } + if b.SoftDeletePolicy != nil { + c.SoftDeletePolicy = new(*b.SoftDeletePolicy) + } + if b.Billing != nil { + c.Billing = new(*b.Billing) + } c.CORS = slices.Clone(b.CORS) if b.CustomPlacementConfig != nil { c.CustomPlacementConfig = &CustomPlacementConfig{DataLocations: slices.Clone(b.CustomPlacementConfig.DataLocations)} @@ -87,6 +141,16 @@ func (b *bucket) clone() Bucket { func (b *bucket) versioned() bool { return b.Versioning != nil && b.Versioning.Enabled } +// softDeleteDuration is how long b keeps what is deleted from it. +func (b *bucket) softDeleteDuration() time.Duration { + if b.SoftDeletePolicy == nil { + return 0 + } + return time.Duration(b.SoftDeletePolicy.RetentionDurationSeconds) * time.Second +} + +func (b *bucket) requesterPays() bool { return b.Billing != nil && b.Billing.RequesterPays } + func (b *bucket) empty() bool { return len(b.objects) == 0 } // touch records a change to b's metadata. The caller holds s.mu. @@ -190,6 +254,7 @@ func forbidden(permission string) *apiError { // lookup returns the named bucket, or the error a call needing permission fails with. The caller holds s.mu. func (s *Server) lookup(name, permission string) (*bucket, *apiError) { if b, ok := s.buckets[name]; ok { + s.purgeTrash(b) return b, nil } if s.foreign[name] { @@ -213,6 +278,33 @@ func normalCORS(cors json.RawMessage) (json.RawMessage, *apiError) { return slices.Clone(cors), nil } +// int64Value is an int64 the API takes as a JSON string or number. +type int64Value int64 + +func (v *int64Value) UnmarshalJSON(data []byte) error { + text := string(data) + if text == "null" { + return nil + } + if unquoted, err := strconv.Unquote(text); err == nil { + text = unquoted + } + n, err := strconv.ParseInt(text, 10, 64) + if err != nil { + return fmt.Errorf("invalid value for an int64 field: %s", data) + } + *v = int64Value(n) + return nil +} + +type retentionPolicyInput struct { + RetentionPeriod *int64Value `json:"retentionPeriod"` +} + +type softDeletePolicyInput struct { + RetentionDurationSeconds *int64Value `json:"retentionDurationSeconds"` +} + // bucketInput is what buckets.insert and buckets.patch take. type bucketInput struct { Name string `json:"name"` @@ -224,6 +316,10 @@ type bucketInput struct { CustomPlacementConfig *CustomPlacementConfig `json:"customPlacementConfig"` Encryption *Encryption `json:"encryption"` RPO string `json:"rpo"` + RetentionPolicy *retentionPolicyInput `json:"retentionPolicy"` + DefaultEventBasedHold *bool `json:"defaultEventBasedHold"` + SoftDeletePolicy *softDeletePolicyInput `json:"softDeletePolicy"` + Billing *Billing `json:"billing"` // Lifecycle is accepted without rules, since the client sends it empty. Lifecycle *struct { Rule []json.RawMessage `json:"rule"` @@ -255,10 +351,9 @@ func (s *Server) insertBucket(r *http.Request) (any, *apiError) { if err := checkACLs(r); err != nil { return nil, err } - if retention, err := boolParam(r, "enableObjectRetention"); err != nil { + objectRetention, err := boolParam(r, "enableObjectRetention") + if err != nil { return nil, err - } else if retention { - return nil, invalid("squall: object retention is not supported") } body, err := readBody(r, maxBody) if err != nil { @@ -280,6 +375,25 @@ func (s *Server) insertBucket(r *http.Request) (any, *apiError) { b := Bucket{ Name: in.Name, Location: in.Location, StorageClass: in.StorageClass, Labels: labels, Versioning: in.Versioning, CORS: in.CORS, CustomPlacementConfig: in.CustomPlacementConfig, Encryption: in.Encryption, RPO: in.RPO, Project: project, + Billing: in.Billing, + } + if in.DefaultEventBasedHold != nil { + b.DefaultEventBasedHold = *in.DefaultEventBasedHold + } + if objectRetention { + b.ObjectRetention = &BucketObjectRetention{Mode: "Enabled"} + } + if p := in.RetentionPolicy; p != nil { + if p.RetentionPeriod == nil { + return nil, badRetentionPeriod() + } + b.RetentionPolicy = &RetentionPolicy{RetentionPeriod: int64(*p.RetentionPeriod)} + } + if p := in.SoftDeletePolicy; p != nil { + if p.RetentionDurationSeconds == nil { + return nil, badSoftDelete() + } + b.SoftDeletePolicy = &SoftDeletePolicy{RetentionDurationSeconds: int64(*p.RetentionDurationSeconds)} } s.mu.Lock() defer s.mu.Unlock() @@ -313,6 +427,15 @@ func (s *Server) store(b Bucket) (*bucket, *apiError) { if c := b.CustomPlacementConfig; c != nil && len(c.DataLocations) != 2 { return nil, invalid("Invalid argument: a custom dual-region takes exactly two data locations.") } + if p := b.RetentionPolicy; p != nil && !validRetentionPeriod(p.RetentionPeriod) { + return nil, badRetentionPeriod() + } + if p := b.SoftDeletePolicy; p != nil && !validSoftDelete(p.RetentionDurationSeconds) { + return nil, badSoftDelete() + } + if o := b.ObjectRetention; o != nil && o.Mode != "Enabled" { + return nil, invalid("Invalid value for: %s is not a valid value", o.Mode) + } if s.foreign[b.Name] { return nil, &apiError{ Status: http.StatusConflict, Reason: "conflict", @@ -325,10 +448,22 @@ func (s *Server) store(b Bucket) (*bucket, *apiError) { Message: "You already own this bucket. Please select another name.", } } - now := s.now().UTC().Format(timeFormat) + t, now := s.timestamp() b.Kind, b.ID, b.SelfLink = "storage#bucket", b.Name, apiBase+"b/"+b.Name b.ProjectNumber = projectNumber(b.Project) b.TimeCreated, b.Updated, b.Metageneration, b.Etag = now, now, 1, etag(1) + b.Generation = s.nextGeneration(t) + b.SoftDeleteTime, b.HardDeleteTime = "", "" + if b.RetentionPolicy != nil { + b.RetentionPolicy.EffectiveTime = cmp.Or(b.RetentionPolicy.EffectiveTime, now) + } + if b.SoftDeletePolicy == nil { + b.SoftDeletePolicy = &SoftDeletePolicy{RetentionDurationSeconds: minSoftDelete} + } + b.SoftDeletePolicy.EffectiveTime = "" + if b.SoftDeletePolicy.RetentionDurationSeconds > 0 { + b.SoftDeletePolicy.EffectiveTime = now + } b.Location = strings.ToUpper(cmp.Or(b.Location, "US")) b.LocationType = locationType(b.Location, b.CustomPlacementConfig) if len(b.Labels) == 0 { @@ -341,8 +476,19 @@ func (s *Server) store(b Bucket) (*bucket, *apiError) { } func (s *Server) getBucket(r *http.Request) (any, *apiError) { + soft, gen, err := softDeletedGeneration(r) + if err != nil { + return nil, err + } s.mu.Lock() defer s.mu.Unlock() + if soft { + b := s.softDeletedBucket(r.PathValue("bucket"), gen) + if b == nil { + return nil, notFoundBucket() + } + return b.clone(), nil + } b, err := s.lookup(r.PathValue("bucket"), "storage.buckets.get") if err != nil { return nil, err @@ -369,7 +515,8 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { return nil, err } if in.Name != "" || in.Location != "" || in.CustomPlacementConfig != nil || in.RPO != "" { - return nil, invalid("squall: a patch may change only labels, versioning, cors, storageClass and encryption") + return nil, invalid("squall: a patch may change only labels, versioning, cors, storageClass, encryption, " + + "retentionPolicy, defaultEventBasedHold, softDeletePolicy and billing") } cors, err := normalCORS(in.CORS) if err != nil { @@ -380,6 +527,9 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { return nil, invalid("Invalid JSON payload received: %v", err) } cleared := func(field string) bool { return string(fields[field]) == "null" } + if cleared("softDeletePolicy") { + return nil, invalid("squall: softDeletePolicy cannot be null; a retentionDurationSeconds of 0 turns soft delete off") + } s.mu.Lock() defer s.mu.Unlock() b, err := s.lookup(r.PathValue("bucket"), "storage.buckets.update") @@ -410,10 +560,28 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { return nil, invalid("Invalid argument: storage class '%s' is not valid.", class) } } + retention, err := s.nextRetentionPolicy(b, in.RetentionPolicy, cleared("retentionPolicy")) + if err != nil { + return nil, err + } + softDelete, err := s.nextSoftDeletePolicy(b, in.SoftDeletePolicy) + if err != nil { + return nil, err + } if len(labels) == 0 { labels = nil } - b.Labels, b.StorageClass = labels, class + b.Labels, b.StorageClass, b.RetentionPolicy, b.SoftDeletePolicy = labels, class, retention, softDelete + if in.DefaultEventBasedHold != nil { + b.DefaultEventBasedHold = *in.DefaultEventBasedHold + } else if cleared("defaultEventBasedHold") { + b.DefaultEventBasedHold = false + } + if in.Billing != nil { + b.Billing = new(*in.Billing) + } else if cleared("billing") { + b.Billing = nil + } if in.Versioning != nil { b.Versioning = new(*in.Versioning) } else if cleared("versioning") { @@ -446,6 +614,7 @@ func (s *Server) deleteBucket(r *http.Request) (any, *apiError) { return nil, &apiError{Status: http.StatusConflict, Reason: "conflict", Message: "The bucket you tried to delete is not empty."} } delete(s.buckets, name) + s.softDeleteBucket(b) return nil, nil } @@ -463,16 +632,31 @@ func (s *Server) listBuckets(r *http.Request) (any, *apiError) { if _, err := boolParam(r, "returnPartialSuccess"); err != nil { return nil, err } + soft, err := boolParam(r, "softDeleted") + if err != nil { + return nil, err + } prefix := r.URL.Query().Get("prefix") s.mu.Lock() defer s.mu.Unlock() var all []Bucket - for _, name := range slices.Sorted(maps.Keys(s.buckets)) { - if b := s.buckets[name]; b.Project == project && strings.HasPrefix(name, prefix) { - all = append(all, b.clone()) + key := func(b Bucket) string { return b.Name } + if soft { + s.purgeBuckets() + for _, b := range s.softDeletedBuckets() { + if b.Project == project && strings.HasPrefix(b.Name, prefix) { + all = append(all, b) + } + } + key = func(b Bucket) string { return fmt.Sprintf("%s\x00%020d", b.Name, b.Generation) } + } else { + for _, name := range slices.Sorted(maps.Keys(s.buckets)) { + if b := s.buckets[name]; b.Project == project && strings.HasPrefix(name, prefix) { + all = append(all, b.clone()) + } } } - items, next, err := page(r, all, func(b Bucket) string { return b.Name }) + items, next, err := page(r, all, key) if err != nil { return nil, err } @@ -504,7 +688,9 @@ func (s *Server) LookupBucket(name string) (Bucket, bool) { return b.clone(), true } -// PutBucket creates b in its Project from the fields buckets.insert takes, and returns it as stored. +// PutBucket creates b in its Project from the fields buckets.insert takes, and returns it as stored. A +// RetentionPolicy may be locked already. With no SoftDeletePolicy, the bucket keeps deletions for 7 days, as +// Cloud Storage's default is. func (s *Server) PutBucket(b Bucket) (Bucket, error) { if b.Project == "" { return Bucket{}, errors.New("squall: a bucket needs a project") @@ -512,6 +698,8 @@ func (s *Server) PutBucket(b Bucket) (Bucket, error) { in := Bucket{ Name: b.Name, Location: b.Location, StorageClass: b.StorageClass, Labels: b.Labels, Versioning: b.Versioning, CORS: b.CORS, CustomPlacementConfig: b.CustomPlacementConfig, Encryption: b.Encryption, RPO: b.RPO, Project: b.Project, + RetentionPolicy: b.RetentionPolicy, DefaultEventBasedHold: b.DefaultEventBasedHold, ObjectRetention: b.ObjectRetention, + SoftDeletePolicy: b.SoftDeletePolicy, Billing: b.Billing, } s.mu.Lock() defer s.mu.Unlock() @@ -541,3 +729,9 @@ func (s *Server) timestamp() (time.Time, string) { t := s.now().UTC() return t, t.Format(timeFormat) } + +// nextGeneration returns a generation number above every earlier one, from t where it can. The caller holds s.mu. +func (s *Server) nextGeneration(t time.Time) int64 { + s.lastGeneration = max(s.lastGeneration+1, t.UnixMicro()) + return s.lastGeneration +} diff --git a/gcp/storage/http_test.go b/gcp/storage/http_test.go index d41eeaa..8895875 100644 --- a/gcp/storage/http_test.go +++ b/gcp/storage/http_test.go @@ -3,6 +3,7 @@ package storage_test import ( "bytes" "encoding/json" + "fmt" "net/http" "net/http/httptest" "strings" @@ -121,6 +122,41 @@ func TestPatchClearsNullFields(t *testing.T) { } } +func TestRetentionAndRestoreRoutes(t *testing.T) { + s := storage.New() + if code, body := do(t, s, http.MethodPost, "/storage/v1/b?project=p", "application/json", `{"name":"routes","retentionPolicy":{"retentionPeriod":"60"}}`); code != http.StatusOK || + !strings.Contains(body, `"retentionPolicy":{"retentionPeriod":"60","effectiveTime":"`) || !strings.Contains(body, `"softDeletePolicy":{"retentionDurationSeconds":"604800"`) { + t.Fatalf("insert = %d %s", code, body) + } + code, body := do(t, s, http.MethodPost, "/storage/v1/b/routes/lockRetentionPolicy", "", "") + if code != http.StatusBadRequest || !strings.Contains(body, `"message":"Required parameter: ifMetagenerationMatch"`) { + t.Fatalf("lock without a metageneration = %d %s", code, body) + } + if code, body := do(t, s, http.MethodPost, "/storage/v1/b/routes/lockRetentionPolicy?ifMetagenerationMatch=1", "", ""); code != http.StatusOK || !strings.Contains(body, `"isLocked":true`) { + t.Fatalf("lock = %d %s", code, body) + } + if _, err := s.PutObject("routes", "a/restore", []byte("x")); err != nil { + t.Fatal(err) + } + gen := s.Objects("routes")[0].Generation + if _, err := s.SetRetentionPolicy("routes", nil); err != nil { + t.Fatal(err) + } + if code, body := do(t, s, http.MethodDelete, "/storage/v1/b/routes/o/a%2Frestore", "", ""); code != http.StatusNoContent { + t.Fatalf("delete = %d %s", code, body) + } + target := fmt.Sprintf("/storage/v1/b/routes/o/a%%2Frestore/restore?generation=%d", gen) + if code, body := do(t, s, http.MethodPost, target, "", ""); code != http.StatusOK || !strings.Contains(body, `"name":"a/restore"`) { + t.Fatalf("restore = %d %s", code, body) + } + if code, body := do(t, s, http.MethodPost, "/storage/v1/b/routes/o/a%2Frestore/compose", "application/json", "{}"); code != http.StatusNotFound { + t.Fatalf("compose = %d %s", code, body) + } + if code, body := do(t, s, http.MethodPost, "/storage/v1/b/routes/o/a/restore", "", ""); code != http.StatusBadRequest || !strings.Contains(body, `"reason":"required"`) { + t.Fatalf("restore without a generation = %d %s", code, body) + } +} + func FuzzServeHTTP(f *testing.F) { multipartBody := "--x\r\nContent-Type: application/json\r\n\r\n{\"name\":\"o\"}\r\n--x\r\nContent-Type: text/plain\r\n\r\nhi\r\n--x--\r\n" for _, seed := range []struct{ method, target, contentType, body string }{ @@ -130,6 +166,10 @@ func FuzzServeHTTP(f *testing.F) { {http.MethodPost, "/upload/storage/v1/b/fuzz/o?uploadType=multipart", "multipart/related; boundary=x", multipartBody}, {http.MethodPost, "/upload/storage/v1/b/fuzz/o?uploadType=media&name=a%2F..%2Fb&ifGenerationMatch=0", "text/plain", "x"}, {http.MethodDelete, "/storage/v1/b/fuzz/o/k?generation=1", "", ""}, + {http.MethodPatch, "/storage/v1/b/fuzz/o/k?overrideUnlockedRetention=true", "application/json", `{"temporaryHold":true,"retention":null,"metadata":{"a":null}}`}, + {http.MethodPost, "/storage/v1/b/fuzz/o/k/restore?generation=1", "", ""}, + {http.MethodPost, "/storage/v1/b/fuzz/lockRetentionPolicy?ifMetagenerationMatch=1", "", ""}, + {http.MethodGet, "/storage/v1/b?project=p&softDeleted=true", "", ""}, } { f.Add(seed.method, seed.target, seed.contentType, []byte(seed.body)) } diff --git a/gcp/storage/objects.go b/gcp/storage/objects.go index a4caafd..d4af684 100644 --- a/gcp/storage/objects.go +++ b/gcp/storage/objects.go @@ -6,6 +6,7 @@ import ( "crypto/md5" //nolint:gosec // the API reports MD5 digests; nothing here relies on them for security "encoding/base64" "encoding/binary" + "encoding/json" "errors" "fmt" "hash/crc32" @@ -18,10 +19,12 @@ import ( "slices" "strconv" "strings" + "time" "unicode/utf8" ) -// Object is one generation of an object, in the API's JSON shape; a noncurrent one has TimeDeleted. +// Object is one generation of an object, in the API's JSON shape; a noncurrent one has TimeDeleted, and a +// soft-deleted one SoftDeleteTime. type Object struct { Kind string `json:"kind"` ID string `json:"id"` @@ -43,14 +46,57 @@ type Object struct { TimeStorageClassUpdated string `json:"timeStorageClassUpdated"` TimeDeleted string `json:"timeDeleted,omitzero"` Metadata map[string]string `json:"metadata,omitzero"` + TemporaryHold *bool `json:"temporaryHold,omitzero"` + EventBasedHold *bool `json:"eventBasedHold,omitzero"` + RetentionExpirationTime string `json:"retentionExpirationTime,omitzero"` + Retention *Retention `json:"retention,omitzero"` + SoftDeleteTime string `json:"softDeleteTime,omitzero"` + HardDeleteTime string `json:"hardDeleteTime,omitzero"` + + // retentionStart is when the object's time in its bucket began, for a retention policy. + retentionStart time.Time + hardDelete time.Time } func (o *Object) clone() Object { c := *o c.Metadata = maps.Clone(o.Metadata) + if o.TemporaryHold != nil { + c.TemporaryHold = new(*o.TemporaryHold) + } + if o.EventBasedHold != nil { + c.EventBasedHold = new(*o.EventBasedHold) + } + if o.Retention != nil { + c.Retention = new(*o.Retention) + } + return c +} + +// view is o as the API shows it in b. The caller holds s.mu. +func (s *Server) view(b *bucket, o *Object) Object { + c := o.clone() + if p := b.RetentionPolicy; p != nil && o.SoftDeleteTime == "" && !isSet(o.EventBasedHold) { + c.RetentionExpirationTime = o.retentionStart.Add(time.Duration(p.RetentionPeriod) * time.Second).UTC().Format(timeFormat) + } return c } +// setHolds sets o's holds, starting its retention period again when an event-based hold is released. The caller holds s.mu. +func (s *Server) setHolds(o *Object, temporary, eventBased *bool) { + if isSet(o.EventBasedHold) && !isSet(eventBased) { + o.retentionStart = s.now() + } + o.TemporaryHold, o.EventBasedHold = temporary, eventBased +} + +// touchObject records a change to o's metadata. The caller holds s.mu. +func (s *Server) touchObject(o *Object) { + o.Metageneration++ + _, o.Updated = s.timestamp() + o.Etag = etag(o.Generation, o.Metageneration) +} + func (o *Object) live() bool { return o.TimeDeleted == "" } // liveObject returns name's live generation in b, or nil. The caller holds s.mu. @@ -62,6 +108,16 @@ func (b *bucket) liveObject(name string) *Object { return gens[len(gens)-1] } +// generation returns generation gen of name in b, live or noncurrent, or nil. +func (b *bucket) generation(name string, gen int64) *Object { + for _, o := range b.objects[name] { + if o.Generation == gen { + return o + } + } + return nil +} + // retire makes name's live generation noncurrent if b is versioned, or removes it. The caller holds s.mu. func (s *Server) retire(b *bucket, name string) { live := b.liveObject(name) @@ -73,17 +129,26 @@ func (s *Server) retire(b *bucket, name string) { live.Updated = live.TimeDeleted return } - b.drop(name, live.Generation) + s.discard(b, live) } -// drop removes generation gen of name. The caller holds s.mu. -func (b *bucket) drop(name string, gen int64) { - gens := slices.DeleteFunc(b.objects[name], func(o *Object) bool { return o.Generation == gen }) +// discard removes generation o from b, keeping it soft-deleted if b keeps deletions. The caller holds s.mu. +func (s *Server) discard(b *bucket, o *Object) { + gens := slices.DeleteFunc(b.objects[o.Name], func(g *Object) bool { return g == o }) if len(gens) == 0 { - delete(b.objects, name) + delete(b.objects, o.Name) + } else { + b.objects[o.Name] = gens + } + keep := b.softDeleteDuration() + if keep == 0 { return } - b.objects[name] = gens + t, now := s.timestamp() + o.SoftDeleteTime, o.hardDelete = now, t.Add(keep) + o.HardDeleteTime = o.hardDelete.Format(timeFormat) + b.trash = append(b.trash, o) + b.keepUntil(o.hardDelete) } func noSuchObject(bucket, name string) *apiError { @@ -104,6 +169,9 @@ type objectInput struct { Metadata map[string]string `json:"metadata"` CRC32C string `json:"crc32c"` MD5Hash string `json:"md5Hash"` + TemporaryHold *bool `json:"temporaryHold"` + EventBasedHold *bool `json:"eventBasedHold"` + Retention *Retention `json:"retention"` } // splitMultipart returns the metadata and media of a multipart/related upload. @@ -180,7 +248,14 @@ func (s *Server) insertObject(r *http.Request) (any, *apiError) { if err != nil { return nil, err } - return o.clone(), nil + return s.view(s.buckets[bucketName], o), nil +} + +// checksums returns data's MD5 and CRC32C, as the API encodes them. +func checksums(data []byte) (md5Hash, crc string) { + sum := md5.Sum(data) //nolint:gosec // a reported digest, not a security check + crc32c := binary.BigEndian.AppendUint32(nil, crc32.Checksum(data, crc32.MakeTable(crc32.Castagnoli))) + return base64.StdEncoding.EncodeToString(sum[:]), base64.StdEncoding.EncodeToString(crc32c) } // writeObject stores data as in.Name's newest generation once precondition passes. The caller holds s.mu. @@ -191,9 +266,7 @@ func (s *Server) writeObject(bucketName string, in objectInput, data []byte, pre if !validObjectName(in.Name) { return nil, invalid("The specified object name is not valid.") } - sum := md5.Sum(data) //nolint:gosec // a reported digest, not a security check - md5Hash := base64.StdEncoding.EncodeToString(sum[:]) - crc := base64.StdEncoding.EncodeToString(binary.BigEndian.AppendUint32(nil, crc32.Checksum(data, crc32.MakeTable(crc32.Castagnoli)))) + md5Hash, crc := checksums(data) if in.CRC32C != "" && in.CRC32C != crc { return nil, invalid("Provided CRC32C \"%s\" doesn't match calculated CRC32C \"%s\".", in.CRC32C, crc) } @@ -213,22 +286,49 @@ func (s *Server) writeObject(bucketName string, in objectInput, data []byte, pre return nil, err } } - s.retire(b, in.Name) + eventHold := in.EventBasedHold + if eventHold == nil && b.DefaultEventBasedHold { + eventHold = new(true) + } + retention, err := s.checkRetention(b, nil, in.Retention, false, isSet(eventHold)) + if err != nil { + return nil, err + } + if err := s.replaceLive(b, in.Name); err != nil { + return nil, err + } + o := s.newGeneration(b, in.Name, b.StorageClass) + o.ContentType, o.ContentEncoding = cmp.Or(in.ContentType, "application/octet-stream"), in.ContentEncoding + o.Size, o.MD5Hash, o.CRC32C, o.Metadata = uint64(len(data)), md5Hash, crc, maps.Clone(in.Metadata) + o.TemporaryHold, o.EventBasedHold, o.Retention = in.TemporaryHold, eventHold, retention + return o, nil +} + +// replaceLive retires name's live generation in b, unless what protects it refuses. The caller holds s.mu. +func (s *Server) replaceLive(b *bucket, name string) *apiError { + if live := b.liveObject(name); live != nil { + if err := s.guard(b, live, b.versioned()); err != nil { + return err + } + } + s.retire(b, name) + return nil +} + +// newGeneration adds a live generation of name to b, to be filled in. The caller holds s.mu. +func (s *Server) newGeneration(b *bucket, name, storageClass string) *Object { t, now := s.timestamp() - s.lastGeneration = max(s.lastGeneration+1, t.UnixMicro()) - gen := s.lastGeneration - escaped := url.PathEscape(in.Name) + gen := s.nextGeneration(t) + escaped := url.PathEscape(name) o := &Object{ - Kind: "storage#object", ID: fmt.Sprintf("%s/%s/%d", bucketName, in.Name, gen), - SelfLink: apiBase + "b/" + bucketName + "/o/" + escaped, - MediaLink: downloadBase + "b/" + bucketName + "/o/" + escaped + "?generation=" + strconv.FormatInt(gen, 10) + "&alt=media", - Name: in.Name, Bucket: bucketName, Generation: gen, Metageneration: 1, - ContentType: cmp.Or(in.ContentType, "application/octet-stream"), ContentEncoding: in.ContentEncoding, - StorageClass: b.StorageClass, Size: uint64(len(data)), MD5Hash: md5Hash, CRC32C: crc, Etag: etag(gen, 1), - TimeCreated: now, Updated: now, TimeStorageClassUpdated: now, Metadata: maps.Clone(in.Metadata), - } - b.objects[in.Name] = append(b.objects[in.Name], o) - return o, nil + Kind: "storage#object", ID: fmt.Sprintf("%s/%s/%d", b.Name, name, gen), + SelfLink: apiBase + "b/" + b.Name + "/o/" + escaped, + MediaLink: downloadBase + "b/" + b.Name + "/o/" + escaped + "?generation=" + strconv.FormatInt(gen, 10) + "&alt=media", + Name: name, Bucket: b.Name, Generation: gen, Metageneration: 1, StorageClass: storageClass, Etag: etag(gen, 1), + TimeCreated: now, Updated: now, TimeStorageClassUpdated: now, retentionStart: t, + } + b.objects[name] = append(b.objects[name], o) + return o } func (s *Server) deleteObject(r *http.Request) (any, *apiError) { @@ -243,15 +343,9 @@ func (s *Server) deleteObject(r *http.Request) (any, *apiError) { if err != nil { return nil, err } - var target *Object + target := b.liveObject(name) if byGeneration { - for _, o := range b.objects[name] { - if o.Generation == gen { - target = o - } - } - } else { - target = b.liveObject(name) + target = b.generation(name, gen) } if target == nil { return nil, noSuchObject(bucketName, name) @@ -259,14 +353,172 @@ func (s *Server) deleteObject(r *http.Request) (any, *apiError) { if err := checkMatch(r, "ifGenerationMatch", "ifGenerationNotMatch", target.Generation); err != nil { return nil, err } + if err := s.guard(b, target, !byGeneration && b.versioned()); err != nil { + return nil, err + } if byGeneration { - b.drop(name, gen) + s.discard(b, target) } else { s.retire(b, name) } return nil, nil } +// target returns the generation a call on an object names, live unless it has a generation parameter. +func (s *Server) target(r *http.Request, permission string) (*bucket, *Object, *apiError) { + gen, byGeneration, err := int64Param(r, "generation") + if err != nil { + return nil, nil, err + } + bucketName, name := r.PathValue("bucket"), r.PathValue("object") + b, err := s.lookup(bucketName, permission) + if err != nil { + return nil, nil, err + } + o := b.liveObject(name) + if byGeneration { + o = b.generation(name, gen) + } + if o == nil { + return nil, nil, noSuchObject(bucketName, name) + } + if err := checkMatch(r, "ifGenerationMatch", "ifGenerationNotMatch", o.Generation); err != nil { + return nil, nil, err + } + if err := checkMatch(r, "ifMetagenerationMatch", "ifMetagenerationNotMatch", o.Metageneration); err != nil { + return nil, nil, err + } + return b, o, nil +} + +func (s *Server) getObject(r *http.Request) (any, *apiError) { + soft, gen, err := softDeletedGeneration(r) + if err != nil { + return nil, err + } + s.mu.Lock() + defer s.mu.Unlock() + if !soft { + b, o, err := s.target(r, "storage.objects.get") + if err != nil { + return nil, err + } + return s.view(b, o), nil + } + bucketName, name := r.PathValue("bucket"), r.PathValue("object") + b, err := s.lookup(bucketName, "storage.objects.get") + if err != nil { + return nil, err + } + o := b.trashed(name, gen) + if o == nil { + return nil, noSuchObject(bucketName, name) + } + if err := checkMatch(r, "ifMetagenerationMatch", "ifMetagenerationNotMatch", o.Metageneration); err != nil { + return nil, err + } + return o.clone(), nil +} + +// objectPatch is what objects.patch and objects.update take. +type objectPatch struct { + Bucket string `json:"bucket"` + Name string `json:"name"` + ContentType *string `json:"contentType"` + ContentEncoding *string `json:"contentEncoding"` + Metadata map[string]*string `json:"metadata"` + TemporaryHold *bool `json:"temporaryHold"` + EventBasedHold *bool `json:"eventBasedHold"` + Retention *Retention `json:"retention"` +} + +func (s *Server) patchObject(r *http.Request) (any, *apiError) { return s.changeObject(r, false) } + +func (s *Server) updateObject(r *http.Request) (any, *apiError) { return s.changeObject(r, true) } + +// changeObject applies objects.patch, or with replace objects.update, which sets every writable field. +func (s *Server) changeObject(r *http.Request, replace bool) (any, *apiError) { + if err := checkACLs(r); err != nil { + return nil, err + } + override, err := boolParam(r, "overrideUnlockedRetention") + if err != nil { + return nil, err + } + body, err := readBody(r, maxBody) + if err != nil { + return nil, err + } + var in objectPatch + if err := unmarshalStrict(body, &in); err != nil { + return nil, err + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(body, &fields); err != nil { + return nil, invalid("Invalid JSON payload received: %v", err) + } + cleared := func(field string) bool { return string(fields[field]) == "null" } + if !replace && (cleared("temporaryHold") || cleared("eventBasedHold")) { + return nil, invalid("squall: a hold cannot be null; set it to false to release it") + } + s.mu.Lock() + defer s.mu.Unlock() + b, o, err := s.target(r, "storage.objects.update") + if err != nil { + return nil, err + } + if in.Bucket != "" && in.Bucket != b.Name || in.Name != "" && in.Name != o.Name { + return nil, invalid("squall: an object's bucket and name cannot be changed") + } + temporary, eventBased, metadata := o.TemporaryHold, o.EventBasedHold, maps.Clone(o.Metadata) + contentType, contentEncoding := o.ContentType, o.ContentEncoding + if replace { + temporary, eventBased, metadata = in.TemporaryHold, in.EventBasedHold, nil + contentType, contentEncoding = "application/octet-stream", "" + } + if in.TemporaryHold != nil { + temporary = in.TemporaryHold + } + if in.EventBasedHold != nil { + eventBased = in.EventBasedHold + } + if in.ContentType != nil { + contentType = cmp.Or(*in.ContentType, "application/octet-stream") + } else if cleared("contentType") { + contentType = "application/octet-stream" + } + if in.ContentEncoding != nil { + contentEncoding = *in.ContentEncoding + } else if cleared("contentEncoding") { + contentEncoding = "" + } + if metadata == nil || cleared("metadata") { + metadata = map[string]string{} + } + for k, v := range in.Metadata { + if v == nil { + delete(metadata, k) + } else { + metadata[k] = *v + } + } + retention := o.Retention + if replace || in.Retention != nil || cleared("retention") { + if retention, err = s.checkRetention(b, o.Retention, in.Retention, override, isSet(eventBased)); err != nil { + return nil, err + } + } else if retention != nil && isSet(eventBased) { + return nil, retentionConflict() + } + if len(metadata) == 0 { + metadata = nil + } + s.setHolds(o, temporary, eventBased) + o.Retention, o.Metadata, o.ContentType, o.ContentEncoding = retention, metadata, contentType, contentEncoding + s.touchObject(o) + return s.view(b, o), nil +} + type objectList struct { Kind string `json:"kind"` Items []Object `json:"items,omitzero"` @@ -285,7 +537,11 @@ func (s *Server) listObjects(r *http.Request) (any, *apiError) { return nil, invalid("squall: objects.list does not support %s", p) } } - for _, p := range []string{"includeTrailingDelimiter", "includeFoldersAsPrefixes", "softDeleted"} { + soft, err := boolParam(r, "softDeleted") + if err != nil { + return nil, err + } + for _, p := range []string{"includeTrailingDelimiter", "includeFoldersAsPrefixes"} { if on, err := boolParam(r, p); err != nil { return nil, err } else if on { @@ -303,16 +559,31 @@ func (s *Server) listObjects(r *http.Request) (any, *apiError) { if err != nil { return nil, err } + listed := func(name string) bool { + return strings.HasPrefix(name, prefix) && name >= start && (end == "" || name < end) + } var all []Object - for _, name := range slices.Sorted(maps.Keys(b.objects)) { - if !strings.HasPrefix(name, prefix) || name < start || end != "" && name >= end { - continue + if soft { + if b.softDeleteDuration() == 0 { + return nil, invalid("Soft delete policy is required to list soft-deleted versions") } - for _, o := range b.objects[name] { - if versions || o.live() { + for _, o := range b.trash { + if listed(o.Name) { all = append(all, o.clone()) } } + slices.SortFunc(all, func(a, b Object) int { return strings.Compare(versionKey(a), versionKey(b)) }) + } else { + for _, name := range slices.Sorted(maps.Keys(b.objects)) { + if !listed(name) { + continue + } + for _, o := range b.objects[name] { + if versions || o.live() { + all = append(all, s.view(b, o)) + } + } + } } items, next, err := page(r, all, versionKey) if err != nil { @@ -332,7 +603,7 @@ func (s *Server) Objects(bucketName string) []Object { var out []Object for _, name := range slices.Sorted(maps.Keys(b.objects)) { for _, o := range b.objects[name] { - out = append(out, o.clone()) + out = append(out, s.view(b, o)) } } return out @@ -346,5 +617,5 @@ func (s *Server) PutObject(bucketName, name string, data []byte) (Object, error) if err != nil { return Object{}, err } - return o.clone(), nil + return s.view(s.buckets[bucketName], o), nil } diff --git a/gcp/storage/permissions.go b/gcp/storage/permissions.go index 6532e81..51cd36b 100644 --- a/gcp/storage/permissions.go +++ b/gcp/storage/permissions.go @@ -1,8 +1,10 @@ package storage import ( + "encoding/json" "fmt" "net/http" + "slices" "strings" "github.com/parallelworks/squall/access" @@ -18,16 +20,29 @@ func WithPolicy(p *access.Policy) Option { // basePermissions is what Cloud Storage checks for each method, from each // method's "Required permissions" in the JSON API reference. var basePermissions = map[string][]string{ - "storage.buckets.insert": {"storage.buckets.create"}, - "storage.buckets.list": {"storage.buckets.list"}, - "storage.buckets.get": {"storage.buckets.get"}, - "storage.buckets.patch": {"storage.buckets.update"}, - "storage.buckets.delete": {"storage.buckets.delete"}, - "storage.objects.list": {"storage.objects.list"}, - "storage.objects.delete": {"storage.objects.delete"}, - "storage.objects.insert": {"storage.objects.create"}, + "storage.buckets.insert": {"storage.buckets.create"}, + "storage.buckets.list": {"storage.buckets.list"}, + "storage.buckets.get": {"storage.buckets.get"}, + "storage.buckets.patch": {"storage.buckets.update"}, + "storage.buckets.delete": {"storage.buckets.delete"}, + "storage.buckets.lockRetentionPolicy": {"storage.buckets.update"}, + "storage.buckets.restore": {"storage.buckets.restore"}, + "storage.objects.list": {"storage.objects.list"}, + "storage.objects.get": {"storage.objects.get"}, + "storage.objects.patch": {"storage.objects.update"}, + "storage.objects.update": {"storage.objects.update"}, + "storage.objects.delete": {"storage.objects.delete"}, + "storage.objects.insert": {"storage.objects.create"}, + "storage.objects.restore": {"storage.objects.restore", "storage.objects.create"}, } +// The permissions a call on a Requester Pays bucket needs: one naming a user project to bill needs +// serviceUsage, and one naming none createBillingAssignment on the bucket's project. +const ( + serviceUsage = "serviceusage.services.use" + createBillingAssignment = "resourcemanager.projects.createBillingAssignment" +) + // needs returns every permission a call to method on resource needs. func (s *Server) needs(method, resource string, r *http.Request) []string { perms := append([]string(nil), basePermissions[method]...) @@ -44,10 +59,47 @@ func (s *Server) needs(method, resource string, r *http.Request) []string { add(q.Get("predefinedAcl") != "" || q.Get("predefinedDefaultObjectAcl") != "", "storage.buckets.setIamPolicy") case "storage.objects.insert": add(s.exists(resource), "storage.objects.delete") + add(uploadSetsRetention(r), "storage.objects.setRetention") + case "storage.objects.patch", "storage.objects.update": + add(bodySetsRetention(peekBody(r, maxBody)), "storage.objects.setRetention") + add(q.Get("overrideUnlockedRetention") == "true", "storage.objects.overrideUnlockedRetention") + case "storage.objects.restore": + add(s.exists(resource), "storage.objects.delete") + } + if method != "storage.buckets.insert" && method != "storage.buckets.list" && s.requesterPays(resource) { + add(q.Get("userProject") != "", serviceUsage) + add(q.Get("userProject") == "", createBillingAssignment) } return perms } +// bodySetsRetention reports whether an object's metadata, as JSON, includes a retention. +func bodySetsRetention(meta []byte) bool { + var fields map[string]json.RawMessage + if json.Unmarshal(meta, &fields) != nil { + return false + } + retention, ok := fields["retention"] + return ok && string(retention) != "null" +} + +func uploadSetsRetention(r *http.Request) bool { + if r.URL.Query().Get("uploadType") != "multipart" { + return false + } + meta, _, err := splitMultipart(r.Header.Get("Content-Type"), peekBody(r, maxObject+maxBody)) + return err == nil && bodySetsRetention(meta) +} + +// requesterPays reports whether resource, as "b/" or a path below it, is in a Requester Pays bucket. +func (s *Server) requesterPays(resource string) bool { + name, _, _ := strings.Cut(strings.TrimPrefix(resource, "b/"), "/") + s.mu.Lock() + defer s.mu.Unlock() + b := s.buckets[name] + return b != nil && b.requesterPays() +} + // exists reports whether resource, as "b//o/", names a live object. func (s *Server) exists(resource string) bool { bucketName, name, ok := strings.Cut(strings.TrimPrefix(resource, "b/"), "/o/") @@ -61,6 +113,9 @@ func (s *Server) exists(resource string) bool { } func permissionDenied(missing []string, resource string) *apiError { + if slices.Contains(missing, createBillingAssignment) { + return &apiError{Status: http.StatusBadRequest, Reason: "required", Message: "Bucket is a requester pays bucket but no user project provided."} + } perm, _, _ := strings.Cut(missing[0], "|") return &apiError{ Status: http.StatusForbidden, diff --git a/gcp/storage/protection.go b/gcp/storage/protection.go new file mode 100644 index 0000000..c8879d8 --- /dev/null +++ b/gcp/storage/protection.go @@ -0,0 +1,280 @@ +package storage + +import ( + "cmp" + "errors" + "fmt" + "net/http" + "time" +) + +// Retention is an object's own retention, with Mode "Unlocked" or "Locked". +type Retention struct { + Mode string `json:"mode"` + RetainUntilTime string `json:"retainUntilTime"` +} + +const untilFormat = "2006-01-02T15:04:05.000000Z07:00" + +func validRetentionPeriod(seconds int64) bool { return seconds > 0 && seconds < maxRetentionPeriod } + +func validSoftDelete(seconds int64) bool { + return seconds == 0 || seconds >= minSoftDelete && seconds <= maxSoftDelete +} + +func badRetentionPeriod() *apiError { + return invalid("Retention policy must have a retention period greater than 0 and less than 100 years.") +} + +func badSoftDelete() *apiError { + return invalid("Soft delete policy must have a retention duration between 7 days and 90 days.") +} + +// nextRetentionPolicy is b's retention policy after a patch sets it to in, or removes it. The caller holds s.mu. +func (s *Server) nextRetentionPolicy(b *bucket, in *retentionPolicyInput, removed bool) (*RetentionPolicy, *apiError) { + old := b.RetentionPolicy + locked := old != nil && old.IsLocked + if removed { + if locked { + return nil, &apiError{ + Status: http.StatusForbidden, Reason: "retentionPolicyNotMet", + Message: fmt.Sprintf("Bucket '%s' has a locked Retention Policy which cannot be removed.", b.Name), + } + } + return nil, nil + } + if in == nil { + return old, nil + } + if in.RetentionPeriod == nil || !validRetentionPeriod(int64(*in.RetentionPeriod)) { + return nil, badRetentionPeriod() + } + period := int64(*in.RetentionPeriod) + if locked && period < old.RetentionPeriod { + return nil, &apiError{ + Status: http.StatusForbidden, Reason: "forbidden", + Message: fmt.Sprintf("Cannot reduce retention duration of a locked Retention Policy for bucket '%s'.", b.Name), + } + } + if old != nil && old.RetentionPeriod == period { + return new(*old), nil + } + _, now := s.timestamp() + return &RetentionPolicy{RetentionPeriod: period, EffectiveTime: now, IsLocked: locked}, nil +} + +// nextSoftDeletePolicy is b's soft delete policy after a patch sets it to in. The caller holds s.mu. +func (s *Server) nextSoftDeletePolicy(b *bucket, in *softDeletePolicyInput) (*SoftDeletePolicy, *apiError) { + if in == nil { + return b.SoftDeletePolicy, nil + } + if in.RetentionDurationSeconds == nil || !validSoftDelete(int64(*in.RetentionDurationSeconds)) { + return nil, badSoftDelete() + } + seconds := int64(*in.RetentionDurationSeconds) + if old := b.SoftDeletePolicy; old != nil && old.RetentionDurationSeconds == seconds { + return new(*old), nil + } + p := &SoftDeletePolicy{RetentionDurationSeconds: seconds} + if seconds > 0 { + _, p.EffectiveTime = s.timestamp() + } + return p, nil +} + +func (s *Server) lockRetentionPolicy(r *http.Request) (any, *apiError) { + metageneration, ok, err := int64Param(r, "ifMetagenerationMatch") + if err != nil { + return nil, err + } + if !ok { + return nil, required("ifMetagenerationMatch") + } + s.mu.Lock() + defer s.mu.Unlock() + b, err := s.lookup(r.PathValue("bucket"), "storage.buckets.update") + if err != nil { + return nil, err + } + if metageneration != b.Metageneration { + return nil, conditionNotMet() + } + if b.RetentionPolicy == nil || b.RetentionPolicy.IsLocked { + return nil, invalid("Bucket '%s' does not have an unlocked retention policy.", b.Name) + } + b.RetentionPolicy.IsLocked = true + s.touch(b) + return b.clone(), nil +} + +func isSet(hold *bool) bool { return hold != nil && *hold } + +// retainedUntil is when o may leave b: the later of the end of b's retention policy and o's own retention. +func (b *bucket) retainedUntil(o *Object) time.Time { + var until time.Time + if p := b.RetentionPolicy; p != nil && !isSet(o.EventBasedHold) { + until = o.retentionStart.Add(time.Duration(p.RetentionPeriod) * time.Second) + } + if o.Retention != nil { + if t, err := time.Parse(time.RFC3339Nano, o.Retention.RetainUntilTime); err == nil && t.After(until) { + until = t + } + } + return until +} + +// guard returns the error deleting or replacing o fails with, or with archive, making it noncurrent, which +// only a hold prevents. The caller holds s.mu. +func (s *Server) guard(b *bucket, o *Object, archive bool) *apiError { + kind := "" + switch { + case isSet(o.TemporaryHold): + kind = "Temporary" + case isSet(o.EventBasedHold): + kind = "Event-Based" + } + if kind != "" { + return &apiError{ + Status: http.StatusForbidden, Reason: "forbidden", + Message: fmt.Sprintf("Object '%s/%s' is under active %s hold and cannot be deleted, overwritten or archived until hold is removed.", b.Name, o.Name, kind), + } + } + if archive { + return nil + } + if until := b.retainedUntil(o); s.now().Before(until) { + return &apiError{ + Status: http.StatusForbidden, Reason: "retentionPolicyNotMet", + Message: fmt.Sprintf("Object '%s/%s' is subject to bucket's retention policy or object retention and cannot be deleted or overwritten until %s", + b.Name, o.Name, until.UTC().Format(untilFormat)), + } + } + return nil +} + +func retentionConflict() *apiError { + return invalid("An object cannot have both an event-based hold and a retention configuration.") +} + +// checkRetention reports whether an object in b whose retention is old may take next, nil removing it. +// The caller holds s.mu. +func (s *Server) checkRetention(b *bucket, old, next *Retention, override, eventHold bool) (*Retention, *apiError) { + var until time.Time + if next != nil { + if b.ObjectRetention == nil { + return nil, &apiError{ + Status: http.StatusForbidden, Reason: "forbidden", + Message: "The caller does not have storage.objects.setRetention access to the Google Cloud Storage object.", + } + } + switch { + case next.Mode == "": + return nil, invalid("Missing retention mode. Both mode and retain until time are required.") + case next.RetainUntilTime == "": + return nil, invalid("Missing retain until time. Both mode and retain until time are required.") + case next.Mode != "Unlocked" && next.Mode != "Locked": + return nil, invalid("Invalid value for: %s is not a valid value", next.Mode) + } + t, err := time.Parse(time.RFC3339Nano, next.RetainUntilTime) + if err != nil { + return nil, invalid("Invalid value for: %s is not a valid value", next.RetainUntilTime) + } + if t.Before(s.now()) { + return nil, invalid("Retain until time cannot be in the past.") + } + if eventHold { + return nil, retentionConflict() + } + until = t + next = &Retention{Mode: next.Mode, RetainUntilTime: t.UTC().Format(timeFormat)} + } + if old != nil { + oldUntil, _ := time.Parse(time.RFC3339Nano, old.RetainUntilTime) + shortened := next == nil || until.Before(oldUntil) + switch { + case old.Mode == "Locked" && (shortened || next.Mode != "Locked"): + return nil, &apiError{ + Status: http.StatusForbidden, Reason: "forbidden", + Message: "The locked object retention cannot be removed. Its retention mode cannot be changed and its retention period cannot be shortened.", + } + case old.Mode != "Locked" && shortened && !override: + return nil, &apiError{ + Status: http.StatusForbidden, Reason: "forbidden", + Message: "The unlocked object retention cannot be removed and its retention period cannot be shortened without overriding unlocked retention intent and permission.", + } + } + } + return next, nil +} + +// SetRetentionPolicy sets the named bucket's retention policy, locked or not, or with nil removes it, without the +// checks a patch makes. An empty EffectiveTime is now. +func (s *Server) SetRetentionPolicy(bucketName string, p *RetentionPolicy) (Bucket, error) { + s.mu.Lock() + defer s.mu.Unlock() + b, ok := s.buckets[bucketName] + if !ok { + return Bucket{}, notFoundBucket() + } + if p != nil { + if !validRetentionPeriod(p.RetentionPeriod) { + return Bucket{}, badRetentionPeriod() + } + _, now := s.timestamp() + p = &RetentionPolicy{RetentionPeriod: p.RetentionPeriod, EffectiveTime: cmp.Or(p.EffectiveTime, now), IsLocked: p.IsLocked} + } + b.RetentionPolicy = p + s.touch(b) + return b.clone(), nil +} + +// SetObjectHolds sets the holds on name's live generation in the named bucket. Releasing an event-based hold +// starts the object's retention period again, as Cloud Storage does. +func (s *Server) SetObjectHolds(bucketName, name string, temporary, eventBased bool) (Object, error) { + s.mu.Lock() + defer s.mu.Unlock() + b, o, err := s.liveForHelper(bucketName, name) + if err != nil { + return Object{}, err + } + if eventBased && o.Retention != nil { + return Object{}, retentionConflict() + } + s.setHolds(o, &temporary, &eventBased) + s.touchObject(o) + return s.view(b, o), nil +} + +// SetObjectRetention sets the retention of name's live generation in the named bucket, or with nil removes it, +// without the checks a patch makes, so a test can start from locked retention or one already expired. +func (s *Server) SetObjectRetention(bucketName, name string, r *Retention) (Object, error) { + s.mu.Lock() + defer s.mu.Unlock() + b, o, err := s.liveForHelper(bucketName, name) + if err != nil { + return Object{}, err + } + if r != nil { + t, err := time.Parse(time.RFC3339Nano, r.RetainUntilTime) + if err != nil || r.Mode != "Unlocked" && r.Mode != "Locked" { + return Object{}, fmt.Errorf("squall: invalid retention %+v", *r) + } + r = &Retention{Mode: r.Mode, RetainUntilTime: t.UTC().Format(timeFormat)} + } + o.Retention = r + s.touchObject(o) + return s.view(b, o), nil +} + +// liveForHelper returns the named bucket and name's live generation in it. The caller holds s.mu. +func (s *Server) liveForHelper(bucketName, name string) (*bucket, *Object, error) { + b, ok := s.buckets[bucketName] + if !ok { + return nil, nil, notFoundBucket() + } + o := b.liveObject(name) + if o == nil { + return nil, nil, errors.New("squall: " + noSuchObject(bucketName, name).Message) + } + return b, o, nil +} diff --git a/gcp/storage/server.go b/gcp/storage/server.go index cddc27e..9838fc1 100644 --- a/gcp/storage/server.go +++ b/gcp/storage/server.go @@ -11,8 +11,9 @@ // // It covers: // -// - buckets: insert, get, list, patch, delete; -// - objects: insert by multipart or media upload, list, delete. +// - buckets: insert, get, list, patch, delete, lockRetentionPolicy, restore; +// - objects: insert by multipart or media upload, get (metadata only), +// patch, update, list, delete, restore. // // Other methods answer 404. Where it differs from Cloud Storage: // @@ -31,29 +32,102 @@ // and ASIA1 and any location with a customPlacementConfig dual-regions, // and the rest regions. Project numbers are derived from project IDs. // - Bucket fields other than name, location, storageClass, labels, -// versioning, cors, customPlacementConfig, encryption and rpo are -// rejected with 400, as are ACLs other than private and projectPrivate. -// cors, encryption and rpo are recorded, not enforced. There are no IAM -// policies on buckets or objects, and no soft delete, retention, lifecycle, autoclass or hierarchical namespace. -// buckets.patch has patch semantics: it merges labels, deleting one set -// to null, and clears labels, versioning, cors or encryption set to null. +// versioning, cors, customPlacementConfig, encryption, rpo, +// retentionPolicy, defaultEventBasedHold, softDeletePolicy and billing +// are rejected with 400, as are ACLs other than private and +// projectPrivate. cors, encryption and rpo are recorded, not enforced. +// There are no IAM policies on buckets or objects, and no lifecycle, +// autoclass, hardDeletePause or hierarchical namespace. buckets.patch has +// patch semantics: it merges labels, deleting one set to null, and clears +// labels, versioning, cors, encryption, retentionPolicy, +// defaultEventBasedHold or billing set to null; a null softDeletePolicy +// is rejected with 400. Every bucket reports a generation (inferred). // - Objects record their size, MD5 and CRC32C, not their data; a crc32c or // md5Hash in the upload's metadata must match the data. Resumable -// uploads, reads, copies, composes and object ACLs are not supported. +// uploads, media reads, copies, composes and object ACLs are not +// supported. // Uploads take up to 16 MiB. // - With versioning on, writing or deleting an object makes its live // generation noncurrent, and objects.list with versions=true lists those // too, ordered by name and then generation. Deleting a bucket fails with // 409 conflict while any generation, live or noncurrent, remains. -// - objects.list takes prefix, startOffset, endOffset, versions, pageToken -// and maxResults; a delimiter, matchGlob, filter, softDeleted, +// - A retention policy's retentionPeriod is greater than 0 and less than +// 100 years, and its effectiveTime is when it was set or its period last +// changed (inferred). Each object reports retentionExpirationTime, its +// time in the bucket plus the period. Deleting or replacing an object +// before then, or before its own retention's retainUntilTime, fails with +// 403 retentionPolicyNotMet and Cloud Storage's message; the time in it +// is in UTC, where Cloud Storage writes Pacific time. Making a live +// object noncurrent in a versioned bucket is allowed, as documented. +// lockRetentionPolicy needs ifMetagenerationMatch (400 required without +// it, 412 conditionNotMet on a mismatch), fails with 400 invalid without +// an unlocked policy, and changes the bucket's metageneration +// (inferred). A locked policy cannot be removed (403 +// retentionPolicyNotMet) or shortened (403 forbidden); the bucket lock +// page says these fail with 400, so the statuses, reasons and messages +// here are inferred. Only an emptied bucket can be deleted, so a locked +// one needs no further check. +// - temporaryHold and eventBasedHold are set by an upload's metadata, +// objects.patch and objects.update, and defaultEventBasedHold puts an +// event-based hold on each new object, a restored one included +// (inferred). A held generation, live or noncurrent, cannot be deleted, +// replaced or made noncurrent: 403 forbidden, with Cloud Storage's +// message. While an event-based hold is on, an object reports no +// retentionExpirationTime, and releasing it starts the object's time in +// the bucket again. objects.update replaces every writable field, so +// leaving a hold out releases it. +// - enableObjectRetention on buckets.insert reports objectRetention.mode +// Enabled, and objects in such a bucket take retention with mode +// Unlocked or Locked and a future retainUntilTime, with Cloud Storage's +// 400s for a missing or invalid one. Unlocked retention can be shortened +// or removed only with overrideUnlockedRetention=true, and locked +// retention only extended, each refused with 403 forbidden. Retention in +// a bucket without objectRetention fails with 403 forbidden naming +// storage.objects.setRetention, and retention beside an event-based hold +// with 400 invalid (both inferred). The 100-year cap on retainUntilTime +// is not checked. +// - softDeletePolicy.retentionDurationSeconds is 0, which turns soft delete +// off, or 7 to 90 days, and a new bucket keeps deletions for 7 days. A +// generation that leaves the bucket, by a delete, by being replaced in a +// bucket without versioning or by a delete of a noncurrent generation, +// is kept soft-deleted with softDeleteTime and hardDeleteTime, on the +// policy at the time. objects.list with softDeleted=true lists them (400 +// invalid with soft delete off), objects.get with softDeleted=true and a +// generation reads one, and objects.restore makes a copy the live +// generation, in STANDARD, retiring the live one as an overwrite would; +// the soft-deleted generation stays until its hardDeleteTime (inferred). +// A restore of a live or noncurrent generation fails with 412 +// objectNotSoftDeleted, and in a bucket with soft delete off with 400 +// SoftDeletePolicyRequired; both messages are inferred. A soft-deleted +// generation cannot be deleted by objects.delete, which answers 404 +// (inferred). A bucket holding only soft-deleted generations can be +// deleted, and is then kept soft-deleted with its policy's duration: +// buckets.list and buckets.get with softDeleted=true (and a generation +// for get) read it, and buckets.restore brings it back with its +// generation, metageneration and soft-deleted objects (inferred), +// failing with 409 conflict while a live bucket has its name (message +// inferred). Everything soft-deleted goes for good at its hardDeleteTime +// on the server's clock. +// - billing.requesterPays is recorded and, under [WithPolicy], enforced: a +// call on a Requester Pays bucket with a userProject also needs +// serviceusage.services.use, and one without needs +// resourcemanager.projects.createBillingAssignment, lacking which it +// fails with 400 and Cloud Storage's message "Bucket is a requester pays +// bucket but no user project provided." (reason required, inferred). +// Without a policy, the caller may bill the bucket's project, so nothing +// is refused. +// - objects.list takes prefix, startOffset, endOffset, versions, +// softDeleted, pageToken and maxResults; a delimiter, matchGlob, filter, // includeTrailingDelimiter or includeFoldersAsPrefixes is rejected with -// 400. Page tokens are positions, so a page may follow one whose items +// 400. objects.patch and objects.update change contentType, +// contentEncoding, metadata, the holds and retention. Page tokens are positions, so a page may follow one whose items // were deleted. Bucket preconditions are ifMetagenerationMatch and -// ifMetagenerationNotMatch, and object ones ifGenerationMatch and -// ifGenerationNotMatch. The fields and userProject parameters are -// accepted and ignored: responses are always whole. Other parameters -// the server does not know are rejected with 400. +// ifMetagenerationNotMatch, and object ones ifGenerationMatch, +// ifGenerationNotMatch and, but for uploads and deletes, the +// metageneration pair. The fields parameter is accepted and ignored: +// responses are always whole, and userProject is checked only as the +// Requester Pays item says. Other parameters the server does not know +// are rejected with 400. // // Fault method names are the API's method IDs, such as // "storage.buckets.insert". A call's resource is its path below @@ -67,7 +141,10 @@ // its method's permission, and those its parameters add, as each method's // reference lists them: a predefined ACL on a patch needs // storage.buckets.setIamPolicy, and an upload that replaces a live object -// storage.objects.delete. projection=full needs nothing more: Cloud Storage +// storage.objects.delete, as does a restore that replaces one; retention in +// an upload or patch needs storage.objects.setRetention, and +// overrideUnlockedRetention=true storage.objects.overrideUnlockedRetention. +// projection=full needs nothing more: Cloud Storage // leaves out the ACLs a caller may not read rather than refusing the call, // and the official clients ask for it on every get and list. Permissions are // not checked against the bucket they act on: a grant covers every bucket. @@ -117,6 +194,7 @@ type Server struct { mu sync.Mutex buckets map[string]*bucket + softBuckets map[string][]*bucket foreign map[string]bool lastGeneration int64 } @@ -136,7 +214,7 @@ func WithClock(now func() time.Time) Option { // New returns an empty server. func New(opts ...Option) *Server { - s := &Server{mux: http.NewServeMux(), now: time.Now, buckets: map[string]*bucket{}, foreign: map[string]bool{}} + s := &Server{mux: http.NewServeMux(), now: time.Now, buckets: map[string]*bucket{}, softBuckets: map[string][]*bucket{}, foreign: map[string]bool{}} for _, opt := range opts { opt(s) } @@ -174,24 +252,49 @@ func (s *Server) registerRoutes() { const ( buckets = "/storage/v1/b" bucket = buckets + "/{bucket}" + object = bucket + "/o/{object...}" ) + objectConds := []string{"generation", "ifGenerationMatch", "ifGenerationNotMatch", "ifMetagenerationMatch", "ifMetagenerationNotMatch"} s.route("POST "+buckets, "storage.buckets.insert", s.insertBucket, insertTarget, "project", "projection", "predefinedAcl", "predefinedDefaultObjectAcl", "enableObjectRetention") s.route("GET "+buckets, "storage.buckets.list", s.listBuckets, listTarget, - "project", "projection", "prefix", "pageToken", "maxResults", "returnPartialSuccess") - s.route("GET "+bucket, "storage.buckets.get", s.getBucket, nil, "projection", "ifMetagenerationMatch", "ifMetagenerationNotMatch") + "project", "projection", "prefix", "pageToken", "maxResults", "returnPartialSuccess", "softDeleted") + s.route("GET "+bucket, "storage.buckets.get", s.getBucket, nil, + "projection", "ifMetagenerationMatch", "ifMetagenerationNotMatch", "generation", "softDeleted") s.route("PATCH "+bucket, "storage.buckets.patch", s.patchBucket, nil, "projection", "ifMetagenerationMatch", "ifMetagenerationNotMatch", "predefinedAcl", "predefinedDefaultObjectAcl") s.route("DELETE "+bucket, "storage.buckets.delete", s.deleteBucket, nil, "ifMetagenerationMatch", "ifMetagenerationNotMatch") + s.route("POST "+bucket+"/lockRetentionPolicy", "storage.buckets.lockRetentionPolicy", s.lockRetentionPolicy, bucketTarget, + "ifMetagenerationMatch") + s.route("POST "+bucket+"/restore", "storage.buckets.restore", s.restoreBucket, bucketTarget, "generation", "projection") s.route("GET "+bucket+"/o", "storage.objects.list", s.listObjects, nil, "projection", "prefix", "delimiter", "pageToken", "maxResults", "versions", "startOffset", "endOffset", "includeTrailingDelimiter", "matchGlob", "includeFoldersAsPrefixes", "softDeleted", "filter") - s.route("DELETE "+bucket+"/o/{object...}", "storage.objects.delete", s.deleteObject, objectTarget, + s.route("GET "+object, "storage.objects.get", s.getObject, objectTarget, slices.Concat(objectConds, []string{"projection", "softDeleted"})...) + s.route("PATCH "+object, "storage.objects.patch", s.patchObject, objectTarget, + slices.Concat(objectConds, []string{"projection", "predefinedAcl", "overrideUnlockedRetention"})...) + s.route("PUT "+object, "storage.objects.update", s.updateObject, objectTarget, + slices.Concat(objectConds, []string{"projection", "predefinedAcl", "overrideUnlockedRetention"})...) + s.route("DELETE "+object, "storage.objects.delete", s.deleteObject, objectTarget, "generation", "ifGenerationMatch", "ifGenerationNotMatch") + restore := s.handle("storage.objects.restore", s.restoreObject, objectTarget, + slices.Concat(objectConds, []string{"projection", "copySourceAcl"})...) + // ServeMux allows a wildcard only at the end, and objects.restore puts a verb after the object name. + s.mux.HandleFunc("POST "+object, func(w http.ResponseWriter, r *http.Request) { + name, ok := strings.CutSuffix(r.PathValue("object"), "/restore") + if !ok { + notImplemented(w, r) + return + } + r.SetPathValue("object", name) + restore(w, r) + }) s.route("POST /upload"+bucket+"/o", "storage.objects.insert", s.insertObject, uploadTarget, "uploadType", "name", "projection", "predefinedAcl", "ifGenerationMatch", "ifGenerationNotMatch", "contentEncoding") - // Without this, ServeMux would redirect an object path with no name to the one with an empty name. - s.mux.HandleFunc("DELETE "+bucket+"/o", notImplemented) + // Without these, ServeMux would redirect an object path with no name to the one with an empty name. + for _, method := range []string{http.MethodDelete, http.MethodPatch, http.MethodPut, http.MethodPost} { + s.mux.HandleFunc(method+" "+bucket+"/o", notImplemented) + } s.mux.HandleFunc("/", notImplemented) } @@ -200,7 +303,12 @@ type handler func(r *http.Request) (any, *apiError) // route registers h for pattern as method, which takes params; a non-nil target names the call's resource. func (s *Server) route(pattern, method string, h handler, target func(*http.Request) string, params ...string) { - s.mux.HandleFunc(pattern, func(w http.ResponseWriter, r *http.Request) { + s.mux.HandleFunc(pattern, s.handle(method, h, target, params...)) +} + +// handle answers method with h, which takes params; a non-nil target names the call's resource. +func (s *Server) handle(method string, h handler, target func(*http.Request) string, params ...string) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { for name := range r.URL.Query() { if !slices.Contains(params, name) && !slices.Contains(commonParams, name) { writeError(w, invalid("squall: %s does not take the parameter %s", method, name)) @@ -236,7 +344,7 @@ func (s *Server) route(pattern, method string, h handler, target func(*http.Requ writeJSON(w, http.StatusOK, body) }, true }) - }) + } } // insertTarget names the bucket a buckets.insert creates, from its body. @@ -250,6 +358,10 @@ func insertTarget(r *http.Request) string { return "b/" + named.Name } +func bucketTarget(r *http.Request) string { + return "b/" + r.PathValue("bucket") +} + func listTarget(r *http.Request) string { return "projects/" + r.URL.Query().Get("project") + "/b" } diff --git a/gcp/storage/softdelete.go b/gcp/storage/softdelete.go new file mode 100644 index 0000000..8f7c779 --- /dev/null +++ b/gcp/storage/softdelete.go @@ -0,0 +1,241 @@ +package storage + +import ( + "errors" + "fmt" + "maps" + "net/http" + "slices" + "strings" + "time" +) + +// softDeletedGeneration reads the softDeleted and generation parameters of a get, each of which needs the other. +func softDeletedGeneration(r *http.Request) (bool, int64, *apiError) { + soft, err := boolParam(r, "softDeleted") + if err != nil { + return false, 0, err + } + gen, ok, err := int64Param(r, "generation") + if err != nil { + return false, 0, err + } + switch { + case soft && !ok: + return false, 0, required("generation") + case ok && !soft && r.PathValue("object") == "": + return false, 0, required("softDeleted") + } + return soft, gen, nil +} + +// trashed returns soft-deleted generation gen of name in b, or nil. +func (b *bucket) trashed(name string, gen int64) *Object { + for _, o := range b.trash { + if o.Name == name && o.Generation == gen { + return o + } + } + return nil +} + +// purgeTrash drops the soft-deleted generations in b whose retention has ended. The caller holds s.mu. +func (s *Server) purgeTrash(b *bucket) { + now := s.now() + if len(b.trash) == 0 || now.Before(b.nextPurge) { + return + } + b.trash = slices.DeleteFunc(b.trash, func(o *Object) bool { return !now.Before(o.hardDelete) }) + b.nextPurge = time.Time{} + for _, o := range b.trash { + b.keepUntil(o.hardDelete) + } +} + +// keepUntil records that b holds a soft-deleted generation until t. +func (b *bucket) keepUntil(t time.Time) { + if b.nextPurge.IsZero() || t.Before(b.nextPurge) { + b.nextPurge = t + } +} + +// softDeleteBucket keeps b, which was just deleted, if it keeps deletions. The caller holds s.mu. +func (s *Server) softDeleteBucket(b *bucket) { + keep := b.softDeleteDuration() + if keep == 0 { + return + } + t, now := s.timestamp() + b.SoftDeleteTime, b.hardDelete = now, t.Add(keep) + b.HardDeleteTime = b.hardDelete.Format(timeFormat) + s.softBuckets[b.Name] = append(s.softBuckets[b.Name], b) +} + +// purgeBuckets drops the soft-deleted buckets whose retention has ended. The caller holds s.mu. +func (s *Server) purgeBuckets() { + now := s.now() + for name, gens := range s.softBuckets { + gens = slices.DeleteFunc(gens, func(b *bucket) bool { return !now.Before(b.hardDelete) }) + if len(gens) == 0 { + delete(s.softBuckets, name) + } else { + s.softBuckets[name] = gens + } + } +} + +// softDeletedBucket returns soft-deleted generation gen of the named bucket, or nil. The caller holds s.mu. +func (s *Server) softDeletedBucket(name string, gen int64) *bucket { + s.purgeBuckets() + for _, b := range s.softBuckets[name] { + if b.Generation == gen { + return b + } + } + return nil +} + +// softDeletedBuckets returns every soft-deleted bucket, ordered by name and generation. The caller holds s.mu. +func (s *Server) softDeletedBuckets() []Bucket { + var out []Bucket + for _, name := range slices.Sorted(maps.Keys(s.softBuckets)) { + gens := slices.SortedFunc(slices.Values(s.softBuckets[name]), func(a, b *bucket) int { return int(a.Generation - b.Generation) }) + for _, b := range gens { + out = append(out, b.clone()) + } + } + return out +} + +func (s *Server) restoreBucket(r *http.Request) (any, *apiError) { + gen, ok, err := int64Param(r, "generation") + if err != nil { + return nil, err + } + if !ok { + return nil, required("generation") + } + name := r.PathValue("bucket") + s.mu.Lock() + defer s.mu.Unlock() + b := s.softDeletedBucket(name, gen) + if b == nil { + if s.foreign[name] { + return nil, forbidden("storage.buckets.restore") + } + return nil, notFoundBucket() + } + if _, ok := s.buckets[name]; ok || s.foreign[name] { + return nil, &apiError{Status: http.StatusConflict, Reason: "conflict", Message: "The requested bucket name is not available."} + } + s.softBuckets[name] = slices.DeleteFunc(s.softBuckets[name], func(g *bucket) bool { return g == b }) + if len(s.softBuckets[name]) == 0 { + delete(s.softBuckets, name) + } + b.SoftDeleteTime, b.HardDeleteTime = "", "" + s.buckets[name] = b + s.purgeTrash(b) + return b.clone(), nil +} + +func (s *Server) restoreObject(r *http.Request) (any, *apiError) { + gen, ok, err := int64Param(r, "generation") + if err != nil { + return nil, err + } + if !ok { + return nil, required("generation") + } + if _, err := boolParam(r, "copySourceAcl"); err != nil { + return nil, err + } + bucketName, name := r.PathValue("bucket"), r.PathValue("object") + s.mu.Lock() + defer s.mu.Unlock() + b, err := s.lookup(bucketName, "storage.objects.restore") + if err != nil { + return nil, err + } + if b.softDeleteDuration() == 0 { + return nil, &apiError{Status: http.StatusBadRequest, Reason: "SoftDeletePolicyRequired", Message: "Soft delete policy is required to restore soft-deleted objects."} + } + src := b.trashed(name, gen) + if src == nil { + if b.generation(name, gen) != nil { + return nil, &apiError{Status: http.StatusPreconditionFailed, Reason: "objectNotSoftDeleted", Message: fmt.Sprintf("Object '%s/%s' is not soft-deleted.", bucketName, name)} + } + return nil, noSuchObject(bucketName, name) + } + var liveGen, liveMeta int64 + live := b.liveObject(name) + if live != nil { + liveGen, liveMeta = live.Generation, live.Metageneration + } + if err := checkMatch(r, "ifGenerationMatch", "ifGenerationNotMatch", liveGen); err != nil { + return nil, err + } + if live == nil && (r.URL.Query().Has("ifMetagenerationMatch") || r.URL.Query().Has("ifMetagenerationNotMatch")) { + return nil, conditionNotMet() + } + if err := checkMatch(r, "ifMetagenerationMatch", "ifMetagenerationNotMatch", liveMeta); err != nil { + return nil, err + } + if err := s.replaceLive(b, name); err != nil { + return nil, err + } + o := s.newGeneration(b, name, "STANDARD") + o.ContentType, o.ContentEncoding, o.Metadata = src.ContentType, src.ContentEncoding, maps.Clone(src.Metadata) + o.Size, o.MD5Hash, o.CRC32C = src.Size, src.MD5Hash, src.CRC32C + if b.DefaultEventBasedHold { + o.EventBasedHold = new(true) + } + return s.view(b, o), nil +} + +// SoftDeletedObjects returns the soft-deleted generations in the named bucket whose retention has not ended, +// ordered by name and generation. +func (s *Server) SoftDeletedObjects(bucketName string) []Object { + s.mu.Lock() + defer s.mu.Unlock() + b, ok := s.buckets[bucketName] + if !ok { + return nil + } + s.purgeTrash(b) + out := make([]Object, 0, len(b.trash)) + for _, o := range b.trash { + out = append(out, o.clone()) + } + slices.SortFunc(out, func(a, b Object) int { return strings.Compare(versionKey(a), versionKey(b)) }) + return out +} + +// SoftDeletedBuckets returns every soft-deleted bucket whose retention has not ended, ordered by name and generation. +func (s *Server) SoftDeletedBuckets() []Bucket { + s.mu.Lock() + defer s.mu.Unlock() + s.purgeBuckets() + return s.softDeletedBuckets() +} + +// PutSoftDeletedObject writes data to name in the named bucket as a soft-deleted generation, as if a live one had +// just been deleted, and returns it. The bucket must keep deletions. +func (s *Server) PutSoftDeletedObject(bucketName, name string, data []byte) (Object, error) { + s.mu.Lock() + defer s.mu.Unlock() + b, ok := s.buckets[bucketName] + if !ok { + return Object{}, notFoundBucket() + } + if b.softDeleteDuration() == 0 { + return Object{}, errors.New("squall: bucket " + bucketName + " keeps no soft-deleted objects") + } + if !validObjectName(name) { + return Object{}, invalid("The specified object name is not valid.") + } + o := s.newGeneration(b, name, b.StorageClass) + o.ContentType, o.Size = "application/octet-stream", uint64(len(data)) + o.MD5Hash, o.CRC32C = checksums(data) + s.discard(b, o) + return o.clone(), nil +} diff --git a/sdktest/gcp/storage/permissions_test.go b/sdktest/gcp/storage/permissions_test.go index 79f4576..d90356d 100644 --- a/sdktest/gcp/storage/permissions_test.go +++ b/sdktest/gcp/storage/permissions_test.go @@ -6,7 +6,9 @@ import ( "slices" "strings" "testing" + "time" + gcs "cloud.google.com/go/storage" "google.golang.org/api/googleapi" "github.com/parallelworks/squall/access" @@ -74,3 +76,24 @@ func TestPolicyRefusesAMethodItDoesNotGrant(t *testing.T) { t.Errorf("Denials() = %+v", denials) } } + +func TestPolicyNeedsRetentionAndRestorePermissions(t *testing.T) { + policy := access.Allow("storage.buckets.create", "storage.buckets.enableObjectRetention", "storage.objects.create", "storage.objects.update", "storage.objects.delete") + c := newClient(t, storage.New(storage.WithPolicy(policy))) + b := c.Bucket("data") + if err := b.SetObjectRetention(true).Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + first := write(t, c, "data", "a.txt", "one") + write(t, c, "data", "a.txt", "two") + + _, err := b.Object("a.txt").Update(t.Context(), gcs.ObjectAttrsToUpdate{Retention: &gcs.ObjectRetention{Mode: "Unlocked", RetainUntil: time.Now().Add(time.Hour)}}) + wantErr(t, "set retention", err, http.StatusForbidden, "forbidden", "Permission 'storage.objects.setRetention' denied") + _, err = b.Object("a.txt").OverrideUnlockedRetention(true).Update(t.Context(), gcs.ObjectAttrsToUpdate{ContentType: "text/csv"}) + wantErr(t, "override retention", err, http.StatusForbidden, "forbidden", "Permission 'storage.objects.overrideUnlockedRetention' denied") + _, err = b.Object("a.txt").Generation(first.Generation).Restore(t.Context(), &gcs.RestoreOptions{}) + wantErr(t, "restore", err, http.StatusForbidden, "forbidden", "Permission 'storage.objects.restore' denied") + if _, err := b.Object("a.txt").Update(t.Context(), gcs.ObjectAttrsToUpdate{ContentType: "text/csv"}); err != nil { + t.Fatalf("update metadata: %v", err) + } +} diff --git a/sdktest/gcp/storage/protection_test.go b/sdktest/gcp/storage/protection_test.go new file mode 100644 index 0000000..227f199 --- /dev/null +++ b/sdktest/gcp/storage/protection_test.go @@ -0,0 +1,553 @@ +package storage_test + +import ( + "errors" + "net/http" + "strings" + "sync" + "testing" + "time" + + gcs "cloud.google.com/go/storage" + "google.golang.org/api/googleapi" + "google.golang.org/api/option" + rawstorage "google.golang.org/api/storage/v1" + + "github.com/parallelworks/squall/access" + "github.com/parallelworks/squall/gcp/storage" +) + +type clock struct { + mu sync.Mutex + now time.Time +} + +func newClock() *clock { return &clock{now: time.Date(2026, 3, 1, 12, 0, 0, 0, time.UTC)} } + +func (c *clock) Now() time.Time { + c.mu.Lock() + defer c.mu.Unlock() + return c.now +} + +func (c *clock) Advance(d time.Duration) { + c.mu.Lock() + defer c.mu.Unlock() + c.now = c.now.Add(d) +} + +// apiErr returns the status, reason and message of a Google API error. +func apiErr(t *testing.T, err error) (int, string, string) { + t.Helper() + e, ok := errors.AsType[*googleapi.Error](err) + if !ok { + t.Fatalf("error = %v, want a Google API error", err) + } + reason := "" + if len(e.Errors) > 0 { + reason = e.Errors[0].Reason + } + return e.Code, reason, e.Message +} + +func wantErr(t *testing.T, what string, err error, code int, reason, message string) { + t.Helper() + if err == nil { + t.Fatalf("%s succeeded, want %d %s", what, code, reason) + } + gotCode, gotReason, gotMessage := apiErr(t, err) + if gotCode != code || gotReason != reason || !strings.Contains(gotMessage, message) { + t.Fatalf("%s = %d %s %q, want %d %s %q", what, gotCode, gotReason, gotMessage, code, reason, message) + } +} + +func writeWith(t *testing.T, c *gcs.Client, bucket, name, data string, attrs gcs.ObjectAttrs) (*gcs.ObjectAttrs, error) { + t.Helper() + w := c.Bucket(bucket).Object(name).NewWriter(t.Context()) + w.ObjectAttrs = attrs + w.Name = name + if _, err := w.Write([]byte(data)); err != nil { + return nil, err + } + if err := w.Close(); err != nil { + return nil, err + } + return w.Attrs(), nil +} + +func overwrite(t *testing.T, c *gcs.Client, bucket, name string) error { + t.Helper() + _, err := writeWith(t, c, bucket, name, "again", gcs.ObjectAttrs{}) + return err +} + +func TestRetentionPolicyAndBucketLock(t *testing.T) { + clk := newClock() + s := storage.New(storage.WithClock(clk.Now)) + c := newClient(t, s) + b := c.Bucket("retained") + if err := b.Create(t.Context(), project, &gcs.BucketAttrs{RetentionPolicy: &gcs.RetentionPolicy{RetentionPeriod: time.Hour}}); err != nil { + t.Fatal(err) + } + attrs, err := b.Attrs(t.Context()) + if err != nil { + t.Fatal(err) + } + if p := attrs.RetentionPolicy; p == nil || p.RetentionPeriod != time.Hour || !p.EffectiveTime.Equal(clk.Now()) || p.IsLocked { + t.Fatalf("retention policy = %+v", p) + } + o := write(t, c, "retained", "kept", "data") + if !o.RetentionExpirationTime.Equal(o.Created.Add(time.Hour)) { + t.Fatalf("retention expiration = %v, created %v", o.RetentionExpirationTime, o.Created) + } + wantErr(t, "delete a retained object", b.Object("kept").Delete(t.Context()), http.StatusForbidden, "retentionPolicyNotMet", + "Object 'retained/kept' is subject to bucket's retention policy or object retention and cannot be deleted or overwritten until 2026-03-01T13:00:00.000000Z") + wantErr(t, "overwrite a retained object", overwrite(t, c, "retained", "kept"), http.StatusForbidden, "retentionPolicyNotMet", "cannot be deleted or overwritten") + + wantErr(t, "lock with a stale metageneration", b.If(gcs.BucketConditions{MetagenerationMatch: 7}).LockRetentionPolicy(t.Context()), + http.StatusPreconditionFailed, "conditionNotMet", "pre-conditions") + if err := b.If(gcs.BucketConditions{MetagenerationMatch: attrs.MetaGeneration}).LockRetentionPolicy(t.Context()); err != nil { + t.Fatalf("lock: %v", err) + } + locked, err := b.Attrs(t.Context()) + if err != nil || !locked.RetentionPolicy.IsLocked || locked.MetaGeneration != attrs.MetaGeneration+1 { + t.Fatalf("locked = %+v, %v", locked, err) + } + wantErr(t, "lock again", b.If(gcs.BucketConditions{MetagenerationMatch: locked.MetaGeneration}).LockRetentionPolicy(t.Context()), + http.StatusBadRequest, "invalid", "Bucket 'retained' does not have an unlocked retention policy.") + + _, err = b.Update(t.Context(), gcs.BucketAttrsToUpdate{RetentionPolicy: &gcs.RetentionPolicy{RetentionPeriod: time.Minute}}) + wantErr(t, "shorten a locked policy", err, http.StatusForbidden, "forbidden", + "Cannot reduce retention duration of a locked Retention Policy for bucket 'retained'.") + _, err = b.Update(t.Context(), gcs.BucketAttrsToUpdate{RetentionPolicy: &gcs.RetentionPolicy{}}) + wantErr(t, "remove a locked policy", err, http.StatusForbidden, "retentionPolicyNotMet", + "Bucket 'retained' has a locked Retention Policy which cannot be removed.") + longer, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{RetentionPolicy: &gcs.RetentionPolicy{RetentionPeriod: 2 * time.Hour}}) + if err != nil || longer.RetentionPolicy.RetentionPeriod != 2*time.Hour || !longer.RetentionPolicy.IsLocked { + t.Fatalf("lengthen a locked policy = %+v, %v", longer, err) + } + + clk.Advance(90 * time.Minute) + wantErr(t, "delete before the longer period ends", b.Object("kept").Delete(t.Context()), http.StatusForbidden, "retentionPolicyNotMet", "14:00:00") + clk.Advance(time.Hour) + if err := b.Object("kept").Delete(t.Context()); err != nil { + t.Fatalf("delete after the period: %v", err) + } + if err := b.Delete(t.Context()); err != nil { + t.Fatalf("delete the emptied locked bucket: %v", err) + } +} + +func TestUnlockedRetentionPolicyChanges(t *testing.T) { + s := storage.New() + c := newClient(t, s) + b := c.Bucket("unlocked") + if err := b.Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + for _, period := range []time.Duration{-time.Second, 100 * 366 * 24 * time.Hour} { + _, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{RetentionPolicy: &gcs.RetentionPolicy{RetentionPeriod: period}}) + wantErr(t, "an out of range period", err, http.StatusBadRequest, "invalid", "greater than 0 and less than 100 years") + } + write(t, c, "unlocked", "k", "v") + if _, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{RetentionPolicy: &gcs.RetentionPolicy{RetentionPeriod: time.Hour}}); err != nil { + t.Fatal(err) + } + wantErr(t, "delete an object older than the policy", b.Object("k").Delete(t.Context()), http.StatusForbidden, "retentionPolicyNotMet", "unlocked/k") + if _, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{RetentionPolicy: &gcs.RetentionPolicy{}}); err != nil { + t.Fatalf("remove an unlocked policy: %v", err) + } + if attrs, _ := b.Attrs(t.Context()); attrs.RetentionPolicy != nil { + t.Fatalf("policy after removal = %+v", attrs.RetentionPolicy) + } + if err := b.Object("k").Delete(t.Context()); err != nil { + t.Fatalf("delete after removing the policy: %v", err) + } +} + +func TestObjectHolds(t *testing.T) { + clk := newClock() + s := storage.New(storage.WithClock(clk.Now)) + c := newClient(t, s) + b := c.Bucket("holds") + if err := b.Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + o, err := writeWith(t, c, "holds", "temp", "x", gcs.ObjectAttrs{TemporaryHold: true}) + if err != nil || !o.TemporaryHold || o.EventBasedHold { + t.Fatalf("upload with a hold = %+v, %v", o, err) + } + wantErr(t, "delete a held object", b.Object("temp").Delete(t.Context()), http.StatusForbidden, "forbidden", + "Object 'holds/temp' is under active Temporary hold and cannot be deleted, overwritten or archived until hold is removed.") + wantErr(t, "overwrite a held object", overwrite(t, c, "holds", "temp"), http.StatusForbidden, "forbidden", "Temporary hold") + released, err := b.Object("temp").Update(t.Context(), gcs.ObjectAttrsToUpdate{TemporaryHold: false, Metadata: map[string]string{"k": "v"}}) + if err != nil || released.TemporaryHold || released.Metageneration != 2 || released.Metadata["k"] != "v" { + t.Fatalf("release = %+v, %v", released, err) + } + if err := b.Object("temp").Delete(t.Context()); err != nil { + t.Fatalf("delete a released object: %v", err) + } + + if _, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{DefaultEventBasedHold: true, RetentionPolicy: &gcs.RetentionPolicy{RetentionPeriod: time.Hour}}); err != nil { + t.Fatal(err) + } + held := write(t, c, "holds", "event", "x") + if !held.EventBasedHold || !held.RetentionExpirationTime.IsZero() { + t.Fatalf("an object under a default event-based hold = %+v", held) + } + wantErr(t, "delete an object under an event-based hold", b.Object("event").Delete(t.Context()), http.StatusForbidden, "forbidden", "active Event-Based hold") + clk.Advance(3 * time.Hour) + released, err = b.Object("event").Update(t.Context(), gcs.ObjectAttrsToUpdate{EventBasedHold: false}) + if err != nil || released.EventBasedHold || !released.RetentionExpirationTime.Equal(clk.Now().Add(time.Hour)) { + t.Fatalf("release the event-based hold = %+v, %v", released, err) + } + wantErr(t, "delete once the hold restarts retention", b.Object("event").Delete(t.Context()), http.StatusForbidden, "retentionPolicyNotMet", "holds/event") + clk.Advance(time.Hour) + if err := b.Object("event").Delete(t.Context()); err != nil { + t.Fatalf("delete after retention: %v", err) + } +} + +func TestHoldsAndRetentionInVersionedBuckets(t *testing.T) { + s := storage.New() + c := newClient(t, s) + b := c.Bucket("versioned-retained") + if err := b.Create(t.Context(), project, &gcs.BucketAttrs{ + VersioningEnabled: true, RetentionPolicy: &gcs.RetentionPolicy{RetentionPeriod: time.Hour}, + }); err != nil { + t.Fatal(err) + } + first := write(t, c, "versioned-retained", "k", "one") + write(t, c, "versioned-retained", "k", "two") + wantErr(t, "delete a retained noncurrent generation", b.Object("k").Generation(first.Generation).Delete(t.Context()), + http.StatusForbidden, "retentionPolicyNotMet", "versioned-retained/k") + if _, err := s.SetObjectHolds("versioned-retained", "k", true, false); err != nil { + t.Fatal(err) + } + wantErr(t, "archive a held object", b.Object("k").Delete(t.Context()), http.StatusForbidden, "forbidden", "Temporary hold") + if _, err := s.SetObjectHolds("versioned-retained", "k", false, false); err != nil { + t.Fatal(err) + } + if err := b.Object("k").Delete(t.Context()); err != nil { + t.Fatalf("make a retained object noncurrent: %v", err) + } +} + +func TestObjectRetention(t *testing.T) { + clk := newClock() + s := storage.New(storage.WithClock(clk.Now)) + c := newClient(t, s) + b := c.Bucket("object-retention") + if err := b.SetObjectRetention(true).Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + if attrs, err := b.Attrs(t.Context()); err != nil || attrs.ObjectRetentionMode != "Enabled" { + t.Fatalf("attrs = %+v, %v", attrs, err) + } + until := clk.Now().Add(time.Hour) + o, err := writeWith(t, c, "object-retention", "k", "x", gcs.ObjectAttrs{Retention: &gcs.ObjectRetention{Mode: "Unlocked", RetainUntil: until}}) + if err != nil || o.Retention == nil || o.Retention.Mode != "Unlocked" || !o.Retention.RetainUntil.Equal(until) { + t.Fatalf("upload with retention = %+v, %v", o, err) + } + wantErr(t, "delete a retained object", b.Object("k").Delete(t.Context()), http.StatusForbidden, "retentionPolicyNotMet", "object-retention/k") + shorter := gcs.ObjectAttrsToUpdate{Retention: &gcs.ObjectRetention{Mode: "Unlocked", RetainUntil: until.Add(-time.Minute)}} + _, err = b.Object("k").Update(t.Context(), shorter) + wantErr(t, "shorten without override", err, http.StatusForbidden, "forbidden", "without overriding unlocked retention intent") + _, err = b.Object("k").Update(t.Context(), gcs.ObjectAttrsToUpdate{EventBasedHold: true}) + wantErr(t, "an event-based hold beside retention", err, http.StatusBadRequest, "invalid", "event-based hold") + if _, err := b.Object("k").OverrideUnlockedRetention(true).Update(t.Context(), shorter); err != nil { + t.Fatalf("shorten with override: %v", err) + } + removed, err := b.Object("k").OverrideUnlockedRetention(true).Update(t.Context(), gcs.ObjectAttrsToUpdate{Retention: &gcs.ObjectRetention{}}) + if err != nil || removed.Retention != nil { + t.Fatalf("remove with override = %+v, %v", removed, err) + } + + locked := gcs.ObjectAttrsToUpdate{Retention: &gcs.ObjectRetention{Mode: "Locked", RetainUntil: until}} + if _, err := b.Object("k").Update(t.Context(), locked); err != nil { + t.Fatalf("lock retention: %v", err) + } + _, err = b.Object("k").OverrideUnlockedRetention(true).Update(t.Context(), gcs.ObjectAttrsToUpdate{Retention: &gcs.ObjectRetention{Mode: "Unlocked", RetainUntil: until}}) + wantErr(t, "unlock locked retention", err, http.StatusForbidden, "forbidden", "The locked object retention cannot be removed.") + extended := gcs.ObjectAttrsToUpdate{Retention: &gcs.ObjectRetention{Mode: "Locked", RetainUntil: until.Add(time.Hour)}} + if _, err := b.Object("k").Update(t.Context(), extended); err != nil { + t.Fatalf("extend locked retention: %v", err) + } + clk.Advance(90 * time.Minute) + wantErr(t, "delete before the extended time", b.Object("k").Delete(t.Context()), http.StatusForbidden, "retentionPolicyNotMet", "14:00:00") + + _, err = b.Object("k").Update(t.Context(), gcs.ObjectAttrsToUpdate{Retention: &gcs.ObjectRetention{Mode: "Locked", RetainUntil: clk.Now().Add(-time.Minute)}}) + wantErr(t, "a retain-until time in the past", err, http.StatusBadRequest, "invalid", "Retain until time cannot be in the past.") + + if err := c.Bucket("plain").Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + _, err = writeWith(t, c, "plain", "k", "x", gcs.ObjectAttrs{Retention: &gcs.ObjectRetention{Mode: "Unlocked", RetainUntil: clk.Now().Add(time.Hour)}}) + wantErr(t, "retention where the bucket does not enable it", err, http.StatusForbidden, "forbidden", "storage.objects.setRetention") +} + +func TestSoftDeletedObjects(t *testing.T) { + clk := newClock() + s := storage.New(storage.WithClock(clk.Now)) + c := newClient(t, s) + b := c.Bucket("soft") + if err := b.Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + attrs, err := b.Attrs(t.Context()) + if err != nil || attrs.SoftDeletePolicy == nil || attrs.SoftDeletePolicy.RetentionDuration != 7*24*time.Hour || !attrs.SoftDeletePolicy.EffectiveTime.Equal(clk.Now()) { + t.Fatalf("soft delete policy = %+v, %v", attrs.SoftDeletePolicy, err) + } + first := write(t, c, "soft", "k", "one") + second := write(t, c, "soft", "k", "two") + if err := b.Object("k").Delete(t.Context()); err != nil { + t.Fatal(err) + } + if live := objects(t, b.Objects(t.Context(), nil)); len(live) != 0 { + t.Fatalf("live = %+v", live) + } + soft := objects(t, b.Objects(t.Context(), &gcs.Query{SoftDeleted: true})) + if len(soft) != 2 || soft[0].Generation != first.Generation || soft[1].Generation != second.Generation || + !soft[0].SoftDeleteTime.Equal(clk.Now()) || !soft[0].HardDeleteTime.Equal(clk.Now().Add(7*24*time.Hour)) { + t.Fatalf("soft-deleted = %+v", soft) + } + got, err := b.Object("k").Generation(first.Generation).SoftDeleted().Attrs(t.Context()) + if err != nil || got.Size != 3 || got.SoftDeleteTime.IsZero() { + t.Fatalf("get a soft-deleted generation = %+v, %v", got, err) + } + if err := b.Object("k").Generation(first.Generation).Delete(t.Context()); !errors.Is(err, gcs.ErrObjectNotExist) { + t.Fatalf("hard delete a soft-deleted generation = %v", err) + } + if err := b.Delete(t.Context()); err != nil { + t.Fatalf("delete a bucket holding only soft-deleted objects: %v", err) + } + if err := c.Bucket("soft").Create(t.Context(), project, nil); err != nil { + t.Fatalf("create the name again: %v", err) + } + if err := c.Bucket("soft").Delete(t.Context()); err != nil { + t.Fatal(err) + } + if got := s.SoftDeletedBuckets(); len(got) != 2 || got[0].Name != "soft" || got[0].HardDeleteTime == "" { + t.Fatalf("soft-deleted buckets = %+v", got) + } + clk.Advance(7 * 24 * time.Hour) + if got := s.SoftDeletedBuckets(); len(got) != 0 { + t.Fatalf("soft-deleted buckets after their retention = %+v", got) + } +} + +func TestRestoreObjects(t *testing.T) { + clk := newClock() + s := storage.New(storage.WithClock(clk.Now)) + c := newClient(t, s) + b := c.Bucket("restorable") + if err := b.Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + first := write(t, c, "restorable", "k", "one") + second := write(t, c, "restorable", "k", "two") + restored, err := b.Object("k").Generation(first.Generation).Restore(t.Context(), &gcs.RestoreOptions{}) + if err != nil || restored.Generation <= second.Generation || restored.Size != 3 || restored.Metageneration != 1 || restored.StorageClass != "STANDARD" { + t.Fatalf("restore = %+v, %v", restored, err) + } + soft := s.SoftDeletedObjects("restorable") + if len(soft) != 2 || soft[0].Generation != first.Generation || soft[1].Generation != second.Generation { + t.Fatalf("soft-deleted after restore = %+v", soft) + } + _, err = b.Object("k").Generation(restored.Generation).Restore(t.Context(), &gcs.RestoreOptions{}) + wantErr(t, "restore a live generation", err, http.StatusPreconditionFailed, "objectNotSoftDeleted", "") + _, err = b.Object("k").Generation(second.Generation).If(gcs.Conditions{DoesNotExist: true}).Restore(t.Context(), &gcs.RestoreOptions{}) + wantErr(t, "restore over a live object with DoesNotExist", err, http.StatusPreconditionFailed, "conditionNotMet", "") + if _, err := s.SetObjectHolds("restorable", "k", true, false); err != nil { + t.Fatal(err) + } + _, err = b.Object("k").Generation(second.Generation).Restore(t.Context(), &gcs.RestoreOptions{}) + wantErr(t, "restore over a held object", err, http.StatusForbidden, "forbidden", "Temporary hold") + + clk.Advance(7 * 24 * time.Hour) + if _, err := b.Object("k").Generation(first.Generation).Restore(t.Context(), &gcs.RestoreOptions{}); !errors.Is(err, gcs.ErrObjectNotExist) { + t.Fatalf("restore after retention = %v", err) + } + + _, err = b.Update(t.Context(), gcs.BucketAttrsToUpdate{SoftDeletePolicy: &gcs.SoftDeletePolicy{RetentionDuration: 24 * time.Hour}}) + wantErr(t, "a one-day soft delete policy", err, http.StatusBadRequest, "invalid", "between 7 days and 90 days") + off, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{SoftDeletePolicy: &gcs.SoftDeletePolicy{}}) + if err != nil || off.SoftDeletePolicy == nil || off.SoftDeletePolicy.RetentionDuration != 0 { + t.Fatalf("turn soft delete off = %+v, %v", off, err) + } + _, err = b.Objects(t.Context(), &gcs.Query{SoftDeleted: true}).Next() + wantErr(t, "list soft-deleted objects with soft delete off", err, http.StatusBadRequest, "invalid", "Soft delete policy is required") +} + +func TestPutSoftDeletedObject(t *testing.T) { + s := storage.New() + if _, err := s.PutBucket(storage.Bucket{Name: "seeded", Project: project}); err != nil { + t.Fatal(err) + } + if _, err := s.PutObject("seeded", "k", []byte("live")); err != nil { + t.Fatal(err) + } + gone, err := s.PutSoftDeletedObject("seeded", "k", []byte("gone")) + if err != nil || gone.SoftDeleteTime == "" { + t.Fatalf("put soft-deleted = %+v, %v", gone, err) + } + if got := s.Objects("seeded"); len(got) != 1 || got[0].Size != 4 || got[0].TimeDeleted != "" { + t.Fatalf("objects = %+v", got) + } + restored, err := newClient(t, s).Bucket("seeded").Object("k").Generation(gone.Generation).Restore(t.Context(), &gcs.RestoreOptions{}) + if err != nil || restored.Size != 4 { + t.Fatalf("restore = %+v, %v", restored, err) + } + if _, err := s.PutBucket(storage.Bucket{Name: "keeps-nothing", Project: project, SoftDeletePolicy: &storage.SoftDeletePolicy{}}); err != nil { + t.Fatal(err) + } + if _, err := s.PutSoftDeletedObject("keeps-nothing", "k", nil); err == nil { + t.Fatal("a soft-deleted object in a bucket that keeps none was put") + } +} + +func TestSetRetentionHelpers(t *testing.T) { + clk := newClock() + s := storage.New(storage.WithClock(clk.Now)) + if _, err := s.PutBucket(storage.Bucket{Name: "helpers", Project: project, ObjectRetention: &storage.BucketObjectRetention{Mode: "Enabled"}}); err != nil { + t.Fatal(err) + } + if _, err := s.PutObject("helpers", "k", []byte("v")); err != nil { + t.Fatal(err) + } + if b, err := s.SetRetentionPolicy("helpers", &storage.RetentionPolicy{RetentionPeriod: 60, IsLocked: true}); err != nil || !b.RetentionPolicy.IsLocked { + t.Fatalf("set a locked policy = %+v, %v", b, err) + } + c := newClient(t, s) + _, err := c.Bucket("helpers").Update(t.Context(), gcs.BucketAttrsToUpdate{RetentionPolicy: &gcs.RetentionPolicy{}}) + wantErr(t, "remove a policy a helper locked", err, http.StatusForbidden, "retentionPolicyNotMet", "locked") + if _, err := s.SetRetentionPolicy("helpers", nil); err != nil { + t.Fatal(err) + } + until := clk.Now().Add(time.Hour).Format(time.RFC3339) + o, err := s.SetObjectRetention("helpers", "k", &storage.Retention{Mode: "Locked", RetainUntilTime: until}) + if err != nil || o.Retention == nil || o.Retention.Mode != "Locked" { + t.Fatalf("set retention = %+v, %v", o, err) + } + wantErr(t, "delete under retention a helper set", c.Bucket("helpers").Object("k").Delete(t.Context()), http.StatusForbidden, "retentionPolicyNotMet", "helpers/k") + if _, err := s.SetObjectRetention("helpers", "k", nil); err != nil { + t.Fatal(err) + } + if _, err := s.SetObjectHolds("helpers", "k", false, true); err != nil { + t.Fatal(err) + } + wantErr(t, "delete under a hold a helper set", c.Bucket("helpers").Object("k").Delete(t.Context()), http.StatusForbidden, "forbidden", "Event-Based") + if _, err := s.SetObjectHolds("helpers", "missing", true, false); err == nil { + t.Fatal("held a missing object") + } +} + +func rawService(t *testing.T, url string) *rawstorage.Service { + t.Helper() + svc, err := rawstorage.NewService(t.Context(), option.WithEndpoint(url+"/storage/v1/"), option.WithoutAuthentication()) + if err != nil { + t.Fatal(err) + } + return svc +} + +// The Go client has no bucket soft delete calls, so this goes through the generated JSON API client. +func TestSoftDeletedBuckets(t *testing.T) { + s := storage.New() + url := serve(t, s) + c, svc := clientAt(t, url), rawService(t, url) + if err := c.Bucket("gone").Create(t.Context(), project, &gcs.BucketAttrs{Labels: map[string]string{"a": "b"}}); err != nil { + t.Fatal(err) + } + write(t, c, "gone", "k", "v") + if err := c.Bucket("gone").Object("k").Delete(t.Context()); err != nil { + t.Fatal(err) + } + if err := c.Bucket("gone").Delete(t.Context()); err != nil { + t.Fatal(err) + } + if err := c.Bucket("kept-nothing").Create(t.Context(), project, &gcs.BucketAttrs{SoftDeletePolicy: &gcs.SoftDeletePolicy{}}); err != nil { + t.Fatal(err) + } + if err := c.Bucket("kept-nothing").Delete(t.Context()); err != nil { + t.Fatal(err) + } + if live := names(t, c.Buckets(t.Context(), project)); len(live) != 0 { + t.Fatalf("live buckets = %v", live) + } + list, err := svc.Buckets.List(project).SoftDeleted(true).Context(t.Context()).Do() + if err != nil || len(list.Items) != 1 || list.Items[0].Name != "gone" || list.Items[0].SoftDeleteTime == "" || list.Items[0].HardDeleteTime == "" { + t.Fatalf("soft-deleted buckets = %+v, %v", list, err) + } + gen := list.Items[0].Generation + got, err := svc.Buckets.Get("gone").SoftDeleted(true).Generation(gen).Context(t.Context()).Do() + if err != nil || got.Labels["a"] != "b" { + t.Fatalf("get a soft-deleted bucket = %+v, %v", got, err) + } + _, err = svc.Buckets.Get("gone").SoftDeleted(true).Context(t.Context()).Do() + wantErr(t, "get a soft-deleted bucket without its generation", err, http.StatusBadRequest, "required", "generation") + + if err := c.Bucket("gone").Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + _, err = svc.Buckets.Restore("gone", gen).Context(t.Context()).Do() + wantErr(t, "restore over a live bucket", err, http.StatusConflict, "conflict", "") + if err := c.Bucket("gone").Delete(t.Context()); err != nil { + t.Fatal(err) + } + restored, err := svc.Buckets.Restore("gone", gen).Context(t.Context()).Do() + if err != nil || restored.Name != "gone" || restored.Generation != gen || restored.SoftDeleteTime != "" { + t.Fatalf("restore = %+v, %v", restored, err) + } + soft := objects(t, c.Bucket("gone").Objects(t.Context(), &gcs.Query{SoftDeleted: true})) + if len(soft) != 1 { + t.Fatalf("soft-deleted objects in the restored bucket = %+v", soft) + } + if _, err := c.Bucket("gone").Object("k").Generation(soft[0].Generation).Restore(t.Context(), &gcs.RestoreOptions{}); err != nil { + t.Fatalf("restore an object in the restored bucket: %v", err) + } + _, err = svc.Buckets.Restore("kept-nothing", 1).Context(t.Context()).Do() + wantErr(t, "restore a bucket that was not kept", err, http.StatusNotFound, "notFound", "") +} + +func TestObjectsUpdateReplacesMetadata(t *testing.T) { + s := storage.New() + url := serve(t, s) + c, svc := clientAt(t, url), rawService(t, url) + if err := c.Bucket("replaced").Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + if _, err := writeWith(t, c, "replaced", "k", "v", gcs.ObjectAttrs{TemporaryHold: true, Metadata: map[string]string{"a": "1"}}); err != nil { + t.Fatal(err) + } + got, err := svc.Objects.Update("replaced", "k", &rawstorage.Object{Metadata: map[string]string{"b": "2"}}).Context(t.Context()).Do() + if err != nil || got.TemporaryHold || len(got.Metadata) != 1 || got.Metadata["b"] != "2" || got.Metageneration != 2 { + t.Fatalf("update = %+v, %v", got, err) + } + if err := c.Bucket("replaced").Object("k").Delete(t.Context()); err != nil { + t.Fatalf("delete after an update released the hold: %v", err) + } +} + +func TestRequesterPays(t *testing.T) { + policy := access.Allow("storage.buckets.create", "storage.buckets.get", "storage.objects.create", serviceUsage) + s := storage.New(storage.WithPolicy(policy)) + c := newClient(t, s) + if err := c.Bucket("payer").Create(t.Context(), project, &gcs.BucketAttrs{RequesterPays: true}); err != nil { + t.Fatal(err) + } + _, err := c.Bucket("payer").Attrs(t.Context()) + wantErr(t, "get without a user project", err, http.StatusBadRequest, "required", "Bucket is a requester pays bucket but no user project provided.") + attrs, err := c.Bucket("payer").UserProject("billed").Attrs(t.Context()) + if err != nil || !attrs.RequesterPays { + t.Fatalf("get with a user project = %+v, %v", attrs, err) + } + if _, err := s.PutBucket(storage.Bucket{Name: "free", Project: project}); err != nil { + t.Fatal(err) + } + if _, err := c.Bucket("free").Attrs(t.Context()); err != nil { + t.Fatalf("get a bucket that is not Requester Pays: %v", err) + } +} + +const serviceUsage = "serviceusage.services.use" From 55b29944bde0074864672cb92a823fc05cce9925 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Thu, 5 Nov 2026 11:23:35 -0600 Subject: [PATCH 2/8] fix(storage): soft-deleted buckets outlive their objects, restore keeps holds and retention, and media reads answer 404 --- gcp/storage/buckets.go | 9 +- gcp/storage/http_test.go | 3 + gcp/storage/objects.go | 12 +-- gcp/storage/permissions.go | 3 +- gcp/storage/protection.go | 15 ++-- gcp/storage/server.go | 55 +++++++----- gcp/storage/softdelete.go | 43 +++++----- sdktest/gcp/storage/protection_test.go | 112 +++++++++++++++++++++++++ 8 files changed, 185 insertions(+), 67 deletions(-) diff --git a/gcp/storage/buckets.go b/gcp/storage/buckets.go index ad3ff4d..53bd9ee 100644 --- a/gcp/storage/buckets.go +++ b/gcp/storage/buckets.go @@ -476,7 +476,7 @@ func (s *Server) store(b Bucket) (*bucket, *apiError) { } func (s *Server) getBucket(r *http.Request) (any, *apiError) { - soft, gen, err := softDeletedGeneration(r) + soft, gen, hasGen, err := softDeletedGeneration(r) if err != nil { return nil, err } @@ -493,6 +493,9 @@ func (s *Server) getBucket(r *http.Request) (any, *apiError) { if err != nil { return nil, err } + if hasGen && gen != b.Generation { + return nil, notFoundBucket() + } if err := checkMatch(r, "ifMetagenerationMatch", "ifMetagenerationNotMatch", b.Metageneration); err != nil { return nil, err } @@ -688,9 +691,7 @@ func (s *Server) LookupBucket(name string) (Bucket, bool) { return b.clone(), true } -// PutBucket creates b in its Project from the fields buckets.insert takes, and returns it as stored. A -// RetentionPolicy may be locked already. With no SoftDeletePolicy, the bucket keeps deletions for 7 days, as -// Cloud Storage's default is. +// PutBucket creates b in its Project from the fields buckets.insert takes (a locked RetentionPolicy too) and returns it. func (s *Server) PutBucket(b Bucket) (Bucket, error) { if b.Project == "" { return Bucket{}, errors.New("squall: a bucket needs a project") diff --git a/gcp/storage/http_test.go b/gcp/storage/http_test.go index 8895875..4454a06 100644 --- a/gcp/storage/http_test.go +++ b/gcp/storage/http_test.go @@ -149,6 +149,9 @@ func TestRetentionAndRestoreRoutes(t *testing.T) { if code, body := do(t, s, http.MethodPost, target, "", ""); code != http.StatusOK || !strings.Contains(body, `"name":"a/restore"`) { t.Fatalf("restore = %d %s", code, body) } + if code, body := do(t, s, http.MethodGet, "/storage/v1/b/routes/o/a%2Frestore?alt=media", "", ""); code != http.StatusNotFound || !strings.Contains(body, `"reason":"notFound"`) { + t.Fatalf("a media read = %d %s", code, body) + } if code, body := do(t, s, http.MethodPost, "/storage/v1/b/routes/o/a%2Frestore/compose", "application/json", "{}"); code != http.StatusNotFound { t.Fatalf("compose = %d %s", code, body) } diff --git a/gcp/storage/objects.go b/gcp/storage/objects.go index d4af684..e417939 100644 --- a/gcp/storage/objects.go +++ b/gcp/storage/objects.go @@ -23,8 +23,7 @@ import ( "unicode/utf8" ) -// Object is one generation of an object, in the API's JSON shape; a noncurrent one has TimeDeleted, and a -// soft-deleted one SoftDeleteTime. +// Object is one generation of an object in the API's JSON shape; noncurrent has TimeDeleted, soft-deleted SoftDeleteTime. type Object struct { Kind string `json:"kind"` ID string `json:"id"` @@ -392,7 +391,7 @@ func (s *Server) target(r *http.Request, permission string) (*bucket, *Object, * } func (s *Server) getObject(r *http.Request) (any, *apiError) { - soft, gen, err := softDeletedGeneration(r) + soft, gen, _, err := softDeletedGeneration(r) if err != nil { return nil, err } @@ -458,9 +457,6 @@ func (s *Server) changeObject(r *http.Request, replace bool) (any, *apiError) { return nil, invalid("Invalid JSON payload received: %v", err) } cleared := func(field string) bool { return string(fields[field]) == "null" } - if !replace && (cleared("temporaryHold") || cleared("eventBasedHold")) { - return nil, invalid("squall: a hold cannot be null; set it to false to release it") - } s.mu.Lock() defer s.mu.Unlock() b, o, err := s.target(r, "storage.objects.update") @@ -476,10 +472,10 @@ func (s *Server) changeObject(r *http.Request, replace bool) (any, *apiError) { temporary, eventBased, metadata = in.TemporaryHold, in.EventBasedHold, nil contentType, contentEncoding = "application/octet-stream", "" } - if in.TemporaryHold != nil { + if in.TemporaryHold != nil || cleared("temporaryHold") { temporary = in.TemporaryHold } - if in.EventBasedHold != nil { + if in.EventBasedHold != nil || cleared("eventBasedHold") { eventBased = in.EventBasedHold } if in.ContentType != nil { diff --git a/gcp/storage/permissions.go b/gcp/storage/permissions.go index 51cd36b..2da979e 100644 --- a/gcp/storage/permissions.go +++ b/gcp/storage/permissions.go @@ -36,8 +36,7 @@ var basePermissions = map[string][]string{ "storage.objects.restore": {"storage.objects.restore", "storage.objects.create"}, } -// The permissions a call on a Requester Pays bucket needs: one naming a user project to bill needs -// serviceUsage, and one naming none createBillingAssignment on the bucket's project. +// Requester Pays calls need serviceUsage with a userProject and createBillingAssignment without one. const ( serviceUsage = "serviceusage.services.use" createBillingAssignment = "resourcemanager.projects.createBillingAssignment" diff --git a/gcp/storage/protection.go b/gcp/storage/protection.go index c8879d8..2ad59a5 100644 --- a/gcp/storage/protection.go +++ b/gcp/storage/protection.go @@ -123,8 +123,7 @@ func (b *bucket) retainedUntil(o *Object) time.Time { return until } -// guard returns the error deleting or replacing o fails with, or with archive, making it noncurrent, which -// only a hold prevents. The caller holds s.mu. +// guard returns why o cannot be deleted or replaced, or with archive, made noncurrent. func (s *Server) guard(b *bucket, o *Object, archive bool) *apiError { kind := "" switch { @@ -156,8 +155,7 @@ func retentionConflict() *apiError { return invalid("An object cannot have both an event-based hold and a retention configuration.") } -// checkRetention reports whether an object in b whose retention is old may take next, nil removing it. -// The caller holds s.mu. +// checkRetention returns next as stored if an object with retention old may take it; nil removes it. func (s *Server) checkRetention(b *bucket, old, next *Retention, override, eventHold bool) (*Retention, *apiError) { var until time.Time if next != nil { @@ -207,8 +205,7 @@ func (s *Server) checkRetention(b *bucket, old, next *Retention, override, event return next, nil } -// SetRetentionPolicy sets the named bucket's retention policy, locked or not, or with nil removes it, without the -// checks a patch makes. An empty EffectiveTime is now. +// SetRetentionPolicy sets or, with nil, removes a bucket's retention policy, locked or not, without a patch's checks. func (s *Server) SetRetentionPolicy(bucketName string, p *RetentionPolicy) (Bucket, error) { s.mu.Lock() defer s.mu.Unlock() @@ -228,8 +225,7 @@ func (s *Server) SetRetentionPolicy(bucketName string, p *RetentionPolicy) (Buck return b.clone(), nil } -// SetObjectHolds sets the holds on name's live generation in the named bucket. Releasing an event-based hold -// starts the object's retention period again, as Cloud Storage does. +// SetObjectHolds sets the holds on a live object; releasing an event-based hold restarts its retention period. func (s *Server) SetObjectHolds(bucketName, name string, temporary, eventBased bool) (Object, error) { s.mu.Lock() defer s.mu.Unlock() @@ -245,8 +241,7 @@ func (s *Server) SetObjectHolds(bucketName, name string, temporary, eventBased b return s.view(b, o), nil } -// SetObjectRetention sets the retention of name's live generation in the named bucket, or with nil removes it, -// without the checks a patch makes, so a test can start from locked retention or one already expired. +// SetObjectRetention sets or, with nil, removes a live object's retention without a patch's checks. func (s *Server) SetObjectRetention(bucketName, name string, r *Retention) (Object, error) { s.mu.Lock() defer s.mu.Unlock() diff --git a/gcp/storage/server.go b/gcp/storage/server.go index 9838fc1..0be16cb 100644 --- a/gcp/storage/server.go +++ b/gcp/storage/server.go @@ -44,9 +44,8 @@ // is rejected with 400. Every bucket reports a generation (inferred). // - Objects record their size, MD5 and CRC32C, not their data; a crc32c or // md5Hash in the upload's metadata must match the data. Resumable -// uploads, media reads, copies, composes and object ACLs are not -// supported. -// Uploads take up to 16 MiB. +// uploads, media reads (objects.get with alt=media answers 404), copies, +// composes and object ACLs are not supported. Uploads take up to 16 MiB. // - With versioning on, writing or deleting an object makes its live // generation noncurrent, and objects.list with versions=true lists those // too, ordered by name and then generation. Deleting a bucket fails with @@ -63,14 +62,14 @@ // it, 412 conditionNotMet on a mismatch), fails with 400 invalid without // an unlocked policy, and changes the bucket's metageneration // (inferred). A locked policy cannot be removed (403 -// retentionPolicyNotMet) or shortened (403 forbidden); the bucket lock -// page says these fail with 400, so the statuses, reasons and messages -// here are inferred. Only an emptied bucket can be deleted, so a locked -// one needs no further check. +// retentionPolicyNotMet) or shortened (403 forbidden), with Cloud +// Storage's messages: the bucket lock page says these fail with 400, but +// Cloud Storage answers 403. Only an emptied bucket can be deleted, so a +// locked one needs no further check. // - temporaryHold and eventBasedHold are set by an upload's metadata, -// objects.patch and objects.update, and defaultEventBasedHold puts an -// event-based hold on each new object, a restored one included -// (inferred). A held generation, live or noncurrent, cannot be deleted, +// objects.patch and objects.update, where null releases one, and +// defaultEventBasedHold puts an event-based hold on each uploaded +// object. A held generation, live or noncurrent, cannot be deleted, // replaced or made noncurrent: 403 forbidden, with Cloud Storage's // message. While an event-based hold is on, an object reports no // retentionExpirationTime, and releasing it starts the object's time in @@ -94,34 +93,43 @@ // policy at the time. objects.list with softDeleted=true lists them (400 // invalid with soft delete off), objects.get with softDeleted=true and a // generation reads one, and objects.restore makes a copy the live -// generation, in STANDARD, retiring the live one as an overwrite would; -// the soft-deleted generation stays until its hardDeleteTime (inferred). +// generation, in STANDARD with the metadata, holds and retention it had +// when deleted, retiring the live one as an overwrite would; the +// soft-deleted generation stays until its hardDeleteTime (inferred). // A restore of a live or noncurrent generation fails with 412 // objectNotSoftDeleted, and in a bucket with soft delete off with 400 // SoftDeletePolicyRequired; both messages are inferred. A soft-deleted // generation cannot be deleted by objects.delete, which answers 404 // (inferred). A bucket holding only soft-deleted generations can be -// deleted, and is then kept soft-deleted with its policy's duration: -// buckets.list and buckets.get with softDeleted=true (and a generation -// for get) read it, and buckets.restore brings it back with its +// deleted, and is then kept soft-deleted for its policy's duration or +// until the last of its soft-deleted generations goes, whichever is +// later, even with soft delete since turned off. buckets.list and +// buckets.get with softDeleted=true (and a generation for get) read it; +// a get of a live bucket with another generation answers 404. +// buckets.restore brings it back with its // generation, metageneration and soft-deleted objects (inferred), // failing with 409 conflict while a live bucket has its name (message // inferred). Everything soft-deleted goes for good at its hardDeleteTime -// on the server's clock. +// on the server's clock, and is held in memory until then: with the +// 7-day default, every deleted or replaced generation stays, so a +// long-running test that churns objects may set +// softDeletePolicy.retentionDurationSeconds to 0 to keep none. // - billing.requesterPays is recorded and, under [WithPolicy], enforced: a // call on a Requester Pays bucket with a userProject also needs // serviceusage.services.use, and one without needs // resourcemanager.projects.createBillingAssignment, lacking which it // fails with 400 and Cloud Storage's message "Bucket is a requester pays -// bucket but no user project provided." (reason required, inferred). +// bucket but no user project provided." The page names the error +// UserProjectMissing, which is the XML API's code; the JSON API's reason +// here, required, is inferred. // Without a policy, the caller may bill the bucket's project, so nothing // is refused. // - objects.list takes prefix, startOffset, endOffset, versions, // softDeleted, pageToken and maxResults; a delimiter, matchGlob, filter, // includeTrailingDelimiter or includeFoldersAsPrefixes is rejected with // 400. objects.patch and objects.update change contentType, -// contentEncoding, metadata, the holds and retention. Page tokens are positions, so a page may follow one whose items -// were deleted. Bucket preconditions are ifMetagenerationMatch and +// contentEncoding, metadata, the holds and retention. Page tokens are +// positions, so a page may follow one whose items were deleted. Bucket preconditions are ifMetagenerationMatch and // ifMetagenerationNotMatch, and object ones ifGenerationMatch, // ifGenerationNotMatch and, but for uploads and deletes, the // metageneration pair. The fields parameter is accepted and ignored: @@ -270,7 +278,14 @@ func (s *Server) registerRoutes() { s.route("GET "+bucket+"/o", "storage.objects.list", s.listObjects, nil, "projection", "prefix", "delimiter", "pageToken", "maxResults", "versions", "startOffset", "endOffset", "includeTrailingDelimiter", "matchGlob", "includeFoldersAsPrefixes", "softDeleted", "filter") - s.route("GET "+object, "storage.objects.get", s.getObject, objectTarget, slices.Concat(objectConds, []string{"projection", "softDeleted"})...) + get := s.handle("storage.objects.get", s.getObject, objectTarget, slices.Concat(objectConds, []string{"projection", "softDeleted"})...) + s.mux.HandleFunc("GET "+object, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("alt") == "media" { + notImplemented(w, r) + return + } + get(w, r) + }) s.route("PATCH "+object, "storage.objects.patch", s.patchObject, objectTarget, slices.Concat(objectConds, []string{"projection", "predefinedAcl", "overrideUnlockedRetention"})...) s.route("PUT "+object, "storage.objects.update", s.updateObject, objectTarget, diff --git a/gcp/storage/softdelete.go b/gcp/storage/softdelete.go index 8f7c779..c025496 100644 --- a/gcp/storage/softdelete.go +++ b/gcp/storage/softdelete.go @@ -10,23 +10,18 @@ import ( "time" ) -// softDeletedGeneration reads the softDeleted and generation parameters of a get, each of which needs the other. -func softDeletedGeneration(r *http.Request) (bool, int64, *apiError) { - soft, err := boolParam(r, "softDeleted") - if err != nil { - return false, 0, err +// softDeletedGeneration reads a get's softDeleted and generation parameters; softDeleted needs a generation. +func softDeletedGeneration(r *http.Request) (soft bool, gen int64, hasGen bool, err *apiError) { + if soft, err = boolParam(r, "softDeleted"); err != nil { + return false, 0, false, err } - gen, ok, err := int64Param(r, "generation") - if err != nil { - return false, 0, err + if gen, hasGen, err = int64Param(r, "generation"); err != nil { + return false, 0, false, err } - switch { - case soft && !ok: - return false, 0, required("generation") - case ok && !soft && r.PathValue("object") == "": - return false, 0, required("softDeleted") + if soft && !hasGen { + return false, 0, false, required("generation") } - return soft, gen, nil + return soft, gen, hasGen, nil } // trashed returns soft-deleted generation gen of name in b, or nil. @@ -59,14 +54,19 @@ func (b *bucket) keepUntil(t time.Time) { } } -// softDeleteBucket keeps b, which was just deleted, if it keeps deletions. The caller holds s.mu. +// softDeleteBucket keeps b, which was just deleted, for its policy's duration or while it holds soft-deleted objects. func (s *Server) softDeleteBucket(b *bucket) { keep := b.softDeleteDuration() - if keep == 0 { + if keep == 0 && len(b.trash) == 0 { return } t, now := s.timestamp() b.SoftDeleteTime, b.hardDelete = now, t.Add(keep) + for _, o := range b.trash { + if o.hardDelete.After(b.hardDelete) { + b.hardDelete = o.hardDelete + } + } b.HardDeleteTime = b.hardDelete.Format(timeFormat) s.softBuckets[b.Name] = append(s.softBuckets[b.Name], b) } @@ -186,14 +186,12 @@ func (s *Server) restoreObject(r *http.Request) (any, *apiError) { o := s.newGeneration(b, name, "STANDARD") o.ContentType, o.ContentEncoding, o.Metadata = src.ContentType, src.ContentEncoding, maps.Clone(src.Metadata) o.Size, o.MD5Hash, o.CRC32C = src.Size, src.MD5Hash, src.CRC32C - if b.DefaultEventBasedHold { - o.EventBasedHold = new(true) - } + restored := src.clone() + o.TemporaryHold, o.EventBasedHold, o.Retention = restored.TemporaryHold, restored.EventBasedHold, restored.Retention return s.view(b, o), nil } -// SoftDeletedObjects returns the soft-deleted generations in the named bucket whose retention has not ended, -// ordered by name and generation. +// SoftDeletedObjects returns a bucket's unexpired soft-deleted generations, ordered by name and generation. func (s *Server) SoftDeletedObjects(bucketName string) []Object { s.mu.Lock() defer s.mu.Unlock() @@ -218,8 +216,7 @@ func (s *Server) SoftDeletedBuckets() []Bucket { return s.softDeletedBuckets() } -// PutSoftDeletedObject writes data to name in the named bucket as a soft-deleted generation, as if a live one had -// just been deleted, and returns it. The bucket must keep deletions. +// PutSoftDeletedObject adds data as a just soft-deleted generation of name, in a bucket that keeps deletions. func (s *Server) PutSoftDeletedObject(bucketName, name string, data []byte) (Object, error) { s.mu.Lock() defer s.mu.Unlock() diff --git a/sdktest/gcp/storage/protection_test.go b/sdktest/gcp/storage/protection_test.go index 227f199..0826784 100644 --- a/sdktest/gcp/storage/protection_test.go +++ b/sdktest/gcp/storage/protection_test.go @@ -551,3 +551,115 @@ func TestRequesterPays(t *testing.T) { } const serviceUsage = "serviceusage.services.use" + +func TestSoftDeletedBucketsOutliveTheirObjects(t *testing.T) { + clk := newClock() + s := storage.New(storage.WithClock(clk.Now)) + url := serve(t, s) + c, svc := clientAt(t, url), rawService(t, url) + for _, name := range []string{"shortened", "turned-off"} { + if err := c.Bucket(name).Create(t.Context(), project, &gcs.BucketAttrs{SoftDeletePolicy: &gcs.SoftDeletePolicy{RetentionDuration: 90 * 24 * time.Hour}}); err != nil { + t.Fatal(err) + } + write(t, c, name, "k", "v") + if err := c.Bucket(name).Object("k").Delete(t.Context()); err != nil { + t.Fatal(err) + } + } + if _, err := c.Bucket("shortened").Update(t.Context(), gcs.BucketAttrsToUpdate{SoftDeletePolicy: &gcs.SoftDeletePolicy{RetentionDuration: 7 * 24 * time.Hour}}); err != nil { + t.Fatal(err) + } + if _, err := c.Bucket("turned-off").Update(t.Context(), gcs.BucketAttrsToUpdate{SoftDeletePolicy: &gcs.SoftDeletePolicy{}}); err != nil { + t.Fatal(err) + } + for _, name := range []string{"shortened", "turned-off"} { + if err := c.Bucket(name).Delete(t.Context()); err != nil { + t.Fatal(err) + } + } + clk.Advance(30 * 24 * time.Hour) + kept := s.SoftDeletedBuckets() + want := clk.Now().Add(60 * 24 * time.Hour).Format("2006-01-02T15:04:05.000Z") + if len(kept) != 2 || kept[0].Name != "shortened" || kept[1].Name != "turned-off" || kept[0].HardDeleteTime != want || kept[1].HardDeleteTime != want { + t.Fatalf("soft-deleted buckets = %+v, want both until %s", kept, want) + } + if _, err := svc.Buckets.Restore("shortened", kept[0].Generation).Context(t.Context()).Do(); err != nil { + t.Fatal(err) + } + soft := objects(t, c.Bucket("shortened").Objects(t.Context(), &gcs.Query{SoftDeleted: true})) + if len(soft) != 1 { + t.Fatalf("soft-deleted objects in the restored bucket = %+v", soft) + } + if _, err := c.Bucket("shortened").Object("k").Generation(soft[0].Generation).Restore(t.Context(), &gcs.RestoreOptions{}); err != nil { + t.Fatalf("restore an object that outlived the shortened policy: %v", err) + } + clk.Advance(60 * 24 * time.Hour) + if kept := s.SoftDeletedBuckets(); len(kept) != 0 { + t.Fatalf("soft-deleted buckets after their objects expired = %+v", kept) + } +} + +func TestRestoreKeepsHoldsAndRetention(t *testing.T) { + clk := newClock() + s := storage.New(storage.WithClock(clk.Now)) + c := newClient(t, s) + b := c.Bucket("restored-metadata") + if err := b.SetObjectRetention(true).Create(t.Context(), project, &gcs.BucketAttrs{DefaultEventBasedHold: true}); err != nil { + t.Fatal(err) + } + if _, err := writeWith(t, c, "restored-metadata", "k", "v", gcs.ObjectAttrs{Metadata: map[string]string{"a": "1"}}); err != nil { + t.Fatal(err) + } + until := clk.Now().Add(time.Hour) + if _, err := b.Object("k").Update(t.Context(), gcs.ObjectAttrsToUpdate{ + EventBasedHold: false, Retention: &gcs.ObjectRetention{Mode: "Unlocked", RetainUntil: until}, + }); err != nil { + t.Fatal(err) + } + clk.Advance(2 * time.Hour) + if err := b.Object("k").Delete(t.Context()); err != nil { + t.Fatal(err) + } + gen := s.SoftDeletedObjects("restored-metadata")[0].Generation + restored, err := b.Object("k").Generation(gen).Restore(t.Context(), &gcs.RestoreOptions{}) + if err != nil || restored.EventBasedHold || restored.Metadata["a"] != "1" || restored.Retention == nil || !restored.Retention.RetainUntil.Equal(until) { + t.Fatalf("restore = %+v, %v", restored, err) + } +} + +func TestLiveBucketGetByGeneration(t *testing.T) { + s := storage.New() + url := serve(t, s) + c, svc := clientAt(t, url), rawService(t, url) + if err := c.Bucket("live").Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + got, err := svc.Buckets.Get("live").Context(t.Context()).Do() + if err != nil { + t.Fatal(err) + } + if _, err := svc.Buckets.Get("live").Generation(got.Generation).Context(t.Context()).Do(); err != nil { + t.Fatalf("get the live generation: %v", err) + } + _, err = svc.Buckets.Get("live").Generation(got.Generation + 1).Context(t.Context()).Do() + wantErr(t, "get another generation", err, http.StatusNotFound, "notFound", "") +} + +func TestNullHoldReleases(t *testing.T) { + s := storage.New() + url := serve(t, s) + c, svc := clientAt(t, url), rawService(t, url) + if err := c.Bucket("nulls").Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + if _, err := writeWith(t, c, "nulls", "k", "v", gcs.ObjectAttrs{TemporaryHold: true}); err != nil { + t.Fatal(err) + } + got, err := svc.Objects.Patch("nulls", "k", &rawstorage.Object{NullFields: []string{"TemporaryHold"}}).Context(t.Context()).Do() + if err != nil || got.TemporaryHold { + t.Fatalf("patch a null hold = %+v, %v", got, err) + } + if err := c.Bucket("nulls").Object("k").Delete(t.Context()); err != nil { + t.Fatalf("delete after a null hold: %v", err) + } +} From c5934dd58e8a44c9beb94344da98fa5586ff8942 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Thu, 5 Nov 2026 11:32:42 -0600 Subject: [PATCH 3/8] fix(storage): restore preconditions, soft delete effective times and bucket gets follow the API reference --- gcp/storage/buckets.go | 6 +- gcp/storage/http_test.go | 3 + gcp/storage/objects.go | 2 +- gcp/storage/protection.go | 8 +- gcp/storage/server.go | 103 ++++++++++++++----------- gcp/storage/softdelete.go | 4 +- sdktest/gcp/storage/protection_test.go | 62 +++++++++++++-- 7 files changed, 127 insertions(+), 61 deletions(-) diff --git a/gcp/storage/buckets.go b/gcp/storage/buckets.go index 53bd9ee..9b203fe 100644 --- a/gcp/storage/buckets.go +++ b/gcp/storage/buckets.go @@ -480,6 +480,9 @@ func (s *Server) getBucket(r *http.Request) (any, *apiError) { if err != nil { return nil, err } + if hasGen && !soft { + return nil, required("softDeleted") + } s.mu.Lock() defer s.mu.Unlock() if soft { @@ -493,9 +496,6 @@ func (s *Server) getBucket(r *http.Request) (any, *apiError) { if err != nil { return nil, err } - if hasGen && gen != b.Generation { - return nil, notFoundBucket() - } if err := checkMatch(r, "ifMetagenerationMatch", "ifMetagenerationNotMatch", b.Metageneration); err != nil { return nil, err } diff --git a/gcp/storage/http_test.go b/gcp/storage/http_test.go index 4454a06..ef7fe7d 100644 --- a/gcp/storage/http_test.go +++ b/gcp/storage/http_test.go @@ -152,6 +152,9 @@ func TestRetentionAndRestoreRoutes(t *testing.T) { if code, body := do(t, s, http.MethodGet, "/storage/v1/b/routes/o/a%2Frestore?alt=media", "", ""); code != http.StatusNotFound || !strings.Contains(body, `"reason":"notFound"`) { t.Fatalf("a media read = %d %s", code, body) } + if code, body := do(t, s, http.MethodPost, "/storage/v1/b/routes/o/src/rewriteTo/b/routes/o/dst/restore", "application/json", "{}"); code != http.StatusNotFound { + t.Fatalf("a rewrite to an object named dst/restore = %d %s", code, body) + } if code, body := do(t, s, http.MethodPost, "/storage/v1/b/routes/o/a%2Frestore/compose", "application/json", "{}"); code != http.StatusNotFound { t.Fatalf("compose = %d %s", code, body) } diff --git a/gcp/storage/objects.go b/gcp/storage/objects.go index e417939..820e9c3 100644 --- a/gcp/storage/objects.go +++ b/gcp/storage/objects.go @@ -560,7 +560,7 @@ func (s *Server) listObjects(r *http.Request) (any, *apiError) { } var all []Object if soft { - if b.softDeleteDuration() == 0 { + if b.softDeleteDuration() == 0 && len(b.trash) == 0 { return nil, invalid("Soft delete policy is required to list soft-deleted versions") } for _, o := range b.trash { diff --git a/gcp/storage/protection.go b/gcp/storage/protection.go index 2ad59a5..b97a04a 100644 --- a/gcp/storage/protection.go +++ b/gcp/storage/protection.go @@ -16,7 +16,7 @@ type Retention struct { const untilFormat = "2006-01-02T15:04:05.000000Z07:00" -func validRetentionPeriod(seconds int64) bool { return seconds > 0 && seconds < maxRetentionPeriod } +func validRetentionPeriod(seconds int64) bool { return seconds > 0 && seconds <= maxRetentionPeriod } func validSoftDelete(seconds int64) bool { return seconds == 0 || seconds >= minSoftDelete && seconds <= maxSoftDelete @@ -76,7 +76,11 @@ func (s *Server) nextSoftDeletePolicy(b *bucket, in *softDeletePolicyInput) (*So return new(*old), nil } p := &SoftDeletePolicy{RetentionDurationSeconds: seconds} - if seconds > 0 { + switch old := b.SoftDeletePolicy; { + case seconds == 0: + case old != nil && old.RetentionDurationSeconds > seconds: + p.EffectiveTime = old.EffectiveTime + default: _, p.EffectiveTime = s.timestamp() } return p, nil diff --git a/gcp/storage/server.go b/gcp/storage/server.go index 0be16cb..8654a47 100644 --- a/gcp/storage/server.go +++ b/gcp/storage/server.go @@ -50,22 +50,23 @@ // generation noncurrent, and objects.list with versions=true lists those // too, ordered by name and then generation. Deleting a bucket fails with // 409 conflict while any generation, live or noncurrent, remains. -// - A retention policy's retentionPeriod is greater than 0 and less than -// 100 years, and its effectiveTime is when it was set or its period last -// changed (inferred). Each object reports retentionExpirationTime, its -// time in the bucket plus the period. Deleting or replacing an object -// before then, or before its own retention's retainUntilTime, fails with -// 403 retentionPolicyNotMet and Cloud Storage's message; the time in it -// is in UTC, where Cloud Storage writes Pacific time. Making a live -// object noncurrent in a versioned bucket is allowed, as documented. -// lockRetentionPolicy needs ifMetagenerationMatch (400 required without -// it, 412 conditionNotMet on a mismatch), fails with 400 invalid without -// an unlocked policy, and changes the bucket's metageneration -// (inferred). A locked policy cannot be removed (403 -// retentionPolicyNotMet) or shortened (403 forbidden), with Cloud -// Storage's messages: the bucket lock page says these fail with 400, but -// Cloud Storage answers 403. Only an emptied bucket can be deleted, so a -// locked one needs no further check. +// - A retention policy's retentionPeriod is greater than 0 and at most +// 3,155,760,000 seconds (100 years), and its effectiveTime is when it +// was set or its period last changed (inferred). Each object reports +// retentionExpirationTime, its time in the bucket plus the period. +// Deleting or replacing an object before then, or before its own +// retention's retainUntilTime, fails with 403 retentionPolicyNotMet and +// Cloud Storage's message; the time in it is in UTC, where Cloud Storage +// writes Pacific time. Making a live object noncurrent in a versioned +// bucket is allowed, as documented. lockRetentionPolicy needs +// ifMetagenerationMatch (400 required without it, 412 conditionNotMet on +// a mismatch), fails with 400 invalid without an unlocked policy, and +// changes the bucket's metageneration (inferred). A locked policy cannot +// be removed (403 retentionPolicyNotMet) or shortened (403 forbidden), +// with Cloud Storage's messages: the bucket lock page says these fail +// with 400, but Cloud Storage answers 403, as the API reference's +// PERMISSION_DENIED for them says. Only an emptied bucket can be +// deleted, so a locked one needs no further check. // - temporaryHold and eventBasedHold are set by an upload's metadata, // objects.patch and objects.update, where null releases one, and // defaultEventBasedHold puts an event-based hold on each uploaded @@ -85,34 +86,39 @@ // storage.objects.setRetention, and retention beside an event-based hold // with 400 invalid (both inferred). The 100-year cap on retainUntilTime // is not checked. -// - softDeletePolicy.retentionDurationSeconds is 0, which turns soft delete -// off, or 7 to 90 days, and a new bucket keeps deletions for 7 days. A +// - softDeletePolicy.retentionDurationSeconds is 0, which turns soft +// delete off, or 7 to 90 days, and a new bucket keeps deletions for 7 +// days. Its effectiveTime is when the duration last rose, since lowering +// it leaves a policy with greater retention in effect from then. A // generation that leaves the bucket, by a delete, by being replaced in a // bucket without versioning or by a delete of a noncurrent generation, // is kept soft-deleted with softDeleteTime and hardDeleteTime, on the // policy at the time. objects.list with softDeleted=true lists them (400 -// invalid with soft delete off), objects.get with softDeleted=true and a -// generation reads one, and objects.restore makes a copy the live -// generation, in STANDARD with the metadata, holds and retention it had -// when deleted, retiring the live one as an overwrite would; the -// soft-deleted generation stays until its hardDeleteTime (inferred). -// A restore of a live or noncurrent generation fails with 412 -// objectNotSoftDeleted, and in a bucket with soft delete off with 400 -// SoftDeletePolicyRequired; both messages are inferred. A soft-deleted -// generation cannot be deleted by objects.delete, which answers 404 -// (inferred). A bucket holding only soft-deleted generations can be -// deleted, and is then kept soft-deleted for its policy's duration or -// until the last of its soft-deleted generations goes, whichever is -// later, even with soft delete since turned off. buckets.list and -// buckets.get with softDeleted=true (and a generation for get) read it; -// a get of a live bucket with another generation answers 404. -// buckets.restore brings it back with its -// generation, metageneration and soft-deleted objects (inferred), -// failing with 409 conflict while a live bucket has its name (message -// inferred). Everything soft-deleted goes for good at its hardDeleteTime -// on the server's clock, and is held in memory until then: with the -// 7-day default, every deleted or replaced generation stays, so a -// long-running test that churns objects may set +// invalid with soft delete off and none kept), objects.get with +// softDeleted=true and a generation reads one, and objects.restore makes +// a copy the live generation, in STANDARD with the metadata, holds and +// retention it had when deleted, retiring the live one as an overwrite +// would; the soft-deleted generation stays until its hardDeleteTime +// (inferred). A restore of a live or noncurrent generation fails with +// 412 objectNotSoftDeleted, and in a bucket with soft delete off and +// none kept with 400 SoftDeletePolicyRequired; both messages are +// inferred. ifGenerationNotMatch and the metageneration pair fail with +// 412 when there is no live object. A soft-deleted generation cannot be +// deleted by objects.delete, which answers 404 (inferred). A bucket +// holding only soft-deleted generations can be deleted, and is then kept +// soft-deleted for its policy's duration or until the last of its +// soft-deleted generations goes, whichever is later, even with soft +// delete since turned off (inferred); once restored, those generations +// can still be listed and restored. buckets.list and buckets.get with +// softDeleted=true read it; buckets.get takes a generation only with +// softDeleted=true and answers 400 required otherwise, as its reference +// says. buckets.restore brings it back with its generation, +// metageneration and soft-deleted objects (inferred), failing with 409 +// conflict while a live bucket has its name (message inferred). +// Everything soft-deleted goes for good at its hardDeleteTime on the +// server's clock, and is held in memory until then: with the 7-day +// default, every deleted or replaced generation stays, so a long-running +// test that churns objects may set // softDeletePolicy.retentionDurationSeconds to 0 to keep none. // - billing.requesterPays is recorded and, under [WithPolicy], enforced: a // call on a Requester Pays bucket with a userProject also needs @@ -121,16 +127,16 @@ // fails with 400 and Cloud Storage's message "Bucket is a requester pays // bucket but no user project provided." The page names the error // UserProjectMissing, which is the XML API's code; the JSON API's reason -// here, required, is inferred. -// Without a policy, the caller may bill the bucket's project, so nothing -// is refused. +// here, required, is inferred. Without a policy, the caller may bill the +// bucket's project, so nothing is refused. // - objects.list takes prefix, startOffset, endOffset, versions, // softDeleted, pageToken and maxResults; a delimiter, matchGlob, filter, // includeTrailingDelimiter or includeFoldersAsPrefixes is rejected with // 400. objects.patch and objects.update change contentType, // contentEncoding, metadata, the holds and retention. Page tokens are -// positions, so a page may follow one whose items were deleted. Bucket preconditions are ifMetagenerationMatch and -// ifMetagenerationNotMatch, and object ones ifGenerationMatch, +// positions, so a page may follow one whose items were deleted. Bucket +// preconditions are ifMetagenerationMatch and ifMetagenerationNotMatch, +// and object ones ifGenerationMatch, // ifGenerationNotMatch and, but for uploads and deletes, the // metageneration pair. The fields parameter is accepted and ignored: // responses are always whole, and userProject is checked only as the @@ -171,6 +177,7 @@ import ( "fmt" "io" "net/http" + "net/url" "path" "slices" "strconv" @@ -296,8 +303,10 @@ func (s *Server) registerRoutes() { slices.Concat(objectConds, []string{"projection", "copySourceAcl"})...) // ServeMux allows a wildcard only at the end, and objects.restore puts a verb after the object name. s.mux.HandleFunc("POST "+object, func(w http.ResponseWriter, r *http.Request) { - name, ok := strings.CutSuffix(r.PathValue("object"), "/restore") - if !ok { + _, escaped, _ := strings.Cut(r.URL.EscapedPath(), "/o/") + escaped, ok := strings.CutSuffix(escaped, "/restore") + name, err := url.PathUnescape(escaped) + if !ok || strings.Contains(escaped, "/") || err != nil { notImplemented(w, r) return } diff --git a/gcp/storage/softdelete.go b/gcp/storage/softdelete.go index c025496..6bb6412 100644 --- a/gcp/storage/softdelete.go +++ b/gcp/storage/softdelete.go @@ -156,7 +156,7 @@ func (s *Server) restoreObject(r *http.Request) (any, *apiError) { if err != nil { return nil, err } - if b.softDeleteDuration() == 0 { + if b.softDeleteDuration() == 0 && len(b.trash) == 0 { return nil, &apiError{Status: http.StatusBadRequest, Reason: "SoftDeletePolicyRequired", Message: "Soft delete policy is required to restore soft-deleted objects."} } src := b.trashed(name, gen) @@ -174,7 +174,7 @@ func (s *Server) restoreObject(r *http.Request) (any, *apiError) { if err := checkMatch(r, "ifGenerationMatch", "ifGenerationNotMatch", liveGen); err != nil { return nil, err } - if live == nil && (r.URL.Query().Has("ifMetagenerationMatch") || r.URL.Query().Has("ifMetagenerationNotMatch")) { + if live == nil && (r.URL.Query().Has("ifGenerationNotMatch") || r.URL.Query().Has("ifMetagenerationMatch") || r.URL.Query().Has("ifMetagenerationNotMatch")) { return nil, conditionNotMet() } if err := checkMatch(r, "ifMetagenerationMatch", "ifMetagenerationNotMatch", liveMeta); err != nil { diff --git a/sdktest/gcp/storage/protection_test.go b/sdktest/gcp/storage/protection_test.go index 0826784..506febe 100644 --- a/sdktest/gcp/storage/protection_test.go +++ b/sdktest/gcp/storage/protection_test.go @@ -145,7 +145,13 @@ func TestUnlockedRetentionPolicyChanges(t *testing.T) { if err := b.Create(t.Context(), project, nil); err != nil { t.Fatal(err) } - for _, period := range []time.Duration{-time.Second, 100 * 366 * 24 * time.Hour} { + if _, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{RetentionPolicy: &gcs.RetentionPolicy{RetentionPeriod: 3_155_760_000 * time.Second}}); err != nil { + t.Fatalf("the maximum period: %v", err) + } + if _, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{RetentionPolicy: &gcs.RetentionPolicy{}}); err != nil { + t.Fatal(err) + } + for _, period := range []time.Duration{-time.Second, 3_155_760_001 * time.Second} { _, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{RetentionPolicy: &gcs.RetentionPolicy{RetentionPeriod: period}}) wantErr(t, "an out of range period", err, http.StatusBadRequest, "invalid", "greater than 0 and less than 100 years") } @@ -364,6 +370,15 @@ func TestRestoreObjects(t *testing.T) { _, err = b.Object("k").Generation(second.Generation).Restore(t.Context(), &gcs.RestoreOptions{}) wantErr(t, "restore over a held object", err, http.StatusForbidden, "forbidden", "Temporary hold") + if _, err := s.SetObjectHolds("restorable", "k", false, false); err != nil { + t.Fatal(err) + } + if err := b.Object("k").Delete(t.Context()); err != nil { + t.Fatal(err) + } + _, err = b.Object("k").Generation(first.Generation).If(gcs.Conditions{GenerationNotMatch: 5}).Restore(t.Context(), &gcs.RestoreOptions{}) + wantErr(t, "restore with ifGenerationNotMatch and no live object", err, http.StatusPreconditionFailed, "conditionNotMet", "") + clk.Advance(7 * 24 * time.Hour) if _, err := b.Object("k").Generation(first.Generation).Restore(t.Context(), &gcs.RestoreOptions{}); !errors.Is(err, gcs.ErrObjectNotExist) { t.Fatalf("restore after retention = %v", err) @@ -593,6 +608,24 @@ func TestSoftDeletedBucketsOutliveTheirObjects(t *testing.T) { if _, err := c.Bucket("shortened").Object("k").Generation(soft[0].Generation).Restore(t.Context(), &gcs.RestoreOptions{}); err != nil { t.Fatalf("restore an object that outlived the shortened policy: %v", err) } + if _, err := svc.Buckets.Restore("turned-off", kept[1].Generation).Context(t.Context()).Do(); err != nil { + t.Fatal(err) + } + soft = objects(t, c.Bucket("turned-off").Objects(t.Context(), &gcs.Query{SoftDeleted: true})) + if len(soft) != 1 { + t.Fatalf("soft-deleted objects in a restored bucket with soft delete off = %+v", soft) + } + if _, err := c.Bucket("turned-off").Object("k").Generation(soft[0].Generation).Restore(t.Context(), &gcs.RestoreOptions{}); err != nil { + t.Fatalf("restore an object kept after soft delete was turned off: %v", err) + } + for _, name := range []string{"shortened", "turned-off"} { + if err := c.Bucket(name).Object("k").Delete(t.Context()); err != nil { + t.Fatal(err) + } + if err := c.Bucket(name).Delete(t.Context()); err != nil { + t.Fatal(err) + } + } clk.Advance(60 * 24 * time.Hour) if kept := s.SoftDeletedBuckets(); len(kept) != 0 { t.Fatalf("soft-deleted buckets after their objects expired = %+v", kept) @@ -638,11 +671,8 @@ func TestLiveBucketGetByGeneration(t *testing.T) { if err != nil { t.Fatal(err) } - if _, err := svc.Buckets.Get("live").Generation(got.Generation).Context(t.Context()).Do(); err != nil { - t.Fatalf("get the live generation: %v", err) - } - _, err = svc.Buckets.Get("live").Generation(got.Generation + 1).Context(t.Context()).Do() - wantErr(t, "get another generation", err, http.StatusNotFound, "notFound", "") + _, err = svc.Buckets.Get("live").Generation(got.Generation).Context(t.Context()).Do() + wantErr(t, "get a generation without softDeleted", err, http.StatusBadRequest, "required", "softDeleted") } func TestNullHoldReleases(t *testing.T) { @@ -663,3 +693,23 @@ func TestNullHoldReleases(t *testing.T) { t.Fatalf("delete after a null hold: %v", err) } } + +func TestLoweringSoftDeleteKeepsEffectiveTime(t *testing.T) { + clk := newClock() + c := newClient(t, storage.New(storage.WithClock(clk.Now))) + b := c.Bucket("lowered") + if err := b.Create(t.Context(), project, &gcs.BucketAttrs{SoftDeletePolicy: &gcs.SoftDeletePolicy{RetentionDuration: 30 * 24 * time.Hour}}); err != nil { + t.Fatal(err) + } + created := clk.Now() + clk.Advance(time.Hour) + lowered, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{SoftDeletePolicy: &gcs.SoftDeletePolicy{RetentionDuration: 7 * 24 * time.Hour}}) + if err != nil || !lowered.SoftDeletePolicy.EffectiveTime.Equal(created) { + t.Fatalf("lowered = %+v, %v, want effective since %v", lowered.SoftDeletePolicy, err, created) + } + clk.Advance(time.Hour) + raised, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{SoftDeletePolicy: &gcs.SoftDeletePolicy{RetentionDuration: 60 * 24 * time.Hour}}) + if err != nil || !raised.SoftDeletePolicy.EffectiveTime.Equal(clk.Now()) { + t.Fatalf("raised = %+v, %v", raised.SoftDeletePolicy, err) + } +} From ac0b7880b901372adc70b7372d42bd75274573e5 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Thu, 5 Nov 2026 11:33:13 -0600 Subject: [PATCH 4/8] docs(storage): attribute the locked retention policy status to the discovery document --- gcp/storage/server.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gcp/storage/server.go b/gcp/storage/server.go index 8654a47..c208532 100644 --- a/gcp/storage/server.go +++ b/gcp/storage/server.go @@ -64,9 +64,10 @@ // changes the bucket's metageneration (inferred). A locked policy cannot // be removed (403 retentionPolicyNotMet) or shortened (403 forbidden), // with Cloud Storage's messages: the bucket lock page says these fail -// with 400, but Cloud Storage answers 403, as the API reference's -// PERMISSION_DENIED for them says. Only an emptied bucket can be -// deleted, so a locked one needs no further check. +// with 400, but Cloud Storage answers 403, the PERMISSION_DENIED that +// the retentionPolicy field description in the JSON API discovery +// document gives for them. Only an emptied bucket can be deleted, so a +// locked one needs no further check. // - temporaryHold and eventBasedHold are set by an upload's metadata, // objects.patch and objects.update, where null releases one, and // defaultEventBasedHold puts an event-based hold on each uploaded From 8634a40797e25fed3f0c2c7af5589211da115471 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Thu, 5 Nov 2026 11:44:13 -0600 Subject: [PATCH 5/8] fix(storage): locking unlocked retention needs the override, updates take a get's body, and soft delete effective times follow their history --- gcp/storage/buckets.go | 5 +- gcp/storage/objects.go | 33 ++++++++ gcp/storage/protection.go | 36 ++++++-- gcp/storage/server.go | 73 +++++++++-------- gcp/storage/softdelete.go | 4 +- sdktest/gcp/storage/permissions_test.go | 19 +++++ sdktest/gcp/storage/protection_test.go | 104 +++++++++++++++++++++--- 7 files changed, 219 insertions(+), 55 deletions(-) diff --git a/gcp/storage/buckets.go b/gcp/storage/buckets.go index 9b203fe..a69ab43 100644 --- a/gcp/storage/buckets.go +++ b/gcp/storage/buckets.go @@ -104,7 +104,8 @@ type bucket struct { // objects holds each name's generations, oldest first; the last is live unless it has TimeDeleted. objects map[string][]*Object // trash holds soft-deleted generations, in the order they were deleted. - trash []*Object + trash []*Object + softDeleteHistory []policyStep // nextPurge is when the first of trash goes for good. nextPurge time.Time // hardDelete is when a soft-deleted bucket goes for good. @@ -471,6 +472,7 @@ func (s *Server) store(b Bucket) (*bucket, *apiError) { } stored := &bucket{Bucket: b, objects: map[string][]*Object{}} stored.Bucket = stored.clone() + s.recordSoftDelete(stored) s.buckets[b.Name] = stored return stored, nil } @@ -575,6 +577,7 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { labels = nil } b.Labels, b.StorageClass, b.RetentionPolicy, b.SoftDeletePolicy = labels, class, retention, softDelete + s.recordSoftDelete(b) if in.DefaultEventBasedHold != nil { b.DefaultEventBasedHold = *in.DefaultEventBasedHold } else if cleared("defaultEventBasedHold") { diff --git a/gcp/storage/objects.go b/gcp/storage/objects.go index 820e9c3..aff2e07 100644 --- a/gcp/storage/objects.go +++ b/gcp/storage/objects.go @@ -242,6 +242,9 @@ func (s *Server) insertObject(r *http.Request) (any, *apiError) { s.mu.Lock() defer s.mu.Unlock() o, err := s.writeObject(bucketName, in, data, func(live int64) *apiError { + if live == 0 && r.URL.Query().Has("ifGenerationNotMatch") { + return conditionNotMet() + } return checkMatch(r, "ifGenerationMatch", "ifGenerationNotMatch", live) }) if err != nil { @@ -429,6 +432,33 @@ type objectPatch struct { TemporaryHold *bool `json:"temporaryHold"` EventBasedHold *bool `json:"eventBasedHold"` Retention *Retention `json:"retention"` + StorageClass string `json:"storageClass"` + outputOnly +} + +// outputOnly holds the Object fields the server sets, which a read-modify-write sends back and which are ignored. +type outputOnly struct { + Kind json.RawMessage `json:"kind"` + ID json.RawMessage `json:"id"` + SelfLink json.RawMessage `json:"selfLink"` + MediaLink json.RawMessage `json:"mediaLink"` + Generation json.RawMessage `json:"generation"` + Metageneration json.RawMessage `json:"metageneration"` + Size json.RawMessage `json:"size"` + MD5Hash json.RawMessage `json:"md5Hash"` + CRC32C json.RawMessage `json:"crc32c"` + Etag json.RawMessage `json:"etag"` + TimeCreated json.RawMessage `json:"timeCreated"` + Updated json.RawMessage `json:"updated"` + TimeStorageClassUpdated json.RawMessage `json:"timeStorageClassUpdated"` + TimeFinalized json.RawMessage `json:"timeFinalized"` + TimeDeleted json.RawMessage `json:"timeDeleted"` + SoftDeleteTime json.RawMessage `json:"softDeleteTime"` + HardDeleteTime json.RawMessage `json:"hardDeleteTime"` + RetentionExpirationTime json.RawMessage `json:"retentionExpirationTime"` + ComponentCount json.RawMessage `json:"componentCount"` + Owner json.RawMessage `json:"owner"` + RestoreToken json.RawMessage `json:"restoreToken"` } func (s *Server) patchObject(r *http.Request) (any, *apiError) { return s.changeObject(r, false) } @@ -466,6 +496,9 @@ func (s *Server) changeObject(r *http.Request, replace bool) (any, *apiError) { if in.Bucket != "" && in.Bucket != b.Name || in.Name != "" && in.Name != o.Name { return nil, invalid("squall: an object's bucket and name cannot be changed") } + if in.StorageClass != "" && in.StorageClass != o.StorageClass { + return nil, invalid("squall: changing an object's storage class needs objects.rewrite, which is not supported") + } temporary, eventBased, metadata := o.TemporaryHold, o.EventBasedHold, maps.Clone(o.Metadata) contentType, contentEncoding := o.ContentType, o.ContentEncoding if replace { diff --git a/gcp/storage/protection.go b/gcp/storage/protection.go index b97a04a..8349d63 100644 --- a/gcp/storage/protection.go +++ b/gcp/storage/protection.go @@ -76,16 +76,35 @@ func (s *Server) nextSoftDeletePolicy(b *bucket, in *softDeletePolicyInput) (*So return new(*old), nil } p := &SoftDeletePolicy{RetentionDurationSeconds: seconds} - switch old := b.SoftDeletePolicy; { - case seconds == 0: - case old != nil && old.RetentionDurationSeconds > seconds: - p.EffectiveTime = old.EffectiveTime - default: - _, p.EffectiveTime = s.timestamp() + if seconds > 0 { + p.EffectiveTime = effectiveSince(b.softDeleteHistory, seconds, s.now()).UTC().Format(timeFormat) } return p, nil } +// policyStep is a soft delete duration and when it took effect. +type policyStep struct { + seconds int64 + since time.Time +} + +// effectiveSince is when the latest unbroken run of durations at least seconds began, ending with one set at now. +func effectiveSince(history []policyStep, seconds int64, now time.Time) time.Time { + since := now + for i := len(history) - 1; i >= 0 && history[i].seconds >= seconds; i-- { + since = history[i].since + } + return since +} + +// recordSoftDelete adds b's soft delete duration to its history if it changed. The caller holds s.mu. +func (s *Server) recordSoftDelete(b *bucket) { + seconds := b.SoftDeletePolicy.RetentionDurationSeconds + if n := len(b.softDeleteHistory); n == 0 || b.softDeleteHistory[n-1].seconds != seconds { + b.softDeleteHistory = append(b.softDeleteHistory, policyStep{seconds: seconds, since: s.now()}) + } +} + func (s *Server) lockRetentionPolicy(r *http.Request) (any, *apiError) { metageneration, ok, err := int64Param(r, "ifMetagenerationMatch") if err != nil { @@ -181,6 +200,9 @@ func (s *Server) checkRetention(b *bucket, old, next *Retention, override, event if err != nil { return nil, invalid("Invalid value for: %s is not a valid value", next.RetainUntilTime) } + if old != nil && !eventHold && old.Mode == next.Mode && old.RetainUntilTime == t.UTC().Format(timeFormat) { + return old, nil + } if t.Before(s.now()) { return nil, invalid("Retain until time cannot be in the past.") } @@ -199,7 +221,7 @@ func (s *Server) checkRetention(b *bucket, old, next *Retention, override, event Status: http.StatusForbidden, Reason: "forbidden", Message: "The locked object retention cannot be removed. Its retention mode cannot be changed and its retention period cannot be shortened.", } - case old.Mode != "Locked" && shortened && !override: + case old.Mode != "Locked" && (shortened || next.Mode == "Locked") && !override: return nil, &apiError{ Status: http.StatusForbidden, Reason: "forbidden", Message: "The unlocked object retention cannot be removed and its retention period cannot be shortened without overriding unlocked retention intent and permission.", diff --git a/gcp/storage/server.go b/gcp/storage/server.go index c208532..818b4b0 100644 --- a/gcp/storage/server.go +++ b/gcp/storage/server.go @@ -80,22 +80,25 @@ // - enableObjectRetention on buckets.insert reports objectRetention.mode // Enabled, and objects in such a bucket take retention with mode // Unlocked or Locked and a future retainUntilTime, with Cloud Storage's -// 400s for a missing or invalid one. Unlocked retention can be shortened -// or removed only with overrideUnlockedRetention=true, and locked -// retention only extended, each refused with 403 forbidden. Retention in -// a bucket without objectRetention fails with 403 forbidden naming -// storage.objects.setRetention, and retention beside an event-based hold -// with 400 invalid (both inferred). The 100-year cap on retainUntilTime -// is not checked. +// 400s for a missing or invalid one. Unlocked retention can be +// shortened, removed or changed to Locked only with +// overrideUnlockedRetention=true, and locked retention only extended, +// each refused with 403 forbidden; the message for a change to Locked is +// inferred. Retention in a bucket without objectRetention fails with 403 +// forbidden naming storage.objects.setRetention, and retention beside an +// event-based hold with 400 invalid (both inferred). The 100-year cap on +// retainUntilTime is not checked. // - softDeletePolicy.retentionDurationSeconds is 0, which turns soft // delete off, or 7 to 90 days, and a new bucket keeps deletions for 7 -// days. Its effectiveTime is when the duration last rose, since lowering -// it leaves a policy with greater retention in effect from then. A -// generation that leaves the bucket, by a delete, by being replaced in a -// bucket without versioning or by a delete of a noncurrent generation, -// is kept soft-deleted with softDeleteTime and hardDeleteTime, on the -// policy at the time. objects.list with softDeleted=true lists them (400 -// invalid with soft delete off and none kept), objects.get with +// days. Its effectiveTime is when the latest unbroken run of durations +// at least the current one began, as "the policy, or one with a greater +// retention" in the discovery document defines it. A generation that +// leaves the bucket, by a delete, by being replaced in a bucket without +// versioning or by a delete of a noncurrent generation, is kept +// soft-deleted with softDeleteTime and hardDeleteTime, on the policy at +// the time; it has a timeDeleted only if it was noncurrent first, as +// Cloud Storage answers. objects.list with softDeleted=true lists them +// (400 invalid with soft delete off and none kept), objects.get with // softDeleted=true and a generation reads one, and objects.restore makes // a copy the live generation, in STANDARD with the metadata, holds and // retention it had when deleted, retiring the live one as an overwrite @@ -103,23 +106,23 @@ // (inferred). A restore of a live or noncurrent generation fails with // 412 objectNotSoftDeleted, and in a bucket with soft delete off and // none kept with 400 SoftDeletePolicyRequired; both messages are -// inferred. ifGenerationNotMatch and the metageneration pair fail with -// 412 when there is no live object. A soft-deleted generation cannot be -// deleted by objects.delete, which answers 404 (inferred). A bucket -// holding only soft-deleted generations can be deleted, and is then kept -// soft-deleted for its policy's duration or until the last of its -// soft-deleted generations goes, whichever is later, even with soft -// delete since turned off (inferred); once restored, those generations -// can still be listed and restored. buckets.list and buckets.get with -// softDeleted=true read it; buckets.get takes a generation only with -// softDeleted=true and answers 400 required otherwise, as its reference -// says. buckets.restore brings it back with its generation, -// metageneration and soft-deleted objects (inferred), failing with 409 -// conflict while a live bucket has its name (message inferred). -// Everything soft-deleted goes for good at its hardDeleteTime on the -// server's clock, and is held in memory until then: with the 7-day -// default, every deleted or replaced generation stays, so a long-running -// test that churns objects may set +// inferred. ifGenerationNotMatch fails with 412 when there is no live +// object, and so does the metageneration pair (inferred). A soft-deleted +// generation cannot be deleted by objects.delete, which answers 404 +// (inferred). A bucket holding only soft-deleted generations can be +// deleted, and is then kept soft-deleted for its policy's duration or +// until the last of its soft-deleted generations goes, whichever is +// later, even with soft delete since turned off (inferred); once +// restored, those generations can still be listed and restored. +// buckets.list and buckets.get with softDeleted=true read it; +// buckets.get takes a generation only with softDeleted=true and answers +// 400 required otherwise, as its reference says. buckets.restore brings +// it back with its generation, metageneration and soft-deleted objects +// (inferred), failing with 409 conflict while a live bucket has its name +// (message inferred). Everything soft-deleted goes for good at its +// hardDeleteTime on the server's clock, and is held in memory until +// then: with the 7-day default, every deleted or replaced generation +// stays, so a long-running test that churns objects may set // softDeletePolicy.retentionDurationSeconds to 0 to keep none. // - billing.requesterPays is recorded and, under [WithPolicy], enforced: a // call on a Requester Pays bucket with a userProject also needs @@ -134,11 +137,13 @@ // softDeleted, pageToken and maxResults; a delimiter, matchGlob, filter, // includeTrailingDelimiter or includeFoldersAsPrefixes is rejected with // 400. objects.patch and objects.update change contentType, -// contentEncoding, metadata, the holds and retention. Page tokens are +// contentEncoding, metadata, the holds and retention, and ignore the +// fields the server sets, so the body a get returns can be sent back; a +// different storageClass is rejected with 400. Page tokens are // positions, so a page may follow one whose items were deleted. Bucket // preconditions are ifMetagenerationMatch and ifMetagenerationNotMatch, -// and object ones ifGenerationMatch, -// ifGenerationNotMatch and, but for uploads and deletes, the +// and object ones ifGenerationMatch, ifGenerationNotMatch (which fails +// an upload with no live object) and, but for uploads and deletes, the // metageneration pair. The fields parameter is accepted and ignored: // responses are always whole, and userProject is checked only as the // Requester Pays item says. Other parameters the server does not know diff --git a/gcp/storage/softdelete.go b/gcp/storage/softdelete.go index 6bb6412..d0a4a71 100644 --- a/gcp/storage/softdelete.go +++ b/gcp/storage/softdelete.go @@ -1,6 +1,7 @@ package storage import ( + "cmp" "errors" "fmt" "maps" @@ -99,7 +100,7 @@ func (s *Server) softDeletedBucket(name string, gen int64) *bucket { func (s *Server) softDeletedBuckets() []Bucket { var out []Bucket for _, name := range slices.Sorted(maps.Keys(s.softBuckets)) { - gens := slices.SortedFunc(slices.Values(s.softBuckets[name]), func(a, b *bucket) int { return int(a.Generation - b.Generation) }) + gens := slices.SortedFunc(slices.Values(s.softBuckets[name]), func(a, b *bucket) int { return cmp.Compare(a.Generation, b.Generation) }) for _, b := range gens { out = append(out, b.clone()) } @@ -174,6 +175,7 @@ func (s *Server) restoreObject(r *http.Request) (any, *apiError) { if err := checkMatch(r, "ifGenerationMatch", "ifGenerationNotMatch", liveGen); err != nil { return nil, err } + // The reference says only ifGenerationNotMatch fails without a live object; the metageneration pair is inferred. if live == nil && (r.URL.Query().Has("ifGenerationNotMatch") || r.URL.Query().Has("ifMetagenerationMatch") || r.URL.Query().Has("ifMetagenerationNotMatch")) { return nil, conditionNotMet() } diff --git a/sdktest/gcp/storage/permissions_test.go b/sdktest/gcp/storage/permissions_test.go index d90356d..71b8911 100644 --- a/sdktest/gcp/storage/permissions_test.go +++ b/sdktest/gcp/storage/permissions_test.go @@ -97,3 +97,22 @@ func TestPolicyNeedsRetentionAndRestorePermissions(t *testing.T) { t.Fatalf("update metadata: %v", err) } } + +func TestPolicyNeedsOverrideToLockUnlockedRetention(t *testing.T) { + policy := access.Allow("storage.buckets.create", "storage.buckets.enableObjectRetention", "storage.objects.create", + "storage.objects.update", "storage.objects.setRetention") + c := newClient(t, storage.New(storage.WithPolicy(policy))) + b := c.Bucket("data") + if err := b.SetObjectRetention(true).Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + until := time.Now().Add(time.Hour) + if _, err := writeWith(t, c, "data", "a.txt", "one", gcs.ObjectAttrs{Retention: &gcs.ObjectRetention{Mode: "Unlocked", RetainUntil: until}}); err != nil { + t.Fatal(err) + } + lock := gcs.ObjectAttrsToUpdate{Retention: &gcs.ObjectRetention{Mode: "Locked", RetainUntil: until}} + _, err := b.Object("a.txt").OverrideUnlockedRetention(true).Update(t.Context(), lock) + wantErr(t, "lock with override", err, http.StatusForbidden, "forbidden", "Permission 'storage.objects.overrideUnlockedRetention' denied") + _, err = b.Object("a.txt").Update(t.Context(), lock) + wantErr(t, "lock without override", err, http.StatusForbidden, "forbidden", "without overriding unlocked retention intent") +} diff --git a/sdktest/gcp/storage/protection_test.go b/sdktest/gcp/storage/protection_test.go index 506febe..6cd12c8 100644 --- a/sdktest/gcp/storage/protection_test.go +++ b/sdktest/gcp/storage/protection_test.go @@ -2,6 +2,7 @@ package storage_test import ( "errors" + "fmt" "net/http" "strings" "sync" @@ -261,6 +262,8 @@ func TestObjectRetention(t *testing.T) { wantErr(t, "shorten without override", err, http.StatusForbidden, "forbidden", "without overriding unlocked retention intent") _, err = b.Object("k").Update(t.Context(), gcs.ObjectAttrsToUpdate{EventBasedHold: true}) wantErr(t, "an event-based hold beside retention", err, http.StatusBadRequest, "invalid", "event-based hold") + _, err = b.Object("k").Update(t.Context(), gcs.ObjectAttrsToUpdate{Retention: &gcs.ObjectRetention{Mode: "Locked", RetainUntil: until}}) + wantErr(t, "lock unlocked retention without override", err, http.StatusForbidden, "forbidden", "without overriding unlocked retention intent") if _, err := b.Object("k").OverrideUnlockedRetention(true).Update(t.Context(), shorter); err != nil { t.Fatalf("shorten with override: %v", err) } @@ -694,22 +697,99 @@ func TestNullHoldReleases(t *testing.T) { } } -func TestLoweringSoftDeleteKeepsEffectiveTime(t *testing.T) { +func TestSoftDeleteEffectiveTimeFollowsHistory(t *testing.T) { clk := newClock() c := newClient(t, storage.New(storage.WithClock(clk.Now))) - b := c.Bucket("lowered") - if err := b.Create(t.Context(), project, &gcs.BucketAttrs{SoftDeletePolicy: &gcs.SoftDeletePolicy{RetentionDuration: 30 * 24 * time.Hour}}); err != nil { + b := c.Bucket("history") + if err := b.Create(t.Context(), project, nil); err != nil { t.Fatal(err) } - created := clk.Now() - clk.Advance(time.Hour) - lowered, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{SoftDeletePolicy: &gcs.SoftDeletePolicy{RetentionDuration: 7 * 24 * time.Hour}}) - if err != nil || !lowered.SoftDeletePolicy.EffectiveTime.Equal(created) { - t.Fatalf("lowered = %+v, %v, want effective since %v", lowered.SoftDeletePolicy, err, created) + day := 24 * time.Hour + start := clk.Now() + var marks []time.Time + for _, step := range []struct { + keep time.Duration + want int + }{ + {30 * day, 1}, + {7 * day, 0}, + {30 * day, 3}, + {14 * day, 3}, + {0, -1}, + {7 * day, 6}, + } { + clk.Advance(time.Hour) + marks = append(marks, clk.Now()) + got, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{SoftDeletePolicy: &gcs.SoftDeletePolicy{RetentionDuration: step.keep}}) + if err != nil { + t.Fatal(err) + } + want := time.Time{} + switch { + case step.want == 0: + want = start + case step.want > 0: + want = marks[step.want-1] + } + if !got.SoftDeletePolicy.EffectiveTime.Equal(want) { + t.Fatalf("after %v at %v: effective %v, want %v", step.keep, clk.Now(), got.SoftDeletePolicy.EffectiveTime, want) + } } - clk.Advance(time.Hour) - raised, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{SoftDeletePolicy: &gcs.SoftDeletePolicy{RetentionDuration: 60 * 24 * time.Hour}}) - if err != nil || !raised.SoftDeletePolicy.EffectiveTime.Equal(clk.Now()) { - t.Fatalf("raised = %+v, %v", raised.SoftDeletePolicy, err) +} + +func TestReadModifyWriteUpdate(t *testing.T) { + s := storage.New() + url := serve(t, s) + c, svc := clientAt(t, url), rawService(t, url) + if err := c.Bucket("rmw").SetObjectRetention(true).Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + if _, err := writeWith(t, c, "rmw", "k", "v", gcs.ObjectAttrs{ + Metadata: map[string]string{"a": "1"}, Retention: &gcs.ObjectRetention{Mode: "Unlocked", RetainUntil: time.Now().Add(time.Hour)}, + }); err != nil { + t.Fatal(err) + } + o, err := svc.Objects.Get("rmw", "k").Context(t.Context()).Do() + if err != nil { + t.Fatal(err) + } + o.Metadata["b"] = "2" + got, err := svc.Objects.Update("rmw", "k", o).Context(t.Context()).Do() + if err != nil || got.Metadata["a"] != "1" || got.Metadata["b"] != "2" || got.Retention == nil || got.Metageneration != 2 { + t.Fatalf("update with the body a get returned = %+v, %v", got, err) + } +} + +func TestSoftDeletedTimeDeleted(t *testing.T) { + c := newClient(t, storage.New()) + for _, versioned := range []bool{false, true} { + name := fmt.Sprintf("deleted-%t", versioned) + b := c.Bucket(name) + if err := b.Create(t.Context(), project, &gcs.BucketAttrs{VersioningEnabled: versioned}); err != nil { + t.Fatal(err) + } + o := write(t, c, name, "k", "v") + if err := b.Object("k").Delete(t.Context()); err != nil { + t.Fatal(err) + } + if versioned { + if err := b.Object("k").Generation(o.Generation).Delete(t.Context()); err != nil { + t.Fatal(err) + } + } + soft := objects(t, b.Objects(t.Context(), &gcs.Query{SoftDeleted: true})) + if len(soft) != 1 || soft[0].Deleted.IsZero() != !versioned { + t.Fatalf("versioned %t: soft-deleted = %+v", versioned, soft) + } + } +} + +func TestUploadIfGenerationNotMatchNeedsALiveObject(t *testing.T) { + c := newClient(t, storage.New()) + if err := c.Bucket("not-match").Create(t.Context(), project, nil); err != nil { + t.Fatal(err) } + w := c.Bucket("not-match").Object("k").If(gcs.Conditions{GenerationNotMatch: 5}).NewWriter(t.Context()) + _, _ = w.Write([]byte("x")) + wantErr(t, "an upload with ifGenerationNotMatch and no live object", w.Close(), http.StatusPreconditionFailed, "conditionNotMet", "") } From 4dff1134d19d051df9375a69b75c7edf98818400 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Thu, 5 Nov 2026 11:53:57 -0600 Subject: [PATCH 6/8] fix(storage): bucket writes take a get's body, and removing object retention needs setRetention --- gcp/storage/buckets.go | 44 +++++++++++++++---- gcp/storage/http_test.go | 57 +++++++++++++++++++++++++ gcp/storage/permissions.go | 27 ++++++++++-- gcp/storage/protection.go | 20 +++++---- gcp/storage/server.go | 40 ++++++++++------- sdktest/gcp/storage/permissions_test.go | 20 +++++++++ sdktest/gcp/storage/protection_test.go | 22 ++++++++++ 7 files changed, 192 insertions(+), 38 deletions(-) diff --git a/gcp/storage/buckets.go b/gcp/storage/buckets.go index a69ab43..c576f08 100644 --- a/gcp/storage/buckets.go +++ b/gcp/storage/buckets.go @@ -299,11 +299,34 @@ func (v *int64Value) UnmarshalJSON(data []byte) error { } type retentionPolicyInput struct { - RetentionPeriod *int64Value `json:"retentionPeriod"` + RetentionPeriod *int64Value `json:"retentionPeriod"` + EffectiveTime json.RawMessage `json:"effectiveTime"` + IsLocked json.RawMessage `json:"isLocked"` } type softDeletePolicyInput struct { - RetentionDurationSeconds *int64Value `json:"retentionDurationSeconds"` + RetentionDurationSeconds *int64Value `json:"retentionDurationSeconds"` + EffectiveTime json.RawMessage `json:"effectiveTime"` +} + +// bucketOutputOnly holds the Bucket fields the server sets, which a read-modify-write sends back and which are ignored. +type bucketOutputOnly struct { + Kind json.RawMessage `json:"kind"` + ID json.RawMessage `json:"id"` + SelfLink json.RawMessage `json:"selfLink"` + ProjectNumber json.RawMessage `json:"projectNumber"` + TimeCreated json.RawMessage `json:"timeCreated"` + Updated json.RawMessage `json:"updated"` + Metageneration json.RawMessage `json:"metageneration"` + Etag json.RawMessage `json:"etag"` + LocationType json.RawMessage `json:"locationType"` + Generation json.RawMessage `json:"generation"` + SoftDeleteTime json.RawMessage `json:"softDeleteTime"` + HardDeleteTime json.RawMessage `json:"hardDeleteTime"` + Owner json.RawMessage `json:"owner"` + ObjectRetention json.RawMessage `json:"objectRetention"` + SatisfiesPZS json.RawMessage `json:"satisfiesPZS"` + SatisfiesPZI json.RawMessage `json:"satisfiesPZI"` } // bucketInput is what buckets.insert and buckets.patch take. @@ -321,6 +344,7 @@ type bucketInput struct { DefaultEventBasedHold *bool `json:"defaultEventBasedHold"` SoftDeletePolicy *softDeletePolicyInput `json:"softDeletePolicy"` Billing *Billing `json:"billing"` + bucketOutputOnly // Lifecycle is accepted without rules, since the client sends it empty. Lifecycle *struct { Rule []json.RawMessage `json:"rule"` @@ -472,7 +496,7 @@ func (s *Server) store(b Bucket) (*bucket, *apiError) { } stored := &bucket{Bucket: b, objects: map[string][]*Object{}} stored.Bucket = stored.clone() - s.recordSoftDelete(stored) + recordSoftDelete(stored, t) s.buckets[b.Name] = stored return stored, nil } @@ -519,10 +543,6 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { if err := in.check(); err != nil { return nil, err } - if in.Name != "" || in.Location != "" || in.CustomPlacementConfig != nil || in.RPO != "" { - return nil, invalid("squall: a patch may change only labels, versioning, cors, storageClass, encryption, " + - "retentionPolicy, defaultEventBasedHold, softDeletePolicy and billing") - } cors, err := normalCORS(in.CORS) if err != nil { return nil, err @@ -544,6 +564,11 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { if err := checkMatch(r, "ifMetagenerationMatch", "ifMetagenerationNotMatch", b.Metageneration); err != nil { return nil, err } + if in.Name != "" && in.Name != b.Name || in.Location != "" && !strings.EqualFold(in.Location, b.Location) || in.RPO != "" && in.RPO != b.RPO || + in.CustomPlacementConfig != nil && (b.CustomPlacementConfig == nil || !slices.Equal(in.CustomPlacementConfig.DataLocations, b.CustomPlacementConfig.DataLocations)) { + return nil, invalid("squall: a patch may change only labels, versioning, cors, storageClass, encryption, " + + "retentionPolicy, defaultEventBasedHold, softDeletePolicy and billing") + } labels := maps.Clone(b.Labels) if labels == nil || cleared("labels") { labels = map[string]string{} @@ -569,7 +594,8 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { if err != nil { return nil, err } - softDelete, err := s.nextSoftDeletePolicy(b, in.SoftDeletePolicy) + changed := s.now() + softDelete, err := nextSoftDeletePolicy(b, in.SoftDeletePolicy, changed) if err != nil { return nil, err } @@ -577,7 +603,7 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { labels = nil } b.Labels, b.StorageClass, b.RetentionPolicy, b.SoftDeletePolicy = labels, class, retention, softDelete - s.recordSoftDelete(b) + recordSoftDelete(b, changed) if in.DefaultEventBasedHold != nil { b.DefaultEventBasedHold = *in.DefaultEventBasedHold } else if cleared("defaultEventBasedHold") { diff --git a/gcp/storage/http_test.go b/gcp/storage/http_test.go index ef7fe7d..0ef3f84 100644 --- a/gcp/storage/http_test.go +++ b/gcp/storage/http_test.go @@ -7,7 +7,9 @@ import ( "net/http" "net/http/httptest" "strings" + "sync" "testing" + "time" "github.com/parallelworks/squall/gcp/storage" ) @@ -163,6 +165,61 @@ func TestRetentionAndRestoreRoutes(t *testing.T) { } } +func TestBucketBodiesWithServerSetFields(t *testing.T) { + s := storage.New() + code, body := do(t, s, http.MethodPost, "/storage/v1/b?project=p", "application/json", + `{"name":"py-off","softDeletePolicy":{"retentionDurationSeconds":0,"effectiveTime":null},"retentionPolicy":{"retentionPeriod":"60","effectiveTime":null,"isLocked":null}}`) + if code != http.StatusOK || !strings.Contains(body, `"softDeletePolicy":{"retentionDurationSeconds":"0"}`) { + t.Fatalf("insert = %d %s", code, body) + } + code, body = do(t, s, http.MethodPatch, "/storage/v1/b/py-off", "application/json", + `{"retentionPolicy":{"retentionPeriod":"120","effectiveTime":"2020-01-01T00:00:00.000Z","isLocked":false},"softDeletePolicy":{"retentionDurationSeconds":"0","effectiveTime":null}}`) + if code != http.StatusOK || !strings.Contains(body, `"retentionPeriod":"120"`) { + t.Fatalf("patch = %d %s", code, body) + } + _, got := do(t, s, http.MethodGet, "/storage/v1/b/py-off", "", "") + var fields map[string]any + if err := json.Unmarshal([]byte(got), &fields); err != nil { + t.Fatal(err) + } + fields["labels"] = map[string]string{"team": "data"} + sent, _ := json.Marshal(fields) + if code, body := do(t, s, http.MethodPatch, "/storage/v1/b/py-off", "application/json", string(sent)); code != http.StatusOK || !strings.Contains(body, `"team":"data"`) { + t.Fatalf("patch with a get's body = %d %s", code, body) + } +} + +func TestSoftDeleteEffectiveTimeOnAMovingClock(t *testing.T) { + var mu sync.Mutex + now := time.Date(2026, 3, 1, 12, 0, 0, 999_000, time.UTC) + s := storage.New(storage.WithClock(func() time.Time { + mu.Lock() + defer mu.Unlock() + now = now.Add(700 * time.Microsecond) + return now + })) + effective := func(body string) string { + t.Helper() + var b struct { + SoftDeletePolicy struct { + EffectiveTime string `json:"effectiveTime"` + } `json:"softDeletePolicy"` + } + if err := json.Unmarshal([]byte(body), &b); err != nil { + t.Fatal(err) + } + return b.SoftDeletePolicy.EffectiveTime + } + _, body := do(t, s, http.MethodPost, "/storage/v1/b?project=p", "application/json", `{"name":"moving"}`) + created := effective(body) + for _, seconds := range []string{"2592000", "604800", "1209600", "604800"} { + _, body = do(t, s, http.MethodPatch, "/storage/v1/b/moving", "application/json", `{"softDeletePolicy":{"retentionDurationSeconds":"`+seconds+`"}}`) + } + if got := effective(body); got != created { + t.Fatalf("effectiveTime = %s, want %s", got, created) + } +} + func FuzzServeHTTP(f *testing.F) { multipartBody := "--x\r\nContent-Type: application/json\r\n\r\n{\"name\":\"o\"}\r\n--x\r\nContent-Type: text/plain\r\n\r\nhi\r\n--x--\r\n" for _, seed := range []struct{ method, target, contentType, body string }{ diff --git a/gcp/storage/permissions.go b/gcp/storage/permissions.go index 2da979e..c76d453 100644 --- a/gcp/storage/permissions.go +++ b/gcp/storage/permissions.go @@ -59,9 +59,12 @@ func (s *Server) needs(method, resource string, r *http.Request) []string { case "storage.objects.insert": add(s.exists(resource), "storage.objects.delete") add(uploadSetsRetention(r), "storage.objects.setRetention") - case "storage.objects.patch", "storage.objects.update": + case "storage.objects.patch": add(bodySetsRetention(peekBody(r, maxBody)), "storage.objects.setRetention") add(q.Get("overrideUnlockedRetention") == "true", "storage.objects.overrideUnlockedRetention") + case "storage.objects.update": + add(bodySetsRetention(peekBody(r, maxBody)) || s.retained(resource), "storage.objects.setRetention") + add(q.Get("overrideUnlockedRetention") == "true", "storage.objects.overrideUnlockedRetention") case "storage.objects.restore": add(s.exists(resource), "storage.objects.delete") } @@ -72,14 +75,30 @@ func (s *Server) needs(method, resource string, r *http.Request) []string { return perms } -// bodySetsRetention reports whether an object's metadata, as JSON, includes a retention. +// bodySetsRetention reports whether an object's metadata, as JSON, includes retention, null included. func bodySetsRetention(meta []byte) bool { var fields map[string]json.RawMessage if json.Unmarshal(meta, &fields) != nil { return false } - retention, ok := fields["retention"] - return ok && string(retention) != "null" + _, ok := fields["retention"] + return ok +} + +// retained reports whether resource, as "b//o/", names a live object with retention. +func (s *Server) retained(resource string) bool { + bucketName, name, ok := strings.Cut(strings.TrimPrefix(resource, "b/"), "/o/") + if !ok { + return false + } + s.mu.Lock() + defer s.mu.Unlock() + b := s.buckets[bucketName] + if b == nil { + return false + } + o := b.liveObject(name) + return o != nil && o.Retention != nil } func uploadSetsRetention(r *http.Request) bool { diff --git a/gcp/storage/protection.go b/gcp/storage/protection.go index 8349d63..dbe2b95 100644 --- a/gcp/storage/protection.go +++ b/gcp/storage/protection.go @@ -63,8 +63,8 @@ func (s *Server) nextRetentionPolicy(b *bucket, in *retentionPolicyInput, remove return &RetentionPolicy{RetentionPeriod: period, EffectiveTime: now, IsLocked: locked}, nil } -// nextSoftDeletePolicy is b's soft delete policy after a patch sets it to in. The caller holds s.mu. -func (s *Server) nextSoftDeletePolicy(b *bucket, in *softDeletePolicyInput) (*SoftDeletePolicy, *apiError) { +// nextSoftDeletePolicy is b's soft delete policy after a patch at now sets it to in. The caller holds s.mu. +func nextSoftDeletePolicy(b *bucket, in *softDeletePolicyInput, now time.Time) (*SoftDeletePolicy, *apiError) { if in == nil { return b.SoftDeletePolicy, nil } @@ -77,7 +77,7 @@ func (s *Server) nextSoftDeletePolicy(b *bucket, in *softDeletePolicyInput) (*So } p := &SoftDeletePolicy{RetentionDurationSeconds: seconds} if seconds > 0 { - p.EffectiveTime = effectiveSince(b.softDeleteHistory, seconds, s.now()).UTC().Format(timeFormat) + p.EffectiveTime = effectiveSince(b.softDeleteHistory, seconds, now).UTC().Format(timeFormat) } return p, nil } @@ -97,12 +97,14 @@ func effectiveSince(history []policyStep, seconds int64, now time.Time) time.Tim return since } -// recordSoftDelete adds b's soft delete duration to its history if it changed. The caller holds s.mu. -func (s *Server) recordSoftDelete(b *bucket) { - seconds := b.SoftDeletePolicy.RetentionDurationSeconds - if n := len(b.softDeleteHistory); n == 0 || b.softDeleteHistory[n-1].seconds != seconds { - b.softDeleteHistory = append(b.softDeleteHistory, policyStep{seconds: seconds, since: s.now()}) - } +// recordSoftDelete pushes b's soft delete duration, set at now, onto its history, which strictly increases. +func recordSoftDelete(b *bucket, now time.Time) { + step := policyStep{seconds: b.SoftDeletePolicy.RetentionDurationSeconds, since: now} + for n := len(b.softDeleteHistory); n > 0 && b.softDeleteHistory[n-1].seconds >= step.seconds; n-- { + step.since = b.softDeleteHistory[n-1].since + b.softDeleteHistory = b.softDeleteHistory[:n-1] + } + b.softDeleteHistory = append(b.softDeleteHistory, step) } func (s *Server) lockRetentionPolicy(r *http.Request) (any, *apiError) { diff --git a/gcp/storage/server.go b/gcp/storage/server.go index 818b4b0..080fca7 100644 --- a/gcp/storage/server.go +++ b/gcp/storage/server.go @@ -37,11 +37,15 @@ // are rejected with 400, as are ACLs other than private and // projectPrivate. cors, encryption and rpo are recorded, not enforced. // There are no IAM policies on buckets or objects, and no lifecycle, -// autoclass, hardDeletePause or hierarchical namespace. buckets.patch has -// patch semantics: it merges labels, deleting one set to null, and clears -// labels, versioning, cors, encryption, retentionPolicy, +// autoclass, hardDeletePause or hierarchical namespace. buckets.patch +// has patch semantics: it merges labels, deleting one set to null, and +// clears labels, versioning, cors, encryption, retentionPolicy, // defaultEventBasedHold or billing set to null; a null softDeletePolicy -// is rejected with 400. Every bucket reports a generation (inferred). +// is rejected with 400. Insert and patch ignore the fields the server +// sets, effectiveTime and isLocked in the policies included, and a patch +// takes name, location, customPlacementConfig and rpo unchanged, so the +// body a get returns can be sent back. Every bucket reports a generation +// (inferred). // - Objects record their size, MD5 and CRC32C, not their data; a crc32c or // md5Hash in the upload's metadata must match the data. Resumable // uploads, media reads (objects.get with alt=media answers 404), copies, @@ -76,18 +80,21 @@ // message. While an event-based hold is on, an object reports no // retentionExpirationTime, and releasing it starts the object's time in // the bucket again. objects.update replaces every writable field, so -// leaving a hold out releases it. +// leaving a hold out releases it and leaving retention out removes it, +// which fails with 403 for Locked retention and for Unlocked retention +// without overrideUnlockedRetention=true. // - enableObjectRetention on buckets.insert reports objectRetention.mode // Enabled, and objects in such a bucket take retention with mode // Unlocked or Locked and a future retainUntilTime, with Cloud Storage's // 400s for a missing or invalid one. Unlocked retention can be // shortened, removed or changed to Locked only with // overrideUnlockedRetention=true, and locked retention only extended, -// each refused with 403 forbidden; the message for a change to Locked is -// inferred. Retention in a bucket without objectRetention fails with 403 -// forbidden naming storage.objects.setRetention, and retention beside an -// event-based hold with 400 invalid (both inferred). The 100-year cap on -// retainUntilTime is not checked. +// each refused with 403 forbidden; a change to Locked gets the message +// for shortening, as no message for it is known (inferred). Retention in +// a bucket without objectRetention fails with 403 forbidden naming +// storage.objects.setRetention, and retention beside an event-based hold +// with 400 invalid (both inferred). The 100-year cap on retainUntilTime +// is not checked. // - softDeletePolicy.retentionDurationSeconds is 0, which turns soft // delete off, or 7 to 90 days, and a new bucket keeps deletions for 7 // days. Its effectiveTime is when the latest unbroken run of durations @@ -161,13 +168,14 @@ // its method's permission, and those its parameters add, as each method's // reference lists them: a predefined ACL on a patch needs // storage.buckets.setIamPolicy, and an upload that replaces a live object -// storage.objects.delete, as does a restore that replaces one; retention in -// an upload or patch needs storage.objects.setRetention, and +// storage.objects.delete, as does a restore that replaces one; a body that +// includes retention, even null, needs storage.objects.setRetention, as does +// an objects.update that removes it by leaving it out (inferred), and // overrideUnlockedRetention=true storage.objects.overrideUnlockedRetention. -// projection=full needs nothing more: Cloud Storage -// leaves out the ACLs a caller may not read rather than refusing the call, -// and the official clients ask for it on every get and list. Permissions are -// not checked against the bucket they act on: a grant covers every bucket. +// projection=full needs nothing more: Cloud Storage leaves out the ACLs a +// caller may not read rather than refusing the call, and the official +// clients ask for it on every get and list. Permissions are not checked +// against the bucket they act on: a grant covers every bucket. // // A lost response aborts the handler with [http.ErrAbortHandler] after // sending the response header, so the [http.Server] serving it closes the diff --git a/sdktest/gcp/storage/permissions_test.go b/sdktest/gcp/storage/permissions_test.go index 71b8911..e61d6b2 100644 --- a/sdktest/gcp/storage/permissions_test.go +++ b/sdktest/gcp/storage/permissions_test.go @@ -10,6 +10,7 @@ import ( gcs "cloud.google.com/go/storage" "google.golang.org/api/googleapi" + rawstorage "google.golang.org/api/storage/v1" "github.com/parallelworks/squall/access" "github.com/parallelworks/squall/gcp/storage" @@ -116,3 +117,22 @@ func TestPolicyNeedsOverrideToLockUnlockedRetention(t *testing.T) { _, err = b.Object("a.txt").Update(t.Context(), lock) wantErr(t, "lock without override", err, http.StatusForbidden, "forbidden", "without overriding unlocked retention intent") } + +func TestPolicyNeedsSetRetentionToRemoveRetention(t *testing.T) { + policy := access.Allow("storage.buckets.create", "storage.buckets.enableObjectRetention", "storage.objects.create", + "storage.objects.update", "storage.objects.overrideUnlockedRetention") + s := storage.New(storage.WithPolicy(policy)) + url := serve(t, s) + c, svc := clientAt(t, url), rawService(t, url) + if err := c.Bucket("data").SetObjectRetention(true).Create(t.Context(), project, nil); err != nil { + t.Fatal(err) + } + write(t, c, "data", "a.txt", "one") + if _, err := s.SetObjectRetention("data", "a.txt", &storage.Retention{Mode: "Unlocked", RetainUntilTime: time.Now().Add(time.Hour).Format(time.RFC3339)}); err != nil { + t.Fatal(err) + } + _, err := c.Bucket("data").Object("a.txt").OverrideUnlockedRetention(true).Update(t.Context(), gcs.ObjectAttrsToUpdate{Retention: &gcs.ObjectRetention{}}) + wantErr(t, "remove retention with null", err, http.StatusForbidden, "forbidden", "Permission 'storage.objects.setRetention' denied") + _, err = svc.Objects.Update("data", "a.txt", &rawstorage.Object{}).OverrideUnlockedRetention(true).Context(t.Context()).Do() + wantErr(t, "remove retention by leaving it out", err, http.StatusForbidden, "forbidden", "Permission 'storage.objects.setRetention' denied") +} diff --git a/sdktest/gcp/storage/protection_test.go b/sdktest/gcp/storage/protection_test.go index 6cd12c8..ef21348 100644 --- a/sdktest/gcp/storage/protection_test.go +++ b/sdktest/gcp/storage/protection_test.go @@ -793,3 +793,25 @@ func TestUploadIfGenerationNotMatchNeedsALiveObject(t *testing.T) { _, _ = w.Write([]byte("x")) wantErr(t, "an upload with ifGenerationNotMatch and no live object", w.Close(), http.StatusPreconditionFailed, "conditionNotMet", "") } + +func TestBucketReadModifyWrite(t *testing.T) { + s := storage.New() + url := serve(t, s) + c, svc := clientAt(t, url), rawService(t, url) + if err := c.Bucket("rmw-bucket").Create(t.Context(), project, &gcs.BucketAttrs{ + Location: "us-east1", RetentionPolicy: &gcs.RetentionPolicy{RetentionPeriod: time.Hour}, + }); err != nil { + t.Fatal(err) + } + b, err := svc.Buckets.Get("rmw-bucket").Context(t.Context()).Do() + if err != nil { + t.Fatal(err) + } + b.Labels = map[string]string{"team": "data"} + b.SoftDeletePolicy.RetentionDurationSeconds = 0 + b.SoftDeletePolicy.ForceSendFields = []string{"RetentionDurationSeconds"} + got, err := svc.Buckets.Patch("rmw-bucket", b).Context(t.Context()).Do() + if err != nil || got.Labels["team"] != "data" || got.SoftDeletePolicy.RetentionDurationSeconds != 0 || got.RetentionPolicy.RetentionPeriod != 3600 { + t.Fatalf("patch with a get's body = %+v, %v", got, err) + } +} From 3ac7fb861eeae6b83c7693891bbea360ec799b02 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Thu, 5 Nov 2026 12:02:34 -0600 Subject: [PATCH 7/8] fix(storage): bucket bodies must match objectRetention, patches can change rpo, and update permissions check the named generation --- gcp/storage/buckets.go | 64 +++++++++++++++---------- gcp/storage/http_test.go | 59 +++++++++++++++++++++++ gcp/storage/permissions.go | 10 ++-- gcp/storage/protection.go | 7 ++- gcp/storage/server.go | 34 +++++++------ sdktest/gcp/storage/permissions_test.go | 38 +++++++++++++++ sdktest/gcp/storage/protection_test.go | 12 +++++ 7 files changed, 178 insertions(+), 46 deletions(-) diff --git a/gcp/storage/buckets.go b/gcp/storage/buckets.go index c576f08..5036b79 100644 --- a/gcp/storage/buckets.go +++ b/gcp/storage/buckets.go @@ -155,12 +155,20 @@ func (b *bucket) requesterPays() bool { return b.Billing != nil && b.Billing.Req func (b *bucket) empty() bool { return len(b.objects) == 0 } // touch records a change to b's metadata. The caller holds s.mu. -func (s *Server) touch(b *bucket) { +func (s *Server) touch(b *bucket) { touchAt(b, s.now()) } + +// touchAt records a change to b's metadata made at t. +func touchAt(b *bucket, t time.Time) { b.Metageneration++ - b.Updated = s.now().UTC().Format(timeFormat) + b.Updated = t.UTC().Format(timeFormat) b.Etag = etag(b.Metageneration) } +// sameObjectRetention reports whether a body's objectRetention, nil for none, matches have. +func sameObjectRetention(in, have *BucketObjectRetention) bool { + return in == nil && have == nil || in != nil && have != nil && in.Mode == have.Mode +} + // etag encodes its numbers as tagged varints, so it changes whenever they do. func etag(numbers ...int64) string { var buf []byte @@ -311,22 +319,21 @@ type softDeletePolicyInput struct { // bucketOutputOnly holds the Bucket fields the server sets, which a read-modify-write sends back and which are ignored. type bucketOutputOnly struct { - Kind json.RawMessage `json:"kind"` - ID json.RawMessage `json:"id"` - SelfLink json.RawMessage `json:"selfLink"` - ProjectNumber json.RawMessage `json:"projectNumber"` - TimeCreated json.RawMessage `json:"timeCreated"` - Updated json.RawMessage `json:"updated"` - Metageneration json.RawMessage `json:"metageneration"` - Etag json.RawMessage `json:"etag"` - LocationType json.RawMessage `json:"locationType"` - Generation json.RawMessage `json:"generation"` - SoftDeleteTime json.RawMessage `json:"softDeleteTime"` - HardDeleteTime json.RawMessage `json:"hardDeleteTime"` - Owner json.RawMessage `json:"owner"` - ObjectRetention json.RawMessage `json:"objectRetention"` - SatisfiesPZS json.RawMessage `json:"satisfiesPZS"` - SatisfiesPZI json.RawMessage `json:"satisfiesPZI"` + Kind json.RawMessage `json:"kind"` + ID json.RawMessage `json:"id"` + SelfLink json.RawMessage `json:"selfLink"` + ProjectNumber json.RawMessage `json:"projectNumber"` + TimeCreated json.RawMessage `json:"timeCreated"` + Updated json.RawMessage `json:"updated"` + Metageneration json.RawMessage `json:"metageneration"` + Etag json.RawMessage `json:"etag"` + LocationType json.RawMessage `json:"locationType"` + Generation json.RawMessage `json:"generation"` + SoftDeleteTime json.RawMessage `json:"softDeleteTime"` + HardDeleteTime json.RawMessage `json:"hardDeleteTime"` + Owner json.RawMessage `json:"owner"` + SatisfiesPZS json.RawMessage `json:"satisfiesPZS"` + SatisfiesPZI json.RawMessage `json:"satisfiesPZI"` } // bucketInput is what buckets.insert and buckets.patch take. @@ -344,6 +351,7 @@ type bucketInput struct { DefaultEventBasedHold *bool `json:"defaultEventBasedHold"` SoftDeletePolicy *softDeletePolicyInput `json:"softDeletePolicy"` Billing *Billing `json:"billing"` + ObjectRetention *BucketObjectRetention `json:"objectRetention"` bucketOutputOnly // Lifecycle is accepted without rules, since the client sends it empty. Lifecycle *struct { @@ -408,6 +416,9 @@ func (s *Server) insertBucket(r *http.Request) (any, *apiError) { if objectRetention { b.ObjectRetention = &BucketObjectRetention{Mode: "Enabled"} } + if in.ObjectRetention != nil && !sameObjectRetention(in.ObjectRetention, b.ObjectRetention) { + return nil, invalid("squall: objectRetention is set by the enableObjectRetention parameter, which this body does not match") + } if p := in.RetentionPolicy; p != nil { if p.RetentionPeriod == nil { return nil, badRetentionPeriod() @@ -564,10 +575,12 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { if err := checkMatch(r, "ifMetagenerationMatch", "ifMetagenerationNotMatch", b.Metageneration); err != nil { return nil, err } - if in.Name != "" && in.Name != b.Name || in.Location != "" && !strings.EqualFold(in.Location, b.Location) || in.RPO != "" && in.RPO != b.RPO || + if in.Name != "" && in.Name != b.Name || in.Location != "" && !strings.EqualFold(in.Location, b.Location) || in.CustomPlacementConfig != nil && (b.CustomPlacementConfig == nil || !slices.Equal(in.CustomPlacementConfig.DataLocations, b.CustomPlacementConfig.DataLocations)) { - return nil, invalid("squall: a patch may change only labels, versioning, cors, storageClass, encryption, " + - "retentionPolicy, defaultEventBasedHold, softDeletePolicy and billing") + return nil, invalid("squall: name, location and customPlacementConfig are set on insert and cannot change") + } + if (in.ObjectRetention != nil || cleared("objectRetention")) && !sameObjectRetention(in.ObjectRetention, b.ObjectRetention) { + return nil, invalid("squall: object retention can be turned on for an existing bucket only in the console, and never off") } labels := maps.Clone(b.Labels) if labels == nil || cleared("labels") { @@ -590,11 +603,11 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { return nil, invalid("Invalid argument: storage class '%s' is not valid.", class) } } - retention, err := s.nextRetentionPolicy(b, in.RetentionPolicy, cleared("retentionPolicy")) + changed := s.now() + retention, err := nextRetentionPolicy(b, in.RetentionPolicy, cleared("retentionPolicy"), changed) if err != nil { return nil, err } - changed := s.now() softDelete, err := nextSoftDeletePolicy(b, in.SoftDeletePolicy, changed) if err != nil { return nil, err @@ -627,7 +640,10 @@ func (s *Server) patchBucket(r *http.Request) (any, *apiError) { } else if cleared("encryption") { b.Encryption = nil } - s.touch(b) + if in.RPO != "" || cleared("rpo") { + b.RPO = in.RPO + } + touchAt(b, changed) return b.clone(), nil } diff --git a/gcp/storage/http_test.go b/gcp/storage/http_test.go index 0ef3f84..1c0b819 100644 --- a/gcp/storage/http_test.go +++ b/gcp/storage/http_test.go @@ -220,6 +220,65 @@ func TestSoftDeleteEffectiveTimeOnAMovingClock(t *testing.T) { } } +func TestBucketObjectRetentionInBodies(t *testing.T) { + s := storage.New() + for _, tc := range []struct { + target, body string + status int + }{ + {"/storage/v1/b?project=p&enableObjectRetention=true", `{"name":"or-on","objectRetention":{"mode":"Enabled"}}`, http.StatusOK}, + {"/storage/v1/b?project=p", `{"name":"or-off","objectRetention":null}`, http.StatusOK}, + {"/storage/v1/b?project=p", `{"name":"or-mismatch","objectRetention":{"mode":"Enabled"}}`, http.StatusBadRequest}, + } { + if code, body := do(t, s, http.MethodPost, tc.target, "application/json", tc.body); code != tc.status { + t.Fatalf("insert %s = %d %s, want %d", tc.body, code, body, tc.status) + } + } + for _, tc := range []struct { + bucket, body string + status int + }{ + {"or-on", `{"objectRetention":{"mode":"Enabled"},"labels":{"a":"b"}}`, http.StatusOK}, + {"or-on", `{"objectRetention":null}`, http.StatusBadRequest}, + {"or-off", `{"objectRetention":{"mode":"Enabled"}}`, http.StatusBadRequest}, + {"or-off", `{"objectRetention":null}`, http.StatusOK}, + } { + if code, body := do(t, s, http.MethodPatch, "/storage/v1/b/"+tc.bucket, "application/json", tc.body); code != tc.status { + t.Fatalf("patch %s with %s = %d %s, want %d", tc.bucket, tc.body, code, body, tc.status) + } + } +} + +func TestBucketPatchReadsTheClockOnce(t *testing.T) { + var mu sync.Mutex + now := time.Date(2026, 3, 1, 12, 0, 0, 0, time.UTC) + s := storage.New(storage.WithClock(func() time.Time { + mu.Lock() + defer mu.Unlock() + now = now.Add(time.Second) + return now + })) + do(t, s, http.MethodPost, "/storage/v1/b?project=p", "application/json", `{"name":"once"}`) + _, body := do(t, s, http.MethodPatch, "/storage/v1/b/once", "application/json", + `{"retentionPolicy":{"retentionPeriod":"60"},"softDeletePolicy":{"retentionDurationSeconds":"1209600"},"rpo":"ASYNC_TURBO"}`) + var b struct { + Updated string `json:"updated"` + RPO string `json:"rpo"` + RetentionPolicy struct { + EffectiveTime string `json:"effectiveTime"` + } `json:"retentionPolicy"` + SoftDeletePolicy struct { + EffectiveTime string `json:"effectiveTime"` + } `json:"softDeletePolicy"` + } + if err := json.Unmarshal([]byte(body), &b); err != nil { + t.Fatal(err) + } + if b.RPO != "ASYNC_TURBO" || b.RetentionPolicy.EffectiveTime != b.Updated || b.SoftDeletePolicy.EffectiveTime != b.Updated { + t.Fatalf("patch = %s", body) + } +} + func FuzzServeHTTP(f *testing.F) { multipartBody := "--x\r\nContent-Type: application/json\r\n\r\n{\"name\":\"o\"}\r\n--x\r\nContent-Type: text/plain\r\n\r\nhi\r\n--x--\r\n" for _, seed := range []struct{ method, target, contentType, body string }{ diff --git a/gcp/storage/permissions.go b/gcp/storage/permissions.go index c76d453..b84cf27 100644 --- a/gcp/storage/permissions.go +++ b/gcp/storage/permissions.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "slices" + "strconv" "strings" "github.com/parallelworks/squall/access" @@ -63,7 +64,7 @@ func (s *Server) needs(method, resource string, r *http.Request) []string { add(bodySetsRetention(peekBody(r, maxBody)), "storage.objects.setRetention") add(q.Get("overrideUnlockedRetention") == "true", "storage.objects.overrideUnlockedRetention") case "storage.objects.update": - add(bodySetsRetention(peekBody(r, maxBody)) || s.retained(resource), "storage.objects.setRetention") + add(bodySetsRetention(peekBody(r, maxBody)) || s.retained(resource, q.Get("generation")), "storage.objects.setRetention") add(q.Get("overrideUnlockedRetention") == "true", "storage.objects.overrideUnlockedRetention") case "storage.objects.restore": add(s.exists(resource), "storage.objects.delete") @@ -85,8 +86,8 @@ func bodySetsRetention(meta []byte) bool { return ok } -// retained reports whether resource, as "b//o/", names a live object with retention. -func (s *Server) retained(resource string) bool { +// retained reports whether resource, as "b//o/", at generation or else live, has retention. +func (s *Server) retained(resource, generation string) bool { bucketName, name, ok := strings.Cut(strings.TrimPrefix(resource, "b/"), "/o/") if !ok { return false @@ -98,6 +99,9 @@ func (s *Server) retained(resource string) bool { return false } o := b.liveObject(name) + if gen, err := strconv.ParseInt(generation, 10, 64); err == nil { + o = b.generation(name, gen) + } return o != nil && o.Retention != nil } diff --git a/gcp/storage/protection.go b/gcp/storage/protection.go index dbe2b95..b67727a 100644 --- a/gcp/storage/protection.go +++ b/gcp/storage/protection.go @@ -30,8 +30,8 @@ func badSoftDelete() *apiError { return invalid("Soft delete policy must have a retention duration between 7 days and 90 days.") } -// nextRetentionPolicy is b's retention policy after a patch sets it to in, or removes it. The caller holds s.mu. -func (s *Server) nextRetentionPolicy(b *bucket, in *retentionPolicyInput, removed bool) (*RetentionPolicy, *apiError) { +// nextRetentionPolicy is b's retention policy after a patch at now sets it to in, or removes it. The caller holds s.mu. +func nextRetentionPolicy(b *bucket, in *retentionPolicyInput, removed bool, now time.Time) (*RetentionPolicy, *apiError) { old := b.RetentionPolicy locked := old != nil && old.IsLocked if removed { @@ -59,8 +59,7 @@ func (s *Server) nextRetentionPolicy(b *bucket, in *retentionPolicyInput, remove if old != nil && old.RetentionPeriod == period { return new(*old), nil } - _, now := s.timestamp() - return &RetentionPolicy{RetentionPeriod: period, EffectiveTime: now, IsLocked: locked}, nil + return &RetentionPolicy{RetentionPeriod: period, EffectiveTime: now.UTC().Format(timeFormat), IsLocked: locked}, nil } // nextSoftDeletePolicy is b's soft delete policy after a patch at now sets it to in. The caller holds s.mu. diff --git a/gcp/storage/server.go b/gcp/storage/server.go index 080fca7..d2db1f8 100644 --- a/gcp/storage/server.go +++ b/gcp/storage/server.go @@ -31,21 +31,25 @@ // default; US, EU and ASIA are multi-regions, NAM4, EUR4, EUR5, EUR7, EUR8 // and ASIA1 and any location with a customPlacementConfig dual-regions, // and the rest regions. Project numbers are derived from project IDs. -// - Bucket fields other than name, location, storageClass, labels, -// versioning, cors, customPlacementConfig, encryption, rpo, -// retentionPolicy, defaultEventBasedHold, softDeletePolicy and billing -// are rejected with 400, as are ACLs other than private and -// projectPrivate. cors, encryption and rpo are recorded, not enforced. -// There are no IAM policies on buckets or objects, and no lifecycle, -// autoclass, hardDeletePause or hierarchical namespace. buckets.patch -// has patch semantics: it merges labels, deleting one set to null, and -// clears labels, versioning, cors, encryption, retentionPolicy, -// defaultEventBasedHold or billing set to null; a null softDeletePolicy -// is rejected with 400. Insert and patch ignore the fields the server -// sets, effectiveTime and isLocked in the policies included, and a patch -// takes name, location, customPlacementConfig and rpo unchanged, so the -// body a get returns can be sent back. Every bucket reports a generation -// (inferred). +// - Writable bucket fields other than name, location, storageClass, +// labels, versioning, cors, customPlacementConfig, encryption, rpo, +// retentionPolicy, defaultEventBasedHold, softDeletePolicy, billing and +// objectRetention are rejected with 400, as are other unknown fields and +// ACLs other than private and projectPrivate. cors, encryption and rpo +// are recorded, not enforced. There are no IAM policies on buckets or +// objects, and no lifecycle, autoclass, hardDeletePause or hierarchical +// namespace. buckets.patch has patch semantics: it merges labels, +// deleting one set to null, and clears labels, versioning, cors, +// encryption, rpo, retentionPolicy, defaultEventBasedHold or billing set +// to null; a null softDeletePolicy is rejected with 400. Insert and +// patch ignore the fields the server sets, effectiveTime and isLocked in +// the policies included, and a patch accepts the insert-only name, +// location and customPlacementConfig only unchanged, so the body a get +// returns can be sent back. objectRetention is set only by +// enableObjectRetention on insert, since the JSON API offers no way to +// turn it on later, so a body's objectRetention must match it: on +// insert what the parameter gives, and on patch what the bucket has, +// with 400 otherwise. Every bucket reports a generation (inferred). // - Objects record their size, MD5 and CRC32C, not their data; a crc32c or // md5Hash in the upload's metadata must match the data. Resumable // uploads, media reads (objects.get with alt=media answers 404), copies, diff --git a/sdktest/gcp/storage/permissions_test.go b/sdktest/gcp/storage/permissions_test.go index e61d6b2..1c3158b 100644 --- a/sdktest/gcp/storage/permissions_test.go +++ b/sdktest/gcp/storage/permissions_test.go @@ -136,3 +136,41 @@ func TestPolicyNeedsSetRetentionToRemoveRetention(t *testing.T) { _, err = svc.Objects.Update("data", "a.txt", &rawstorage.Object{}).OverrideUnlockedRetention(true).Context(t.Context()).Do() wantErr(t, "remove retention by leaving it out", err, http.StatusForbidden, "forbidden", "Permission 'storage.objects.setRetention' denied") } + +func TestPolicyChecksTheGenerationAnUpdateNames(t *testing.T) { + policy := access.Allow("storage.buckets.create", "storage.buckets.enableObjectRetention", "storage.objects.create", + "storage.objects.update", "storage.objects.overrideUnlockedRetention") + s := storage.New(storage.WithPolicy(policy)) + url := serve(t, s) + c, svc := clientAt(t, url), rawService(t, url) + if err := c.Bucket("versions").SetObjectRetention(true).Create(t.Context(), project, &gcs.BucketAttrs{VersioningEnabled: true}); err != nil { + t.Fatal(err) + } + put := func(name, data string) storage.Object { + t.Helper() + o, err := s.PutObject("versions", name, []byte(data)) + if err != nil { + t.Fatal(err) + } + return o + } + first := put("a.txt", "one") + if _, err := s.SetObjectRetention("versions", "a.txt", &storage.Retention{Mode: "Unlocked", RetainUntilTime: time.Now().Add(time.Hour).Format(time.RFC3339)}); err != nil { + t.Fatal(err) + } + put("a.txt", "two") + _, err := svc.Objects.Update("versions", "a.txt", &rawstorage.Object{}).Generation(first.Generation).OverrideUnlockedRetention(true).Context(t.Context()).Do() + wantErr(t, "remove a noncurrent generation's retention", err, http.StatusForbidden, "forbidden", "Permission 'storage.objects.setRetention' denied") + if _, err := svc.Objects.Update("versions", "a.txt", &rawstorage.Object{}).Context(t.Context()).Do(); err != nil { + t.Fatalf("update the live generation, which has no retention: %v", err) + } + + older := put("b.txt", "one") + put("b.txt", "two") + if _, err := s.SetObjectRetention("versions", "b.txt", &storage.Retention{Mode: "Unlocked", RetainUntilTime: time.Now().Add(time.Hour).Format(time.RFC3339)}); err != nil { + t.Fatal(err) + } + if _, err := svc.Objects.Update("versions", "b.txt", &rawstorage.Object{}).Generation(older.Generation).Context(t.Context()).Do(); err != nil { + t.Fatalf("update a noncurrent generation without retention while the live one has it: %v", err) + } +} diff --git a/sdktest/gcp/storage/protection_test.go b/sdktest/gcp/storage/protection_test.go index ef21348..3a74def 100644 --- a/sdktest/gcp/storage/protection_test.go +++ b/sdktest/gcp/storage/protection_test.go @@ -815,3 +815,15 @@ func TestBucketReadModifyWrite(t *testing.T) { t.Fatalf("patch with a get's body = %+v, %v", got, err) } } + +func TestPatchRPO(t *testing.T) { + c := newClient(t, storage.New()) + b := c.Bucket("rpo") + if err := b.Create(t.Context(), project, &gcs.BucketAttrs{Location: "nam4"}); err != nil { + t.Fatal(err) + } + got, err := b.Update(t.Context(), gcs.BucketAttrsToUpdate{RPO: gcs.RPOAsyncTurbo}) + if err != nil || got.RPO != gcs.RPOAsyncTurbo { + t.Fatalf("patch rpo = %+v, %v", got, err) + } +} From 4f41db2b96a1438f5f066d0d4d1418b7ce31efea Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Thu, 5 Nov 2026 12:17:29 -0600 Subject: [PATCH 8/8] fix(storage): an update with an invalid generation answers 400 under a policy --- gcp/storage/http_test.go | 18 ++++++++++++++++++ gcp/storage/permissions.go | 6 +++++- gcp/storage/server.go | 14 +++++++++----- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/gcp/storage/http_test.go b/gcp/storage/http_test.go index 1c0b819..7b7736c 100644 --- a/gcp/storage/http_test.go +++ b/gcp/storage/http_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/parallelworks/squall/access" "github.com/parallelworks/squall/gcp/storage" ) @@ -279,6 +280,23 @@ func TestBucketPatchReadsTheClockOnce(t *testing.T) { } } +func TestUpdateWithAnInvalidGenerationUnderAPolicy(t *testing.T) { + s := storage.New(storage.WithPolicy(access.Allow("storage.objects.update"))) + if _, err := s.PutBucket(storage.Bucket{Name: "gens", Project: "p", ObjectRetention: &storage.BucketObjectRetention{Mode: "Enabled"}}); err != nil { + t.Fatal(err) + } + if _, err := s.PutObject("gens", "k", []byte("v")); err != nil { + t.Fatal(err) + } + if _, err := s.SetObjectRetention("gens", "k", &storage.Retention{Mode: "Unlocked", RetainUntilTime: "2099-01-01T00:00:00Z"}); err != nil { + t.Fatal(err) + } + code, body := do(t, s, http.MethodPut, "/storage/v1/b/gens/o/k?generation=soon", "application/json", "{}") + if code != http.StatusBadRequest || !strings.Contains(body, "Invalid value for generation") { + t.Fatalf("update = %d %s", code, body) + } +} + func FuzzServeHTTP(f *testing.F) { multipartBody := "--x\r\nContent-Type: application/json\r\n\r\n{\"name\":\"o\"}\r\n--x\r\nContent-Type: text/plain\r\n\r\nhi\r\n--x--\r\n" for _, seed := range []struct{ method, target, contentType, body string }{ diff --git a/gcp/storage/permissions.go b/gcp/storage/permissions.go index b84cf27..4ff9706 100644 --- a/gcp/storage/permissions.go +++ b/gcp/storage/permissions.go @@ -99,7 +99,11 @@ func (s *Server) retained(resource, generation string) bool { return false } o := b.liveObject(name) - if gen, err := strconv.ParseInt(generation, 10, 64); err == nil { + if generation != "" { + gen, err := strconv.ParseInt(generation, 10, 64) + if err != nil { + return false + } o = b.generation(name, gen) } return o != nil && o.Retention != nil diff --git a/gcp/storage/server.go b/gcp/storage/server.go index d2db1f8..eaa46fc 100644 --- a/gcp/storage/server.go +++ b/gcp/storage/server.go @@ -46,10 +46,11 @@ // the policies included, and a patch accepts the insert-only name, // location and customPlacementConfig only unchanged, so the body a get // returns can be sent back. objectRetention is set only by -// enableObjectRetention on insert, since the JSON API offers no way to -// turn it on later, so a body's objectRetention must match it: on -// insert what the parameter gives, and on patch what the bucket has, -// with 400 otherwise. Every bucket reports a generation (inferred). +// enableObjectRetention on insert, since the docs give only the console +// for turning it on for an existing bucket, so a body's objectRetention +// must match it: on insert what the parameter gives, and on patch what +// the bucket has, with 400 otherwise. Every bucket reports a generation +// (inferred). // - Objects record their size, MD5 and CRC32C, not their data; a crc32c or // md5Hash in the upload's metadata must match the data. Resumable // uploads, media reads (objects.get with alt=media answers 404), copies, @@ -179,7 +180,10 @@ // projection=full needs nothing more: Cloud Storage leaves out the ACLs a // caller may not read rather than refusing the call, and the official // clients ask for it on every get and list. Permissions are not checked -// against the bucket they act on: a grant covers every bucket. +// against the bucket they act on: a grant covers every bucket. They are +// worked out before the call takes the server's lock, so a concurrent call +// that changes what they depend on, such as an object's retention, in +// between can leave a call checked against the state just before it. // // A lost response aborts the handler with [http.ErrAbortHandler] after // sending the response header, so the [http.Server] serving it closes the