Skip to content

fix: forward the legal hold as an Object Lock header on federated CopyObject - #172

Open
Aeirx wants to merge 1 commit into
pgsty:mainfrom
Aeirx:fix/federated-copyobject-legal-hold
Open

fix: forward the legal hold as an Object Lock header on federated CopyObject#172
Aeirx wants to merge 1 commit into
pgsty:mainfrom
Aeirx:fix/federated-copyobject-legal-hold

Conversation

@Aeirx

@Aeirx Aeirx commented Sep 9, 2026

Copy link
Copy Markdown

Fixes #166.

The loss

A federated CopyObject requesting x-amz-object-lock-legal-hold: ON answered 200 with the destination carrying no hold. The resolved value arrived at the remote as X-Amz-Meta-X-Amz-Object-Lock-Legal-Hold, which nothing applies. Retention requested on the same copy survived, so the response and the retention state both looked right.

Root cause, re-checked against the selected minio-go

The federation branch hands the resolved metadata map to Core.PutObject as PutObjectOptions.UserMetadata. Header() writes the typed lock fields first and then classifies each UserMetadata key:

for k, v := range opts.UserMetadata {
    if isAmzHeader(k) || isStandardHeader(k) || isStorageClassHeader(k) || isMinioHeader(k) {
        header.Set(k, v)
    } else {
        header.Set("x-amz-meta-"+k, v)
    }
}
  • supportedHeaders lists x-amz-object-lock-mode and x-amz-object-lock-retain-until-date, not x-amz-object-lock-legal-hold
  • isAmzHeader matches only x-amz-meta-, x-amz-grant-, x-amz-acl, SSE and x-amz-checksum-

So the hold falls to the else and retention does not. PutObjectOptions.validate() would have rejected the key, but Core.PutObject goes straight to the low-level PUT and never calls it.

The change

Move only the hold onto the typed field and forward a clone without the raw key.

The clone is load-bearing twice:

  • typed fields are written before the UserMetadata loop, so a leftover raw key would add a bogus x-amz-meta- entry beside the correct header;
  • objInfo.UserDefined (the copy response and the event) is now rebuilt from the resolved values rather than from opts.UserMetadata, which no longer carries the hold. Building it from the forwarding map would under-report a hold the proxy did apply.

Retention deliberately stays in the map. It already passes through as a standard header, and moving it to the typed RetainUntilDate field would format with time.RFC3339 and truncate a retain-until date such as 2030-01-01T00:00:00.789Z to whole seconds.

Test

TestAPIFederatedCopyObjectLegalHold drives the legacy etcd federation branch through setupCopyObjectFederation and asserts the wire, since that is where the value was being lost:

  • the remote receives X-Amz-Object-Lock-Legal-Hold
  • it never receives the x-amz-meta- spelling
  • the destination version actually stores the hold

It fails without the change with legal hold forwarded as user metadata [ON]; the destination stores no hold.

One fixture note: the helper sets the lock configuration through globalBucketMetadataSys and calls parseAllConfigs. Writing ObjectLockConfigXML alone is not enough — BucketMetadata keeps a parsed copy that the lookups read, and without the parse the copy is rejected earlier with 400 InvalidRequest: Bucket is missing ObjectLockConfiguration, which is the second failure mode noted in the issue.

go build ./cmd/...                                          ok
go vet ./cmd/                                               ok
go test ./cmd/ -run TestAPIFederatedCopyObjectLegalHold     ok
go test ./cmd/ -run 'TestAPIFederated|TestAPICopyObject|ObjectLock'   ok  (80s)
gofmt                                                       clean

Scope

Only the federation serialization of the hold, as the issue scopes it. Untouched and tracked separately: #165 (a legal-hold header suppressing the bucket's default retention — after this change a hold-only federated copy stores the hold and no retention, which is the behaviour #165 is about), and the proxy validating Object Lock against its own configuration rather than the destination's.

…yObject

A cross-deployment CopyObject that requests
`x-amz-object-lock-legal-hold: ON` answered 200 while the destination
carried no hold. The resolved value reached the remote as ordinary user
metadata, `X-Amz-Meta-X-Amz-Object-Lock-Legal-Hold`, so nothing applied it.
Retention requested on the same copy survived, which is what made the loss
easy to miss.

The federation branch passes the resolved metadata map straight to
`Core.PutObject` as `PutObjectOptions.UserMetadata`. minio-go's `Header()`
writes the typed lock fields first, then prefixes every UserMetadata key it
does not recognise with `x-amz-meta-`; `supportedHeaders` covers
`x-amz-object-lock-mode` and `x-amz-object-lock-retain-until-date` but not
`x-amz-object-lock-legal-hold`, and `isAmzHeader` does not match it either.
Retention therefore arrives as real headers and the hold does not. The
high-level `validate()` that would have rejected the key never runs, because
`Core.PutObject` goes straight to the low-level PUT.

Carry the hold on the typed `LegalHold` option and forward a cloned map with
the raw key removed. The clone matters twice: typed fields are written before
the UserMetadata loop, so a leftover raw key would add a bogus `x-amz-meta-`
entry beside the correct header, and the proxy's own response and event
metadata are rebuilt from the resolved values rather than the forwarding map,
which no longer carries the hold.

Retention stays in the map deliberately. It already passes through as a
standard header, and moving it to the typed `RetainUntilDate` field would
format with `time.RFC3339` and truncate a retain-until date to whole seconds.

The new test asserts the wire: the remote must receive
`X-Amz-Object-Lock-Legal-Hold` and never the `x-amz-meta-` spelling, and the
destination version must actually store the hold. It fails without the change
with "legal hold forwarded as user metadata [ON]".

Fixes pgsty#166

Signed-off-by: Ayush Sharma <72848455+Aeirx@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P1] Federated CopyObject silently drops an explicit legal hold

1 participant