* 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.
* fix(master): advance maxVolumeId when registering EC shards
After EC encoding the original normal volume is deleted, so a
high-numbered volume can exist only as EC shards. Only regular volumes
advanced maxVolumeId (Disk.doAddOrUpdateVolume), so a master that
rebuilt its state from heartbeats (raft state not resumed) undercounted
the max and NextVolumeId could re-issue an id that EC shards still
occupy. A new volume then gets created on top of the EC volume id; new
writes land on it, but reads route to the old EC shards whose .ecx never
held the new needle, returning 404 and corrupting that object.
Advance maxVolumeId when EC shards are registered, mirroring the
regular-volume path. RegisterEcShards is the chokepoint both the full
and incremental heartbeat sync paths funnel through.
* test: cover incremental heartbeat path for EC maxVolumeId
Both SyncDataNodeEcShards and IncrementalSyncDataNodeEcShards funnel
through RegisterEcShards; assert the invariant on the incremental path
too.
* 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.
* mysql: keep S3 list order byte-lexicographic regardless of name column collation
ORDER BY name and the name > ? pagination predicate follow the column
collation, so a case-insensitive filemeta.name (e.g. utf8mb3_general_ci)
returns S3 keys out of byte order and breaks clients that merge two sorted
listings.
Detect the live name collation at startup; only when it isn't binary, wrap
the list comparison, prefix, and ORDER BY in BINARY name so order and
pagination stay consistent. Correctly configured utf8mb4_bin tables keep
their indexed range scan unchanged, and the operator gets a warning to
convert the column.
* postgres: keep S3 list order byte-lexicographic regardless of name column collation
ORDER BY name and the name > $n pagination predicate follow the column or
database collation, so a locale-aware filemeta.name (e.g. the en_US.UTF-8
database default) returns S3 keys out of byte order and breaks clients that
merge two sorted listings.
Detect the live name collation at startup; only when it isn't byte-ordered,
wrap the list comparison, prefix, and ORDER BY in COLLATE "C" so order and
pagination stay consistent. A byte-ordered (C/POSIX/C.UTF-8) column keeps its
indexed range scan unchanged, and the operator gets a warning to declare the
column COLLATE "C".
* filer: stream persisted log files when serving metadata subscriptions
readFileEntries buffered every LogEntry of a whole log file into memory
before returning them one by one, making a subscription read O(entries in
one log file) instead of O(one entry). On a filer with large per-entry
metadata, many concurrent SubscribeMetadata streams each loading a full
log file exhausted memory.
Keep a current LogFileIterator and return one entry at a time, advancing
files as each is exhausted. The deleted-volume skip is preserved.
* filer: close the log file iterator on read errors too
A genuine read error returned early without closing the current
LogFileIterator, leaving its ChunkStreamReader alive until GC. Close on
every exit path and propagate only a real error.
* filer: close persisted-log iterators when a subscription stops early
The streaming iterator keeps a log file reader open across calls, so a
subscription that returns before EOF (early stop, cancellation) left the
reader alive until GC. Add idempotent Close on LogFileQueueIterator and
OrderedLogVisitor, and have ReadPersistedLogBuffer wait for the readahead
goroutine and close the visitor on the way out.
* fix: log meta backup offset errors
* fix: log meta backup offset errors
* fix: exit on meta backup offset errors
Exit with a non-zero status when the initial metadata backup offset cannot be persisted.
Classify offset-read failures during streaming so the backup process exits instead of retrying forever, allowing supervisors to restart and bootstrap from a missing checkpoint.
* meta backup: read offset in the loop, drop offset error type
Reading the saved offset inside the retry loop makes an offset read
failure a clean exit and a stream error a retry, without a typed error
to tell them apart. streamMetadataBackup now takes the start time.
---------
Co-authored-by: Chris Lu <chris.lu@gmail.com>
* s3: keep dynamic IAM live when -iam.config is set
-iam.config was treated like a static -config identity file: it set
useStaticConfig, which makes the filer metadata subscription skip
reloads. Identities and policies created at runtime (the IAM gRPC API)
then never took effect, so advanced IAM (OIDC/STS) and dynamic IAM were
mutually exclusive.
Gate useStaticConfig on whether inline identities were actually loaded.
An OIDC/STS-only config carries none, so it keeps the dynamic credential
store live; a -config identity file still freezes its identities as
before.
* s3: mark static identities on config reload too
A -config reload (grace.OnReload) re-reads the file, but only the startup
path marked its identities static, so identities added to the file and
reloaded were left unprotected from dynamic filer updates. Move the
marking into loadS3ApiConfigurationFromFile and make it additive and
scoped to the file's identities, so a reload protects newly added ones
without freezing dynamic filer-managed identities.
* s3: sync reloaded static identities into the credential manager
After marking a (re)loaded config file's identities static, push the
updated set into the credential manager so reloaded identities still
appear in listings and survive later dynamic merges. Centralize the sync
in loadS3ApiConfigurationFromFile and drop the now-redundant call in the
reload hook.
2026-06-03 23:28:25 -07:00
Fabian HardtGitHubgemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>Chris Lu
* sftpd: support SSH user certificates signed by a trusted CA
Adds a new "certificate" auth method to weed sftp. When enabled, the server
loads trusted CA public keys from -trustedUserCAKeysFile (OpenSSH
authorized_keys format, one or more keys) and accepts only ssh.Certificate
blobs of type UserCert on the public-key channel. Validation uses
ssh.CertChecker: CA signature, ValidAfter/ValidBefore, non-empty
ValidPrincipals and SSH login user must appear in ValidPrincipals. The
authenticated user must exist in the user store; home dir and permissions
resolve as before.
Behaviour mirrors MinIO's --sftp=trusted-user-ca-key and OpenSSH's
TrustedUserCAKeys: when certificate auth is active, plain (non-cert) public
keys are rejected even if "publickey" is also listed. Default authMethods
remain "password,publickey", so existing deployments are unaffected.
* Update weed/sftpd/auth/certificate.go
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* sftpd: address review feedback on certificate auth
- Pre-marshal trusted CA public keys in IsUserAuthority instead of
re-marshaling on every authentication attempt (gemini-code-assist).
- Differentiate user-not-found from underlying store errors via
errors.As(*user.UserNotFoundError) so backend/read failures are no
longer reported as bad credentials (coderabbitai).
- Fix the corresponding sanity check in the missing-file test to use
errors.As instead of errors.Is (UserNotFoundError has no Is method,
so the previous check never matched) (coderabbitai).
* sftpd: register trustedUserCAKeysFile flag in filer and server commands
The new field on SftpOptions is dereferenced unconditionally in
resolvePaths(), but only the standalone `weed sftp` command was wiring
its flag. `weed filer` and `weed server` both embed an SftpOptions value
and call resolvePaths() on it, so they hit a nil pointer dereference at
startup.
Register `-sftp.trustedUserCAKeysFile` in both commands and update the
-sftp.authMethods help text to mention the new "certificate" method.
Fixes the SFTP Integration Tests CI failure on this PR.
* helm: expose SFTP certificate auth in the SeaweedFS chart
Adds Helm-chart support for the new SSH user-certificate auth method:
- values.yaml (sftp:) gains `trustedUserCAKeys` (inline OpenSSH
authorized_keys-format CA public keys) and `existingCAKeysSecret`
(reference an externally managed Secret). Same pair added under
allInOne.sftp with a null default that falls back to the top-level
sftp.* setting.
- New template templates/sftp/sftp-ca-secret.yaml renders a
chart-managed Secret <release>-sftp-ca-secret with `ca_user.pub`,
but only when SFTP is enabled, "certificate" is in authMethods,
inline keys are provided, and no existingCAKeysSecret is set.
- templates/sftp/sftp-deployment.yaml and the all-in-one deployment
template add `-trustedUserCAKeysFile=/etc/sw/sftp_ca/ca_user.pub`
to the weed sftp command, mount the CA secret at /etc/sw/sftp_ca
and add the corresponding volume. All cert-auth bits are guarded
by `contains "certificate" authMethods` so existing users see no
change.
- authMethods help text updated to mention "certificate".
Verified end-to-end on a local k3d cluster: cert login succeeds,
plain-pubkey login is rejected with "public key without certificate
not allowed".
* helm: fail render when SFTP certificate auth lacks CA keys
When certificate is in authMethods but neither trustedUserCAKeys nor
existingCAKeysSecret is set, the deployment mounted a secret that the
chart never renders, leaving the pod stuck on a missing volume. Fail at
template time with a clear message instead.
* sftpd: fix stale auth-method list in SFTPServiceOptions comment
keyboard-interactive was never implemented; certificate is the new
supported method. Match the CLI help text.
* sftpd: test Manager wiring of certificate vs public-key channel
Cover the channel takeover at the Manager level: certificate auth
displaces plain public-key auth when both are enabled, public-key auth
stays put otherwise, and enabling certificate without a CA file errors.
---------
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
* filer: bound TraverseBfsMetadata memory by queuing directory paths
The BFS enqueued every entry, so it held the whole subtree in memory
including each file's chunk list. A filer serving a peer's first-time
bootstrap traversal of a large tree could exhaust memory and get killed.
Stream each entry as it is visited and queue only directory paths to
descend into. Memory is now bounded by the number of directories rather
than the entire tree, and the streamed output order is unchanged.
* filer: match excluded prefixes on path-component boundaries
Only treat an excluded prefix as a match when it ends at a path
boundary, so excluding /a/b does not also drop a sibling like /a/bc.
Short-circuit the trie walk on the first real match.
Blanking preferred kept route-by-key reads from dialing a flagged owner first,
but withFilerClientFailover always re-adds the current filer, so when the owner
is the gateway's current filer it stayed in the candidate list and got dialed
anyway. Treat a recently-unreachable filer as unhealthy in the health partition
so it is deferred to the last-resort tail instead of tried before healthy
replicas; preferred is still tried first, and a live owner is unaffected.
* s3: route object reads to the key's owner filer
Writes already route by key to the owner filer on the lock ring, where the
entry is created. Reads went to the gateway's local filer and treated its
NotFound as authoritative, so a GET on one gateway could miss an object
another gateway had just written until the filers' metadata replication
caught up.
Resolve an object's entry from the key's owner first, failing over to the
gateway's filer set only on transport errors. An owner NotFound stays
authoritative: no fan-out across filers, and no resurrecting a peer's
not-yet-replicated tombstone, so a delete routed to the owner is visible at
once and a genuine miss costs one lookup. Keys owned by the local filer are
unchanged. Objects written through the non-routed lock path land on a
gateway's local filer, so they can still read as absent on the owner until
they replicate.
withFilerClientFailover takes a preferred start filer; the object-entry
reads pass the owner, every other caller passes "" and keeps the
current-filer fast path.
* s3: consult the prior owner on a rebalance-window read miss
Owner-first reads route a key to its current ring owner. When a filer joins,
~1/N of keys reassign to it, and the new owner may not have replicated a
just-moved key yet, so an owner NotFound would surface a transient 404 for an
object that already exists elsewhere.
Retain the previous ring on the gateway's LockClient for a cooling-off window
(PriorOwnerForKey, mirroring the master's LockRing.PriorOwner) and, on the
owner's NotFound, probe the key's previous owner once before treating the miss
as final. The probe is scoped to keys whose ownership actually moved and only
within the window, so steady-state reads are untouched.
This trades the transient scale-up 404 for a transient stale read if a delete
routed to the new owner races the same window — the same authoritative-NotFound
tradeoff, narrowed to the rebalance.
* s3: try healthy filers before unhealthy ones on failover
The candidate list probed its first entry (usually the current filer)
unconditionally, so a health-flagged current filer cost a transport timeout on
every ordinary call before failover reached a replica. Partition candidates into
healthy and unhealthy, keep priority within each, and fall back to unhealthy
ones only when all healthy ones fail.
* reduce comments on the routed read and lock client paths
* s3: skip a recently-unreachable owner on route-by-key reads
The gateway's filer health tracking no-ops for an owner outside the static
-filer list, so during a sustained owner outage every route-by-key read
re-dials the dead owner before failing over. Flag an owner whose owner-first
read hit a transport error and skip it (read local-first) for a short TTL, so
reads pay one dead dial per TTL instead of one per request; the flag expires so
owner-first reads resume once the owner or the ring recovers.
* s3: always try the preferred owner first, health-order only the rest
The healthy/unhealthy partition also demoted a health-flagged preferred owner
behind healthy replicas, so a replica's authoritative NotFound could mask a
write that had only reached the owner — the read-after-write race this routing
exists to close. Pull preferred out of the partition and keep it first; the
recently-unreachable gate already steers reads away from a genuinely dead owner.
* fix(iam): return a valid user ARN from CreateUser and GetUser
The terraform aws provider 6.41 reads a user back after creating it and
blocks until GetUser returns a value that passes arn.IsARN. We only set
UserName, so the ARN was empty and apply hung until the 2m timeout.
Populate Arn (and Path) via a shared iam.NewUser helper in both the
embedded and standalone IAM handlers.
* fix(iam): use the userName parameter directly in NewUser
Drop the redundant local copy; the value parameter is already function-local.
* fix(iam): return full user objects with ARNs from GetGroup
GetGroup listed members with only UserName set. Build them via the shared
NewUser helper so group members carry a valid Arn and Path like the other
user responses, in both the embedded and standalone IAM handlers.
ReadFromBuffer and HasData() take the read lock separately, so a write
that lands between them can make a subscriber which just read a
momentarily empty buffer return ResumeFromDiskError even though the data
is now servable from memory. Re-read under a fresh lock and only bail
when the position is genuinely behind the in-memory window (flushed to
disk); otherwise loop back and read it.
* fix(iam): implement CreatePolicyVersion for managed policies
The AWS Terraform provider updates a managed policy in place via
CreatePolicyVersion, which returned 501 NotImplemented and broke
terraform apply on any policy change.
Implement CreatePolicyVersion (plus ListPolicyVersions, GetPolicyVersion
and DeletePolicyVersion) on both the standalone IAM server and the
embedded S3 IAM API. Managed policies keep a single current document, so
each is modeled as one default version "v1": CreatePolicyVersion replaces
the document, List/GetPolicyVersion expose it, and DeletePolicyVersion
rejects deleting the default. GetPolicy now reports DefaultVersionId so
the provider's read can fetch the document. The standalone path also
refreshes the cached Identity.Actions of every identity the policy is
attached to so the new document takes effect.
* fix(iam): reject CreatePolicyVersion unless SetAsDefault=true
With a single always-default managed-policy version, a request with
SetAsDefault=false (or omitted) would stage a non-default version on AWS
but here silently replaced the active document. Reject it on both the
standalone and embedded paths.
Isolate the new policy-version tests from the shared package fixtures so
they stay order-independent, and assert IsDefaultVersion on the response.
The username-isolation policy denied s3:ListBucket through an object-path
NotResource. ListBucket is bucket-level, so its resource ARN is the bucket
and never matches an object path: the Deny always fired and a user could
not list their own prefix. Scope the per-user List deny with a StringNotLike
s3:prefix condition instead, the same mechanism the matching Allow uses.
* fix(s3api): keep ListBucket resource ARN at bucket level
ListObjects with ?prefix= was denied for IAM users granted s3:ListBucket
on the bucket ARN. authRequestWithAuthType promotes the prefix into object
so the legacy CanDo path can honor prefix-scoped Action strings, and that
promoted object leaked into the policy resource ARN, producing
arn:aws:s3:::bucket/<prefix> which never matches a bucket-level statement.
Keep the resource bucket-level for List in the bucket-policy and
IAM-attached-policy evaluators; prefix scoping stays in the s3:prefix
Condition. The CanDo path is untouched.
* fix(s3api): resolve List action at bucket level when prefix is promoted
The IAM evaluator built a bucket-level resource ARN but still passed the
prefix-promoted object to ResolveS3Action, so listing with a prefix made
hasObject true and misresolved ListBucketVersions/ListBucketMultipartUploads
to ListBucket. Resolve the action against the same zeroed object, and trim
the resource-ARN comments.
Bulk DeleteObjects carries the keys in the request body, so the route Auth
middleware ran a single bucket-level check with object="", building the
resource ARN as arn:aws:s3:::<bucket>. That never matches an s3:DeleteObject
policy scoped to <bucket>/*, so the entire batch was denied even though the
single-key DELETE worked with the same credentials.
Defer authorization to the handler and check each key via AuthorizeBatchDeleteKey,
mirroring AuthorizeCopySource: a synthetic DELETE /<bucket>/<key> request resolves
s3:DeleteObject (or s3:DeleteObjectVersion when a versionId is given) against the
object ARN. Denied keys come back as per-key errors while authorized keys still
delete, matching AWS semantics.
* fix(mount): don't strand a directory cached-but-empty when an off-loop wipe races a rebuild
Idle eviction, kernel Forget, and the copy-range fallback cleared a
directory's cached entries directly, off the metaCache apply loop, after
resetting the cached flag in inodeToPath as a separate step. A concurrent
rebuild could publish a fresh listing (markCachedFn) in between, so the late
DeleteFolderChildren left the directory flagged cached over an empty store.
lookupEntry then returns an authoritative ENOENT and ReadDir returns nothing,
so every file in the directory disappears from the mount although it is still
present on the filer.
Route those wipes through a new apply-loop step that resets the flag and wipes
the store together, serialized with a build's markCachedFn, and skips a
directory while it is building.
* fix(mount): route the meta-event retry cleanup through the apply-loop purge
The subscription-retry callback wiped the mount root's cached children
directly off the apply loop and reset the cache flags as a separate step — the
same pattern that can leave a concurrently-rebuilding root cached-but-empty.
Invalidate all flags (safe on its own, it never deletes entries) then purge the
root's children through the apply loop.
* [CheckDisk][GRPC]: implement MVP for disk health detection, added timeout for new grpc connections
* fix(volume): build disk health check on every platform
setDiskStatus only existed behind the statfs build tag, so disk.go failed
to compile on windows, openbsd, solaris, netbsd and plan9. Move the timeout
wrapper and failure tracking into the shared disk.go and have each platform's
fillInDiskStatus return an error, so every platform gets the same protection
from a stuck filesystem.
Also restore the uint64(fs.Bavail) cast: Bavail is int64 on freebsd, so the
unguarded multiply broke the freebsd build.
* fix(volume): keep one outstanding statfs probe per disk
A stuck statfs used to leave isChecking cleared by the timeout path, so the
next check spawned another goroutine while the previous one was still blocked
in the syscall, leaking one goroutine per minute on a hung disk. Clear the
flag only when statfs returns and treat an overlapping check as a failure, so
a hung filesystem keeps a single outstanding probe and still gets reported.
* fix(volume): assume disk available until the first health check
isDiskAvailable defaulted to false, and CollectHeartbeat skips locations that
are not available. A freshly started volume server would therefore omit every
volume from its first heartbeats until the async CheckDiskSpace ran, so the
master could briefly treat all of them as missing.
* fix(volume): label the disk error metric by data directory
The new gauge tagged the series with IdxDirectory while every neighbouring
resource gauge uses Directory, so the error series would not line up with them
in dashboards. Also log the underlying error instead of a generic message.
* test(volume): cover disk health success and repeated-failure paths
* fix(volume): make a healthy disk the zero-value default
Track the disk as isDiskUnavailable instead of isDiskAvailable so the safe
state is the zero value, matching isDiskSpaceLow. CollectHeartbeat only skips a
location once a check has actively marked it unavailable, so any DiskLocation
built without running CheckDiskSpace (tests, future call sites) still reports
its volumes instead of silently dropping them.
* feat(disk): detect degraded disks using IO latency probes
* feat(stats): introduce configurable disk I/O health probe with EWMA-based latency detection
* feat(disk): replace EWMA with sliding window algorithm for disk health detection and added user-friendly options
* feat(disk): improve disk health probing and recovery
* feat(volume): configure disk health checks via volume.toml
* fix(volume): Remove disk IO probe CLI options
---------
Co-authored-by: ptukha <ptukha@tochka.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
Explain:
- problem: Delete and DeleteProxied could panic on malformed URLs when a JWT was provided.
- root cause: maybeAddAuth was called before checking the error returned by http.NewRequest, so req could be nil.
- fix: return the request construction error before adding the Authorization header.
- validation: go test ./weed/util/http -run 'TestDelete(ReturnsInvalidRequestErrorBeforeAddingAuth|ProxiedReturnsInvalidRequestErrorBeforeAddingAuth)' -count=1; git diff --check
* fix(seaweed-volume): bound request body and stored-content expansion to prevent OOM
The Rust volume server buffered the entire upload body with
to_bytes(usize::MAX) and only checked the file-size limit afterward, so a
single large upload — or many concurrent uploads, since the in-flight byte
throttle defaults to 0 (unlimited) — could exhaust memory and get the process
OOM-killed under load. The read path had two more single-request OOM vectors:
`vec![0u8; manifest.size]` allocated from an attacker-controlled chunk-manifest
size, and gzip decompression was unbounded (gzip bomb).
- Bound the upload body read by file_size_limit_bytes (plus a margin for
multipart framing), mirroring Go's io.LimitReader(sizeLimit+1), and reject
oversize before the whole body is buffered.
- Validate manifest.size (reject negative / oversized) before allocating.
- Cap gzip output in maybe_decompress_gzip and route the inline GzDecoder sites
through it.
* fix(seaweed-volume): address review - chunk offset, 32-bit cast, decompress errors
- Validate chunk.offset before indexing in chunk-manifest expansion: a negative
offset wrapped to a huge usize and underflowed `end - offset` (panic from a
crafted manifest). Reject negative, skip out-of-range, use saturating math.
- Use usize::try_from for the upload body limit instead of `as usize`, so a
>usize::MAX file_size_limit on 32-bit caps at usize::MAX rather than silently
truncating to a tiny value.
- maybe_decompress_gzip now returns Result<_, GunzipError> distinguishing a
decode failure (callers fall back to raw bytes, as before) from hitting the
size cap (TooLarge), which now returns 413 instead of silently serving the
still-compressed bytes.
* fix(seaweed-volume): inflate manifest chunks into the result window to cap peak memory
The chunk-manifest expansion still doubled memory: `result` was already allocated
at manifest.size (<=2 GiB) and each compressed chunk was inflated into a separate
Vec (also up to 2 GiB), so a single request could peak near 4 GiB.
Decompress compressed chunks directly into their result[offset..] window (bounded
by the remaining space) so a chunk never allocates a second large buffer; peak
stays at ~manifest.size. Bytes past the window are dropped (matching the prior
truncation), and a fully-undecodable chunk still falls back to its raw bytes.
* fix(seaweed-volume): fall back to raw chunk bytes on any decode failure
Per review: the gzip fallback must run on any decode error, not only when no
bytes were decoded. Clear the partially-written output and copy the chunk's raw
bytes (truncated to the window), restoring the prior decode-failure behavior.
* docker: cross-compile the Go binary instead of emulating it under QEMU
The builder stage ran as the target platform, so arm64/arm/386 images
emulated the whole Go compile (and the full git clone) under QEMU. The
binary is CGO-free, so pin the builder to $BUILDPLATFORM and cross-compile
with GOOS/GOARCH (GOARM for v7), keeping every target's compile native.
* ci: build all release container variants in parallel
The build matrix throttled to two variants at a time on a stale rate-limit
worry. Pulls go through mirror.gcr.io and pushes target GHCR only, so the
five variants can all build at once.
* ci: copy each variant to Docker Hub from its build job
The separate copy-to-dockerhub job waited on the whole build matrix before
any GHCR -> Docker Hub copy could start. Move the crane copy into the build
job so each variant copies as soon as it is built, overlapping with the
others still compiling. tag-latest and helm-release now depend on build.
GetObject on a versioned object returned NoSuchKey forever when the
.versions directory existed but carried no latest-version pointer (empty
Extended metadata) while real version files remained inside it. The
self-heal path only fired for a dangling pointer (present but referencing
a missing file), not an absent one, so doGetLatestObjectVersion fell
straight through and errored on every read.
- doGetLatestObjectVersion now calls recoverLatestVersionWithoutPointer
when the pointer is missing or empty. An absent pointer is the legitimate
signal that a pre-versioning or suspended-versioning "null" object is
current, so that object wins; only when it is absent do we rescan
.versions/ and rebuild the pointer from the version files present.
Transient rescan failures propagate instead of being masked as NotFound.
- selectLatestVersion derives the version id from the v_<versionId> file
name when the Seaweed-X-Amz-Version-Id attribute is absent, so version
files written outside the normal versioned-PUT path (replicated or
restored entries) are still promotable. The orphan diagnostic uses the
same detection so an entry can't be both promoted and counted an orphan.
* fix(ec): pass per-volume data-shard count to the parity-shard split
ShardsInfo.DeleteParityShards/MinusParityShards looped ids 10..13, assuming
the fixed 10+4 layout. For a non-default ratio this splits data vs parity
wrong — a wide ratio (12+4, 16+6) drops real data ids >= 10, which breaks
ec.decode. They now take a dataShards argument (<= 0 falls back to
DataShardsCount) and clear ids dataShards..MaxShardCount. ec.decode threads
the data-shard count from collectEcNodeShardsInfo to both split call sites,
and admin LogicalSize passes DataShardsCount.
Also: EC cleanup now sets an explicit per-disk storage impact
(-len(ShardIds)) instead of falling back to the TotalShardsCount constant,
so freed-capacity accounting matches the shards actually removed.
OSS is always 10+4, so behavior is unchanged here; this keeps the split
ratio-correct and the API aligned with the enterprise per-volume override.
Adds parity-split ratio tests.
* ec: clear parity shards in one locked pass
Address review: DeleteParityShards looped si.Delete, taking the lock once per
id. shards is sorted by Id and shardBits is a bitmap, so mask off the high
bits and truncate the sorted slice at the first parity id (binary search) under
a single lock. Preserves the dataShards<=0 -> DataShardsCount default.
* fix(ec): resolve EC data-shard count from the volume's .vif on reboot
A volume server never loads a cluster EC config into memory, so startup
decisions that assumed 10 data shards mishandled volumes whose .vif
records a different ratio:
- validateEcVolume sized the expected shard against 10 data shards and
required >=10 local shards, so a volume with a non-default ratio and a
coexisting .dat could be wiped on reboot. Read the ratio from the .vif.
- pruneIncompleteEcWithSiblingDat used the hardcoded 10-shard threshold,
so a full data set for a non-default ratio with a healthy sibling .dat
was wiped as a partial leftover. Use the EcVolume's .vif-derived ratio.
Behavior is unchanged for the standard 10+4 layout (the .vif resolves to
10). Adds storage-level reboot tests.
* ec: avoid per-call allocations in ecDataShardsFromVif
Address review: the helper runs once per EC volume at startup. Replace the
slice+map dedup of the two dirs with direct conditional checks via a small
ecDataShardsFromVifDir helper, eliminating the heap allocations and GC
pressure when loading many volumes.
A Canceled/DeadlineExceeded from the caller's per-request context was
treated like a dead channel: it closed the shared cached ClientConn and
cancelled every other in-flight RPC on it with "the client connection is
closing". Under a burst of concurrent chunk assigns (e.g. a large S3
multipart upload) one slow assign hitting its 10s attempt timeout could
poison the connection for all the rest, cascading into a flood of 500s.
Thread the caller's context into shouldInvalidateConnection and only
invalidate on Canceled/DeadlineExceeded while that context is still live,
which isolates the genuine stale-channel signal (a peer restart behind a
k8s Service VIP). To carry the context, add a ctx parameter to the
existing WithGrpcClient, WithMasterClient, and WithMasterServerClient; the
master assign and volume-lookup paths pass their per-attempt context and
every other caller passes context.Background().
makeupDiff replays post-snapshot changes onto the compacted volume. For a
replayed deletion it appended a tombstone to the new .dat but recorded the
.idx entry with offset 0. When that deletion is the last replayed change the
tombstone lands at the .dat tail, and the post-commit integrity check skips
offset-0 entries, so it sees 32 trailing bytes it can't account for and flips
the volume read-only, reloading it as a SortedFileNeedleMap instead of the
writable map.
Record the tombstone's real .dat offset, matching the normal delete path; the
needle map still treats it as deleted off the negative size, so lookups are
unchanged. Mirror the same fix into the Rust volume server.
* s3: auto-enforce bucket quota read-only both ways
Quota read-only only ever flipped when an admin re-ran
s3.bucket.quota.enforce, so a bucket that went over quota stayed
read-only forever even after usage dropped back under.
Fold enforcement into the per-minute, leader-locked bucket-size loop
the s3 gateway already runs for metrics: it now flips each bucket's
read-only flag to match its quota in both directions, rewriting
filer.conf only when a flag actually changes. The set/clear decision
lives in one shared FilerConf.ApplyBucketQuotaReadOnly helper so the
shell command and the gateway can't drift.
* only manage read-only when a quota is set, never clobber manual locks
* trim comments
When the destination's stored mtime is newer than the incoming source version, UpdateEntry skips the update (last-writer-wins). A copy left truncated by an earlier failed replication trips this: the source kept the file's original mtime while the partial copy was written recently, so it looks "newer" and is never corrected. When the destination is strictly shorter than the source, re-replicate the full source content and replace the chunk list instead of skipping. Same shorter-than-source bypass for CreateEntry.
* s3api: use getEtagFromEntry for multipart part ETag to prefer Extended metadata
* s3api: add tests for getEtagFromEntry Extended ETag preference in multipart upload
* s3api: avoid double-quoting ETags in ListParts output
* s3api: add docstring for filer_multipart_etag_test.go
* fix(http): handle invalid gzip stream errors
Explain:
- problem: ReadUrlAsStream could panic when a response claimed gzip encoding but the body was not a valid gzip stream.
- root cause: the gzip reader error was ignored and a nil reader was deferred and read from.
- fix: return the gzip.NewReader error before registering Close or reading.
- validation: go test ./weed/util/http -run TestReadUrlAsStreamReturnsGzipReaderError -count=1; git diff --check.
* test: avoid closing shared global HTTP client in unit test
* filer: name the read-only path in the write rejection
The write path rejected creates under a read-only rule with a bare
"read only", giving no hint which path was locked or why. Wrap the
error with the matched location prefix and a quota hint so a FUSE
mkdir or S3 put points straight at the offending bucket.
* return the read-only reason over HTTP and drop any query string from the fallback prefix
A destination volume server that hits its idle deadline while reading a large upload body under load returns 400 "read tcp ...: i/o timeout". fetchAndWrite retried that on the flat ~1s RetryUntil backoff, hammering the already-overloaded destination. Route i/o timeout, connection reset, broken pipe and net.Error timeouts through the same escalating 10s-2min backoff already used for EOF so it can recover.
shell: only skip system-log subtree in fs.meta.save, not fsck/verify
The SystemLogDir skip lived in the shared BFS traversal, so volume.fsck
built its in-use set without the /topic/.system/log chunks and flagged
every referenced log needle as orphan. -reallyDeleteFromVolume would then
delete live log data and leave dangling filer entries. Gate the skip
behind a flag that only fs.meta.save sets.
* fix(ec_bitrot): cap sidecar block_size in ValidateBitrotManifest
A sidecar loaded from disk (or supplied via a backfill/peer RPC) could carry a
huge power-of-two block_size that passed validation, then force a multi-GiB
scratch-buffer allocation in scrub/verify. Add a shared MaxBitrotBlockSize
(64 MiB) constant, enforce it as an upper bound in isPow2MultipleOf1MiB, and
derive the volume flag cap from the same constant so they cannot drift.
* fix(ec_bitrot): don't destroy a valid destination sidecar on an optional copy
writeToFile opened the destination with O_TRUNC before knowing whether the
source had the file, so an optional copy (ignoreSourceFileNotFound) from a source
that lacks the .ecsum truncated and then removed a valid pre-existing destination
sidecar. Stage the optional copy into a temp sibling and commit it with an atomic
rename only when the source actually delivered the file; a missing source is now
a no-op. Mandatory copies keep their in-place behavior.
Chris LuandGitHub
Konstantin LebedevandGitHub
Lars LehtonenandGitHub
ahalaunandGitHub
steve.weiandGitHub
Nguyễn Lộc PhúcandGitHub