mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-17 19:48:35 +02:00
268424a5e1c9e793a9305373540e64db7fe7eeae
1488
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Chris Lu
|
268424a5e1 |
s3: extract zip members on PutObject (#11786)
* filer: add zip archive extraction helpers SpoolZipReader makes an upload body seekable for archive/zip, SanitizeZipMemberName rejects zip-slip and non-regular members, and CheckZipLimits caps member count and total uncompressed size. For seaweedfs/seaweedfs#11722 * filer: extract zip members on ?extract=zip upload POST/PUT to a directory path with ?extract=zip (or X-Seaweedfs-Extract: zip) writes each archive member as its own entry, so a bundle of small files lands in one request instead of one roundtrip each. Members keep their mtime, mode and mime-by-extension; directories are created; unsafe names are skipped and reported. extract.keep=true also stores the archive itself. For seaweedfs/seaweedfs#11722 * filer: bound zip spool memory by actual bytes A chunked upload has no Content-Length, so the old size hint let an unbounded body go through io.ReadAll. Read up to the memory threshold plus one byte first, then spill the prefix and the rest to a temp file, capped at ZipExtractMaxBytes total. * filer: guard zip member size total against overflow Declared member sizes summing past math.MaxUint64 wrapped the running total and slipped past the 1 TiB cap. * server: only take the inline fast path at a short first read uploadReaderToChunks read at most chunkSize bytes, then treated any read below SaveToFilerLimit as the whole file. When SaveToFilerLimit exceeded chunkSize, longer uploads were stored truncated. * server: resolve storage options per extracted zip member A member can land under a storage rule more specific than the request path. Re-running detectStorageOption0 per destination applies that rule's collection, replication, TTL, filename limit, and read-only guard to files and directories alike, while explicit query overrides still win. * server: accept raw request body for POST extract uploads Every POST was forced through MultipartReader, so a raw zip body always got a 400. Only parse multipart when the Content-Type is actually multipart. * server: extract zip directories before files and stop on cancel Processing directory members first keeps their stored mtime/mode from being overwritten by parents auto-created for an earlier file member. Checking ctx.Err() between members keeps a disconnected client from driving thousands more writes. * server: assert stored mtime and mode for extracted members * s3: add zip extraction header constants * s3: extract zip members on PutObject with extract header x-amz-meta-snowball-auto-extract or x-seaweedfs-extract: zip on a PutObject fans out the archive to one object per member under the object's directory, reusing putToFiler so chunking, collections, storage rules, versioning, and bucket-default encryption apply to each member. The archive itself is not stored. Explicit SSE request headers are rejected since one encryption config cannot represent independently created member objects. * s3: bound member fan-out by request cancellation Stop starting new member writes once the request context is done, matching the filer extract loop. * filer: bound compressed zip spool and reject non-local member names The 1 TiB uncompressed limit was also capping the compressed bytes spooled to the host's temp directory, so a handful of concurrent extractions could fill the filer's disk before any ZIP validation ran. SpoolZipReader now takes a separate compressed-size limit (the filer passes its TUS max upload size) with a 5 GiB default. SanitizeZipMemberName gains a filepath.IsLocal check: the segment loop already rejects "..", but CodeQL only treats member names as sanitized through a recognized guard, and the extra check also covers reserved Windows names. * server: honor destination rules for zip members and clamp maxMB An x-seaweedfs-destination header picks the storage rules for a logical destination while members still land under the request path, matching PostHandler. Directory members now also enforce the actual path's read-only rule instead of relying on option resolution to fail. maxMB up to 2048 overflowed the int32 chunk size, so an oversized query value made extraction (and ordinary autoChunk uploads) store members empty yet report success. The parsing is shared and clamped at 2047 MB. * s3: scope zip member writes through the per-key checks PutObject applies The extraction loop bypassed most of what a standalone PutObject would face: members were written without IAM authorization for their key, directory members replaced markers without entry lock checks, and lock headers persisted retention without the s3:PutObjectRetention or s3:PutObjectLegalHold permissions. authorizeMember now runs IAM, table-bucket path rules, bucket policy, lock-header validation, and non-versioned lock enforcement per member key, and directory members take the same guarded mkdir path as an explicit dir marker. The archive's own Content-Md5 and x-amz-checksum-* values are verified while the body spools instead of being silently dropped, members stop inheriting the archive's Content-Encoding, and non-versioned members resolve their lifecycle TTL like normal uploads. * s3: fail malformed lock headers before spooling the archive Per-member re-validation turned a bad object-lock header into one error entry per member inside a 200 response. Validate once up front like PutObjectHandler does, and keep only the dedicated retention/legal-hold permission checks scoped to each member key. * filer: return 413 for oversized archives and honor saveInside in extract Map filer.ErrZipTooLarge to 413 instead of 500 for client-side archive size violations. Extracted members written under saveInside now store as inline entry content instead of unreachable volume chunks. * s3: write extracted members under their own request context Point member writes at the member key via mux.SetURLVars so SSE-KMS builds the encryption context from the member ARN, not the archive's. Map the spool limit error to ErrEntityTooLarge. |
||
|
Chris Lu
|
a8affc851d |
filer: extract zip members on ?extract=zip upload (#11782)
* filer: add zip archive extraction helpers SpoolZipReader makes an upload body seekable for archive/zip, SanitizeZipMemberName rejects zip-slip and non-regular members, and CheckZipLimits caps member count and total uncompressed size. For seaweedfs/seaweedfs#11722 * filer: extract zip members on ?extract=zip upload POST/PUT to a directory path with ?extract=zip (or X-Seaweedfs-Extract: zip) writes each archive member as its own entry, so a bundle of small files lands in one request instead of one roundtrip each. Members keep their mtime, mode and mime-by-extension; directories are created; unsafe names are skipped and reported. extract.keep=true also stores the archive itself. For seaweedfs/seaweedfs#11722 * filer: bound zip spool memory by actual bytes A chunked upload has no Content-Length, so the old size hint let an unbounded body go through io.ReadAll. Read up to the memory threshold plus one byte first, then spill the prefix and the rest to a temp file, capped at ZipExtractMaxBytes total. * filer: guard zip member size total against overflow Declared member sizes summing past math.MaxUint64 wrapped the running total and slipped past the 1 TiB cap. * server: only take the inline fast path at a short first read uploadReaderToChunks read at most chunkSize bytes, then treated any read below SaveToFilerLimit as the whole file. When SaveToFilerLimit exceeded chunkSize, longer uploads were stored truncated. * server: resolve storage options per extracted zip member A member can land under a storage rule more specific than the request path. Re-running detectStorageOption0 per destination applies that rule's collection, replication, TTL, filename limit, and read-only guard to files and directories alike, while explicit query overrides still win. * server: accept raw request body for POST extract uploads Every POST was forced through MultipartReader, so a raw zip body always got a 400. Only parse multipart when the Content-Type is actually multipart. * server: extract zip directories before files and stop on cancel Processing directory members first keeps their stored mtime/mode from being overwritten by parents auto-created for an earlier file member. Checking ctx.Err() between members keeps a disconnected client from driving thousands more writes. * server: assert stored mtime and mode for extracted members * filer: bound compressed zip spool and reject non-local member names The 1 TiB uncompressed limit was also capping the compressed bytes spooled to the host's temp directory, so a handful of concurrent extractions could fill the filer's disk before any ZIP validation ran. SpoolZipReader now takes a separate compressed-size limit (the filer passes its TUS max upload size) with a 5 GiB default. SanitizeZipMemberName gains a filepath.IsLocal check: the segment loop already rejects "..", but CodeQL only treats member names as sanitized through a recognized guard, and the extra check also covers reserved Windows names. * server: honor destination rules for zip members and clamp maxMB An x-seaweedfs-destination header picks the storage rules for a logical destination while members still land under the request path, matching PostHandler. Directory members now also enforce the actual path's read-only rule instead of relying on option resolution to fail. maxMB up to 2048 overflowed the int32 chunk size, so an oversized query value made extraction (and ordinary autoChunk uploads) store members empty yet report success. The parsing is shared and clamped at 2047 MB. * filer: return 413 for oversized archives and honor saveInside in extract Map filer.ErrZipTooLarge to 413 instead of 500 for client-side archive size violations. Extracted members written under saveInside now store as inline entry content instead of unreachable volume chunks. |
||
|
Chris LuandDevin
|
6da432b8bf |
filer: make a retried ObjectTransaction replay its recorded outcome (#11788)
* filer_pb: add request_id to ObjectTransactionRequest Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: replay recorded outcome for a retried ObjectTransaction A failover replay carries the same request_id; under the same path lock the owner replays the recorded outcome instead of re-applying the mutations over whatever superseded them. Only application-level outcomes are recorded — a lookup error leaves the request free to re-run. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: stamp routed object transactions with a request id One id per logical transaction so failover retries replay the recorded outcome instead of re-applying the mutations. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: scope transaction replay tokens per path and keep them for their TTL The dedup key includes the lock path so a token reused on a different object cannot replay this outcome. Records are never evicted early — an early eviction would let a replay re-apply — so the expiry sweep is amortized and the map is bounded by the insert rate over the TTL. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: bound the transaction replay cache at capacity * filer: do not record indeterminate transaction outcomes A mutation error can be transient or follow a partial apply; recording it replays that failure to a retry for the whole TTL instead of letting the same idempotent mutations converge. Only successful transactions and precondition failures are definitive outcomes worth replaying. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: replay recorded mutation failures instead of re-applying --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
00804b32fa |
s3tables: let an explicit policy Deny veto fallback and owner allows (#11772)
* s3tables: let an explicit policy Deny veto fallback and owner allows Table operations evaluated the table or namespace policy and the bucket policy as an OR, so an explicit Deny in one policy was overridden by an Allow in the other. Owner and identity grants also short-circuited before the resource policy was read. Deny statements are now evaluated first at both levels, matching AWS explicit-deny semantics. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3tables: fail closed when a resource side has no owner --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
44657988ea |
s3: apply destination retention on copies instead of inheriting it (#11773)
* s3: apply destination retention on copies instead of inheriting it CopyObject carried the source object's retention mode, retain-until date and legal hold into the destination entry, so a copy onto a bucket with default COMPLIANCE retention produced an unlocked object that could be deleted early. Copied objects now take their retention from the copy request's lock headers or the destination bucket's default, matching PUT semantics; a versioned self-copy gets the same treatment while an unversioned metadata-only self-copy keeps the object's existing lock. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: validate lock headers on metadata-only self-copies --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
2d06e2d6c1 |
s3: evaluate tag conditions on copy-source and tagging subresources (#11771)
* s3: evaluate tag conditions on copy-source and tagging subresources Policy evaluation ran without the object entry on several paths, so a Deny conditioned on s3:ExistingObjectTag never matched: - CopyObject, UploadPartCopy and RenameObject checked the source key before its entry was fetched; repeat the check on a synthesized GET once the entry resolves. - GetObjectAttributes and Get/Put/DeleteObjectTagging never supplied the entry; recheck with the resolved entry. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: fail closed on tag-policy lookups and honor tag-conditioned grants - recheckTagConditionedPolicyWithLookup returned success when the entry lookup errored, letting a tag-gated Deny slip past a transient miss; only ErrNotFound still falls through to the handler's own NoSuchKey. - A read or delete denied in the tagless first phase now gets one more evaluation with the resolved entry's tags in hand, so an s3:ExistingObjectTag-conditioned Allow can still grant a copy-source read or rename. The deferred recheck requires an explicit Allow; indeterminate results keep the original verdict, and the existence of the key never leaks past a deny. - RenameObject now applies the same tag-aware recheck to the source removal, so a tag-gated Deny on s3:DeleteObject blocks a rename the way it blocks a direct DELETE. - GetObjectAttributes evaluates the object-entry policy recheck before conditional headers, so a 304/412 response can no longer bypass a tag-based Deny. * s3: honor identity denies in the tag-aware source recheck The deferred recheck re-evaluates only the bucket policy, so a bucket Allow could lift an explicit Deny in the caller's attached or session policies on a copy/rename source. Apply the same applicable-policy Deny check the first phase uses. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
446e6f3444 |
s3tables: scope OIDC-derived account IDs to the token issuer (#11774)
* s3tables: scope OIDC-derived account IDs to the token issuer Table ownership keyed off the bare sub claim, so two identity providers issuing the same sub collided on one account - a caller could act on tables owned by a different provider's user. OIDC bearer tokens now carry the same issuer-namespaced (sub, iss) hash STS sessions already expose as aws:userid, and getAccountID prefers it over the raw claims. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3tables: keep legacy owners working and scrub client-subject claims - OIDC bearer tokens without a subject are rejected, and a token-supplied aws:userid claim is always overwritten with the verified (sub, iss) hash, so a forged claim can no longer stand in for the principal. - Owners written before issuer scoping hold a bare subject; permission checks now re-anchor that subject under the caller's issuer, so the same (sub, iss) pair keeps its ownership while scoped owners remain issuer-isolated. The token issuer is asserted into claims alongside sub and carried through PolicyContext. * s3tables: anchor legacy subjects to the configured issuer set * s3: enumerate trusted issuers from registered providers too * s3: fail closed when the OIDC provider store cannot be listed trustedOIDCIssuers kept the registered issuers when ListOIDCProviders failed, presenting a partial set as complete. A deployment trusting one registered issuer A plus a stored issuer B would, on a listing error, report only A — the single-issuer shape under which a legacy bare subject re-anchors — letting a caller from A claim a stored subject that belongs to B and gain owner access. With no provider store the registered providers are provably complete, so keep returning them; with a store, a failed listing means the real set is unknown, so return none. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
d3a97f76bd |
s3: fix PutObject If-Match under suspended versioning (#11780)
* s3: keep suspended-versioning conditional writes off the routed condition A suspended PUT writes the null-version slot at the regular path while the entry the conditional headers evaluate lives under .versions/. The routed WriteCondition checked the request's etag against that empty slot, so a correct If-Match failed with 412. Passing uniqueWritePath sends conditional requests through the lock path, whose precondition resolves the same entry the read path serves; unconditional writes still route. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer_pb: add condition_alt_key to ObjectTransactionRequest * filer: evaluate a transaction condition against a fallback entry condition_alt_key names the entry consulted when the primary is absent, so a suspended-versioning write can check 'the null slot if present, else the latest version' atomically under the lock. * s3: route suspended-versioning conditional writes with a fallback key The gateway names the latest version file as condition_alt_key, so the transaction checks the entry the read path serves — null slot first, latest version when it is absent — under the same lock that applies the write. Conditional suspended PUTs route again instead of splitting check and write across a gateway lock that routed writes skip. * filer: resolve the condition fallback under the transaction lock The gateway named the pointed-at version file as condition_alt_key, resolved before the transaction ran: a concurrent version delete could move the latest pointer between the gateway read and the locked check, and a delete-marker latest read as an existing object. The alt key is now the .versions directory and the filer resolves the latest-version pointer under the same lock that serializes the write — a delete marker resolves to absent, and unresolvable pointer state fails closed. * filer: fail closed on alt-key lookups and skip directory primaries A transient FindEntry error returning a nil entry read as "versions dir absent" and let If-None-Match writes pass. Check the error before the entry. And a bare directory at the object path satisfied the primary lookup, so the .versions fallback never ran — directories are not the object, treat them as an absent slot. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: scope directory-as-absent conditions to alt-key callers --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
dependabot[bot]andChris Lu
|
82f0a37efe |
build(deps): bump github.com/Azure/azure-sdk-for-go/sdk/storage/azblob from 1.8.0 to 1.8.2 (#11740)
* build(deps): bump github.com/Azure/azure-sdk-for-go/sdk/storage/azblob Bumps [github.com/Azure/azure-sdk-for-go/sdk/storage/azblob](https://github.com/Azure/azure-sdk-for-go) from 1.8.0 to 1.8.2. - [Release notes](https://github.com/Azure/azure-sdk-for-go/releases) - [Commits](https://github.com/Azure/azure-sdk-for-go/compare/sdk/azcore/v1.8.0...sdk/azidentity/v1.8.2) --- updated-dependencies: - dependency-name: github.com/Azure/azure-sdk-for-go/sdk/storage/azblob dependency-version: 1.8.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * test: restore loadBucketMetadataFromFiler stub after TestGetBucketMetadata TestGetBucketMetadata replaced the package-level loadBucketMetadataFromFiler with a stub that sleeps one second per load and never restored it. Every test running after it in the same binary paid that sleep on each bucket metadata cache miss: TestPutObjectUploadACL (~245s) and TestCopyObjectDestinationACL (~46s) each created a fresh BucketRegistry per subtest, so the stub fired hundreds of times. In CI this stretched the Test job past 11 minutes and left it exposed to runner cancellation. Restore the real loader via t.Cleanup so the slow stub is scoped to the test that needs it. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris Lu
|
3f8035deb7 |
security: share the admin gRPC peer authentication check
The local-peer fallback and bearer-token validation used by the filer IAM gRPC path move into weed/security so every admin-plane gRPC service can enforce the same contract. A client certificate verified by the server's TLS config now also counts as authenticated. |
||
|
Chris Lu
|
a4c8d31cb8 |
s3api: ignore response header overrides on anonymous requests
AWS requires response-* query parameters to be signed; an anonymous caller must not choose how a stored object is served. |
||
|
Chris LuandDevin
|
d69f45ee83 |
s3: validate multipart initiate ACLs like PUT
CreateMultipartUpload accepts canned and custom ACLs on initiation; run them through the same authorization path as PutObject so an explicit ACL requires s3:PutObjectAcl, and persist the validated grants on the upload entry for completion to apply. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
3aaf80b33b |
s3: stop trusting internal ACL headers in ParseS3Metadata
Multipart initiation stored client-supplied Seaweed-X-Amz-Acl and Seaweed-X-Amz-Owner request headers onto the upload entry, and CompleteMultipartUpload copied them to the finished object, letting a caller forge ownership or a public ACL without s3:PutObjectAcl. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
85f04b54fd |
s3tables: check data-path ancestors for catalog markers on DeleteTable
The decoupled-delete guard only inspects the resolved data path itself, but the catalog marker lives on a table's name path, so a location that resolves inside another table's directory (its marker-less data subdirectory) slips through and the recursive purge destroys the other table's data. Walk the data path's ancestors within the bucket and refuse when any carries a table or view catalog entry. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
6f93f87446 |
s3tables: confine caller-supplied metadataLocation to the table's namespace
CreateTable/RegisterTable/UpdateTable (and the view equivalents) validate the location only for bucket and traversal, so a tenant can register a table whose location resolves into another namespace's directory and DeleteTable will purge it. Require the location to live under the caller's namespace subtree, the boundary per-namespace delegation uses. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Zhe ChenandChris Lu
|
547befc070 |
s3api: preserve remote object ETags across cache fills (#11704)
* s3api: preserve remote object ETags across cache fills * s3api: align remote copy validators and reject stale ETags * s3api: keep remote ETag through metadata touches, drop it after resizes The remote-ETag check required Mtime to match RemoteMtime, so a utimens on a cold remote object returned an empty ETag until a later cache fill restored the remote value. It also trusted LastLocalSyncTsNs alone, so a setattr resize that leaves chunks and the stamp untouched kept serving the old remote ETag for different local bytes. Require FileSize to match RemoteSize for both the synced and remote-only paths and stop comparing modification times: content changes already surface through cleared sync stamps, chunks, or inline content, while a metadata-only touch leaves the remote bytes authoritative. Regression tests cover touched cold and cached entries and resized cached entries. --------- Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> |
||
|
|
fac021544e |
s3api: register the azurekms KMS provider under its build tag (#11691)
* s3api: register the azurekms KMS provider under its build tag weed/kms/azure registers itself with weed/kms from an init(), but the blank import that triggers it was commented out in auth_credentials.go (TODO: Fix Azure SDK compatibility issues). Nothing else imports the package, so building with -tags azurekms still ends with "KMS provider 'azure' not registered" at startup for any config using "type": "azure". Move the import into weed/s3api/kms_azure.go behind //go:build azurekms so default builds stay free of the Azure SDK, and drop the dead comment. Closes #11686 * ci: compile and test azurekms-gated code weed/kms/azure is excluded from every default build, so nothing in CI compiled it; that is how the provider shipped unregistered. Build the tree and run the kms tests with -tags azurekms on every Go change. --------- Co-authored-by: Yi-111-a <> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
|
d050464316 |
s3api: apply bucket default encryption when volume data encryption is enabled (#11681)
* s3api: apply bucket default encryption when volume data encryption is enabled When -s3.encryptVolumeData (s3a.cipher) is enabled, putToFiler skipped checking and applying bucket default encryption due to a '!s3a.cipher' guard. Volume-level data encryption and object-level Server-Side Encryption (SSE-S3 / SSE-KMS) operate at different layers, and explicit SSE headers already work alongside volume encryption. Remove the '!s3a.cipher' guard so PutObject without explicit SSE headers inherits bucket default encryption regardless of volume data encryption. Add regression test TestPutObjectAppliesBucketDefaultEncryptionWithVolumeCipher. Signed-off-by: Tyagiquamar <mohdquamartyagi@gmail.com> * s3api: decrypt the volume cipher on direct SSE chunk reads fetchFullChunk, fetchChunkViewData, and createEncryptedChunkReader fetched raw bytes over HTTP, so volume-encrypted chunks reached SSE-S3/KMS/C decryptors still ciphered. Route them through fetchChunkData: encrypted or compressed chunks go through RetriedFetchChunkData (cipher-aware, slices plaintext space for views); plain chunks keep the streaming range read. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: cover cipher-aware chunk reads with a fake volume server The fake volume now serves stored GETs with Range support, and TestFetchChunkDataDecryptsVolumeCipher verifies full-chunk and view reads return plaintext for ciphered chunks while plain chunks still slice via HTTP ranges. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: guard fake volume server stored map with its mutex The HTTP handler goroutine read v.stored while test goroutines wrote it, a data race go test -race can flag. Lock v.mu around the map read and the test writes. --------- Signed-off-by: Tyagiquamar <mohdquamartyagi@gmail.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
|
b2eefb8a73 |
filer: fail the request, not the process, when chunks cannot be resolved (#11685)
* filer: fail the request, not the process, when chunks cannot be resolved CompactFileChunks and ViewFromChunks discarded the error from NonOverlappingVisibleIntervals and iterated the nil interval list it returns on failure. A CreateEntry or UpdateEntry whose context was cancelled therefore panicked in SeparateGarbageChunks and took the whole filer down, losing the metadata log it had not yet flushed. Both functions now return the error. cleanupChunks fails the request with it, the mount flush paths return it, the replication sinks and the S3 SSE range reader return it, and a ChunkStreamReader that cannot build its view fails every read with it and reports it as its SourceError instead of reading as an empty file. Fixes #11682 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * filer: propagate chunk resolution failures in scan paths NonOverlappingVisibleIntervals can fail without a cancelled context; the query engine and log-store callers discarded the error and dereferenced nil intervals. * query: propagate chunk resolution errors instead of skipping files When a manifest cannot be resolved, ReadParquetStatistics, countLiveLogRowsExcludingParquetSources, and computeLiveLogMinMax logged a warning and skipped the failed file, so queries returned understated counts and wrong min/max values. Surface the errors instead: the fast-path aggregation collector now returns a DataSourceError so the engine falls back to a full scan, and the row-count helpers return the error rather than a partial total. --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Bharat Kaurav
|
774c9927a7 |
s3api: validate ListObjectVersions max-keys and honor max-keys=0 (#11703)
* fix(s3api): accept max-keys up to 2147483647 and cap list pages at 1000 * add tests for ListObjects max-keys range and 1000 cap * refactor(s3api): return uint16 from parseMaxKeys to avoid narrowing casts * fix(s3api): validate ListObjectVersions max-keys and honor max-keys=0 * add test for ListObjectVersions max-keys=0 * fix(s3api): widen uint16 max-keys for ListObjectVersions |
||
|
Peter Dodd
|
5f1a742726 |
fix: return NoSuchKey and drop stale entries when a remote-mounted object is gone from the remote (#11680)
A remote-only entry whose object was deleted from the remote storage outside the filer answered GET with 500 and stayed in the filer. The remote's not-found was lost on the way: the backends' ReadFile returned it as an untyped error, so FetchAndWriteNeedle failed with codes.Unknown and nothing downstream could tell it from any other failure. - remote_storage: GCS, S3 (NoSuchKey) and Azure (BlobNotFound) reads return ErrRemoteObjectNotFound. GCS reports a missing bucket the same way as a missing object, so it confirms the bucket with a listing. - volume server and filer: the not-found crosses gRPC as codes.NotFound carrying the sentinel's text, and the filer's cache RPC returns codes.NotFound, which the S3 gateway already maps to NoSuchKey. - s3api: the origin fallback answers NoSuchKey on a confirmed not-found. - filer: a confirmed not-found removes the stale entry, so the lazy remote-metadata cache converges on the remote. Only remote-only files outside .versions and without an active object lock are removed, only if unchanged since the fetch (checked on the object's write owner, under the lock S3 object writes take), and with a metadata-only delete: the filer skips its inline remote delete, and the delete events' entries carry a marker that makes filer.remote.sync and filer.remote.gateway skip their remote delete, while filer.sync still replicates it. The replicated DeleteEntryRequest carries keep_remote_object, so the destination's delete events are marked too. The store drops the marker from every write, so clients cannot plant it. |
||
|
|
d3dc03c85a |
s3api: resolve volume data encryption in CopyObject SSE flows (#11646) (#11683)
* s3api: resolve volume data encryption in CopyObject SSE flows (#11646) * s3api: honor bucket-default KMS key on copy and fix transformed-upload metadata - Synthesize the destination bucket's default encryption as request headers before any SSE evaluation, so the configured KMS key ID and bucket-key setting reach the copy paths instead of only a boolean. - uploadTransformedChunkData returns the upload result so callers record the uploader's cipher key AND compression decision; a wrongly cleared IsCompressed made transformed copies unreadable. - decompressChunkVolumeCipher fails loudly when a compressed chunk does not decompress, instead of uploading still-compressed bytes marked uncompressed. - copyMultipartSSECChunk now strips the volume cipher and re-encrypts on upload like the other transform paths. - The ciphered inner upload now uses the caller's private BytesBuffer. * s3api: extract copy bucket-default header synthesis for coverage Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: test bucket-default encryption header synthesis on copy Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: validate copy encryption headers before bucket defaults and resolve empty KMS key * s3api: name the AWS-managed SSE-KMS default key once --------- Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris Lu
|
c023493cf0 |
s3api: tolerate session tokens on statically configured credentials (#11694)
* s3api: consolidate session token extraction into extractSessionToken Three call sites duplicated the same header/header/query lookup; share one helper named after the existing s3tables equivalent. * s3api: tolerate session tokens on statically configured credentials Credential vendors like Unity Catalog emit a session token with every vended credential, including static ones (UC's StaticAwsCredentialGenerator only engages when s3.sessionToken is set). Requests signed by a configured access key were routed to STS validation and rejected, so static credential vending never worked against SeaweedFS. Resolve the access key first: when it maps to a configured credential the signature alone authenticates the request, and the attached token is marked ignored so authorization does not route it into the STS session-policy path. STS-issued access keys are never in the static map, so temporary credentials still validate their token exactly as before. * s3api: cover static credentials carrying a session token * test: exercise UC static credential vending against SeaweedFS s3.sessionToken.0 selects UC's StaticAwsCredentialGenerator, which vends the configured keys verbatim. The vended session token is foreign to SeaweedFS and previously failed SigV4; now it round-trips through temporary-table-credentials into real S3 I/O. * s3api: reject temporary credentials in GetFederationToken after auth Token presence alone cannot distinguish a temporary credential from a statically configured one carrying a vended token. Move the check behind verifyV4Signature and key it on the operative session token so tolerated tokens keep the caller eligible. * s3api: test GetFederationToken with authenticated temporary credentials Sign the rejection cases with real session credentials so they reach the post-auth check, and cover a vended static credential being accepted. * test: check vended-credential delete error in UC integration test |
||
|
Paolo Valletta
|
4d306e1279 |
s3api: revoke the identities a config reload no longer declares (#11679)
* s3api: revoke the identities a config reload no longer declares A static config file reload merges with isFullState=false, so an identity deleted from -s3.config kept authenticating with all of its access keys until the process restarted. An operator revoking a leaked key got no error and no revocation. The file is the source of truth for the identities it declares, so a reload now drops the ones that have left it, together with their access keys and those of their service accounts. staticIdentityNames was only ever added to, which kept a removed name protected as well; the names the file itself declares are tracked separately from the AWS environment credentials, so a reload never revokes what the file never declared. * s3api: keep a file reload from replacing the dynamic store Removing the last identity from a config file emptied staticIdentityNames, so useStaticConfig turned false and the next reload of that file took the replace path: every filer-managed identity and its access keys disappeared until a dynamic reload brought them back. A static config file is authoritative for the identities it declares and never for the dynamic store, so a file load now always merges. The first file load at startup takes the merge path as well, from an empty state. Reported by the Devin and Greptile reviews of this PR. TestReloadStaticConfigWithoutIdentitiesKeepsDynamic reloads an emptied file twice and asserts that a filer-managed identity and its access key survive both; the existing test now also checks that the service account key was loaded before the reload and that the environment identity's key still works after it. * s3api: clear the environment credentials in the emptied-file test The AWS environment identity is static and is re-added after every merge, so on a runner that has AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY set it kept hasStaticConfig true once the file was emptied, and the next reload still took the merge path: the regression this test guards passed unnoticed. Clear both variables whatever the runner has set. With the pre-fix condition (hasStaticConfig alone) and the variables present in the environment, the test now fails with "reload 2: a filer-managed identity must survive a reload of an emptied file". Reported by the Greptile review of this PR. |
||
|
Chris LuandDevin
|
5d80ac39b8 |
s3api: verify under the write lock before re-committing an ambiguous routed PUT (#11649)
* s3api: verify under the write lock before re-committing an ambiguous routed PUT A routed PUT whose response was lost after the owner committed, or whose transaction returned a store error, fell straight into the lock path and re-sent the same entry. A concurrent PUT that had superseded the commit in the meantime had already deleted this entry's chunks as old, so the re-commit restored metadata pointing at dead needles and the object read back 404 permanently. The lock path now resolves the routed attempt's outcome inside the write lock first: a stored entry with the uploaded chunks means the route landed; a different stored entry or an unresolved lookup refuses the re-commit with ServiceUnavailable so the client retries with a fresh upload; only a proven-absent entry falls through to the normal create. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: tighten ambiguous routed PUT recovery - a match found only after an unanswered filer is not authoritative; the unreachable filer may hold a newer entry, so refuse instead of finalizing on it (both recovery paths) - a failed post-recovery finalization now rolls the recovered entry back, matching the create path's undo - a proven-absent entry is only re-committed after probing that the uploaded chunks' needles are still alive; a committed-and-deleted PUT would otherwise write back an entry pointing at reclaimed needles - the .versions latest pointer no longer flips back to an older version when a newer one committed while the write was uncertain Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: stamp late versions noncurrent when a newer latest pointer wins The keep-newer early return in updateLatestVersionInDirectory left the version just stored without ExtNoncurrentSinceNsKey, so the lifecycle engine could never age it out. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: only a failure past mutation 0 makes a routed PUT ambiguous A deterministic refusal at the PUT mutation (e.g. "existing entry is a directory") applied nothing, but marking it ambiguous sent the lock-path fallback through conservative recovery, which found the directory entry and refused with 503 instead of the correct 409. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: keep all transaction errors ambiguous; route directory conflicts through the lock path * s3: verify uploaded chunks before re-committing over a directory A stored directory does not prove the routed PUT never committed: the route can land, a delete can remove the entry and its chunks, and a nested write can recreate the directory before recovery takes the lock. Re-committing then stores an entry pointing at dead needles. Probe the uploaded chunks first and refuse with ServiceUnavailable when they can no longer be verified. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
|
838c554e33 |
s3api: reject SSE headers PutObject cannot honor, as CopyObject does (#11642)
* s3api: reject SSE headers PutObject cannot honor, as CopyObject does PutObject and CreateMultipartUpload did not check the server-side encryption headers they were given. An x-amz-server-side-encryption value that names no method, such as "aes:kms", matched none of the SSE paths, so the object was stored unencrypted and the request answered 200. SSE-C together with x-amz-server-side-encryption was also accepted, and one of the two silently won. CopyObject already rejects both through validateEncryptionCompatibility. Run the same check, with the same error codes, before PutObject and CreateMultipartUpload store anything. * s3api: answer rejected SSE headers with InvalidArgument, as S3 does S3 rejects an unknown x-amz-server-side-encryption value and SSE-C combined with another method with InvalidArgument. PutObject and CreateMultipartUpload now return that code, with S3's messages, through two new error codes; CopyObject keeps its own. Also run ceph/s3-tests' test_put_obj_enc_conflict_c_s3, _c_kms and _bad_enc_kms in CI, which check both handlers' responses end to end. * s3api: close the remaining ways a PUT could skip requested encryption - A repeated x-amz-server-side-encryption header is rejected: the encryption paths apply only the first value, so extra values could hide the method the client asked for. - KMS options (key id, encryption context, bucket key) are rejected unless the method is aws:kms, matching the S3 InvalidArgument error. - CreateMultipartUpload validates before auto-create, so a refused upload cannot leave a bucket behind. - Directory markers encrypt their inline content through the shared SSE path and record the same entry metadata as regular objects, instead of storing requested-encrypted bytes in plaintext. * s3api: read back what the SSE marker write stores - Repeated SSE-C and KMS option headers are rejected alongside a repeated x-amz-server-side-encryption, closing the same first-value bypass for customer-key and KMS fields. - Algorithm validity is checked before KMS options so an unsupported value keeps the "not supported" error. - serveDirectoryContent decrypts marker content with the stored SSE metadata and returns the SSE headers, so an encrypted marker reads back what was written; its Content-Length now reflects the bytes actually served. * s3api: HEAD of a marker skips decryption and keeps the stored size HEAD returns no body, so it now runs only the SSE-C key check instead of decrypting — matching HeadObjectHandler and avoiding a KMS round-trip — and chunk-backed directory entries report Attributes.FileSize again rather than the length of their (empty) inline content. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: keep GET Content-Length to the bytes serveDirectoryContent writes The FileSize override described chunk-backed markers on HEAD, but GET sends only the inline content, so it promised bytes it never wrote. * s3api: stream a chunk-backed directory on GET like any object A directory promoted over an uploaded object keeps the object chunks with empty inline content, so serving only entry.Content made GET deliver nothing while HEAD reported FileSize. GET now routes those entries through the regular volume-server stream, keeping the two methods consistent. * s3api: answer a busy volume read with RequestBytesExceed --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
9a9bd67efe |
s3api: do not re-run permission checks in the aws-chunked reader (#11655)
calculateSeedSignature called verifyV4Signature with shouldCheckPermissions=true, which runs VerifyActionPermission — a check that does not consult bucket policies. Every caller of newChunkedReader is already behind the Auth middleware, which does evaluate them, so a principal allowed only by the bucket policy was authorized upstream and then denied when the handler built the body reader: aws-chunked PutObject/UploadPart (botocore's default shape over TLS, PyArrow's over HTTP as well) failed with AccessDenied. The reader now verifies only the signature; a wrong secret is still SignatureDoesNotMatch. The non-streaming paths already work this way. Generated with [Devin](https://devin.ai) Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
9a365549c3 |
catalog: refused writes answer 403 to callers that can read the entry (#11650)
* s3tables: answer a denied write with 403 when the caller can read the entry Update/Delete/Rename answered every authorization refusal as not-found so the denial leaked no existence signal. For a caller allowed to GetTable (or GetView) the same entry, the veil hides nothing it could not load — yet a refused write was still answered 404, so clients saw a table they just loaded reported as missing. When the write check fails, re-check read permission on the same entry: readable entries get 403 AccessDenied; invisible ones keep the not-found answer. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * iceberg: map UpdateTable errors through writeManagerError on commit paths CommitTable and CommitTransaction answered any non-conflict UpdateTable failure, including AccessDenied and NoSuchTable, with a bare 500. A 500 reads as outcome-unknown to clients (PyIceberg raises CommitStateUnknownException) where a refused commit is a plain ForbiddenException, matching what the drop and rename handlers already emit through the same mapper. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3tables: keep the not-found veil over views and match the real read check on renames UpdateTable and DeleteTable read shared metadata without checking the entry kind, so a denied write on a view reported 403 to a GetTable-only caller where a missing name reports 404. The rename visibility check also fed resource tags into GetView evaluation that the real GetView path never supplies. * s3tables: gofmt handler_table.go Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
56fbc3deb5 |
s3api: align S3/IAM error responses with AWS (#11632)
* s3err: add InvalidArgument and AuthorizationHeaderMalformed codes Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: answer unrecognized bucket PUT sub-resources with 501 A PUT on a bucket carrying an unrecognized query (logging, metrics, intelligent-tiering, ...) fell through to the bare CreateBucket route and returned BucketAlreadyOwnedByYou or re-created the bucket. AWS answers these with NotImplemented. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: reject malformed copy-source and multipart PUT parameters A malformed X-Amz-Copy-Source or a non-numeric partNumber fell through to the plain PutObject route and stored the body as a regular object. Answer them with InvalidArgument-class errors instead of writing data. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: verify x-amz-content-sha256 against the streamed body A PUT carrying a hex or base64 payload hash now streams through a verifier that reports a mismatch once the stream is exhausted, instead of storing an object that does not match its declared hash. The error is deferred so intermediate reads that drop (n>0, err) results cannot silently swallow it. Sentinel values (unsigned/streaming payloads) remain exempt and malformed values fail fast. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: map truncated SigV4 headers to the error for the missing field AWS answers an Authorization header missing Credential= with InvalidArgument and one missing or malformed Signature= with AuthorizationHeaderMalformed, instead of a generic MissingFields. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: answer IAM/STS failures in the query-protocol envelope Embedded IAM and STS routes now report authentication, form-parse and authorization failures with the IAM ErrorResponse body instead of the S3 Error envelope, so IAM SDK clients can parse them. Requests signed for s3 keep the S3 envelope, keyed off the credential scope. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: return 403 AccessDenied when the request has no Date Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: answer throttling rejections as SlowDown ErrTooManyRequest and ErrRequestBytesExceed reported made-up codes; AWS serves these throttling rejections as SlowDown. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: reject versionId requests on buckets that never had versioning GET, HEAD and DELETE carrying a non-empty versionId on an unversioned bucket now fail with InvalidArgument instead of being answered as a plain object request. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: answer DeleteObjects over 1000 keys with MalformedXML Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: InvalidArgument for non-numeric or out-of-range part numbers partNumber=abc, 0 and >10000 all resolve to InvalidArgument, matching AWS, instead of InvalidPart. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * iam: correct LimitExceeded, InvalidAction and ServiceFailure mappings LimitExceeded is a conflict (409), an unknown Action is InvalidAction (404) rather than NotImplemented, and internal failures report the IAM receiver fault type. Applies to both the embedded IAM endpoint and the standalone iamapi server. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * iam: refuse DeleteUser while access keys remain Deleting a user with live credentials orphaned its access keys; AWS answers DeleteConflict until they are removed first. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test/s3: add S3/IAM error-response compatibility harness * s3: return InvalidArgument for malformed x-amz-content-sha256 A header value that decodes to neither 32-byte hex nor base64 is a malformed argument, not a hash mismatch. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: stop spawned mini when readiness times out A slow-starting server otherwise survives the failure path and keeps the S3 port occupied for the next run. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: register atexit cleanup before setup A failed setup previously skipped cleanup, leaking the bucket and IAM user on persistent servers. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3api: reject unrouted subresources on the DELETE bucket catch-all PutBucketHandler gained the same guard when the route-level check moved into the handlers; DeleteBucketHandler was missed, so an authorized DELETE /bucket?logging could delete the bucket instead of answering NotImplemented. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: tolerate unset fixture variables in cleanup and cover DELETE ?logging Cleanup now runs its IAM/multipart steps only when setup reached them, so an early setup failure still removes the bucket. Added a DELETE bucket-subresource case asserting NotImplemented and bucket survival. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: fail a case when its side-effect check reports a regression A non-empty check note now fails the case, so a deleted bucket or an object created by a malformed request cannot slip through behind a passing status check. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Joel Town RoadandChris Lu
|
079a7d8ba3 |
s3: give each prefix its own hidden-prefix probe budget (#11626)
* s3: give each prefix its own hidden-prefix probe budget The shared cursor.probedEntries counter in dirHoldsOnlyHiddenEntries was exhausted by one large all-deleted subtree, causing every later prefix in the same request to be treated as visible. The fix allocates a fresh budget (hiddenProbePerPrefixBudget, default 1000) for each top-level call and threads it down to recursive calls via a pointer, so cross-prefix budget bleed is impossible. Fixes #10847. * s3: keep 10000 probe budget, now per prefix Restore hiddenProbeBudget = 10000 as a package const (not a mutable var) applied per-prefix instead of per-request. The override hook moves to an unexported probeBudget field on ListingCursor; zero means use the package default. Tests set probeBudget: 3 on the cursor, keeping them fast without touching package state. Also revert the unrelated uint32 cast on the ListEntries Limit field. * s3: cap total hidden-prefix probe work per listing request The per-prefix budget resets for every candidate prefix, but deleted prefixes do not spend maxKeys, so a page can walk an unbounded number of them. Keep a request-wide probedEntries ceiling (hiddenProbeTotalBudget, 10x the per-prefix budget) so total probe work stays bounded. * s3api: pin the probe-budget cutoff and the request-wide ceiling --------- Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> |
||
|
|
576837f1b2 |
fix(s3): make self-heal pointer persist CAS-bound against concurrent writers (#11627)
* fix(s3): make self-heal pointer persist CAS-bound against concurrent writers Follow-up to #11618: pointerless reads of a slash key whose regular-path entry is a physical parent (or a bare-key object) now fall through to healStaleLatestVersionPointer, which rescans .versions and persists a repaired pointer. The persist was an unconditional upsert off the pre-scan snapshot, so a PUT or delete that atomically advanced the pointer on the owner filer while the heal was rescanning could be rolled back, making older content or ACLs current again. Mirror the CAS discipline clearStaleLatestVersionPointer already applies: re-fetch the live .versions entry, require its pointer fields to still match the ones the heal observed, and abandon the persist (still returning the rescanned entry) when a concurrent writer has moved them. Write the live Extended map so concurrently updated fields are preserved. * fix(s3): close the check-then-act window in the self-heal pointer persist The CAS re-fetch added in the previous commit narrows the race but leaves a gateway-side window: after the live .versions entry is re-read and the pointer compared, the repair is still written back through an unconditional RPC, so a PUT or delete committing between the re-fetch and the persist still ends up rolled back by the stale repair. Bind the persist to the live image the heal just re-read with an IF_ENTRY_EQUAL precondition, the same discipline routedSelfCopy applies to stale self-copies: the filer evaluates the condition under the entry's path lock and conditional writes route to the owner filer, so a writer committing inside the window fails the precondition and the winner's pointer stands. FailedPrecondition and NotFound are authoritative replies and are not replayed by the failover layer. The test now also covers a writer committing during the persist, which reverts the pointer on the previous unconditional write-back. * s3api: CAS-bind the stale-pointer clear against concurrent writers The pointer clear re-read the live .versions entry and then wrote it back unconditionally through mkFile, so a writer committing between the re-fetch and the persist was rolled back to a cleared pointer. Persist through the same IF_ENTRY_EQUAL conditional update as the repair path. * s3api: test the CAS contract on the stale-pointer clear * s3api: never clear a pointer the clear did not observe as stale The CAS clear skipped its live-pointer match when the caller's snapshot carried an empty latest-version id, so a writer promoting a version between the clear's rescan and its re-fetch had the fresh pointer CAS-cleared away (expected = the writer's own live entry), briefly making the just-written version appear absent. With an empty observed id, reaching the persist at all implies a concurrent promotion (an idle key short-circuits as already-clear), so make the pointer match unconditional and abort instead. Extend TestClearStaleLatestVersionPointerConcurrentWriter with pointerless-snapshot cases: a post-rescan promotion must survive, and an idle pointerless key must short-circuit as already-clear. The fake filer's proto round-trip drops empty Extended maps, so the snapshot is padded the way real callers do. --------- Co-authored-by: zhaoyuchen <yc.zhao@yinzon.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> |
||
|
Chris Lu
|
ec261c5fbc |
S3: quote ETag in CopyObject and UploadPartCopy XML responses (#11624)
* s3api: extract quoteETag from setEtag Consolidate ETag quoting so the XML response builders can share it. * s3api: quote ETag in CopyObjectResult XML AWS returns the ETag quoted in the copy result body, matching the ETag header. Fixes seaweedfs/seaweedfs#11622 * s3api: quote ETag in CopyPartResult XML UploadPartCopy returned the raw ETag in the XML body while the response header and other APIs return it quoted. Fixes seaweedfs/seaweedfs#11622 * s3api: test ETag quoting in copy responses * s3api: assert quoted ETag on the wire in copy response tests * s3api: use strconv.Quote in quoteETag Addresses CodeQL 'potentially unsafe quoting' on string concatenation. |
||
|
zhao-ycandChris Lu
|
d829275de6 |
fix(s3): recheck cache metadata and validate null objects (#11618)
* fix(s3): recheck cache metadata and read null directory markers Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com> * fix(s3): preserve null object identity and bound test cache Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com> * s3: trim comments on the anonymous read cache path Signed-off-by: Chris Lu <chris.lu@gmail.com> --------- Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com> Signed-off-by: Chris Lu <chris.lu@gmail.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
|
aa5b337716 |
fix(s3): honor object ACLs for anonymous GET and HEAD (#11605)
* fix(s3): honor object ACLs for anonymous reads Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com> * s3: drop unsigned session tokens on deferred anonymous reads An unsigned GET/HEAD carrying X-Amz-Security-Token is anonymous, not a session request; strip the token before authentication and authorization so it cannot influence identity or policy evaluation. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
zhao-ycandChris Lu
|
16e66b1bad |
fix(s3): initialize destination ACLs for CopyObject (#11599)
* fix(s3): initialize destination ACLs for CopyObject Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com> * fix(s3): re-check routed self-copy eligibility on the locked read routeInPlace was decided on the pre-lock entry, but the PATCH body re-reads the entry. A concurrent write changing file mode or MIME in between left the routed PATCH installing new ACL keys while Attributes kept the stale mode. Evaluate eligibility against the re-read entry and retry the self-copy under the distributed lock when it no longer qualifies. * fix(s3): guard metadata self-copies against concurrent writes Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com> * ci: raise s3api unit-test timeout to 9m The suite crossed the 5m binary timeout on the hosted runner (local run is ~4.3m and still growing). The job-level limit is already 10m. * ci: allow setup time before the S3 API test suite Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com> --------- Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> |
||
|
Chris Lu
|
3c17c5146e |
S3: fix ListObjectVersions losing keys across page boundaries (#11598)
* s3api: thread filer client through the versioned-listing collector
findVersionsRecursively now binds one SeaweedFilerClient for the whole
recursive walk instead of re-resolving a filer on every list/lookup call,
and the collector's list/getEntry/scanLatestVersionEntry/getObjectVersionList
helpers go through it. No behavior change; this also lets tests drive
collectVersions with a stubbed client.
* s3api: keep collecting versions while pending names can sort into the page
ListObjectVersions walked the filer in directory-entry name order and
stopped as soon as maxKeys+1 items were collected, sorting only that
partial set. Filer names do not match key order: "a.copy.versions" sorts
before "a.versions" while key "a.copy" sorts after "a", so a page
boundary inside the earlier-walked sibling's versions permanently skipped
the later key.
Track the largest key collected (maxKey) and, once the collector is full,
keep walking until entry names pass the ceiling of names that can still
resolve to keys at or below it; the ceiling reaches through the prefix
versions of maxKey. Entries whose subtree can only hold keys above maxKey
are skipped. Versions of an in-bound object are collected in full so its
position in the sorted page is exact.
Fixes seaweedfs#11594
* s3api: resume versioned listings at the earliest covering name prefix
computeStartFrom mapped the key marker straight to an entry name (or cut
it at the first '/'), which skips sibling directories that are a prefix
of the marker below '0' - for marker "d.x" the listing resumed at name
"d.x", skipping directory "d" whose keys "d/*" all sort after it.
Resume at the earliest remainder prefix ending at a byte below '0' ('/',
'.', '-' and friends), so every directory whose subtree can still hold
keys past the marker is revisited; already-returned keys inside are
filtered by the existing marker checks as before.
* s3api: regression test for versioned-listing pagination order
Drive collectVersions against a stubbed filer holding the issue-11594
layout - "a.copy.versions" listing before "a.versions", plus a "d/"
subtree next to "d.x" - and assert that every page size from 1 up
reproduces the unpaginated ordering with no lost or duplicated entries.
Also updates TestComputeStartFrom for the new earliest-prefix resume and
gives testFilerClient a LookupDirectoryEntry stub.
* s3api: inject list/getEntry functions into the version collector
Pinning one SeaweedFilerClient for the whole walk dropped per-call
failover: previously each s3a.list resolved a filer through
WithFilerClient, so a mid-walk filer failure could fall back to a
healthy peer. Inject s3a.list/s3a.getEntry as function fields instead -
production keeps the failover behavior, tests can still stub.
* s3api: keep scanning marker for a later covering prefix
A leading byte below '0' (marker .hidden/file) has no non-empty prefix
at index 0, but a deeper separator still does - resuming at .hidden/file
skipped the .hidden directory and its keys after file. Continue the scan
instead of bailing on the first byte.
|
||
|
zhao-ycandChris Lu
|
483dd4b12e |
s3api: persist ACLs on PutObject uploads (#11592)
* s3api: persist ACLs on PutObject uploads Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com> * s3api: fix PutObject ACL edge cases found in review - Only enforce BucketOwnerEnforced when explicitly configured; buckets without a stored ownership control keep accepting upload ACLs - Ignore ACL query parameters on SigV2 requests, which do not sign them - Mirror signed-query ACL values into headers after authentication so grant parsing and resolveFileMode agree on presigned uploads - Validate only caller-supplied grantees against the account registry; default grants now work for accounts outside the local registry - Reject unknown grantee keys and accept comma-separated grantee lists without spaces in ParseCustomAclHeader - Guard against identities without an account * s3api: harden upload ACL parsing and authorization Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com> * s3api: evaluate upload ACL grantees individually in policies A comma-joined grant header or a signed query parameter reached policy conditions as one value, so a deny on a later grantee did not fire. Split grant headers into per-grantee values for policy evaluation and share the grantee pair parser with ParseCustomAclHeader. * s3api: keep raw grant header values visible to policy conditions Exact-match conditions written against the signed header value stopped matching once grantees were split for evaluation. Preserve the original wire values alongside the per-grantee values so deny policies fire on either granularity. * s3api: evaluate upload ACL grants as one canonical list in policies Conditions on s3:x-amz-grant-* now see a single comma-separated canonical grant list identical for a single line, repeated header lines, or a signed query parameter. This keeps StringEquals allows and exact-list or allowlist (StringNotEquals) denies accurate regardless of wire encoding. * s3api: preserve upload ACL denies and align policy checks Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com> * s3api: retain upload owner grants and literal policy values Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com> --------- Signed-off-by: zhaoyuchen <43179751+zhao-yc@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris Lu
|
d9b69a7f76 |
s3api: fix PutObjectAcl permission scoping and owner grants (#11587)
PutObjectAcl had four authorization and ownership bugs: - The handler embedded the resource path into the action (WriteAcp:bucket/object), and authRequest/CanDo then scoped it to the request's bucket/object again. A bucket-wide WriteAcp:bucket grant could never match, so legitimate owners got 403. - After authRequest succeeded via an IAM or bucket policy, a leftover identity.CanDo gate re-checked only the legacy Actions list, denying identities authorized purely by policies. - For canned and default ACLs, ExtractAcl generated the FULL_CONTROL grant for the requesting account instead of the object owner. An admin setting private/public-read on another account's object left the owner metadata intact but reassigned full control to the admin. - Objects without stored owner metadata (e.g. written via the filer outside S3) fell back to treating the requester as the owner, so any user with a WriteAcp grant could take them over. Non-admins are now denied; admins keep the takeover fallback. Grantee validation now also accepts the object's stored owner even when that account has been removed from the registry, so canned/XML ACLs for retired owners keep working. |
||
|
|
39bc9cd0ef |
s3api: copy the trailer checksum before reading the next trailer line (#11583)
* s3api: copy the trailer checksum before reading the next trailer line
parseChunkChecksum kept the checksum value as a sub-slice of the line
returned by bufio.Reader.ReadSlice, which is only valid until the next
read. When the trailer lines arrive in separate TCP segments, reading
x-amz-trailer-signature refills the buffer and overwrites the saved
value, so a correct upload fails with InvalidDigest ("The Content-Md5
you specified is not valid").
The AWS SDK for Java v2 (>= 2.30) on a Linux JDK sends the trailer that
way; about half of its signed streaming uploads failed.
Fixes #11582
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* s3api: reuse crc32 writer and trim comments in trailer split test
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
|
||
|
Chris Lu
|
793ce06b10 |
s3api: allow unsigned SSE-C customer key headers on presigned requests (#11578)
* s3api: allow unsigned SSE-C customer key headers on presigned requests AWS requires only x-amz-server-side-encryption-customer-algorithm to be signed on presigned URLs; the key and key-MD5 headers are supplied at request time. Since #9121 rejected any x-amz-* header outside SignedHeaders, SDK-generated presigned SSE-C requests (e.g. .NET GetPreSignedUrlRequest) fail with SignatureDoesNotMatch. Exempt the customer key and copy-source key headers for presigned requests only. * s3api: test presigned SSE-C requests carrying unsigned key headers |
||
|
Peter DoddandChris Lu
|
52fb9f93ff |
s3: track filer joins and leaves pushed by the master (#11563)
* fix(s3): track filer joins and leaves pushed by the master The S3 FilerClient replaced its -filer seed with a master snapshot of filer IPs at boot and refreshed it only every 5 minutes. A rolling restart replaces every filer well inside that window, leaving S3 servers with only dead addresses and failing every write until the next poll. Apply the master's ClusterNodeUpdate pushes to the filer list as they arrive, keeping the poll as a backstop. The last filer is never removed, and a poll snapshot requested before a push was applied is discarded rather than overwriting newer membership. * Defer last-filer leaves; bump the generation only on real changes * fix(s3): cancel deferred filer leaves on rejoin and on discovery A deferred last-filer leave outlived the filer it was recorded for: a rejoin at the same address looked like a duplicate add, and a discovery snapshot left the entry behind. The next join then removed a live filer until the following poll. A join now cancels any deferred leave for its address, and an applied snapshot clears them, since it is the master's current membership. * Bump the push generation when a rejoin cancels a deferred leave --------- Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> |
||
|
Chris LuandDevin
|
90f8c4378f |
s3: restrict admin gRPC to local callers when no signing key (#11530)
* s3: restrict admin gRPC to local callers when no signing key The S3 gateway's gRPC port (default 0.0.0.0:19000, always on) serves the IAM cache and internal lifecycle admin services. checkAdminAuth was a no-op when jwt.filer_signing.key was unset, so any reachable host could PutIdentity an admin identity and take over the bucket data. Without a shared key callers cannot be distinguished, so admin RPCs are now limited to unix-socket, loopback, and the server's own interface addresses. Remote filer-to-S3 propagation and lifecycle workers must set jwt.filer_signing.key; the Bearer-token path is unchanged. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: fail closed on nil guard and refresh local addresses per call Review feedback: a nil filerGuard bypassed all checks — treat it like a missing key and require a local peer. The own-address set was cached forever, so interfaces added later were rejected; enumerate per call instead since admin RPCs are rare. Nil ctx is denied rather than panics. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: read the signing key once and bound interface enumeration Review feedback: reading SigningKey twice could straddle a SIGHUP reload — an old nonempty key skipped the local-peer check while the new empty key verified the token. And enumerating interfaces per no-key call is wasteful for co-located workers dialing the announced address; cache the address set for 30s so new interfaces still become usable promptly. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: enumerate interface addresses per no-key admin call A cached address set keeps trusting an IP after it is removed from the host and reassigned to another machine — that host would then hold unauthenticated admin access for the cache TTL. Per-call enumeration only runs for non-loopback TCP peers on the no-key path, which is low-volume admin traffic, so the freshness is worth the syscall. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Eliah RusinandClaude Opus 5.5
|
164c3db606 |
s3: return 403, not 500, when an over-quota bucket refuses a write (#11552)
* s3: return 403, not 500, when an over-quota bucket refuses a write Filer AssignVolume flattened ErrReadOnly into the free-text AssignVolumeResponse.Error string, so S3 PutObject / PutObjectPart via UploadReaderInChunks could not match it with errors.Is and fell through to 500 InternalError: retryable, and it hides the quota. Add FilerError READ_ONLY and AssignVolumeResponse.error_code, set it alongside the unchanged error text, and rebuild the sentinel with filer_pb.AssignVolumeResponseError. weed_server.ErrReadOnly now aliases filer_pb.ErrReadOnly so errors.Is matches on both sides, and mapChunkedUploadErrorToS3Error maps it to ErrAccessDenied. There is no "read only" substring matching, so a volume server's "volume N is read only" stays retryable. Carrying the verdict as a response code rather than a gRPC status keeps clients from treating it as a transport failure: the S3 gateway does not fail over across filers and the Java client does not retry it. Wrap per-chunk copy errors with %w so CopyObject keeps the sentinel, and map UploadPartCopy chunk errors through mapCopyErrorToS3Error instead of always returning 500. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * ci: re-run integration tests (PyPI download timeout) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> |
||
|
Javier Garcia
|
8fdcf69eb0 |
s3api: report the stored checksum in GetObjectAttributes (#11529)
GetObjectAttributes accepted the Checksum attribute but never filled it in, as its comment said SeaweedFS did not store S3 checksums. PutObject and CompleteMultipartUpload store them now, and HeadObject returns them. Fill in Checksum from the same entry fields, with the ChecksumType and ChecksumCRC64NVME members the response did not have. Also run ceph/s3-tests' test_get_checksum_object_attributes in CI. |
||
|
Javier Garcia
|
988fc4f7ba |
s3api: do not store aws-chunked in an object's Content-Encoding (#11528)
* s3api: do not store aws-chunked in an object's Content-Encoding aws-chunked in Content-Encoding names the SigV4 streaming framing of the request body, which the gateway decodes on upload. PutObject and CreateMultipartUpload stored the header as sent, so an object uploaded with "gzip, aws-chunked" was served with that Content-Encoding, and one uploaded with "aws-chunked" alone was served as aws-chunked. S3 drops aws-chunked and keeps the other encodings. Also run ceph/s3-tests' test_object_content_encoding_aws_chunked in CI. * s3api: read every Content-Encoding field, and drop aws-chunked on copy A client can send aws-chunked and the object's own encoding as separate Content-Encoding fields. Only the first was read, so "aws-chunked" followed by "gzip" left the object without its gzip. Combine all the fields before dropping aws-chunked. CopyObject with the REPLACE directive stored the requested Content-Encoding as sent: drop aws-chunked there too. |
||
|
Khris RichardsonandClaude Opus 5.5
|
895d49b55b |
s3/iam: manage roles through the IAM API, with an opt-in persistent role store (#11522)
* s3/iam: manage roles through the IAM API, with an opt-in persistent role store
Roles could only come from the IAM config file: the S3 server pinned the
role store to memory and the embedded IAM API had no role actions, so a
role could not be created, retrusted or revoked without editing the file
and restarting every gateway.
Role store
- Read the `roleStore` key (the IAMConfig field already existed). With an
IAM config file the default stays memory; with none it is the filer, as
for OIDC providers, so zero-config clusters keep runtime-created roles.
- Roles from the IAM config file never go into a persistent role store,
which outlives the file and may be shared by S3 servers with different
files. They are served from memory beneath the store, as OIDC providers
are: a stored role of the same name takes precedence, and deleting it
restores the file's. A config-file role cannot be changed or deleted
through the API (UnmodifiableEntity), and removing one from the file
removes it at the next start. An in-memory store holds them as records,
as before. They have no creation time, so CreateDate is omitted rather
than reporting when this server started. SetRoleStore installs a store
the same way, so a store set after startup keeps the config-file roles,
as SetOIDCProviderStore does for providers.
- Watch /etc/iam/roles and drop the cached role definitions on change. The
cached filer store otherwise serves a peer's stale role for up to its 5m
TTL, which keeps a revoked trust policy in force on the other gateways.
- Role stores wrap ErrRoleNotFound for a missing role; the filer store
used to report any failed lookup as "role not found". CreateRole proceeds
only on a confirmed absence, so an unreadable store cannot let it write
over an existing role.
IAM actions
- CreateRole, GetRole, ListRoles, DeleteRole, UpdateAssumeRolePolicy,
AttachRolePolicy, DetachRolePolicy, ListAttachedRolePolicies. The reads
are allowed in read-only mode.
- A role defined in the config file is reloaded from it at every start, so
changing or deleting it through the API is refused (UnmodifiableEntity)
rather than silently reverted.
- DeleteRole with policies attached is refused (DeleteConflict), as on AWS.
- Role names follow AWS's rules ([\w+=,.@-]{1,64}); a role is stored as
<name>.json in the filer, so this also keeps a name from leaving the role
store's directory. At most 10 managed policies per role (AWS's default
quota; MaxManagedPoliciesPerUser is 10 too), LimitExceeded beyond.
- DeletePolicy is refused (DeleteConflict) while a role attaches the
policy, as it already is for users and groups: roles attach policies by
name, so a policy created later under the deleted one's name would
otherwise take effect on the role.
- Role paths other than "/" and role tags are not stored, so they are
refused rather than dropped.
Role IDs and sessions
- Roles get a unique RoleId when first stored (random, AWS AROA form),
kept across updates; a config-file role gets a stable ID derived from its
name, since it is created again at every start.
- Sessions issued through AssumeRoleWithWebIdentity, AssumeRoleWithCredentials
and AssumeRole carry the role's ID (claim "rid"), and a request under a role
whose current ID differs is denied. Resolving a session's policies by role
name let a session outlive its role: once a role was deleted, a role later
created under the same name — with a different trust policy and different
policies — revived every unexpired session of the old one with the new
role's permissions. Sessions issued before this change carry no ID and are
unaffected until they expire.
Integration test (test/s3/iam, run with `make start-services`):
TestWebIdentityWithProviderAndRoleManagedThroughIAMAPI configures an OIDC
provider, a managed policy and a role entirely through the IAM API against a
JWKS served by the test, then checks the trusted subject gets credentials
scoped to the attached policy; another subject, a token signed by another
key, an unsigned token and a token for another audience are refused; and UpdateAssumeRolePolicy moves the
trust at once.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* s3/iam: bind every role session to its role, and change roles atomically
Review follow-ups.
Session binding
- The role-ID check ran only when a session carried no policy names, and
AssumeRole embeds the role's attached policies, so those sessions kept
their permissions after the role was deleted or recreated. The check
now runs for every session carrying a role ID, before policy selection.
- A named role that cannot be resolved at issuance gets no session,
instead of one with no role ID (which nothing binds).
- A config-file role's ID is derived from its name and trust policy, not
the name alone: a different role put in the file under the same name
gets a new ID, while an unchanged role keeps its sessions across restarts.
Role writes
- RoleStore gains UpdateRole, a read-modify-write that lands only if the
role is unchanged since the read, and otherwise re-reads and retries. The
filer store uses the filer's write conditions (IF_NOT_EXISTS for a new
role, IF_ENTRY_EQUAL otherwise). CreateRole, UpdateAssumeRolePolicy and
Attach/DetachRolePolicy all go through it, so two gateways no longer
overwrite each other's changes, a change racing a delete no longer
writes the role back, and of two concurrent creates one gets
EntityAlreadyExists.
- The filer store's ListRoles pages past 1,000 entries and fails on a
broken stream instead of returning what arrived, so DeletePolicy's
attachment check sees every role. ListRoles skips a role deleted between
listing and reading it.
- CreateRole validates first; a failed write is ServiceFailure, not
InvalidInput. Any Tags.* parameter is refused, not only the first key.
- ExecuteAction's skipPersist covers the S3ApiConfiguration only; the
comment now says so. Role and OIDC provider actions write their own stores.
Each fix has a test that fails without it. Against a real filer with two
gateways, concurrent AttachRolePolicy calls lost 1-4 of 8 attachments per
run before this change and none after.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* s3/iam: one role snapshot per decision; DeleteRole is atomic; watch a custom role store path
Review follow-ups.
- Authorization evaluates the policies of the role definition the session's
binding was checked against, instead of reading the role again: a role
replaced in between cannot lend a session its policies.
- AssumeRole and AssumeRoleWithLDAPIdentity issue the session from the
definition whose trust admits the caller (IAMManager.ResolveRoleForPrincipal),
and take its ID, duration cap and embedded policies from that same
definition. A role replaced after the caller's trust check by one that does
not trust the caller now yields AccessDenied, not a session bound to the
replacement.
- A RoleUpdate that returns nil deletes the role, on the same condition as a
write: the filer store deletes with ObjectTransaction on IF_ENTRY_EQUAL,
routed and locked like the conditional CreateEntry. DeleteRole decides
against the role it deletes, so a policy attached meanwhile on another
server is a DeleteConflict, and a delete never removes a role written
after its check.
- S3 servers watch the role store's configured basePath, not only
/etc/iam/roles, so a custom path also drops peers' cached roles on change.
Each has a test that fails without it. Live against a real filer: DeleteRole
refuses while a policy is attached and removes the entry once detached; all
test/s3/iam CI stages pass.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* s3/iam: state which roles DeletePolicy's attachment check can see
RolesAttachingPolicy sees the stored roles and this server's config-file
roles. A role defined only in another server's IAM config file is invisible
to it, so a config-file role that attaches a managed policy is protected
only on the servers whose file defines it. The doc comment now says so and
how to avoid it: keep such roles in every server's file, or attach only
config-file policies to config-file roles.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* iam: note that a role store set after startup is not watched for peer changes
S3 servers build their metadata watch list once, at startup, from the role
store installed then. SetRoleStore's doc now says that a filer-backed store
installed later with a different basePath is not watched, so peers' changes
to it reach this server's cached roles only when the cache expires.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
||
|
Alex Hu
|
38ce95d960 |
s3api: always write XML timestamps with three fractional digits (#11520)
CopyObject responses carried LastModified values such as
"2026-09-29T20:30:04.56Z": trailing zeros of the fractional seconds were
trimmed, and a whole-second value had no fraction at all. AWS S3 always
writes exactly three digits ("...04.560Z"), and clients that parse with a
fixed-width pattern reject anything else. minio-java 8.6.0
(yyyy-MM-dd'T'HH:mm:ss.SSS'Z') throws DateTimeParseException, so roughly
one CopyObject in ten fails on the client even though the copy succeeded.
Two causes:
- xsdDateTime marshalled with "2006-01-02T15:04:05.999999999", which
drops trailing zeros. It now writes UTC with ".000Z".
- CopyObjectResult.MarshalXML had a pointer receiver, but the handlers
pass the result by value, so encoding/xml never called it and fell back
to time.Time's RFC 3339 encoding. It now has a value receiver.
CopyPartResult had no custom marshaller at all; it now uses xsdDateTime.
Follow-up to #8394 / #8398, which truncated these timestamps to
milliseconds but kept the trimmed format.
|
||
|
Chris LuandDevin
|
757917f564 |
filer: evict remote-cached objects under storage pressure (#11515)
* filer: identify remote-mounted entries safe to drop under disk pressure ListEvictableRemoteEntries walks every mounted directory directly on the filer store (no lazy remote listing) and returns entries that hold local chunks fully synchronized with remote, ordered oldest-cached first. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: evict remote-cached chunks oldest-first and vacuum the garbage uncacheRemoteEntry applies the same transition remote.uncache does - cleared chunks plus a reset LastLocalSyncTsNs under the entry path lock - and evictRemoteCachedEntries serializes passes over all mounts until a byte target is met. Aged victims are preferred; a second pass accepts any synchronized cached entry when aged ones cannot cover the request, since a failed read is worse than a dropped hot object. Cleared chunks only become disk space after compaction, so reclaimRemoteCacheSpace pairs each pass with a rate-limited VacuumVolume call that also picks up orphaned partial fills. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: trigger remote cache eviction under storage pressure A periodic check (30s) reads disk usage from master topology and evicts remote-mounted cached chunks once any disk crosses -filer.remoteCacheEvictThreshold (default 0.9; 0 disables), with a vacuum pass to reclaim the tombstoned needles. The cold-read cache path also kicks the same reclaim when a fill fails on exhausted volumes - the request still falls back to streaming from the remote, but the cache stops being permanently wedged full. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: flush deletion queue before remote cache vacuum Vacuum ran immediately after eviction while evicted file IDs still sat in the asynchronous deletion queue, so compaction saw no garbage and the cache stayed wedged. Flush the queue synchronously first and shorten the vacuum cooldown so sustained pressure does not wait five minutes between reclaim passes. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: cover remote cache eviction under capacity pressure Unit tests pin the eligibility filter and oldest-first ordering; the integration test runs a constrained two-node setup that saturates the cache, verifies the oldest synced entry is evicted and vacuumed, and that a later read re-caches it. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: coalesce remote cache reclaim passes A failed cache fill used to queue behind any in-flight eviction, stacking full mount traversals during a write-failure storm. Skip the pass when one is already running; the caller falls back to streaming from remote regardless. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: stop the remote cache janitor on shutdown The eviction ticker kept running after Shutdown closed the metadata store and could traverse a closed store. Give the janitor a context cancelled from Shutdown and propagate it into its master RPCs and traversals. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: vacuum only tombstoned volumes and retry deferred passes VacuumVolume with no volume id swept every collection, compacting volumes unrelated to the cache fill that failed. Now the reclaim path collects the vids of file ids actually flushed from the deletion queue and compacts only those. Vids that land inside the vacuum cooldown stay in a pending set the janitor retries on each tick, so chunks evicted just after a sweep are not stranded until the next pressure event. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: count only pressured disks when evicting remote cache The janitor measured the largest excess on one disk but let bytes on healthy disks satisfy the reclaim target. Split the topology disk view per physical disk and count only chunk bytes whose volumes sit on an over-threshold disk; entries contributing nothing there are skipped. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: compare remote cache sync time at nanosecond precision Second-precision mtime comparisons let a local write in the same second as the last sync still qualify as evictable, discarding unsynced changes. Compare LastLocalSyncTsNs against full-precision mtime (mtime_ns round-trips through the entry codec), and apply the same fix to remote.uncache's inline check. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: invalidate remote sync stamp on local content change A local overwrite that keeps the remote entry's LastLocalSyncTsNs looks evictable even though the remote copy no longer matches, and some write paths stamp mtime at second precision so a timestamp comparison cannot catch it. UpdateEntry now clears the stamp when chunks change without a fresh stamp, leaving replicated updates authoritative. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: bound remote cache master rpcs and vacuum all evicted garbage VolumeList and VacuumVolume now run under a 30s context so a stalled master cannot wedge the eviction janitor. The targeted vacuum drops the garbage threshold so volumes with under 10% deleted bytes still compact. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: tolerate straggler fills in remote cache eviction test Detached fills from the concurrent wave keep racing the final checks: live chunks legitimately fill both volumes, and a re-cached object can be evicted again before its commit is observed. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: start remote cache eviction loop after filer init The janitor's first tick dereferences fs.filer; starting the goroutine before NewFiler assigns it could panic when startup exceeds an interval. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: keep remote cache vacuum intent across retries Evicted entries now record their chunk volumes for vacuum directly, so the intent survives whoever consumes the shared deletion queue first. A pending volume keeps several vacuum attempts so tombstones that land late are still compacted, and the janitor retries pending volumes under the reclaim mutex instead of flushing unrelated deletes every tick. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: bound each remote cache vacuum request independently A shared 30s deadline across pending volumes let one slow compaction cancel the rest. Each VacuumVolume now gets its own context, and pending volumes keep more attempts since the master reports request acceptance rather than compaction. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: tighten remote cache reclamation bound Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: treat chunk timestamp changes as content changes chunksEqual now also compares ModifiedTsNs so an update that rewrites a chunk record still invalidates the remote sync stamp. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: run remote cache queue flush under the reclaim context BatchDelete for flushed file ids now uses the caller's context instead of context.Background(), so a reclaim pass bounded by shutdown or timeout stops its deletes too. Other callers keep their existing behavior. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: scope remote cache vacuum to evicted volumes The flush no longer feeds the shared deletion queue's ids into the pending set — only evicted chunks' volumes are tracked, so ordinary deletions no longer pick up repeated vacuum attempts. The flush also runs under a shutdown-immune bounded context and is skipped when no volume is pending. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: retry remote cache vacuums even after unmount Pending volumes were only retried while a remote mount existed; removing the last mount skipped every later pass and left evicted bytes allocated. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: reclaim partial cache fills that run out of capacity A fill that fails midway queues its written chunks for deletion, but when no entries remain evictable the reclaim pass found no pending volumes and skipped the flush and vacuum entirely, leaving the partial garbage to the slow periodic vacuum while the disk stayed full. Mark the failed fill's chunk volumes pending so the pass tombstones and compacts them even when nothing was evicted. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
|
5da137233d |
s3/iam: persist IAM-managed OIDC providers in the filer, and trust them after a restart (#11510)
* s3/iam: persist IAM-managed OIDC providers in the filer, and trust them after a restart
The S3 server's IAM config loader never read the documented
`oidcProviderStore` key, so the OIDC provider store was always in memory:
a provider created with CreateOpenIDConnectProvider lived in one gateway's
process, was lost on restart, and was never seen by peers. The
/etc/iam/oidc-providers metadata subscription refreshed from that empty
in-memory store.
- Read `oidcProviderStore` and pass it to the IAM manager. With an IAM
config file the default stays memory. With no config file (zero-config
IAM, as `weed filer -s3` and operator-managed clusters run) it defaults
to the filer: there is nothing static to shadow, and providers created at
runtime otherwise vanish on restart.
- With a store that outlives the process, load the STS runtime view from it
at startup, so providers created on an earlier boot or on a peer are
trusted without waiting for the next mutation.
- If the store cannot be read at startup (a filer not up yet), the load is
retried in the background with backoff until it succeeds: the metadata
subscription reports only later changes, so providers already stored would
otherwise stay unknown to STS until one of them changed.
- Mark records mirrored from STS.Providers as `source: static-config`, and
at startup delete such records whose provider has left the config, so
removing a provider from the config file still revokes it. Records created
through the IAM API are never pruned.
- The filer store reported every failed lookup, an unreachable filer
included, as ErrOIDCProviderNotFound, which CreateOIDCProvider reads as
"free to create". Only a confirmed absence is now not-found.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* s3/iam: keep config-file OIDC providers out of a persistent store
Review of the previous commit found that mirroring the IAM config file's
providers into a persistent store, and pruning them when they leave the
file, breaks as soon as S3 servers share a filer:
- a server prunes stored config-file providers its own file does not list,
including ones a peer's file still defines (a zero-config server prunes
them all);
- mirroring overwrites an API-created provider with the same ARN and marks
it config-owned, so a later prune deletes it;
- a failed mirror write or a failed prune leaves a stale record trusted;
- a mirrored record is loaded into STS at startup as an IAM-managed provider
and shadows the config-file provider, dropping the settings a record does
not carry (jwksUri, roleMapping, policyClaim, ...).
A persistent store now never receives the config file's providers. STS keeps
serving them from its static configuration, as it always has; the IAM API
lists and returns them from memory, refuses to change or delete them
(UnmodifiableEntity; change them in the file) and to create another provider
with their ARN (EntityAlreadyExists). The store holds only providers created
through the IAM API, and those are what startup loads into STS. There is
nothing to prune, so the source marker is gone. An in-memory store keeps its
behaviour: the config file's providers are records in it, as before.
buildOIDCProviderFromRecord also carries PolicyClaim and
AllowedPrincipalTagKeys now; they were dropped whenever an API-created
provider was loaded into STS.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* s3/iam: send UnmodifiableEntity as a 400, not an internal error
The IAM API's error writer had no case for UnmodifiableEntity, which the
previous commit returns for a change to a config-file provider, so it went
out as a 500 ServiceFailure that clients retry. AWS sends it as a 400.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* s3/iam: document stored-over-config precedence, drop invented CreateDate, cancel superseded retries
Follow-ups from review of
|
||
|
Chris LuandDevin
|
9b3b12c607 |
filer: pin-aware reader cache eviction and stream release (#11503)
* filer: synchronize stream pins and release them on transitions Guard chunkStream.cacher with the ReaderCache lock everywhere: mount sections share one ChunkReadAt across concurrent reads, and unsynchronized release could double-unpin. Reads served from the chunk cache now detach the stream's pin instead of retaining the previous chunk. Eviction prefers unpinned downloaders so a pinned buffer is not dropped mid-stream. A new ReleaseStream lets callers drop their pin without destroying the shared cache; S3 and WebDAV readers use it. lastChunkFid becomes atomic since concurrent mount reads can update it. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: keep eviction bounded when every downloader is pinned Both eviction paths still fall back to a pinned victim when no unpinned one exists, so abandoned stream pins cannot bypass the downloader limit or stall the memory budget. Budget eviction also rechecks the pin under the ReaderCache lock at removal time: a stream that pinned the selected victim in between keeps it mapped and the selection retries. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: restore budget bookkeeping when a victim gets pinned mid-eviction removeUnpinned losing the pin race left the victim out of the idle list while still holding its reservation, making it unevictable even as the pinned fallback. Push it back when the reservation is still live. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |