* s3: invalidate stale reader cache locations on chunk read failure
* filer: share the chunk-read self-heal across reader cache and streaming paths
The reader cache retry added a third copy of the invalidate-relookup-compare-retry
dance already inlined in PrepareStreamContentWithThrottler and duplicated in
retryWithCacheInvalidation. Extract retryFetchWithFreshLocations and route all
three through it, parameterized by the refetch primitive.
* filer: drop redundant completedTimeNew store in reader cache success path
startCaching already stamps completedTimeNew unconditionally before the
fetchErr branch; the second store inside the success branch is dead.
* filer: make NewReaderCache cache invalidator an explicit parameter
The variadic ...CacheInvalidator only ever read the first element, so a caller
could pass two and silently get one. Take a single explicit argument and have
the non-S3 callers pass nil.
* filer: inject reader cache chunk fetch as a struct field
Replace the process-global readerCacheFetchChunkData test seam with a
per-instance fetchChunkDataFn field defaulted in NewReaderCache, matching how
lookupFileIdFn is already wired. Tests set the field on the cache instead of
swapping a shared global.
* filer: log the location count, not full URLs, on self-heal retry
---------
Co-authored-by: Chris Lu <chris.lu@gmail.com>
* s3: support AWS object form for bucket policy Principal, add NotPrincipal
Bucket policy statements only accepted a bare string or array of strings for
the Principal element, so the AWS-documented object form was rejected:
"Principal": { "AWS": "arn:aws:iam::123456789012:root" }
"Principal": { "AWS": ["arn:...", "999999999999"] }
Add a PolicyPrincipal type that parses the bare string, the bare array
(retained for backward compatibility), and the object form keyed by AWS,
Service, Federated or CanonicalUser (each value a string or array). All keyed
values are flattened for principal matching, and the original JSON is preserved
so PutBucketPolicy/GetBucketPolicy returns the exact shape submitted - keeping
infrastructure-as-code tools (Terraform, Ansible) idempotent.
Also add NotPrincipal support (a statement applies to every principal except the
ones named), compiled and evaluated in both policy evaluators, and reject
statements that specify both Principal and NotPrincipal.
* s3: address review - validate principal object form, honor dynamic NotPrincipal
- Reject unsupported Principal object keys (only AWS/Service/Federated/
CanonicalUser) and empty values, so a form like {"AWS":[]} no longer compiles
to zero matchers and silently relies on the match-all fallback.
- Detect both Principal and NotPrincipal by field presence, not by flattened
length, so a present-but-empty field is still rejected.
- Honor dynamic (policy-variable) NotPrincipal/Principal patterns in the
compiled evaluator; previously a NotPrincipal made only of variables was
treated as absent and its exclusion bypassed.
- Add regression tests for the object-form validation and dynamic NotPrincipal.
* sts: enforce session-policy explicit deny during role chaining
A chained AssumeRole caller authenticates with an STS session token whose
inline session policy can explicitly deny sts:AssumeRole. The deny check only
evaluated the caller's named policies, so such a session could still chain into
any role its trust policy admits. Validate the session token in the deny check
and honor an explicit Deny in the inline session policy too.
* test(sts): integration coverage for AssumeRole authorization
Add an end-to-end AssumeRole authorization test (real weed mini + boto3):
a non-admin caller assumes a role its trust policy admits, an explicit
identity-side deny is blocked, and a session policy's explicit deny blocks
role chaining.
* sts: skip OIDC tokens and reject revoked sessions in the chaining deny check
Review follow-ups on the session-policy deny check:
- Guard session validation with !isOIDCToken so a bearer token our STS service
cannot validate does not error into a false deny.
- Reject a revoked session before evaluating its policy, restoring the
revocation enforcement the AssumeRole path lost when it stopped routing
through IsActionAllowed.
* fix(sts): authorize AssumeRole by the role's trust policy
The role's trust policy already declares who may assume it, but the caller
also had to pass an identity-side sts:AssumeRole check that only the Admin
action could satisfy — legacy static identities have no way to express
sts:AssumeRole on a role. So assuming any role required a full admin
identity. Drop the redundant check and let the trust policy be the authority;
scope it to specific principals to restrict who can assume.
* sts: resolve caller principal ARN for the trust-policy check
A legacy static identity can reach AssumeRole without a PrincipalArn set;
passing the empty value would miss a trust policy that names a concrete
principal. Resolve it to the canonical user ARN, sharing the logic
GetCallerIdentity already used inline.
* sts: enforce explicit identity-side deny for AssumeRole
Authorizing a named role by its trust policy alone dropped identity-side
evaluation entirely, so a caller whose attached policy explicitly denies
sts:AssumeRole could still assume any role the trust policy admits. Re-check
the caller's policies through the IAM manager for an explicit deny
(deny-always-wins) without requiring an allow; the trust policy stays the
allow authority.
Introduce security.BearerPrefix ("Bearer ", RFC 6750) and use it
everywhere an "Authorization: Bearer <token>" header is constructed,
replacing the scattered "BEARER "/"Bearer " string literals. SeaweedFS
matches the scheme case-insensitively when parsing (security.GetJwt), so
behavior is unchanged; this removes the magic string and settles the
casing on the standard form. The parser's upper-case comparison stays as
is on purpose.
* s3: replicate by fanning out from the gateway to every holder
The S3 gateway uploaded each chunk to one volume server, which then
relayed the copies to the other replica holders. The gateway now uploads
each chunk to every holder in parallel (type=replicate), removing the
primary volume server's receive-then-resend relay.
AssignVolume returns every replica holder (new repeated Location replicas,
forwarded from the master assign), the s3api captures them, and the
chunked uploader fans out whenever a chunk has more than one holder.
Cipher uploads keep the server-driven path since per-call encryption would
diverge the replicas.
* s3: cancel sibling replica uploads on the first failure
* s3: trim replica fan-out comments
* s3: roll back successful fan-out chunk copies when a holder fails
A failed fan-out records no FileChunk, so copies that landed on the holders
that finished before the cancel were leaked as orphans the caller could not
see. Track the holders that succeeded and delete the needle from each
(type=replicate, local-only) on failure, leaving nothing behind.
A config-file reload (SIGHUP) routed through MergeS3ApiConfiguration,
which skips identities marked static so dynamic admin/filer updates can't
clobber them. That also blocked the config file itself from updating its
own identities, so editing a secretKey and reloading had no effect.
Thread a fromStaticFile flag from the file-load path into the merge: the
authoritative file overwrites its static identities (and reapplies service
accounts under them), while dynamic updates still leave them immutable.
Mark the rebuilt identities static in the merge so a concurrent
RemoveIdentity never observes them as removable mid-reload.
Standalone weed s3 created a master client and registered the receiving
SeaweedS3IamCache gRPC service, but never wrapped its credential store
with the propagating store. Only the filer-embedded path called
SetMasterClient, so IAM mutations on one s3 pod never reached peers; they
served a stale in-memory identity cache and returned InvalidAccessKeyId
until restarted.
Wrap the credential store with the master client when one is available,
mirroring the filer path, so mutations fan out over the existing gRPC
cache service.
* s3tables: allow hyphens in namespace and table names
Iceberg REST clients routinely use hyphenated namespace/table names, but the
S3 Tables charset (a-z, 0-9, _) rejected them with 400. Accept '-' as an
interior character (names must still start, and namespaces end, with a letter
or digit), making the catalog conformant for those clients. A permissive
superset of the AWS S3 Tables charset.
* s3tables: allow hyphens in table ARN parsing too
The ARN regexes still excluded '-', so parseTableFromARN rejected ARNs with
hyphenated namespace/table names and existing reject-the-hyphen tests broke.
Widen the ARN patterns to match the validator, retarget those tests at a
still-invalid leading-hyphen name, and cover ARN parsing with hyphens.
* s3tables: purge decoupled table data without deleting the reused name path
A renamed or created-over-leftover table keeps its data at a location that
differs from its catalog name path. Drop now purges that data location and
clears the marker, instead of recursively deleting the name path, which may
still hold another table's data.
* iceberg: route a table created over a leftover to a unique location
When the default location is occupied by a leftover directory (data kept when
another table was renamed to this name), create the new table at a unique
location so it cannot overwrite that table's metadata. Common case is unchanged.
* iceberg: fail table create when the leftover-path check errors
A transient filer lookup error fell through as "not occupied", routing the
new table back to the default path and risking the very overwrite this check
guards against. Propagate the error and return 500 instead.
* s3tables: assert all catalog xattrs cleared on decoupled drop
Seed the full marker set so the test catches a regression that leaves the
policy, tags, version, or entry-type attribute on the reused name path.
* s3tables: refuse to drop a table whose data path is an ancestor
Corrupt metadata can resolve the data path to the bucket or namespace root,
which the bucket-scope check still admits; a recursive purge there would wipe
sibling tables. Reject an ancestor data path before deleting.
handleCreateTable used a type assertion that fails through WithFilerClient's
'all filers failed' wrap, so a concurrent create that the pre-check missed
fell through instead of returning the existing table. Use errors.As.
* s3tables: add RenameTable operation
* iceberg: support table rename
* iceberg: test table rename
* s3tables: keep table data in place on rename
rename is catalog-only: drop the source's catalog xattrs in place instead of recursively deleting its directory, which wiped the metadata.json and data files the renamed destination still points at. treat a missing table-metadata xattr as NoSuchTable in GetTable so the soft-deleted source name stops resolving.
* s3tables: test rename preserves data
make the in-memory filer honor recursive data deletion and seed the source table's metadata/ and data/ children, then assert a rename leaves them intact, the source name resolves to NoSuchTable, and the destination resolves to the preserved location.
* iceberg: map rename errors through wrapped manager error
* s3tables: authorize rename destination namespace
rename moved a table into the destination namespace after only checking the source, letting a source-authorized caller place tables in namespaces they don't control. require CreateTable on the destination namespace and bucket before writing.
* s3tables: purge renamed table data on drop
* s3tables: test table data dir derivation
s3: don't write 503 to a disconnected client during remote cache wait
When the remote-only cache poll returns without chunks, re-check the
request context before emitting 503 + Retry-After. A client that
disconnected during the wait surfaces as context.Canceled, which the
caller already handles silently; writing to the closed connection only
produced broken-pipe log noise.
* s3tables: tag table entries and exclude views from table listings
* s3tables: add view CRUD operations
* iceberg: support view create, load, exists, drop, and list
* iceberg: support view update
* iceberg: test view error classification and metadata round-trip
* iceberg: pre-check existence and write view metadata only after create
* iceberg: map view namespace-not-found to 404
* iceberg: test view create namespace-404 and duplicate no-clobber
* s3tables: tag view metadata and entry type atomically
CreateView wrote ExtendedKeyMetadata and ExtendedKeyEntryType in two
UpdateEntry calls, so a partial failure could leave a view directory
untagged. Add setExtendedAttributes to set both in one UpdateEntry.
* iceberg: roll back view registration when metadata write fails
The metadata file is written after the catalog registers the view. If
that write fails, drop the just-created view so it doesn't linger
pointing at a missing metadata.json. Reuse the DeleteView path via a
shared dropView helper.
* iceberg: support multi-table transaction commit
Add handleCommitTransaction for POST /v1/transactions/commit. Validation
is atomic across all table-changes (resolve, load, evaluate every
requirement before any write); metadata writes and pointer flips are
best-effort with rollback, so this is not crash-atomic.
* iceberg: route transactions/commit with and without prefix
* iceberg: test transaction commit request decoding
* iceberg: restore full prior table state on transaction rollback
* iceberg: test transaction rollback restores full prior table state
* iceberg: only clean up metadata for rolled-back tables
* s3tables: add RegisterTable op
* iceberg: support table register
* iceberg: test register table
* iceberg: parse engine-written metadata version from location
* iceberg: test metadata version parsing for both filename forms
* iceberg: map register errors through wrapped manager error
* iceberg: validate register metadata-location bucket and reject traversal
* iceberg: log register metadata load failure
* filer: treat a directory carrying object data as an S3 key object
A file promoted to a directory by a child write keeps its chunks, inline
content, or remote-tiered entry. Recognize that as a directory key object,
not only when a Mime is set, so the object still lists, demotes on delete,
and is not reclaimed by cleanup like the object it still is.
* filer: keep the empty-folder cleaner from reclaiming a promoted object
The cleaner skips directory key objects, but its check only looked at the
Mime. Mirror the chunks/content/remote check so a file promoted to a
directory is not deleted once its children are gone.
* s3: serve ranged GET for a directory that carries object data
Reject only zero-size directories so a file promoted to a directory streams
range requests instead of returning 404, while empty directories still 404.
* s3: return HEAD metadata for a directory that carries object data
HEAD now 404s a directory only when it has no data, so a promoted object is
retrievable while empty/implicit directories still fall back to LIST.
* iceberg: return 400 for invalid namespace/table names
The S3 Tables name charset (a-z, 0-9, _) is stricter than the Iceberg
REST spec, so clients sending hyphens or uppercase hit a validation
error. That error fell through to 500; it's client input, so map it to
400 BadRequestException across the namespace and table handlers.
* iceberg: tighten name-validation error matching
Match the validator's own phrasings (invalid/must/cannot) instead of a
bare "namespace name"/"table name" substring, so an unrelated fault that
happens to mention a name isn't misreported as a 400. Lowercase first to
stay robust to message capitalization.
* iceberg: support namespace property updates
Add POST /v1/namespaces/{namespace}/properties to the REST catalog. It
applies the request's removals and updates and returns the removed/updated/
missing summary the spec defines. A new UpdateNamespace op on the S3 Tables
manager rewrites the stored namespace properties; AWS S3 Tables namespaces
have no properties, so this is the SeaweedFS-side backing for the catalog.
* iceberg: dedup namespace property removals
A key repeated in removals was deleted on its first occurrence, then
reported as missing on the next — landing in both removed and missing.
Skip keys already processed.
* iceberg: map namespace-update backend errors to REST statuses
UpdateNamespaceProperties returned 500 for every manager failure, masking
the namespace being dropped between read and write, or a denied caller.
Inspect the typed S3TablesError and answer 404/403 accordingly, 500 only
for the rest. Also replaces the GetNamespace not-found string match.
* iceberg: test the namespace-properties conflict path
Cover the 422 returned when a key appears in both removals and updates.
The check runs before any backend call, so it needs no filer.
* fix(s3api): preserve requested AES256 copy encryption
Problem
CopyObject metadata processing ignored an explicit x-amz-server-side-encryption: AES256 request header. A destination copy could lose the requested SSE-S3 metadata even though KMS requests were handled.
Root cause
processMetadataBytes only wrote the destination SSE header when the requested algorithm was aws:kms. Any other explicit SSE algorithm fell through to the source-preservation branch.
Fix
Write the requested SSE algorithm whenever x-amz-server-side-encryption is present, and keep KMS-specific metadata handling limited to aws:kms.
Co-authored-by: Codex <noreply@openai.com>
* fix(s3api): reject unsupported copy encryption algorithms
A mistyped or unsupported x-amz-server-side-encryption value on a copy
request slipped past validation and got persisted as the destination's
algorithm header, advertising encryption that was never applied. Reject
anything other than AES256 or aws:kms up front.
* fix(s3api): write SSE key metadata for empty encrypted copies
A zero-byte source copied with an explicit SSE request took the
no-content branch and never ran the encryption path, leaving the object
with a bare algorithm header but no key. HEAD then advertised SSE while
the encryption-state machine saw the header as orphaned. Run the inline
encryption path when the destination requests encryption so the key
metadata is written too.
* s3api: use SSEAlgorithmKMS constant in copy metadata handling
* test(s3api): cover source SSE preservation on copy
* test(iam): allow the local client's real source IP in SourceIp tests
The aws:SourceIp allow policies hardcoded the loopback CIDRs, but a CI
runner reaching the server over localhost can be observed with one of the
host's RFC1918 addresses (the S3 endpoint is advertised on a 10.x
interface), so the positive-condition PutObject was denied and the allow
assertion flaked while the deny path passed trivially. Broaden the allow
list to loopback plus private ranges via a shared helper, and log the
denial on each failed attempt so any residual failure is diagnosable.
---------
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
* fix(s3api): sort repeated SigV4 query values
Problem
SigV4 canonical query strings must sort repeated parameter values. SeaweedFS preserved the incoming value order, so a correctly signed request with repeated query parameters could fail verification when the request order differed from canonical order.
Root cause
getCanonicalQueryString delegated directly to url.Values.Encode(), which sorts parameter names but preserves each key's value slice order.
Fix
Sort every query value slice before encoding the canonical query string, while still excluding X-Amz-Signature for presigned URLs.
Reproduction
go test ./weed/s3api -run TestGetCanonicalQueryStringSortsRepeatedValues -count=1 failed before the fix with partNumber=2 before partNumber=10.
Co-authored-by: Codex <noreply@openai.com>
* test(s3api): expand canonical query coverage
Co-authored-by: Codex <noreply@openai.com>
---------
Co-authored-by: Codex <noreply@openai.com>
* s3: add streaming reader interface for remote storage
Add RemoteStorageStreamReader optional interface to support efficient
streaming of large remote objects without buffering entire file in memory.
This enables future stream-through caching where data can be served to
clients while simultaneously writing to volume servers.
Implement ReadFileAsStream() for S3, GCS, and Azure backends using their
native streaming APIs. This provides the foundation for improving TTFB
on large remote file access by serving data directly from remote storage
while background cache operation populates local chunks.
The streaming interface allows remote storage backends to return io.ReadCloser,
enabling efficient memory usage for multi-GB objects compared to the
current ReadFile() approach which buffers entire ranges in memory.
* s3: adaptive timeout for remote object caching to improve TTFB
Use size-aware cache polling timeout to balance cache-hit rate against
time-to-first-byte:
- Small files (<50MB): 10s timeout - more likely to complete caching
before timeout, improving subsequent request performance
- Medium files (50-500MB): 5s timeout - default balance
- Large files (>500MB): 2s timeout - fail-fast to improve initial TTFB
for very large downloads
This reduces waiting time for large remote files while maintaining
high cache-hit rate for smaller files that cache quickly.
* s3: address code review feedback for stream-through cache
- Move startBackgroundRemoteCache call after policy recheck to avoid
cache side effects for denied requests (authorization first)
- Make startBackgroundRemoteCache version-aware by accepting versionId
parameter and using buildVersionedRemoteObjectPath
- Add timeout (5 minutes) to background cache context to prevent
goroutine pile-up if RPC stalls under load
- Update cacheRemoteObjectForStreamingWithShortTimeout to return both
entry and error, allowing callers to distinguish transient errors
(timeout/cancellation) from permanent errors (not found, denied)
- Update streamFromVolumeServers to handle permanent cache errors with
appropriate HTTP status codes (404 for not found, 503 for transient)
* s3api: add optional request interceptor to circuit breaker
Add an optional Interceptor func(next http.HandlerFunc, action string) http.HandlerFunc
field on CircuitBreaker, applied at the very top of Limit() -- before upload
concurrency limiting and before the 'if !cb.Enabled' early return -- so it runs
for every route regardless of breaker state.
It is nil by default (no behavior change). An interceptor may reject a request
(write its own response and skip next) or wrap next to observe/shape it. This
provides a single per-request extension point at the existing per-route
chokepoint (cb.Limit already wraps nearly every S3 route), useful for tracing,
auditing, or request rate limiting without touching the route table.
* s3api: evaluate circuit breaker interceptor per request
Move the Interceptor nil-check into the returned handler so it is consulted
per request instead of captured at registration time. This:
- lets the interceptor be installed after registerRouter runs (handlers are
built during construction, before request-time dependencies exist), and
- avoids dereferencing a nil CircuitBreaker at registration time, which
panicked tests that register routes on a server with a nil cb
(e.g. TestRouting_STSWithQueryParams).
Adds a regression test for installing the interceptor after Limit() returns.
* s3: bound streaming remote-cache wait so large cold GetObject returns 503, not a hang
A remote-only object whose download outlasts the 30s dedup attempt fell through
to cacheRemoteObjectForStreaming, which waited on the raw request context with no
bound. For multi-GB blobs the client read timeout fires first, so the request was
canceled and surfaced as InternalError rather than the 503 + Retry-After the
handler already emits. The filer keeps caching on a detached context, so the
retry would have streamed from the cached chunks.
Bound the wait under common client read timeouts: the 503 fires while the client
is still connected, and a retry picks up the finished cache.
* s3: make the streaming remote-cache timeout atomic
The bound is a package-level var so tests can shorten it. Hold it as an
atomic int64 so a test that stores a short value can never race the request
path that loads it.
* s3: assert the streaming-cache timeout test waits on the bound
The upper-bound-only check would also pass if the RPC returned immediately on a
setup error. Add a lower bound so the test fails unless the bounded wait fired.
* s3: register account-less identities' synthesized account in the lookup
#9962 gave each account-less identity a distinct account id derived from
its name (instead of collapsing into admin), but never registered that
account in the id->account map. GetAccountNameById then returned empty
for such ids, so ACL grantee validation rejected canonical grants to the
caller's own account with InvalidRequest, and bucket/object owner display
was dropped as 'owner is invalid'.
This broke a canned PutObjectAcl by an account-less identity (e.g.
TestVersionedObjectAcl with the default 'some_admin_user' identity):
ValidateAndTransferGrants -> GetAccountNameById -> 'account id is not
exists' -> 400 InvalidRequest.
Register the synthesized account at config load so its id resolves to a
display name. Add a regression test.
* s3: reuse explicitly-configured account for account-less identity
Address review: if an account with the same id as an account-less
identity's synthesized account is explicitly configured (custom display
name/email), reuse it instead of the synthesized one. Add a test.
* s3tables: resolve account-less identities to a distinct principal
Static identities with no account block default to the shared admin
account, so getAccountID returned "admin" for every such user and the
permission checks treated them all as the admin principal. Only keep the
admin account when the identity actually carries an admin action;
otherwise fall back to the unique identity name.
* s3tables: limit the open-by-default fallback to anonymous access
The legacy permission path allowed any request that no policy explicitly
denied whenever default-allow was on, which is the zero-config default.
That let an authenticated identity without table permissions reach table
resources owned by others. Restrict the fallback to requests with no
identity or the anonymous identity; authenticated callers must pass an
explicit action or policy check. Zero-config and anonymous access are
unchanged.
* s3tables: drop the no-op ListTableBuckets account gate
The top-level check passed the principal as its own owner, so it always
allowed. Per-bucket filtering in the loop is the real authority; remove
the dead gate and the now-unused locals.
* s3tables: derive the Iceberg catalog's default-allow from auth state
The Iceberg catalog reuses the S3 Tables Manager, which hardcoded
default-allow on. Authenticated callers were enforced only because the
identity struct happens to propagate into the handler; if it were ever
dropped, a secured catalog would fall open. Mirror the S3 port and set
the Manager's default-allow from the authenticator, so an authenticated
caller is enforced regardless. Shell and admin keep their own trusted
Manager. Regression test covers the struct, name-only, and admin paths.
* s3tables: drop redundant ACTION_ADMIN string conversion
ACTION_ADMIN is an untyped string constant, so the conversion is a no-op.
* s3tables: enforce name-only authenticated callers, add trusted bypass
defaultAllowFor treated a request with no identity object as anonymous,
but the Manager path forwards only the identity name (not the struct).
A name-only authenticated caller could therefore be misclassified as
anonymous and allowed under the open default. Treat a server-set identity
name as authenticated too, and add an explicit trusted flag for the local
shell/admin tooling that legitimately bypasses authorization.
* s3tables: trim verbose comments
* s3: give STS sessions a distinct owner account, not admin
STS sessions were built with Account: &AccountAdmin, so every assumed-role
session shared the admin account for ownership and ACL checks. Use the
assumed-role user as the account id instead, matching the JWT auth path.
Session permissions are unchanged: they come from the session policies,
and admin is granted only through Actions.
* s3: resolve STS session identity to the OIDC subject
Use sessionInfo.Subject (falling back to the assumed-role user when
absent) for the session identity name and account id, so the SigV4 and
JWT auth paths resolve the same session to the same identity instead of
diverging on AssumedRoleUser vs Subject.
* s3: trim verbose comments
* s3: stop collapsing account-less identities into the admin account
Identities configured without an account block all defaulted to the
shared admin account, so distinct users got the same owner id and
ownership checks could not tell them apart. checkAccessByOwnership also
treated that id as an admin bypass, so any account-less caller passed
ownership for any bucket. Give such identities a distinct account id from
their name, and decide the ownership admin bypass by Admin capability
rather than by the account id. isUserAdmin is now nil-safe.
* s3: use the context identity in isUserAdmin before re-authenticating
The Auth middleware already verifies and stores the identity in the
request context. Read it there first so the ownership/admin checks don't
re-run signature verification, which is redundant and fails once the
request body has been consumed.
* s3: nil-guard the context identity in isUserAdmin
A non-nil interface wrapping a typed-nil *Identity passes the type
assertion; guard against it before calling isAdmin().
* s3: trim verbose comments
* s3: preserve checksums for copied multipart parts
* s3: return checksums from multipart copy
* s3: pin the upload's checksum algorithm on copy-part re-stream
* s3: note why UploadPartCopy uses the re-stream slow path
* s3: explain the TLS proxy in the multipart copy checksum test
* s3: cover nil and unknown-algorithm edge cases in copy checksum tests
* s3: cover all checksum algorithms in the multipart copy test
* s3: run all checksum integration tests, not just presigned
* s3: validate indirect filer path inputs
* s3: avoid query parsing on common request path
* filer: scope copy/move source against JWT AllowedPrefixes
maybeCheckJwtAuthorization only checked r.URL.Path, but copy and move read
their source from the cp.from / mv.from query params. A prefix-restricted
token could copy or move data out of a subtree it cannot otherwise reach.
Check every path the request touches, reusing pathHasComponentPrefix so
`..` in the source is collapsed before the prefix match.
* s3: confine iceberg CreateTable location to the catalog bucket
CreateTable derived the metadata bucket and path from the client-supplied
req.Location / req.Name and wrote there directly, so a caller scoped to one
table bucket could place metadata in another bucket (and path.Join collapsed
any `..`). Require the parsed bucket to equal the request's catalog bucket
and reject traversal segments in the table path.
* webdav: clean client path before subFolder confinement
wrappedFs concatenated subFolder + name before the underlying FileSystem
ran path.Clean, so `..` in the request path or COPY/MOVE Destination
resolved across the FilerRootPath confinement boundary. Clean the name as a
rooted path first so traversal segments collapse below subFolder. Only the
non-default -filer.path (non-empty subFolder) setup was affected.
* filer: enforce read-only rule on real write path with destination header
The x-seaweedfs-destination header overrides the path used for storage-rule
matching while the entry is written at r.URL.Path, letting a caller select a
writable rule for a read-only target. When the header is present, also check
the read-only/quota rule against the actual write path.
Reject copy sources whose bucket/object fail IsValidBucketName /
IsValidObjectKey, the helpers validateRequestPath already applies to the
request URL. The object is joined onto the bucket path and `.`/`..`
segments are collapsed by the filer, so without this the source need not
stay within the parsed bucket. Route UploadPartCopy through
ValidateCopySource too; it previously only checked for empty bucket/object.
The quota enforcement loop already computes each bucket's configured
quota and effective read-only flag every minute, but neither was
visible to monitoring, so operators could not alert before a bucket
flips read-only.
Add two gauges next to the existing bucket size metrics:
SeaweedFS_s3_bucket_quota_bytes configured quota; the series is only
present while the quota is enabled,
so size/quota utilization queries
never divide by zero
SeaweedFS_s3_bucket_read_only 1 when the bucket's location rule is
read-only (over quota or manually
locked), 0 otherwise
Both are cleaned up with the other per-bucket gauges on bucket
deletion and inactivity TTL.
Problem: Signature V4 SignedHeaders parsing accepted empty header name segments such as host; or ;host. Malformed Authorization headers could continue into signature verification instead of failing during header parsing.
Root cause: parseSignedHeader only checked that the SignedHeaders value was non-empty, then split it on semicolons without validating each element.
Fix: reject empty or whitespace-only signed header elements with ErrMissingFields before returning the parsed header list.
Reproduction: go test ./weed/s3api -run TestParseSignedHeaderRejectsEmptyHeaderNames -count=1 failed before the fix because SignedHeaders=host; returned ErrNone.
Validation: gofmt -w weed/s3api/auth_signature_v4.go weed/s3api/auth_signature_v4_test.go; git diff --check; go test ./weed/s3api -run TestParseSignedHeaderRejectsEmptyHeaderNames -count=1; go test ./weed/s3api -count=1
Co-authored-by: Codex <noreply@openai.com>
Problem: SigV4 chunked upload checksum trailer parsing rejected mixed-case checksum header names even though HTTP header field names are case-insensitive.
Root cause: extractChecksumAlgorithm compared the x-amz-trailer value and trailer header key against exact lowercase strings.
Fix: Trim and lowercase checksum trailer header names before matching supported checksum algorithms.
Reproduction: go test ./weed/s3api -run TestExtractChecksumAlgorithmIsCaseInsensitive -count=1 with X-Amz-Checksum-Crc32; before the fix it returned unsupported checksum algorithm.
Validation: gofmt -w weed/s3api/chunked_reader_v4.go weed/s3api/chunked_reader_v4_test.go; git diff --check; go test ./weed/s3api -run TestExtractChecksumAlgorithmIsCaseInsensitive -count=1; go test ./weed/s3api -count=1
Co-authored-by: Codex <noreply@openai.com>
Problem: Re-storing object-lock default retention with Days left a previous Years extended attribute in place, so later loads could see both Days and stale Years.
Root cause: StoreObjectLockConfigurationInExtended only wrote period fields that were set on the new configuration and did not delete old Days or Years keys before writing the replacement rule.
Fix: Clear stored default-retention Days and Years keys before writing the current default retention period fields.
Reproduction: go test ./weed/s3api -run TestStoreObjectLockConfigurationClearsStaleYears -count=1 failed before the fix because the stale years key remained.
Validation: go test ./weed/s3api -run TestStoreObjectLockConfigurationClearsStaleYears -count=1; go test ./weed/s3api -count=1; git diff --check; git diff --cached --check
Co-authored-by: Codex <noreply@openai.com>
processExplicitDirectory appended a directory-key object as a version
without checking it against the prefix. A versioned listing descends
through ancestor markers to reach a deeper prefix, so every ancestor
(Veeam/, Veeam/Backup/, ...) leaked into Versions even though none of
them match the prefix - which makes Veeam's immutable repository scan
abort on an unexpected key. Guard on the prefix so only keys at or under
it surface, matching ListObjectsV2 and AWS.
* fix(s3api): accept HTTP-date conditionals
Problem: Object conditional headers rejected valid HTTP-date values in RFC850 or ANSIC format for If-Modified-Since and If-Unmodified-Since.
Root cause: parseConditionalHeaders used time.Parse(time.RFC1123), accepting only one HTTP-date representation instead of the standard formats accepted by net/http.ParseTime.
Fix: Parse conditional date headers with http.ParseTime so RFC1123, RFC850, and ANSIC HTTP-date forms are accepted.
Reproduction: go test ./weed/s3api -run TestParseConditionalHeadersAcceptsHTTPDateFormats -count=1 failed before the fix with ErrInvalidRequest for RFC850 and ANSIC date values.
Validation: env GOCACHE=/private/tmp/seaweedfs-go-cache go test ./weed/s3api -run TestParseConditionalHeadersAcceptsHTTPDateFormats -count=1; env GOCACHE=/private/tmp/seaweedfs-go-cache go test ./weed/s3api -count=1; git diff --check; git diff --cached --check
* fix(s3api): accept HTTP-date copy-source conditionals
Mirror the put-path http.ParseTime switch onto the copy-source If-Modified-Since / If-Unmodified-Since headers, which still rejected valid RFC850 and ANSIC dates.
* fix(s3api): keep RFC1123 UTC-zone dates working alongside http.ParseTime
http.ParseTime rejects the "UTC" zone that Go clients emit via t.UTC().Format(time.RFC1123), which the old RFC1123 parser accepted. Add a parseHTTPDate helper that tries http.ParseTime first and falls back to RFC1123, so the put and copy-source conditional date headers accept the union of HTTP-date formats plus the UTC zone.
---------
Co-authored-by: Chris Lu <chris.lu@gmail.com>
Problem: Default object-lock retention accepted an explicitly provided Years value of zero, even though a default retention period must be positive when present.
Root cause: validateDefaultRetention rejected zero Days but only rejected negative Years, leaving YearsSet with Years=0 as a successful validation path.
Fix: Treat an explicitly provided zero Years value as ErrInvalidRetentionPeriod, matching the existing Days validation.
Reproduction: go test ./weed/s3api -run TestValidateDefaultRetention -count=1 failed before the fix because the Zero years case returned nil.
Validation: go test ./weed/s3api -run TestValidateDefaultRetention -count=1; go test ./weed/s3api -count=1; git diff --check; git diff --cached --check
* fix(s3api): require space in v2 auth prefix
Problem: Signature V2 Authorization headers with a malformed algorithm token such as AWSX... are accepted as if they were AWS ... headers.
Root cause: validateV2AuthHeader checks HasPrefix("AWS") but then slices past an assumed trailing space, so an extra character after AWS is skipped and the rest is parsed as credentials.
Fix: Require the Authorization header to start with the exact AWS plus space prefix before parsing fields.
Reproduction: go test ./weed/s3api -run 'TestValidateV2AuthHeader/algorithm_prefix_without_space|TestDoesSignV2Match/malformed_auth_-_no_space_after_AWS' -count=1 fails before the fix because AWSXAKIA... is accepted.
Validation: go test ./weed/s3api -run 'TestValidateV2AuthHeader/algorithm_prefix_without_space|TestDoesSignV2Match/malformed_auth_-_no_space_after_AWS' -count=1; go test ./weed/s3api -count=1; git diff --check; git diff --cached --check
* Update weed/s3api/auth_signature_v2.go
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
---------
Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
A suspended-versioning DELETE was recorded with createDeleteMarker, which mints a
fresh real version id each time, so repeated suspended deletes piled up delete
markers instead of overwriting a single null marker as S3 specifies. Record the
suspended delete as a 'null' marker with a fixed file name (v_null) and point the
latest-version pointer at it explicitly; putSuspendedVersioningObject's existing
null-version cleanup removes it on the next suspended PUT, so the object undeletes
cleanly and at most one null marker exists. Enabled-versioning deletes are
unchanged (still distinct historical markers).
Update TestSuspendedVersioningDeleteBehavior to the AWS-correct counts: one null
marker after a suspended delete, and the null marker plus one real marker after a
re-enabled delete.
ListObjectVersions gated explicit directory objects on Mime ==
FolderMimeType, but an SDK PutObject of "dir/" carries a default
Content-Type (e.g. application/octet-stream), so those directory keys
were dropped from the version listing while ListObjectsV2 - which keys
off IsDirectoryKeyObject (any non-empty mime) - still showed them. Use
the same IsDirectoryKeyObject check so the two listings agree.
The directory test's storage-class assertion compared an ObjectStorageClass
constant against ObjectVersion.StorageClass (ObjectVersionStorageClass);
the values matched but the SDK enum types did not, so it only surfaced
once the directories started appearing. Use the matching constant.
* s3: rescan .versions when the cached latest pointer is missing on a list
ListObjectsV2 resolves each versioned object's current version from the
latest-version pointer cached on the .versions directory entry. When that
pointer is absent on the filer serving the list, the object was dropped
from the listing. Fall back to a read-only rescan of .versions/ to pick
the newest version - the version files are present locally even when the
cached pointer is not - so the object still lists. This mirrors the read
path's recoverLatestVersionWithoutPointer; the scan loop is shared.
Read-only by design: a list can touch many objects, so it does not persist
a pointer.
* s3: copy scanned Extended before stamping the version id
Stamping an Expiration.Days rule as a volume TTL at write time bakes an
irreversible TTL into the object: removing or lengthening the rule later
can't un-expire it, unlike worker-driven expiration. The metadata-only
delete it enables also skips per-chunk DeleteFile, so dead bytes linger in
a not-yet-expired TTL volume with no deleted-byte accounting until the
whole volume ages out.
Gate the resolver on a per-bucket flag, off by default; toggle with the
s3.bucket.lifecycle.fastpath shell command. Default writes take the worker
path: real deletes that honor current policy and let vacuum reclaim space.
* fix(s3api): standardize ETag calculation across S3 API handlers
* s3: make copyEntryETag nil-safe
---------
Co-authored-by: Chris Lu <chris.lu@gmail.com>
* security: reload JWT signing keys on SIGHUP
Signing keys were read once in the server constructors and never
refreshed. After a key rotation (Secret update, divergent reads) the
in-memory key stayed stale and every request kept failing "wrong jwt"
until the affected process was restarted.
Add Guard.UpdateSigningKeys and call it from the master, volume and
filer reload paths and the s3 reload hook, next to the existing
whitelist refresh. Make the global chunk-read JWT cache reloadable via
an atomic swap, and register the master's Reload with grace.OnReload --
it was never wired, so the master ignored SIGHUP entirely.
Mirror the same refresh in the Rust volume server's SIGHUP handler.
* security: swap signing keys behind an atomic pointer
Addresses review feedback on the in-place key swap: SigningKey is a
[]byte, so reassigning the Guard fields while a request handler reads
them is a data race that can tear the multi-word slice header and read
out of bounds.
Hold the four signing-key fields in an immutable signingConfig snapshot
behind atomic.Pointer; UpdateSigningKeys swaps the whole pointer, so a
reader sees either the old keys or the new ones. Reads go through new
SigningKey/ExpiresAfterSec/ReadSigningKey/ReadExpiresAfterSec accessors.
The Rust guard is already safe: every read and the SIGHUP write go
through the shared RwLock<Guard>.
* security: fold whitelist + auth state into the atomic snapshot
Review follow-up. UpdateSigningKeys still wrote isWriteActive while the
request path read it (and the whitelist maps) unsynchronized, so a SIGHUP
under load could expose an inconsistent mix of activation bits and
whitelist contents.
Move all hot-reloadable Guard state -- keys, expirations, whitelist, and
the activation flags -- into a single immutable guardState swapped behind
one atomic.Pointer. The Update* methods take a small mutex to serialize
the read-modify-write; readers stay lock-free. The concurrency test now
also rotates the whitelist and probes IsWhiteListed under -race.
Also read each signing key once per branch in the volume/filer JWT auth
checks, so a reload landing mid-check can't take the allow-fast-path
after auth was enabled or verify against a different key than the branch
saw.
* s3: return BucketAlreadyOwnedByYou when recreating your own bucket
PutBucket returned BucketAlreadyExists for every existing bucket, even
when the caller already owns it, so idempotent re-creation (e.g. a
container that creates its bucket on startup) couldn't tell "someone
else took the name" from "it's already mine".
Recreating a bucket you own now returns BucketAlreadyOwnedByYou, unless
the request conflicts with the existing bucket: a different Object Lock
setting, or an ACL on the request or the existing bucket. To detect the
latter, a requested non-default canned/grant ACL is now persisted on
creation instead of being dropped.
* s3: fail PutBucket when the existing bucket's config can't be read
When a bucket already exists, an unreadable config left the recreate
defaulting to BucketAlreadyOwnedByYou, masking the backend error and
possibly accepting a conflicting recreate (Object Lock / ACL unknown).
Surface the read error instead.
* s3: return the stored bucket ACL from GetBucketAcl
GetBucketAcl always returned the owner's default full-control grant and
ignored any stored ACL, so a bucket created with a canned ACL or one set
via PutBucketAcl never read back correctly. Decode the stored grants
instead, sharing one grants-to-XML helper with the object ACL handler.
The shared helper also emits each grantee's real xsi:type (e.g. Group for
public-read) instead of a hardcoded CanonicalUser, so group grants read
back correctly for both bucket and object ACLs.
* s3: resolve the right already-exists error on the concurrent-create race
When two requests create the same bucket at once, the loser's mkdir
fails and the handler fell back to a flat BucketAlreadyExists, bypassing
the same-owner idempotency check. Route both the pre-check and the race
fallback through one existingBucketError helper so a same-owner recreate
still gets BucketAlreadyOwnedByYou.
* s3: record the bucket owner's account id at creation
setBucketOwner only stored the creating identity name, so the canonical
account id wasn't available later. Persist it under ExtAmzOwnerKey too,
the same field PutBucketAcl writes, so the bucket owner can be reported
independently of whoever reads it.
* s3: report the bucket owner from GetBucketAcl, not the caller
GetBucketAcl built the ACL Owner from the caller's account header, so an
admin or cross-account read returned the wrong owner. Use the owner
persisted on the bucket, falling back to the caller only when none is
recorded.
Chris LuandGitHub
Sergey ZinchenkoandGitHub
