SILO Security Chronicle

• 1: CVE-2026-32285: The jsonparser Advisory That Required No Patch
• 2: CVE-2026-33322: OIDC JWT Algorithm Confusion
• 3: CVE-2026-33419: LDAP STS Enumeration and the Throttling Chain
• 4: CVE-2026-34204: Replication Metadata Injection
• 5: CVE-2026-39414: Oversized S3 Select Records and a SIMD Bypass
• 6: CVE-2026-40344: Snowball Auto-Extract Authentication Bypass
• 7: CVE-2026-41145: Unsigned-Trailer Query Authentication Bypass
• 8: CVE-2026-42600: ReadMultiple Storage-REST Path Traversal
• 9: Internode Path Containment Audit: Paying Off What CVE-2026-42600 Left Owing
• 10: The Parser Knew, the Schema Didn't: Config Keys That Could Take Every Notification Down
• 11: Object Grant, Bucket Reach: When 'bucket/*' Could Rewrite the Bucket Itself
• 12: Absent Is Not Empty: A Blank versionid and the Fail-Open It Invites
• 13: Three Headers, One Lie: Making the Client Source Address Mean Something
• 14: Sorted Is Not Increasing: How One Duplicate Part Number Doubled an Object

1 - CVE-2026-32285: The jsonparser Advisory That Required No Patch

Status: Closed without a code change
GitHub issue: pgsty/minio#26

Security maintenance is not always a sequence of “find a vulnerability, then ship a patch.” The initial assessment of CVE-2026-32285 was that the repository might still carry a vulnerable jsonparser; replacing the dependency or maintaining another fork was even considered. Checking the resolved module version and actual reachability changed the conclusion: the tree already used v1.1.2, which contained the fix, and govulncheck found no reachable vulnerable symbol.

The right final action was not to manufacture an upgrade. It was to record the evidence and close the issue.

What was wrong with the initial assumption

The issue was first understood as “this dependency has no fixed version.” Acting on that premise without verification could have produced several changes that looked proactive but made the project worse:

• changing the dependency graph for no security benefit;
• introducing compatibility regressions in the name of a nonexistent fix;
• adding another fork that would need long-term maintenance;
• implying that previous SILO releases were demonstrably exposed when that had not been established.

Security work cannot be measured by whether it produces a diff. Leaving correct code unchanged is itself a security decision, and it needs evidence.

Investigation

The investigation narrowed the question through four layers of evidence:

1. Confirm the version actually selected in the current go.mod and go.sum graph.
2. Check the upstream release and establish that v1.1.2 already contained the relevant fix.
3. Run and inspect govulncheck; it reported no reachable vulnerable symbol.
4. Attribute the discrepancy in the issue to stale advisory or vulnerability-database information, not to a vulnerability still present in the source tree.

Four claims must remain separate: a version was once listed as affected, a package is imported, a vulnerable symbol is reachable in the program, and remote input can actually exploit that path. None of them proves the others.

Why there was no “just in case” upgrade

If the selected version already includes the fix, bumping to an arbitrary newer version does not make the system safer. It only expands the change surface and makes later regressions harder to attribute. That is especially risky in a large Go module graph.

The final decision was therefore to:

• avoid committing a fictitious fix;
• preserve the version and reachability evidence in the issue;
• keep version gates and govulncheck in place to detect a future dependency rollback;
• treat “no change required now” as a dated conclusion, not a permanent exemption.

Verification boundary

This incident established that the checkout examined on 2026-04-15 did not require a code change for CVE-2026-32285. It does not establish that every future branch, module graph, or release will remain unaffected. A dependency downgrade or a change in module selection requires the version and reachability checks to be repeated.

This article records the investigation and the basis for closure. The original govulncheck was not rerun while preparing this chronicle.

The principle this incident left behind

The objective of security maintenance is an accurate risk conclusion, not a patch for every issue. For a dependency CVE, ask in order: which version is actually selected, whether the vulnerable code enters the program, whether the symbol is reachable, and whether a deployed entry point makes it exploitable. Only when those answers require a source change should the investigation produce a diff.

2 - CVE-2026-33322: OIDC JWT Algorithm Confusion

Status: Released
First containing release: RELEASE.2026-04-17T00-00-00Z
Affected entry points: AssumeRoleWithWebIdentity, AssumeRoleWithClientGrants
GitHub issue: pgsty/minio#22

The old implementation placed the OIDC client secret in the JWT verifier keyring while also accepting HMAC signing methods. An attacker who knew that client secret could therefore mint an HS-signed token and exchange it through STS for temporary credentials. The final fix restored asymmetric, JWKS-only verification. It deliberately broke HS256/384/512 compatibility instead of keeping an option that would reintroduce ambiguous trust semantics.

The vulnerability was not the absence of signature verification

At first glance, the old code did verify JWT signatures. The boundary that failed was more specific: which kind of key the verifier would accept, and whether the token header could select an algorithm with semantics that did not match that key’s intended role.

The attack chain required several conditions:

• the attacker obtained the OIDC client secret;
• the attacker constructed an HMAC-signed ID token;
• the verifier treated the client secret as an HMAC signing key;
• the token reached the WebIdentity or ClientGrants STS flow and was exchanged for temporary credentials.

Disclosure of a client secret is already serious, but it should not automatically confer the power to issue arbitrary user ID tokens. Combining those capabilities in one keyring created the algorithm-confusion vulnerability.

A compatibility path was implemented, then deliberately removed

During the fix, an allow_hmac-style compatibility path was implemented. It appeared reasonable: keep the secure default while letting users with a real requirement opt in. But retaining a shared secret in the general verifier keyring meant administrators would need to understand that the option expanded the entire STS trust boundary. Any future drift in the method allowlist could reopen the flaw.

The trade-off became clear:

The most important decision was not what code was added, but that a completed compatibility implementation was removed.

Final invariants

The fix was concentrated in the OIDC JWT verification path and established four rules:

• verifier keys come only from the identity provider’s JWKS;
• the OIDC client secret never enters the JWT verification keyring;
• HS256, HS384, and HS512 are always rejected;
• the ordinary RS256 flow and JWKS refresh/retry use the same method allowlist.

The CVE was not used as a pretext for expanding JOSE support. PS256 and EdDSA remained out of scope.

Verification and release

The development record includes HS256 rejection, RS256 acceptance, JWKS refresh/retry regression tests, and focused go test ./internal/config/identity/openid. Temporary compatibility helpers, configuration, and tests were all removed from the final diff.

The public fix is f1f2239, released with SILO 2026-04-17. This article records the historical verification; those tests were not rerun while preparing the chronicle.

Compatibility cost

This is an explicit breaking change. Identity providers that still issue HS256/384/512 tokens must migrate to JWKS-backed RSA or ECDSA before upgrading SILO. The project chose a narrower trust model that is easier to explain and audit over preserving a configuration that happened to work before.

3 - CVE-2026-33419: LDAP STS Enumeration and the Throttling Chain

Status: Released, followed by two rounds of corrections
First containing release: RELEASE.2026-04-17T00-00-00Z
Complete correction: RELEASE.2026-06-18T00-00-00Z
GitHub issue: pgsty/minio#23

The core vulnerability was straightforward: LDAP STS returned different results for “user does not exist” and “password is wrong,” creating a username oracle. The first fix unified the external authentication failure and added limits by source IP and username. Continued review then showed that success refunds, spoofable source headers, reservation accounting, and the shared username bucket could turn the security control itself into a new attack surface.

The final June design removed the username bucket that enabled precise account lockout, retained only the source-IP bucket, and made proxy attribution an explicit deployment contract.

Initial threat model

The entry point is AssumeRoleWithLDAPIdentity. An attacker needs no existing MinIO account. Access to the LDAP STS endpoint is enough to compare the code, status, or message returned for an unknown user and a wrong password, enumerate valid usernames, and combine that knowledge with password spraying, guesses about organizational naming, or social engineering.

The fix could not simply disguise every error as “wrong password.” LDAP connection, lookup-bind, and directory-service failures still needed to appear as infrastructure errors, or operators would lose the ability to diagnose the service.

First round: uniform responses and a limiter

The initial fix on 2026-04-15 did three things:

• unknown user and bad password returned the same external STS authentication error;
• LDAP infrastructure failures still returned 500, with the real cause retained in the server log;
• a new in-memory limiter initially created buckets for both source IP and normalized username.

This closed the content side channel and raised the cost of brute-force attempts, but the limiter state machine and source attribution exposed more problems.

Second round: success, attribution, and accounting

The follow-up changes on April 16 addressed three classes of defects:

1. Successful authentication must not consume the failure allowance; the reserve/commit/cancel/refund lifecycle had to be explicit.
2. The socket peer must be used by default; X-Forwarded-For, X-Real-IP, and Forwarded cannot be trusted merely because a request supplies them.
3. Refund and capacity need hard bounds so cancel logic cannot mint tokens.

A proxy must be placed on an explicit trusted allowlist before it can influence source attribution.

Third round: remove the username bucket

Adversarial review in June overturned the intuition that “source plus username must be stronger than source alone.” A shared username bucket could be exhausted continuously from arbitrary origins. With only a low request rate, an attacker could lock a targeted account before the legitimate user ever reached an LDAP bind.

The final fix therefore:

• removed the per-username bucket;
• peeled trusted hops from XFF right to left and selected the first untrusted address;
• rejected trusted-proxy footguns such as 0.0.0.0/0 and ::/0;
• stopped using Forwarded for security-sensitive bucketing;
• allowed X-Real-IP only under a contract in which the proxy overwrites rather than forwards client input.

This turn in the review shows that a security control needs its own threat model. More dimensions of throttling do not automatically mean more security.

Rejected alternatives

Verification and release

The historical record covers limiter reserve/commit/cancel/refund behavior, concurrency, success and infrastructure failures, external equivalence of unknown-user and bad-password responses, and RemoteAddr, spoofed-header, trusted-proxy, multi-hop, and catch-all-CIDR cases. Focused package tests and builds were recorded as passing.

The LDAP security end-to-end test skips when _MINIO_LDAP_TEST_SERVER is absent, so an outer ok cannot be presented as proof of the full LDAP scenario.

The first public fix was 6619d0c. Follow-up corrections include c55b52c, 817a457, 084a154, and 5e40665.

Final cost and residual risk

• The limiter now keys only on source IP and gives up a hard per-account throttle across different origins.
• It is per-node and in-memory, not a cluster-wide password defense.
• Botnets, distributed origins, IPv6 address rotation, and LDAP bind timing remain.
• Incorrect trusted-proxy configuration can still destroy source attribution.
• A Forwarded-only deployment falls back to the peer bucket and loses granularity.

Rate limiting can reduce attempts from one source. The uniform external authentication response is what actually conceals whether a username exists.

4 - CVE-2026-34204: Replication Metadata Injection

Status: Released
First containing release: RELEASE.2026-04-17T00-00-00Z
GitHub issue: pgsty/minio#24

Ordinary PUT and COPY requests could smuggle X-Minio-Replication-* headers into internal X-Minio-Internal-* SSE metadata, creating objects whose replication state did not match the authorized path and could even make them unreadable. The final fix stopped accepting replication-only metadata by default, restored it only in a trusted flow authorized for ReplicateObjectAction, and sanitized CopyObject before any header consumer ran.

Threat model

An attacker needed only ordinary object-write permission, not internode credentials. The input came entirely from client-controlled X-Minio-Replication-* headers, but metadata extraction converted it into internal replication or SSE state.

Later read paths interpreted the object according to that false internal state. The result could be an unreadable object: an integrity and availability failure. Almost every production server accepting untrusted writes needed to be treated as affected.

The root problem was not the header name. It was that data from an untrusted source acquired internal semantics without passing replication authorization.

Reject the whole request, or sanitize precisely?

Rejecting an ordinary request whenever it contains a replication header is the most obvious fix. It would also turn a header clients were previously allowed to send and have ignored into a hard failure. The final design was more precise:

• the default extraction path does not accept replication-only metadata;
• ordinary PUT and COPY strip those fields first;
• only a path authorized for ReplicateObjectAction restores them;
• replica-status writes use the same trusted condition;
• legitimate multipart and Snowball replication flows explicitly restore the SSE metadata they require.

That keeps the compatibility change inside internal semantics instead of expanding it to every client carrying an extraneous header.

Why CopyObject had to sanitize early

CopyObject headers are not used only for the final metadata map. They can be consumed earlier by precondition logic and SSE-C source handling. Removing them immediately before the object write is too late: earlier consumers have already been contaminated.

The final sanitization occurs before those consumers. “Untrusted replication headers never enter internal semantics” becomes one invariant instead of a convention every downstream function must remember to enforce.

Implementation and verification

The change covered handler utilities, object handlers, and multipart handlers, with tests at several layers:

• trusted and untrusted metadata extraction at the helper layer;
• malicious PUT and COPY cases at the handler layer;
• CopyObject header sanitization;
• red/green comparison between the vulnerable parent and the patched tree;
• live-server before/after behavior showing that a malicious header no longer made an object unreadable;
• continued operation of legitimate replication, multipart, and Snowball flows.

The public fix is fcb8f24. This article preserves the historical verification boundary; no live server was started again while preparing the chronicle.

Cost and residual risk

• Internal replication headers supplied by ordinary clients are now ignored or stripped.
• Replication-only metadata must be restored explicitly inside an authorized branch.
• If a future replication entry point forgets to restore it, the result should be a functional regression rather than another untrusted write path.
• The audit focused on replication headers; it does not establish that every X-Minio-Internal-* field has undergone the same trust review.

This incident leaves a simple review question: a field that looks “internal” is not necessarily trusted. Ask where it came from and which authorization decision allowed it to acquire internal meaning.

5 - CVE-2026-39414: Oversized S3 Select Records and a SIMD Bypass

Status: Released; the second-round fix was completed in June
Initial fix release: RELEASE.2026-04-17T00-00-00Z
Complete fix release: RELEASE.2026-06-18T00-00-00Z
GitHub issue: pgsty/minio#25

The first fix in April reused the existing 1 MiB maxCharsPerRecord limit for both CSV and ordinary JSON Lines. This prevented unbounded buffering while waiting for a delimiter and returned the explicit OverMaxRecordSize error to clients. A June review then found that CPUs with SIMD support took a different simdjson fast path that bypassed the limit completely.

The final solution sent JSON Lines through the bounded reader on every CPU. The same review also corrected error mapping, parser errors, and the flush of completed records before a terminal error. SILO temporarily gave up the SIMD fast path in exchange for consistent security semantics.

Threat model

An attacker can submit or query an object containing an extremely long single record. The reader continues buffering until it sees a record delimiter, allowing memory and CPU denial of service. More subtly, the same input can select a different implementation according to the machine’s CPU features. Safe behavior on a test machine does not necessarily prove safe behavior in production.

Error semantics are part of the fix. If an oversized record appears only as a generic InternalError, clients and monitoring systems cannot distinguish an enforced security limit from a server failure.

First round: reuse the existing 1 MiB invariant

The first patch did not invent a new configuration knob. It applied the existing maxCharsPerRecord = 1 MiB rule:

• the CSV splitter and line-delimited JSON rejected oversized records before buffering or parsing them further;
• the earliest splitter error was preserved instead of being overwritten by a partial decode;
• the error propagated as OverMaxRecordSize rather than collapsing into InternalError.

This was a deliberate compatibility contraction. Clients with lines or records larger than 1 MiB now had to split their input.

Second round: a hardware-dependent bypass

Following the call chain again in June exposed this path:

JSON Lines -> simdj.NewReader -> simdjson.ParseNDStream

When simdjson.SupportedCPU() returned true, JSON Lines bypassed the bounded json.PReader. The third-party parser kept reading past a chunk boundary until it found a newline. A generic reader wrapper could not simultaneously preserve already completed records and guarantee a bound on the next record.

The final choice was not another wrapper. JSON Lines temporarily stopped using the SIMD path and always used the bounded PReader. If SIMD support returns, that implementation must enforce the same record limit itself and pass the same CPU-independent regression suite.

Stream semantics corrected in the same round

The review also fixed several adjacent behaviors:

• use errors.As to pass through errors implementing SelectError, not just one concrete type;
• have the JSON worker wrap parser failures as JSONParsingError;
• flush completed records still waiting below the batch threshold before emitting a terminal error event;
• preserve error priority in input order instead of letting a later oversized record overwrite an earlier parse error.

Those details determine whether a client sees the correct failure or a resource-limit fix that quietly broke the streaming protocol.

Deliberately left outside this CVE

• The historical mismatch between CSV AllowQuotedRecordDelimiter and the outer physical-newline splitter.
• Whether \r in CRLF counts toward the record length.
• Restoring SIMD performance without an equivalent bound.

These questions may be real, but they require independent AWS-compatibility evidence or a more complex quote-aware splitter. They did not belong in a security patch based on guesses.

Verification and release

The historical record includes oversized JSON Lines, error-code preservation, and behavior tests that do not depend on the local machine’s SIMD capabilities. go test ./internal/s3select/... -count=1 and git diff --check were recorded as passing.

The initial public fix was c5765dc; the complete June correction is fd69c89. Those tests were not rerun while preparing this article.

Final trade-offs

• JSON Lines performance may decrease; this incident did not produce a benchmark that quantifies it.
• The 1 MiB per-record limit rejects oversized input accepted by previous releases.
• Quoted, multiline CSV semantics still need separate work.
• Any future CPU-specific fast path must share the slow path’s security tests.

The second fix leaves the central lesson: a security invariant must hold across hardware-dependent paths. A green test on one CPU does not prove that another execution engine is protected.

6 - CVE-2026-40344: Snowball Auto-Extract Authentication Bypass

Status: Released
First containing release: RELEASE.2026-04-17T00-00-00Z
GitHub advisory: GHSA-9c4q-hq6p-c237

Snowball’s PutObjectExtractHandler omitted the streaming unsigned-trailer authentication case. A tar stream with a forged signature could enter untar() before authentication completed, and one request could fan out into many object writes. The final fix initialized the correct reader, handled the decoded length, and completed SigV4 verification before any tar byte reached the extractor.

Why the identifier changed

The official CVE had not been assigned when the fix was written, so the commit subject used the temporary identifier fake CVE-2026-40028. The final identifier is CVE-2026-40344. The historical commit was not rewritten; the advisory and this article use the official number.

From one missing authentication case to bulk object writes

The entry point was Snowball / PutObjectExtract auto-extraction. The request used unsigned-trailer streaming, an authentication type the old handler did not cover as ordinary PUT did.

The danger was larger than one incorrectly authorized request. Once the tar stream entered untar(), that request could create multiple attacker-chosen objects. An authentication omission was therefore amplified into a bulk-write problem.

Final invariant: the extractor sees zero bytes on failure

The key statement in the fix was:

If authentication ultimately fails, untar() must have seen zero bytes.

That rule excludes “extract first, then roll back if authentication fails.” Object writes travel through several paths, and proving a complete rollback is much harder than proving that input never crossed the boundary. Authentication had to close before data entered the extractor.

Implementation

The final change:

• recognized authTypeStreamingUnsignedTrailer;
• read X-Amz-Decoded-Content-Length;
• used newUnsignedV4ChunkedReader();
• performed complete SigV4 request verification before entering untar();
• preserved valid signed Snowball requests and CRC32 trailer flows.

Verification

The historical commit and investigation record cover:

• rejection of a forged-signature Snowball unsigned-trailer request;
• rejection of anonymous Snowball writes to a non-public bucket;
• successful extraction with a valid signature and trailing CRC32;
• red/green comparison between the vulnerable parent and the patched tree;
• containerized before/after smoke tests.

The public fix is b50ab58. The container tests were not rerun while preparing this article.

Compatibility and residual risk

• Clients that relied on an unsigned-trailer Snowball combination whose signature was never really verified will fail after upgrading.
• Authentication now closes before extraction, but tar paths, archive-size limits, and object-count limits remain separate security surfaces.
• Snowball and ordinary unsigned-trailer requests now share a reader; future changes must regress both paths together.

The essence of the fix was not another if. It moved the authentication decision in front of the actual amplification boundary.

7 - CVE-2026-41145: Unsigned-Trailer Query Authentication Bypass

Status: Released
First containing release: RELEASE.2026-04-17T00-00-00Z
GitHub advisory: GHSA-hv4r-mvr4-25vw

Query-string SigV4 credentials could enter a STREAMING-UNSIGNED-PAYLOAD-TRAILER data flow, while the old code verified the signature only when an Authorization header was present. A request carrying a valid access-key identifier could therefore complete a write without a correct signature.

The final fix moved presigned rejection and SigV4 verification into newUnsignedV4ChunkedReader(), making every caller consuming that stream share one authentication boundary.

Identifier note

The official CVE had not been assigned when the patch was written, so its commit subject used fake CVE-2026-40027. The final identifier is CVE-2026-41145. The historical commit remains unchanged; public material uses the official identifier.

Root cause: authentication was coupled to transport form

The affected entry points included PutObject and PutObjectPart. The request selected STREAMING-UNSIGNED-PAYLOAD-TRAILER, with its credentials and signature in the query string rather than the Authorization header.

The old handler used header presence to decide whether to verify a signature. The body reader still consumed the data normally, silently degrading query authentication into something close to an anonymous write. The attacker needed to know a valid access-key identifier, but did not need to produce a correct signature.

The problem was not failure to parse the query parameters. It was that authentication depended on how credentials were transported instead of the trust boundary at which the stream was consumed.

Why the patch did not live in each handler

Anonymous unsigned-trailer requests were not prohibited wholesale. If bucket policy explicitly permits anonymous writes, they can still follow the anonymous authorization path. The forbidden state is the mixture of query credentials with no verification of those credentials.

Implementation and verification

The fix performs presigned rejection and SigV4 verification at the reader entry in cmd/streaming-v4-unsigned.go, while removing the gates in the PutObject and multipart handlers that depended on header presence.

New tests cover forged query PUT, multipart, mixed authentication, and anonymous policy. The historical record also includes a vulnerable-parent write that succeeded, failure on the patched tree, and live-server before/after smoke tests showing that header-authenticated and valid anonymous flows continued to work.

The public fix is fa7c579. The live exploit was not rerun while preparing this article.

Compatibility and residual risk

• Presigned/query unsigned-trailer is now explicitly unsupported, an intentional breaking change.
• Moving the fix into the reader significantly reduces the chance that a sibling handler omits the check again.
• Other streaming authentication modes still need their own audits; this reader fix does not establish that every SigV4 streaming combination is safe.

The shape of this fix matters as much as its payload: when several handlers share an authenticated data stream, authentication belongs to the reader rather than to optional checks in each caller.

8 - CVE-2026-42600: ReadMultiple Storage-REST Path Traversal

Status: Released
First containing release: RELEASE.2026-06-18T00-00-00Z
GitHub advisory: GHSA-xh8f-g2qw-gcm7
Affected scope: Distributed erasure only; cluster-root / internode JWT required

The msgpack body of /rmpl carried Bucket, Prefix, and Files. The old code joined those values into filesystem paths without a containment check. The initial fix implemented full preflight validation. Continued call-chain review then found that this API had had no production caller since 2024. The final solution changed from “retain and harden” to removing the route, handler, client, interface, and generated code.

Deleting roughly a thousand lines was a larger source diff than a local validation guard, but it left a smaller long-term attack surface.

Threat model

The vulnerable route was registered only in distributed erasure mode; single-node deployments were unaffected. An attacker needed an internode JWT derived from the root secret, control of a node, or the ability to intercept unencrypted traffic between nodes.

The dangerous fields were inside the msgpack body, not the URL or form data, so upper HTTP path middleware never saw them. xlStorage.ReadMultiple joined and read the resulting paths directly, allowing them to escape the drive root.

This was not an anonymous S3 vulnerability. It crossed the boundary from “cluster root or peer” to “any file readable by the node process.”

First design: retain the API and validate it completely

The initial patch in xlStorage.ReadMultiple:

• rejected absolute paths, . and .. segments, backslashes, Windows drive prefixes, and NUL bytes;
• checked final containment across drive, volume, prefix, and file;
• preserved the historical contract for an empty Bucket and .minio.sys/multipart where possible;
• returned an error before any read or streaming began.

That design could close the known traversal, but review quickly exposed an early-return gap.

MaxResults exposed the danger of validate-as-you-use

The first patch validated each Files item inside the read loop. For Files=[good, bad] with MaxResults=1, the function returned after reading the first item and never validated the second.

It did not read the second malicious file, but it violated the intended invariant that the entire msgpack request must be valid before execution. Validation was therefore moved into a complete preflight pass, with path-length checks also completed before streaming.

The general rule is useful beyond this endpoint: when a request can return early or stream a partial response, checking an element immediately before use is not equivalent to validating the whole request.

Final decision: delete the API

Further call-chain audit established that:

• upstream removed the last production caller in September 2024;
• multipart had moved to ReadParts;
• the current tree had no in-tree production consumer;
• upstream’s final remediation also deleted ReadMultiple.

The final change removed the route, handler, client wrapper, StorageAPI / xlStorage method, metric, datatype, and generated code. storageRESTVersion retained the existing compatibility strategy.

Verification and release

The in-place validation phase ran focused tests for xlStorage, the storage-REST client, msgpack encode/decode, and path edge cases; adversarial review exposed the MaxResults flaw. The deletion phase checked the route, client, interface, generated surfaces, and absence of callers.

The public fix is 73ac524, released with SILO 2026-06-18. The post-deletion full suite was not rerun while preparing this article.

Compatibility and claim boundary

• The external S3 API is unchanged.
• Third-party implementations that privately called the internal /rmpl endpoint will stop working.
• A mixed-version rolling upgrade may encounter a protocol mismatch, so cluster nodes should be kept on the same version during the upgrade.
• Removing this endpoint proves only that ReadMultiple no longer exists; it does not establish that every internal node request carrying body paths has completed a containment audit.

This CVE reached the right final fix, but it also leaves an important distinction: closing one endpoint is not the same claim as closing an entire vulnerability class.

9 - Internode Path Containment Audit: Paying Off What CVE-2026-42600 Left Owing

Status: Fixed on the local pgsty/minio branch, unreleased and not disclosed (no CVE/GHSA requested; the upstream repository is archived) Affected scope: Distributed erasure only; cluster-root / internode JWT required Prerequisite reading: CVE-2026-42600 · ReadMultiple

This article contains complete exploitation vectors and measurements. Publishing it constitutes disclosure. Hold it until the fixed release ships.

The previous entry closed with this sentence:

Deleting the endpoint proves only that ReadMultiple no longer exists. It cannot be extrapolated into a completed containment audit of every internode body path.

That was an IOU, written down in plain sight. This is the record of paying it — and a not-very-flattering construction log.

Conclusions first

• Twelve defects, all inherited from upstream. Verified by per-function md5 comparison: the fork’s diff against upstream on the affected files is pure deletion, zero added lines.
• This is not a new vulnerability. It is the remainder of CVE-2026-42600 — three more protocol surfaces under the same root cause.
• Our failure is not in the code. It is in the record: a point fix was written up as a closure.
• While fixing it we introduced four regressions of our own, every one of them in a rule we invented rather than reused.

“N endpoints” was the wrong frame

Earlier audits kept counting endpoints, arriving at 19, then 21, then 22 — and missing an entire protocol surface each time. The real structure is four surfaces:

The first row matters as much as the rest: it overturns the earlier “all 21 endpoints escapable” claim. Those audits grepped for the validation helper inside handler bodies, found nothing, and concluded there was no protection — missing that the protection lives in the middleware layer.

The fourth row is the one no amount of hardening in storage-REST handlers can reach.

The root cause is three layers, not one bug

Three design facts, none wrong on its own:

1. Validation happens only at the HTTP surface. r.Form is populated from url.ParseQuery(RawQuery) and never from a body (introduced 2017).
2. Grid RPC bypasses the middleware. /minio/grid/v1 upgrades once; subsequent msgpack frames never re-enter the HTTP chain (introduced 2023).
3. The storage layer performs no containment. getVolDir rejects a volume only when it is exactly ""/./.., and pathJoin runs Clean (settled 2018).

In one sentence: the upper layer assumes the lower one validates, the lower assumes the upper already did, and neither can see the channel in between.

ReadMultiple was merely one endpoint that exercised that structure. Removing it left the structure intact.

One line of the timeline deserves singling out. The divide-by-zero in ShardFileSize has been present since 2020, but only when it moved inside xioutil.WithDeadline in 2024-10 — a change meant to fix large-object timeouts — did it escalate from “one failed request” to “the whole process exits”, because WithDeadline runs its work function on a bare goroutine that no recover() can reach. That escalation was not visible at the time.

Vectors, confirmed by execution

Every one reproduced against a real xlStorage through the real REST/grid client, with planted sentinel files. None of this is static inference.

Two of these had never been found before and are worth calling out.

RenameFile with an empty source path hits the volume-root alias and relocates the whole volume. Aimed at .minio.sys, one ordinary S3 GET afterwards yields the cluster’s IAM and configuration. It requires no traversal sequence at all — so any audit that greps for .. misses it by construction.

A negative part size floors both terms of ShardFileSize to zero. checkPart’s only integrity test is st.Size() < expectedSize, so every file that exists is reported intact, including a truncated shard. Worse, this holds whether or not the erasure parameters are valid: metadata that passes FileInfo.IsValid() — the very check healing trusts — is affected. That is not an input-validation problem but a data-integrity one: a legitimate heal reading poisoned metadata concludes the shard is fine and skips the repair.

The fix: two chokepoints, not twenty patches

The invariant to restore is one sentence: a path from an internode payload must resolve inside the volume it names, and a volume must resolve inside the drive root.

Only .. can break the first half (absolute and backslash-prefixed paths are folded under volumeDir by pathJoin’s Clean), and the second half has one independent break: paths that alias the volume root. Two rules, therefore — not a policy matrix.

Under 40 lines of core logic. No handler is modified, no call site in xl-storage.go is touched, and the local erasure path is left alone.

Two details worth recording:

• The check must run before the join, on the raw argument. pathJoin runs Clean against an absolute drivePath, which erases a leading .. entirely — /drive/../../etc becomes /etc, so a check placed after the join reads clean and passes everything. This is the most likely way a future refactor silently undoes the fix.
• NSScanner is the one method that reaches the filesystem without getVolDir. Its guard line is load-bearing, not decorative.

Among the rejected alternatives, the notable one is adding containment at all 33 pathJoin(volumeDir, …) sinks. That would be genuine defence in depth, but it means 33 edits in the most performance-sensitive file in the tree, each needing its own judgement about whether the volume root is a legitimate target. The guard rails buy most of the same resistance to drift for a fraction of the risk. This is an explicitly recorded IOU: if a code path is ever added that reaches the filesystem without getVolDir, the decision must be revisited.

Construction log: four regressions we caused ourselves

This section is unflattering and more informative than the fix.

First: whitespace. The initial rule treated whitespace as a separator, refusing " " and " " — legal S3 object keys. PutObject commits through RenameData, so such a key would fail on every remote drive simultaneously and break write quorum. The irony: the vulnerability needs root credentials; this bug needs a user to send a space.

Second: backslash-only keys. Same function, same root cause. path.Clean never treats \ as a separator, so on Unix "\\" is an ordinary filename. Refusing it made a distributed cluster reject a write a single-node server accepts — the same S3 API behaving differently by deployment topology.

Third: spaces and periods on Windows. The Win32 normalisation layer strips trailing spaces and periods from a path component, so a component made only of those vanishes and the path resolves to its parent. That makes both " " and "..." volume-root aliases on Windows — and "..." was sitting in our own list of legal object names at the time. We had not merely missed the vector; we had asserted it was safe.

Fourth: negative part sizes. The new guard rejected only “positive size with unusable parameters”, equating “non-zero” with “positive”. Negative values take a different route to the same zero.

Two false greens

Writing the AppendFile acceptance test produced two meaningless green runs in a row:

1. Driving it through the REST client — which special-cases *bytes.Reader and derives Content-Length from it, silently overriding the forged value.
2. Switching to an opaque reader — at which point Go’s own HTTP client refuses to send a request whose body is shorter than the declared length.

Only driving the handler directly through httptest reproduced it. The lesson: a client’s self-protection is not a server’s defence, and an attacker with a raw socket has no such scruples.

A third was a design failure. We built a per-field reflection poisoner, then discarded it: it cannot distinguish “should have rejected but delegated” from “correctly allowed a non-path field” (ETag, Algorithm, …), so it reports correct behaviour as failure.

The subtlest lived in the fuzzer. The first property test treated separator-only strings as an exception with an early return. That is not an exception, it is a blind spot — the fuzzer had been shut out of the entire category by hand and could never have found the backslash key in a million executions. A wrong exception is more dangerous than no fuzzer at all, because it creates the impression the space has been searched.

A very concentrated pattern

The reused semantics produced zero regressions; the invented rules produced all of them.

This is not coincidence. hasBadPathComponent is already the object layer’s own rule via IsValidObjectPrefix, validated by real S3 traffic for years, and structurally cannot reject anything creatable through the S3 API. An invented rule has nothing behind it but the author’s imagination.

The actionable form: reuse rather than invent; and when you must invent, write the property test by exclusion rather than enumeration, express exceptions with a predicate independent of the implementation, and keep them as few as possible.

The guard rails matter more than the patch

The final test suite pulls in two directions, and neither alone is enough:

• Falsifiability — remove each guard in turn and confirm the tests actually go red (191 failing subtests with the traversal guards removed; 64 GiB and 10.4 GB reappearing with the allocation guards removed). This is precisely what the rejected community PR lacked: its test asserted err != nil against a target that did not exist, so it passes with the vulnerability fully intact.
• Legal-traffic fuzzing — asserting that any key IsValidObjectName accepts, the guards accept (1.96M executions, no violations), and that any legal bucket name survives getVolDir (810K). This is what our own first two attempts lacked.

Plus a method-level reflection rail that fails by name when a path-taking method is added to StorageAPI unguarded.

History states the case for these rails bluntly: CVE-2026-39414 was also point-fixed on 2026-04-15 and only received a fix: complete ... two months later. Counting this one, “point fix → recorded as closure → completed months later” has now happened twice in this fork. The problem is not that someone was careless. It is that nothing in the tree could tell you a class was still open. Guard rails turn “someone must remember” into “CI fails”.

On adversarial review

This fix went through five rounds of independent adversarial review. Each round found one missed defect, and all five stood: whitespace keys → backslash keys → the AppendFile allocation → Windows spaces and periods → negative part sizes.

Our own review did find two in the same period (the WithDeadline log amplification and ReadParts using the wrong rule), but only after being pushed to that standard.

The hit rate says something plain: the last gate before merge should be independent acceptance, not the author’s own conclusion. During this work the author judged the change ready to ship four times and was overturned three.

Follow-up status

The first draft listed two implementation gaps. Both are now closed on the local branch, but none of these follow-up commits is in a published server release as of 2026-08-03:

• ReadFileHandler is bounded. Commit b6f70ab08 rejects a declared read length above 5 GiB, the maximum size of the S3 part represented by this legacy whole-file bitrot path. Legitimate GiB-scale reads can still allocate on that scale; the change removes caller-controlled allocation above the format’s real ceiling rather than pretending large reads are cheap.
• Negative part sizes cannot be persisted or trusted. Commit 80e8eaa42 rejects them at the AddVersion write funnel and again in CheckParts and VerifyFile, so both new poison and already-written metadata are covered. The internode boundary check uses the same predicate.
• Non-positive erasure block sizes are rejected at construction. Commit 80e8eaa42 validates blockSize in NewErasure, covering the other offset and decode divisions that a single downstream ShardFileSize guard could not. Rebalance’s separate division is guarded at its own boundary.

Two limitations remain and should not be folded into a stronger claim:

• No Windows CI. Windows builds are published; tests run on Ubuntu only. The Windows rule is reasoned from documented Win32 behaviour and has not been verified on the platform.
• Symlinks. The containment check is lexical, as upstream’s is.

Closing

The previous entry said that closing an endpoint and closing a defect class are two different conclusions. This time the known sinks are closed on the local branch, at the cost of four self-inflicted regressions and three overturned declarations that it was ready to ship. Publication remains a separate gate: the fixes above are not in a released server build yet.

If only one sentence survives: the vulnerability was upstream’s; our mistake was treating a point fix as a closure. And what prevents a third occurrence is not a more careful person — it is a test that fails.

10 - The Parser Knew, the Schema Didn't: Config Keys That Could Take Every Notification Down

Status: Fixed on the local pgsty/minio branch as 162ded343, unreleased Classification: Configuration-schema consistency and availability, not a vulnerability; includes one defensive hardening (credential values no longer echoed in validation errors) Affected scope: notify_nats JWT/NKey/TLS-handshake-first options, notify_amqp immediate, and any pre-2020 config migrated with an enabled NATS target — whose failure then silences every notification backend Tracking: pgsty/minio issue #39

This article names two unfixed availability defects in neighbouring code (the Postgres/MySQL migration writes, and kvFields typo folding). Neither is exploitable — both break the operator’s own configuration, loudly or not at all — and both are already named in the committed audit test’s allowlist. Publication needs no hold beyond the release itself.

Conclusions first

• Three notify_nats options — user_credentials, nkey_seed, tls_handshake_first — and one notify_amqp option — immediate — were read by the parser, written by the legacy migration, and registered nowhere. CheckValidKeys rejected exactly what GetNotifyNATS required.
• One constant meant two things. target.NATSUserCredentials held the string "MINIO_NOTIFY_NATS_USER_CREDENTIALS", sat in the environment-variable const block, and was used both as an env var name and as a config key. The snake_case config key for creds-file auth did not exist anywhere in the program.
• The reporter’s error did not come from their command. It came from the legacy migration: the pre-fix migration reproduces the issue’s error text byte for byte, including the notify_nats:ONE target name their command never mentioned. The migration wrote the store once; validation rejects it at every boot thereafter.
• The blast radius is the amplifier: FetchEnabledTargets fails fast on the first bad subsystem, its only caller just logs, and the global target list stays nil — so one broken NATS entry silently switches off Kafka, webhook, MQTT, and everything else.
• Inherited from upstream. Three feature PRs — #19139 (2024-02, user_credentials), #21008 (2025-04, tls_handshake_first), #21231 (2025-04, nkey_seed) — each added the parser and the env var, and each skipped the schema. Upstream is archived; the fork inherits both the defect and the duty.
• The fix registers the keys, splits the two-faced constant, corrects the migration — including a sibling bug that silently wrote immediate’s value under the internal key — tolerates the legacy on-disk spelling on the load path only, stops echoing values in invalid-key errors in both CheckValidKeys forms, and installs an AST audit that mechanically forbids this defect class across all ten notify subsystems.
• The audit found the next instance before the ink dried: the Postgres/MySQL legacy migrations write five unregistered keys, one of which is a plaintext database password. Recorded, allowlisted shrink-only, tracked for follow-up.

The error that named a target nobody asked about

The report (issue #39, by kuldeep-link11, against a NATS cluster using JWT operator/accounts auth) is a clean reproduction: configure notify_nats with a credentials file, watch it bounce.

$ mc admin config set us notify_nats:FITCHECK \
    address=nats-1:4222 subject=events.object.created \
    MINIO_NOTIFY_NATS_USER_CREDENTIALS=/jwt/creds/minio_notifier.creds \
    jetstream=off queue_dir=/data/queue-fitcheck queue_limit=100000

mc: <ERROR> ... found invalid keys
    (MINIO_NOTIFY_NATS_USER_CREDENTIALS=/jwt/creds/minio_notifier.creds
     nkey_seed= tls_handshake_first=off ) for 'notify_nats:ONE' sub-system,
    use 'mc admin config reset myminio notify_nats:ONE' to fix invalid keys

Two things in that error are wrong in ways the command cannot explain. The invalid-key list contains nkey_seed= and tls_handshake_first=off — keys the user never passed. And the rejected sub-system is notify_nats:ONE, while the command configured notify_nats:FITCHECK.

The second oddity is the whole case. Our reviewer established that the mc admin config set path cannot even carry an unregistered key: the server-side tokenizer, kvFields, splits the input line by searching for registered key names, so an unknown token never becomes a key at all — it is absorbed into the preceding key’s value. Probed directly:

input:  subject=s MINIO_NOTIFY_NATS_USER_CREDENTIALS=/jwt/x.creds
stored: subject="s MINIO_NOTIFY_NATS_USER_CREDENTIALS=/jwt/x.creds"

So the rejection could not have been about the command line. It was validateConfig sweeping the whole subsystem and tripping over a different, already-stored target named ONE that carried all three keys. Only one code path in the tree writes those key names into a store: the legacy config migration. Driving the pre-fix migration on an enabled NATS target named ONE reproduces the issue’s error text character for character — including the empty nkey_seed=, which is just what migration writes when the legacy config had no NKey.

That reframes the incident. This was not “the server rejected my command.” It was: an old config was migrated once, the migration wrote three keys the validator does not accept, and the store has been failing validation at every boot since — taking every other notification target down with it, silently, because the failure is logged and swallowed. The reporter’s command merely walked into the blast radius and got handed someone else’s error.

One constant, two meanings

The declaration, as inherited (internal/event/target/nats.go, pre-fix):

const (
    NATSAddress  = "address"
    NATSSubject  = "subject"
    NATSUsername = "username"
    NATSPassword = "password"
    NATSNKeySeed = "nkey_seed"            // config key — correct shape
    // ...
    EnvNATSUsername     = "MINIO_NOTIFY_NATS_USERNAME"
    NATSUserCredentials = "MINIO_NOTIFY_NATS_USER_CREDENTIALS"  // ← in the Env block
    EnvNATSPassword     = "MINIO_NOTIFY_NATS_PASSWORD"
)

NATSUserCredentials is named like a config key, valued like an env var, and shelved with the env vars. The parser used it as both: once as the env var to look up, once as the config key to read from the stored KVS. The migration used it as a key to write. There was no "user_credentials" string anywhere in the program — the config key for creds-file auth simply did not exist, which is why the reporter, finding no documented key, resorted to passing the env var name as one.

A name that means two things will eventually be wrong in one of them. Here it was wrong in both directions at once: as a key it was unregistered garbage; as the only spelling available it taught users and the migration to write garbage.

Four surfaces, no handshake

A notify option in this codebase lives on four surfaces that must agree: the defaults (DefaultNATSKVS — what validation accepts and mc admin config get displays), the help (HelpNATS — what mc admin config documents), the parser (GetNotifyNATS — what the server actually reads), and the migration (SetNotifyNATS — what upgrades write). Nothing ties them together. Three upstream feature PRs each updated the parser and the env plumbing, and each forgot the first two surfaces:

The AMQP row hides the quieter sibling. The AMQP migration did not skip immediate — it wrote immediate’s value under the internal key, and dropped cfg.Internal entirely:

config.KV{
    Key:   target.AmqpInternal,          // wrong key
    Value: config.FormatBool(cfg.Immediate),  // right value
},
// cfg.Internal: written nowhere

Because internal is registered, this one passes validation. The NATS gaps break a migrated config loudly enough to be found eventually; the AMQP gap corrupts it silently — a migrated broker config carries the wrong flag with a clean bill of health. One defect class, two presentations: the unregistered key fails closed, the misrouted value fails wrong.

The amplifier

None of this would deserve the word “outage” without the aggregation semantics. FetchEnabledTargets iterates the ten notify subsystems and returns (nil, err) on the first failure; its only caller logs the error and moves on, leaving the global notification target list nil; every later lookup nil-guards into an empty list. One rejected notify_nats target therefore turns off all bucket notifications — Kafka, webhook, AMQP, MQTT, the lot — with nothing but one line in the server log.

We considered changing this to per-subsystem isolation and decided not to, in this fix. Skip-the-broken-subsystem is a real behavioural change to how operators experience a bad config: today it fails loudly-in-aggregate (everything stops), and configurations that operators have already reasoned about depend on validation being all-or-nothing. Rewiring that is a compatibility decision that deserves its own change, not a rider on a registration fix — and once registration is correct, legal configs no longer trigger the cascade at all. The decision is recorded as a doc comment on FetchEnabledTargets and pinned by a characterization test, so the next person to touch it changes it on purpose or not at all.

The fix

About a hundred lines of production change, carried by nine hundred lines of tests (162ded343: 8 files, +1029/−7).

Registration. All four keys enter their default KVS and help schema, placed where an operator would look for them (user_credentials beside username, nkey_seed after token, tls_handshake_first after tls_skip_verify, immediate beside mandatory). Registration is also what makes a key visible: all four now appear in mc admin config get output where they previously did not.

The constant, split. NATSUserCredentials becomes a real config key, "user_credentials"; a new EnvNATSUserCredentials carries the env string. Every env var name involved — MINIO_NOTIFY_NATS_USER_CREDENTIALS, _NKEY_SEED, _TLS_HANDSHAKE_FIRST, MINIO_NOTIFY_AMQP_IMMEDIATE, and their _TARGET-suffixed forms — is frozen byte-for-byte: they are public interface, they worked throughout (the env route was always the workaround), and a test now pins them as raw string literals, so no rename of a Go constant can drift them silently.

Help flags, by precedent. Both new NATS values are file paths (a .creds file; an NKey seed file), so they are marked Sensitive but not Secret, mirroring cert_authority/client_cert/client_key rather than password/token. Secret would additionally redact them from mc admin config get — hiding an operator’s own configured path from them, which is why the private-key path client_key never had it either.

Migration, corrected. SetNotifyNATS now writes the real key; SetNotifyAMQP writes immediate = cfg.Immediate and internal = cfg.Internal.

If you are affected today, on a pre-fix build: the env var route works and always did, and mc admin config reset myminio notify_nats:<target> un-wedges a poisoned store at the cost of its settings. On the fixed build, poisoned stores simply load again — next section.

Living with what the old migration already wrote

Fixing the migration helps the next upgrade. It does nothing for stores the broken migration already wrote, which contain the literal key MINIO_NOTIFY_NATS_USER_CREDENTIALS — still unregistered, still fatal at every boot. Telling those operators to hand-reset their config would mean punishing them for our write.

So the load path tolerates it, narrowly. Validation accepts the legacy spelling for the NATS subsystem only — a test asserts AMQP still rejects it, so the tolerance cannot become a general escape hatch — and the parser falls back to it only when the real key is empty. Precedence is env > user_credentials > legacy key, and it holds by construction rather than by convention: the fallback result is passed as the default argument of the env lookup. All three orderings are tested. The legacy key stays out of the defaults and the help on purpose: it is tolerated, never advertised, never newly settable (kvFields sees to that).

The constant for it is a package-local literal, not an alias of EnvNATSUserCredentials — deliberately. It names bytes already on disk, so it must not follow any future rename of the env constant. The comment says so.

One trap discovered while wiring this, worth its own paragraph because it will bite someone eventually: the codebase has two CheckValidKeys — a free function and a method — and their deprecatedKeys parameters mean opposite things. The free function tolerates the listed keys (skips them); the method subtracts them from the valid set (rejects them). Refactoring this call from one form to the other would silently invert the tolerance into a ban. That asymmetry is now documented at the call site, which is the best one can do short of renaming an exported API.

The tolerance is written to be retired: the clean end state is to rewrite the legacy key into user_credentials once at load, then delete both the tolerance and the fallback. That is follow-up #2 below — and it also closes a small hole the tolerance leaves open: an unregistered key carries no Sensitive flag, so a tolerated legacy key ships its value (a path) unredacted in health-diagnostics bundles while user_credentials shows *redacted*.

Secrets in error messages

The invalid-keys error that started all this printed the rejected pairs with their values: found invalid keys (MINIO_NOTIFY_NATS_USER_CREDENTIALS=/jwt/creds/minio_notifier.creds ...). Those paths are mild. The mechanism is not: whatever value rides on a rejected key — a mistyped nkey_sed=<seed>, a bind password on a stale LDAP key — lands in the server log and in the mc client’s terminal.

Both CheckValidKeys forms now print key names only, keeping the shape and the mc admin config reset hint. The second site was one step beyond the written task scope — the method form serves LDAP, OpenID, and the policy plugin, where a rejected value can be an actual bind password — and the independent review, asked to judge that extension, said it would have demanded it: fixing one of two identical leaks is a half-fix. Repo-wide, nothing parsed values out of that string and no test asserted the old text; the change is global and intended.

There is a converse worth recording: this redaction is what stands between the next defect of this class and a credential in the logs. The follow-up below found the Postgres/MySQL migrations writing a plaintext database password under an unregistered key — on a pre-redaction build, the resulting rejection prints that password.

A guard that makes the class extinct

Registering four keys fixes four keys. The class — four surfaces, no handshake — stays open unless something ties the surfaces together mechanically. The fix therefore ships an AST-based audit test that parses parse.go and legacy.go, resolves the constants (from the target package sources, so there is no hand-maintained list to rot), and asserts, for all ten notify subsystems:

• every key the parser reads is registered in that subsystem’s defaults;
• every key the migration writes is registered (minus an explicit, shrink-only allowlist — next section);
• every help entry names a registered key.

Run against the pre-fix tree, it fails on exactly the four known gaps and nothing else — which is the red proof that it measures the right thing.

The adversarial review then attacked the audit itself with a mutation harness, on the theory that a guard you cannot watch fail is a guess — the discipline the previous article argued for. Seven of its nine mutations were caught. Two were not, and both blinded the audit silently: rename the parser’s loop variable (the read-collector pattern-matched the receiver name kv), or switch a migration entry to Go’s idiomatic elided composite-literal form (the write-collector demanded a typed config.KV{...}). In both cases the collector returns an empty map, the assertion loop iterates zero keys, and the test passes vacuously. Both are refactors a maintainer would make without a second thought; one of them is what gofumpt nudges you toward.

Two hardenings closed this, each verified in both directions — with the hardening the mutation is caught; with the hardening removed (the counterfactual) the vacuous pass returns:

• A floor assertion in the reverse direction: every registered key must be seen being read. This holds for all ten subsystems today — measured, not assumed, including the deprecated streaming_* keys read inside a nested conditional — so it costs nothing, and a blinded collector now produces one loud error per registered key (22 of them for NATS) instead of a green run.
• A widened literal guard: typed literals that are neither config.KV nor config.KVS are skipped; untyped (elided) literals have no type to check and are now inspected rather than ignored.

Final score: ten mutations, ten caught — the harness gained one variant along the way, and the closure round swept the full suite. The audit also enforces its own allowlist in both directions — removing an entry that is still needed fails, and an entry that goes stale (the migration no longer writes that key) fails too, so the allowlist can neither grow silently nor lie about the present.

What the audit found next

The write-side check refused to go green on two subsystems that had nothing to do with issue #39. SetNotifyPostgres and SetNotifyMySQL write five keys — host, port, username, password, database — that no default KVS registers and no parser reads. These are relics of the pre-DSN configuration shape, and the migration still emits them. Driving the real helpers confirms it: a migrated Postgres or MySQL notify target is rejected on the next load with found invalid keys (host, port, username, password, database) — the same failure mode as NATS, the same every-boot persistence, the same all-notifications blast radius through the fail-fast. And password there is a plaintext database password, which is exactly the value the redaction above now keeps out of the logs.

It is deliberately not fixed in this change. The scope was locked to the NATS and AMQP gaps, and the right treatment (register the five as deprecated, or stop writing them, or both) is a judgment call that deserves its own red/green cycle. It is pinned in the audit’s knownUnregisteredWrites allowlist with a shrink-only comment, so it cannot be quietly forgotten: the day someone fixes it, the stale allowlist entry fails the test and demands its own deletion.

Open items, none in a released build as of 2026-08-04:

1. Postgres/MySQL migration unregistered writes — major, live at every boot for anyone migrating a pre-KV config with those targets enabled.
2. Rewrite-on-load for the legacy NATS key, then retire the F5 tolerance and fallback; also closes the health-bundle redaction gap for tolerated keys.
3. kvFields typo folding — an unknown key name in mc admin config set is silently absorbed into the preceding key’s value instead of erroring. Pre-existing upstream wart; it protected nobody here and will corrupt someone’s subject eventually.

Review record

The change went through three gates before commit:

The honest accounting, in the house tradition: neither demand was a defect in the production fix. One was a lint gate (two British spellings that would have failed make test — and while fixing them, the implementer’s rewritten comment introduced a third, dialled, which the same gate caught; the demand vindicated itself in real time). The other was the audit-blindness pair above — durability of the guard, not correctness of the change. What the review did overturn was the incident’s origin story: the migration-path reproduction, the ONE target, and the kvFields absorption proof all came from the reviewer, and they change what operators should conclude — this was a boot-time outage lying in wait in stored configs, not a CLI validation quirk.

The implementer’s red phase also surfaced three bugs in its own new tests before the fix landed, recorded rather than smoothed over: a fixture that assumed stored targets are layered over defaults when config.Merge actually passes them through verbatim; a characterization test that segfaulted on a nil HTTP transport (FetchEnabledTargets dereferences it unconditionally — hostile to testing, noted, unfixed); and an early draft keyed to the very constant the fix renames, which made it pass green pre-fix — rewritten against the literal string so it pins the on-disk schema rather than the Go symbol.

Declined, and left open

Declined, deliberately:

• Per-subsystem error isolation in FetchEnabledTargets — a compatibility decision, not a rider (above).
• Registering or advertising the legacy key — tolerated on load, absent from defaults and help, impossible to set anew.
• Renaming EnvNatsTLSHandshakeFirst’s odd casing — an aesthetic rename in a fork is diff noise that buys nothing.
• Fixing the Postgres/MySQL migration here — scope-locked, pinned in the allowlist instead (above).

Left open: the three follow-ups above, and one cosmetic consequence — a store that still carries the tolerated legacy key will show it verbatim in mc admin config get until rewrite-on-load lands.

Closing

Every one of these keys worked perfectly through the environment variable, which is why three feature PRs could ship, get reviewed, get used, and never notice that the config-file half of the interface was stillborn. The parser and the schema are two descriptions of the same contract, maintained by hand, four surfaces wide — and for two and a half years nothing in the build checked that they agree.

If only one sentence survives: when two artifacts must stay identical and only convention binds them, the divergence is not a risk but a schedule — put a machine between them, then mutate the machine until you have watched it catch the drift you fear.

11 - Object Grant, Bucket Reach: When 'bucket/*' Could Rewrite the Bucket Itself

Status: Fixed on pgsty/silo-pkg main (3c24ad1, extended by 1f97549, scoped to its final twelve actions in d8b1fa7), released as silo-pkg v3.11.0; consumed by pgsty/minio Classification: Access-control hardening — a privilege boundary, narrowly restored Affected scope: IAM users/roles/service accounts granted only object-scoped (arn:aws:s3:::bucket/*) access, in deployments that share a cluster across tenants Tracking: upstream minio/minio issue #20449 (public since 2024, still open)

Conclusions first

• In IAM policy matching, a bucket-level request carries an empty object name, and the matcher built its resource string as "bucket/". An object-only policy pattern — "arn:aws:s3:::bucket/*" — then matched that string, so a grant that should cover only objects also authorized bucket-level actions.
• The dangerous one is PutBucketPolicy. A tenant holding only s3:* on bucket/* could install a bucket policy with Principal:"*" — making the bucket publicly readable or writable — or grant itself bucket-level control. Same mechanism, same class: DeleteBucket/ForceDeleteBucket (the issue’s own reproduction), PutReplicationConfiguration (exfiltration), PutBucketLifecycle (mass deletion), PutBucketVersioning, PutBucketObjectLockConfiguration, and the rest of the bucket-configuration writes.
• The full correction is a two-directional behavior change: it tightens over-granting Allow statements and loosens over-blocking Deny statements, and it would revoke ListBucket/GetBucketLocation grants that many real deployments write as bucket/* today. That is a compatibility break, not a clean patch.
• So we shipped a narrow fix — first six sensitive bucket-configuration writes, then, in a second pass, twelve: the bucket-level writes that hand the caller something its object access does not already give it, plus four that no handler implements. Only on Allow statements, so no Deny and no NotResource exclusion is ever weakened, with an environment-variable escape hatch. The compatibility-sensitive read/list family, CreateBucket, and three bucket writes with plausible tenant use are left unchanged, by decision.
• Twice we claimed the change could only remove permissions, and twice an untested case said otherwise — the second time found by an independent review of a shipped release. The protected path now requires the resource to match both the bare and the historical form, which makes the property hold by construction rather than by argument.
• The fix is red/green proven at the matcher layer and end to end through the real handlers; the object-scoped hot path is untouched.

The slash, and the empty object name

Every bucket-level S3 operation authorizes with an empty object name — checkRequestAuthType(ctx, r, policy.PutBucketPolicyAction, bucket, ""). The IAM matcher turned that into a resource string, and for the empty-object case it appended a trailing slash:

resource.WriteString(args.BucketName)
if args.ObjectName != "" {
    // "bucket/object"
} else {
    resource.WriteByte('/') // "bucket/"  <-- the defect
}

"bucket/" is matched by the wildcard pattern "bucket/*", because * matches the empty string. So a policy that grants s3:* on arn:aws:s3:::bucket/* — which reads as “anything, but only on the objects in bucket” — was evaluated as granting bucket-level actions too. The bucket-policy evaluation path (for anonymous/public access) never had this slash and is the reference-correct behavior; only the IAM path was wrong, and there was exactly one place it went wrong.

This is upstream minio/minio #20449, filed in 2024. An early upstream attempt deleted the slash outright and was reverted the same day for breaking policies that relied on the old behavior. The lesson we took from that revert shaped the fix below.

What it actually enables

PutBucketPolicyHandler has a single authorization gate and nothing behind it. Once the IAM check passes, the caller may store any well-formed bucket policy for that bucket.

The concrete chain, in a multi-tenant cluster:

1. An administrator grants tenant A the policy Allow s3:* on arn:aws:s3:::bucket-a/*, intending “A may work with the objects in bucket-a, nothing more.”
2. Because of the slash, A may call PutBucketPolicy on bucket-a.
3. A installs { "Principal": "*", "Action": "s3:GetObject", "Resource": "arn:aws:s3:::bucket-a/*" }. Every object in bucket-a is now readable by the anonymous internet. s3:* makes it world-writable. Pointing Principal at an account A controls exfiltrates the data; granting itself bucket-level actions in that policy is self-escalation.

The same object-only grant reaches other bucket-configuration writes with comparable consequences: replication to an attacker’s target, a one-day lifecycle expiry that deletes the bucket’s contents, disabling versioning, tampering with object-lock retention. None of these should be reachable from a grant scoped to objects.

This is not remotely exploitable and requires no missing credential — the caller is an authenticated principal you deliberately gave a scoped policy to. In a single-tenant deployment, that principal is your own trusted user and the practical risk is low. In a shared, multi-tenant cluster it is a real cross-tenant boundary failure.

Why a narrow fix, not the whole boundary

The obvious fix is to stop appending the slash for every bucket-level request. We did not do that, for two reasons that matter more than the one-line diff suggests.

It breaks common, benign usage. The correction does not only revoke the dangerous bucket writes — it also revokes ListBucket, GetBucketLocation, and ListBucketMultipartUploads when they were granted through bucket/*. Many deployments write exactly that and rely on it. The evidence is upstream’s own test suite: eleven STS integration tests grant s3:ListBucket on bucket/* and then assert that listing works. If the projects that wrote the server write it this way, production policies do too. A maintenance upgrade that turns those into AccessDenied is precisely the kind of surprise we refuse to ship.

It cuts both directions. The matcher builds the same resource string for Allow and Deny. So the full correction tightens over-granting Allow statements and simultaneously loosens over-blocking Deny statements: an administrator who locked a bucket with Deny s3:* on bucket/* would silently lose that protection for bucket-level actions. A clean-looking fix that moves security in two directions at once is not a maintenance patch — it is a migration.

So we narrowed the change to where it is unambiguously right and effectively free of compatibility cost:

• Only bucket-level writes are protected. The first pass covered six sensitive configuration writes: PutBucketPolicy, DeleteBucketPolicy, PutReplicationConfiguration, PutBucketLifecycle, PutBucketVersioning, PutBucketObjectLockConfiguration. The second pass (below) extended that to twelve. Almost nobody grants these through an object-only pattern on purpose — you do not accidentally rely on an object grant being able to rewrite a bucket’s policy or delete the bucket — so revoking that path breaks essentially no one.
• Only on Allow statements. Deny statements keep the historical resource string, so no existing Deny is ever weakened. The narrow fix only ever adds a denial.
• The read/list family is left exactly as it was. ListBucket on bucket/* still works. That is the compatibility-sensitive part, and it waits.

The fix

The matcher keeps the trailing slash in every case except one: a bucket-level Allow statement being evaluated for a protected action, with the compatibility shim off.

resource.WriteString(args.BucketName)
if args.ObjectName != "" {
    // "bucket/object" — unchanged
} else if args.BucketName == "" {
    resource.WriteByte('/') // KMS two-phase sentinel — unchanged
} else if legacyBucketResourceMatch.Load() ||
    statement.Effect != Allow ||
    !isSensitiveBucketMutation(args.Action) {
    resource.WriteByte('/') // historical behavior for Deny / non-sensitive / shim-on
}
// else: bare "bucket" — an object-only "bucket/*" no longer authorizes it

Because args.Action is the concrete request action, a wildcard grant (s3:*) is covered too: the wildcard matches at the action step, and by the time the resource string is built the action is the specific PutBucketPolicy. A bare-bucket resource (arn:aws:s3:::bucket) and the * resource still match, so correctly scoped grants — including the built-in readwrite policy — are untouched.

The escape hatch is MINIO_API_LEGACY_BUCKET_RESOURCE_MATCH=on, read once at startup. It restores the full historical behavior — both the over-grant and the over-block — for any operator who needs the old semantics while they adjust their policies.

The second pass, and the question that decided its size

The first round protected six configuration writes and stopped. Reviewing it against the original issue showed that was not enough: the action reproduced in #20449 itself — DeleteBucket — was still reachable through an object-only grant. An end-to-end test against the first-pass build confirmed it: a user holding nothing but s3:* on arn:aws:s3:::bucket/* called RemoveBucket and the bucket was gone.

Extending the set raised the real question — how far? The first instinct was “every bucket-only write except CreateBucket,” fifteen actions. That was the wrong instinct, and the reason is a detail of how the bug fires.

The bug only triggers when the statement already grants the bucket-level action. Resource matching runs after action matching, so a read-only tenant holding s3:GetObject on bucket/* never reaches DeleteBucket — the action never matched. In practice the affected principal holds s3:*, which means they already have full read, write, and delete over every object in the bucket. That reframes the severity of each candidate action, because the question is not “how dangerous is this action in the abstract” but “what does reaching it add to a position that already includes all the data?”

By that test, three groups fall out:

Protected — reaching these grants something the object access does not. PutBucketPolicy and DeleteBucketPolicy hand access to other principals, anonymous included, and can grant the caller bucket-level actions it was never given: self-escalation and public exposure. PutBucketObjectLockConfiguration and PutBucketVersioning defeat protections that exist precisely to stop a holder of write access from destroying data. PutReplicationConfiguration and PutBucketLifecycle act under server credentials and keep acting after the caller’s access is revoked. DeleteBucket and ForceDeleteBucket destroy the bucket entity and its configuration irreversibly.

Protected at zero cost. PutBucketCors, DeleteBucketCors, PutBucketQOS, and PutInventoryConfiguration have no MinIO server behavior attached today — no handler at all, or a handler that returns NotImplemented after the authorization check. Withholding them changes nothing that works, and covers them in advance if a handler is ever wired.

Deliberately not protected. PutBucketTagging, PutBucketEncryption, and PutBucketNotification are bucket-level writes, and the first draft of this pass did protect them. They came back out. None of the three gives the caller access it does not already hold — the harm is to the owner’s posture, not to the access boundary — while a tenant handed s3:* on bucket/* and told “this bucket is yours” may quite reasonably tag it, set default encryption, or wire up event notifications. Low security gain against a real compatibility cost is the wrong trade for a maintenance release. They keep the historical matching, and a test now asserts that they are unprotected, so putting any of them back is a deliberate act with a visible cost rather than an edit to a list.

That leaves twelve actions, shipped as silo-pkg v3.11.0. Two older boundaries stand unchanged: CreateBucket keeps the historical matching (it targets a bucket that does not exist yet, and provisioning flows commonly create a tenant’s bucket with that tenant’s own credentials), and the read/list family still waits for the migration-gated change.

The choice of what to break, in other words, was made by asking would an administrator ever write this on purpose — not by ranking the actions by how dangerous they sound. The first question predicts which upgrades break; the second only sets urgency.

The claim that was wrong twice

Everything above rests on one property: this change may remove permissions and must never add one. Both times we asserted it, we were asserting it about a mechanism we had reasoned through rather than tested through. Both times it was false.

The first pass withheld the slash from the NotResource match as well — and NotResource is an exclusion. An Allow s3:* NotResource bucket/* statement historically did not apply to bucket-level requests on that bucket; matching the exclusion against the bare bucket name made it stop matching, so the Allow it qualified grew, for exactly the writes being protected. Restoring the historical form for NotResource fixed that, and the second pass shipped saying the result was “provably monotone.”

An independent adversarial review of that release produced a counterexample within the hour. Withholding the slash does not merely remove a match — it changes which string patterns are matched against, and a pattern can match "mybucket" without ever having matched "mybucket/". The clean case is a fixed-width wildcard:

Allow s3:PutBucketPolicy on arn:aws:s3:::mybucke?

? matches exactly one character. Against the historical nine-character "mybucket/" it does not match, so this statement never authorized the bucket-level write. Against the new eight-character "mybucket" it matches, so the hardening granted something the buggy matcher refused. Small in reach — you have to write a length-sensitive pattern — but it is precisely the class of defect the property was supposed to exclude, shipped in a release whose notes claimed the property held.

The fix is not another special case. On the protected path the matcher now requires both forms to match: the bare bucket name and the historical "bucket/". The result is an intersection with the historical decision, so it is monotone by construction — there is no pattern it can newly satisfy, and no argument to get wrong next time. mybucket* still grants (it matched both all along); mybucket/* is still withheld; mybucke? is refused exactly as it always was. That shipped as silo-pkg v3.11.0.

Two things are worth taking from this beyond the patch itself. A correctness fix in an authorization path must never make anything newly allowed — and the only way to know is to test both directions, because the reasoning feels airtight in both cases where it wasn’t. And when a security property is load-bearing, build it out of an operation that cannot violate it rather than out of a case analysis you believe is complete.

Regression tests now pin each direction: grants narrowed, Deny untouched, NotResource exclusions untouched, fixed-width wildcards not broadened, the three unprotected writes still reachable, plus an invariant test that every protected action really is bucket-only (ResetBucketReplicationState, despite its name, is an object action and stays out). In the server they run end to end through the real handlers — client, inline session policy, and the S3 router — and every one of them fails against the release that had the bug.

What you will notice

For nearly everyone: nothing. Object access is unchanged, ListBucket via bucket/* is unchanged, and correctly written bucket policies are unchanged.

The one visible change: a request that tries to delete the bucket, or change its policy, replication, lifecycle, versioning, or object-lock configuration using credentials whose only matching grant is an object-only bucket/* pattern now returns AccessDenied. Bucket tagging, default encryption, and event notification are not affected. That is the boundary being enforced. If a deployment genuinely depends on the old behavior, set MINIO_API_LEGACY_BUCKET_RESOURCE_MATCH=on and grant those actions on the bare bucket ARN (arn:aws:s3:::bucket) at your own pace.

What we deliberately left open

The general problem in #20449 — that bucket/* reaches the remaining bucket-level actions: ListBucket, GetBucketLocation, the configuration reads, CreateBucket, and the three tenant-plausible writes above — is not fixed here. Closing it fully means revoking grants that real deployments depend on, so it belongs to a future release that carries a migration path.

What that release owes operators is more than a wider action list, because no one can enumerate every deployment’s policies — which means shrinking or growing the protected set by guessing is an exercise with a hard ceiling. Three things raise it:

• A startup policy audit. Walk the stored policies and name each one whose meaning changes, in both the grant and the deny direction. That turns an upgrade surprise into a pre-upgrade checklist, it is read-only, and it can ship before the enforcement change rather than with it.
• A denial that explains itself. When a request is refused because only an object-scoped grant matched, say exactly that, and name the compatibility switch. A break an operator can diagnose in thirty seconds costs an order of magnitude less than a silent one.
• A switch with a scope. MINIO_API_LEGACY_BUCKET_RESOURCE_MATCH is all-or-nothing today: an operator who needs one action back has to reopen the self-escalation path along with it. Per-action scoping is what makes the change safe to adopt.

Recording the boundary rather than implying it: today twelve bucket-level writes are corrected. Everything else — the read/list family, CreateBucket, and bucket tagging, encryption, and notification — still honors bucket/* as a bucket-level grant, by decision, until that migration-gated change lands.

Closing

A single appended slash turned “only the objects” into “and the bucket too.” The tempting fix removes the slash everywhere and, in doing so, breaks a listing pattern half the world relies on and quietly weakens every Deny written against bucket/*. The fix we shipped removes it in exactly the place where an object-scoped grant should never have reached — the writes that can make a bucket public, and the ones that can delete it — and nowhere else. The rest is written down, waiting for a release where breaking it is something users are told to expect rather than something that happens to them.

12 - Absent Is Not Empty: A Blank versionid and the Fail-Open It Invites

Status: Fixed on the local pgsty/minio branch as 744a9dcd7, unreleased Classification: Policy-enforcement correctness — a fail-closed report, a fail-open trap avoided, and one narrow trim bypass closed. Not a headline CVE — see How we classify this Affected scope: Any deployment with a bucket/IAM policy using Null or StringEquals on s3:versionid; the reported break is on DeleteObject/DeleteObjects Tracking: upstream minio/minio issue #21735 (reporter iTrooz, 2026-01-10); upstream repository archived read-only since 2026-04-25

This article documents an unreleased fix and two unfixed same-class residuals in neighbouring paths (governance-bypass and Snowball). Hold publication until the fix ships and the residuals are triaged.

Conclusions first

• The policy engine decides Null by slice length, not by content. MinIO wrote "versionid": {""} into the condition map unconditionally, so a request that named no version still presented a length-1 slice. Null:{s3:versionid:true} — “match only when the key is absent” — could therefore never match, and Null:false always matched. The reporter’s “allow deletes only of the current object” policy denied every current-object delete (HTTP 200 envelope, per-object AccessDenied).
• The one-line fix is a trap. “Write the key only when it is non-empty” fixes the report and simultaneously opens something worse. DeleteObjects carries each object’s version in the XML body; the condition builder reads only the query string. Drop the empty key and a body version simply vanishes from the map — read as absent, i.e. as null — so a policy meant to protect old versions would authorize deleting a specific one. Fail-closed defect, meet fail-open bypass.
• The real fix has two parts: write the key only when a version is named, and bind it, for DeleteObject, to the effective server-resolved version (ReqInfo.VersionID) — the per-entry body value that the DeleteObjects loop already resolves — rather than to whatever the query string happened to carry.
• A third, adjacent hole closed on the way: the builder read the version untrimmed while the object layer trims it, so a padded ?versionId=V%20 let a Deny StringEquals s3:versionid "V" be sidestepped on the read/tag/copy paths.
• Inherited from upstream, and unfixable there. minio/minio is archived read-only, so the fix lives in the fork; this is the same getConditionValues we hardened in the condition-source work.

Absent is not empty

A condition key in a MinIO policy resolves to a lowercase name in a map[string][]string, and the engine answers Null by asking how long that slice is (silo-pkg .../policy/condition/nullfunc.go):

func (f nullFunc) evaluate(values map[string][]string) bool {
	rvalues := getValuesByKey(values, f.k)
	if f.value { // Null:true — "the key must be absent"
		return len(rvalues) == 0
	}
	return len(rvalues) != 0 // Null:false — "the key must be present"
}

The content of the strings is never read. A slice {""} has length 1. To this function, a present-but-empty value is indistinguishable from a real version ID, and both are the opposite of absent.

Now the value that fed it, as inherited (cmd/bucket-policy.go, getConditionValues):

args := map[string][]string{
	// ...
	"versionid": {vid}, // vid == "" for any request that names no version
	// ...
}

vid is the request’s ?versionId, empty on the overwhelming majority of calls. So every request, versioned or not, arrived at the engine carrying versionid: [""] — permanently length-1, permanently “present.”

The two Null directions then invert:

The reporter wrote the canonical “let clients delete current objects but not roll back versions” policy — Allow s3:DeleteObject with Condition {"Null": {"s3:versionid": "true"}} — and watched every version-less delete return AccessDenied. The Allow never fired because its condition tested “no version named” and the map insisted a version was always named. StringEquals cannot see the difference either ({""} and absent both fail to intersect a non-empty policy value); only Null and ForAllValues:* are sensitive to it, which is why Null is where it surfaced.

The fail-open next door

The obvious fix writes the key only when it is non-empty, and for a single DeleteObject that is completely correct: no version → absent → Null:true matches. Ship that alone, though, and Multi-Delete turns it into an authorization bypass.

DeleteObjects (POST /{bucket}?delete) does not put versions in the query. Each object carries its own optional version in the request body:

<Delete>
  <Object><Key>photo.jpg</Key><VersionId>a1b2…</VersionId></Object>
  <Object><Key>notes.txt</Key></Object>
</Delete>

The condition builder reads r.Form — the query string — and nothing merges an XML body into it. So under the naive fix, an entry that names version a1b2… in the body produces an empty query version, the key is omitted, and the engine sees absent — null. A policy written to allow only null-version deletes now matches, and the specific old version the operator meant to protect is deleted. The fail-closed nuisance from the report has become a fail-open on exactly the operation that most needs to be scoped per object.

This is the crux the reporter’s simple case hides: the condition value must be the version the server will actually act on for this object, and for Multi-Delete that value lives on a channel the condition builder never looked at.

The fix: the effective version, not a convenient one

Two mechanisms, because either alone is wrong.

1 — Represent absence honestly (cmd/bucket-policy.go). Write the key only when the request names a version, so “no version” becomes a length-0 read:

if vid != "" {
	args["versionid"] = []string{vid}
}

2 — Bind DeleteObject to the effective version (cmd/auth-handler.go, authorizeRequestWithTags). The DeleteObjects loop already resolves each entry’s body version into ReqInfo.VersionID (via checkRequestAuthTypeWithVID, cmd/bucket-handlers.go:502, a sequential loop — no shared-state race). Authorization rebinds the condition value to that server-resolved string, and deletes the key when it is empty:

conditionValuesForAuth := func(lc string, cred auth.Credentials) map[string][]string {
	values := getConditionValuesWithTags(r, lc, cred, existingTags, requestTags)
	if action == policy.DeleteObjectAction {
		// DeleteObjects carries the effective version in each XML object,
		// not in the request query. Keep authorization scoped to that entry.
		if versionID == "" {
			delete(values, "versionid")
		} else {
			values["versionid"] = []string{versionID}
		}
	}
	return values
}

An end-to-end test drives a &versionId=query-level-decoy on the DeleteObjects URL and asserts it never reaches any entry’s decision — the per-entry body value wins, the decoy is stripped.

Why DeleteObjectAction only, and not a blanket ReqInfo rebind. The tempting simplification — “always use ReqInfo.VersionID” — breaks copy. For a CopyObject, the source read is authorized as GetObject against the source’s version, which travels in the x-amz-copy-source header, and getConditionValues already extracts it there; ReqInfo.VersionID for a copy holds the destination query (usually empty). A blanket rebind would overwrite the correct copy-source version with the wrong one. Every non-delete version-aware operation (Get, Head, tagging, retention, copy-source read) carries its version in the query or the copy-source header, both of which the builder reads, and both of which are the effective version for a single object. Only Multi-Delete diverges. So the override is precisely as wide as the divergence, and no wider.

The version the server acts on is the trimmed one

One gap remained once the delete paths were correct. The builder read the version raw:

vid := r.Form.Get(xhttp.VersionID) // untrimmed

while every path that actually uses the version trims it first — newContext (cmd/utils.go:806) and getOpts (cmd/object-api-options.go:101) both strings.TrimSpace. So on non-delete version-aware actions, a padded ?versionId=V%20 presented "V " to the policy engine while the object layer read, tagged, or retained version "V". A Deny keyed on StringEquals s3:versionid "V" — “protect this exact version” — saw "V ", failed to match, and did not fire; the operation on "V" proceeded. A narrow bypass (the attacker must know the version and that a space changes nothing downstream), but a real one.

The fix trims both reads, aligning the condition value with the effective version:

vid := strings.TrimSpace(r.Form.Get(xhttp.VersionID))
// ... and the copy-source fallback likewise

DeleteObjectAction was already immune, because it uses the already-trimmed ReqInfo.VersionID. Trimming introduces no new allow: it can only make the condition value equal the version actually operated on, which tightens Deny and corrects Allow in the same direction. We proved it is load-bearing by removing only the trim and watching the padded test case go red.

What it affected

The reported break is on delete, but the underlying key is read by many actions. After the fix, every version-aware chain evaluates s3:versionid against the version the server resolves for that operation:

A forgery route was already closed by the earlier condition-source work and is worth restating: versionid is a reserved internal key (both the versionid and canonical Versionid spellings), so a client cannot inject a second copy through the header/query merge loops. The wire parameter is spelled versionId (capital I) and lands in an inert args["versionId"] the engine never reads.

Two directions of impact, kept distinct because they have different severities:

• Functional (the report): version-less deletes were wrongly denied. Fail-closed — an availability and usability defect, not a grant.
• Security (the trap and the trim): the naive fix would have granted deletes of protected versions on Multi-Delete (fail-open); and the untrimmed value permitted a narrow Deny bypass on read/tag/copy. The fix closes the first before it can exist and the second where it already did.

How we classify this

We are not minting a CVE for this, and the honest reasons are worth stating.

The behaviour the reporter filed is fail-closed: MinIO denied operations the policy meant to allow. A system that is too strict leaks nothing and grants nothing; it is a correctness and usability defect, and inflating a false-deny into a vulnerability would cheapen every real entry in this chronicle, whose neighbours are authentication bypasses and path traversals.

What carries genuine security weight is not the report but its vicinity. The fail-open on Multi-Delete is real, but it is a hazard we would have introduced, not one that shipped — the value of the two-part design is that the dangerous version never existed in a build. The trim bypass did exist, but it is narrow: it requires a Deny keyed on an exact s3:versionid, an attacker who knows the version, and it only ever affected non-delete paths. We closed it because it was in reach, not because it was a headline.

So: policy-enforcement correctness, filed here because that is where we keep silent enforcement failures, with the security interest recorded plainly rather than dressed up.

The boundaries we did not cross

Two same-class residuals remain, recorded rather than silently left:

• Governance-bypass in Multi-Delete. When an entry carries object-lock, enforceRetentionBypassForDelete re-authorizes under BypassGovernanceRetentionAction (cmd/bucket-object-lock.go:153). That action is not DeleteObjectAction, so the effective-version override does not apply, and its s3:versionid is still the query value — absent in a normal Multi-Delete — rather than the per-entry version whose lock is being bypassed.
• Snowball tar extraction. PutObjectExtract takes each member’s version from the tar PAX record minio.versionId after the per-file authorization, so a named version can be written that never appeared in any condition value.

Both are narrow, both are pre-existing, and both would widen the change from “fix the reported key” into “re-plumb every action’s version into ReqInfo.” We scoped to the reported surface and wrote the IOUs down here, for the same reason the previous article recorded its object-layer omission: a deliberate omission that is not written down is indistinguishable from an oversight six months later.

A related decision, declined: the sibling keys username, userid, signatureversion, and authType are still written unconditionally empty, carrying exactly the present-but-empty defect versionid just shed — Null:{aws:username:true} is always false, including for the anonymous caller it should match. Fixing them is a one-liner each and a forty-caller blast radius, and some (principaltype is never empty) do not share the bug at all. We did not bundle a broad presence sweep into a versionid fix; it is named here as the next thread to pull.

Falsification

Three experiments, in the discipline that a test you have not watched fail is not yet a test.

• Revert both source files to HEAD. The end-to-end DeleteObjects test turned red with every version-less entry returning AccessDenied — a faithful reproduction of issue #21735 — and the unit test caught the {""} key directly (“an absent versionId was exposed to policy evaluation”). Reapply, green.
• Remove only the TrimSpace. The padded case went red on the exact assertion — got [7f4b6b5f-…dd8 ] — proving the trim is not decoration. Restore, green.
• The decoy. The Multi-Delete test appends &versionId=query-level-decoy to the URL and asserts it reaches no entry’s decision, which is what distinguishes “reads the query” from “reads the effective per-entry version.”

The change touched only five files (cmd/bucket-policy.go, cmd/auth-handler.go, two tests, one doc example), committed with explicit paths in a working tree that had concurrent unrelated work in it, so nothing from the neighbouring efforts was swept in.

Source and lineage

The report is upstream minio/minio#21735, opened 2026-01-10 against RELEASE.2025-09-07T16-13-09Z: a Null:{s3:versionid:true} policy denying version-less DeleteObjects. The upstream repository went archived and read-only on 2026-04-25, so there is no upstream fix to wait for and no maintainer to coordinate with — the fork is the only venue, and the record here is the resolution.

The defect is old and inherited. getConditionValues has written versionid unconditionally for as long as the key has existed; the length-based Null semantics are upstream’s, in the policy package the fork consumes via silo-pkg. This is the same function and the same lineage as the earlier condition-source hardening that stopped client input from shadowing server-derived condition values — a related read of “what a policy condition is allowed to believe about a request,” continued here into “and it must believe the version the server will actually act on.”

Closing

Absent is not empty. A map that cannot say “no version” by leaving the key out will say it by leaving the value blank, and a Null that counts length will believe a version was named on every request that named none.

If one sentence survives: a fail-closed bug is the dangerous kind to fix, because the obvious repair flips it to fail-open — so bind the condition to the value the server actually acts on, from the same channel the operation reads, not the channel that was convenient; and when you stop at the reported surface, write down the versions you left on the wrong channel, rather than trusting the next person to find them.

13 - Three Headers, One Lie: Making the Client Source Address Mean Something

Status: Landed on pgsty/minio master as fe6dc4780, unreleased Classification: Opt-in hardening plus a documentation defect, not a vulnerability and not a regression; no CVE assigned. The underlying weakness is inherited from upstream and its default behaviour is unchanged here Affected scope: aws:SourceIp policy conditions, the audit log remotehost field, S3 event notification Host, and the client shown by mc admin trace — on any deployment whose S3 API port is reachable without passing through a header-sanitising proxy Upstream: nothing to file — minio/minio is archived. Prior art there: PR #4736 (2017, the concern raised and half-addressed), discussion #17878 (2023, maintainer marks it working as intended), PR #20977 (2025, the partial switch)

This article states plainly that an IpAddress policy condition is not enforceable on a directly-reachable MinIO deployment, and that this remains true by default after the change. That is a property of upstream MinIO as shipped, not a defect introduced by the fork, and it has never been documented anywhere. Publishing it is the point.

Conclusions first

• MinIO reads the client’s address out of three interchangeable headers — X-Forwarded-For, X-Real-IP, RFC 7239 Forwarded — and never from the TCP connection unless all three are absent. That address becomes aws:SourceIp and the audit log’s client field, so whoever controls it controls both IP-based access control and the attribution of every logged action.
• The one switch that existed, _MINIO_API_XFF_HEADER=off, suppresses one of those three. An attacker’s response is to send X-Real-IP instead. Our own code comment was recommending it as the mitigation.
• “Put MinIO behind a reverse proxy” is not sufficient, for two independent reasons: on Kubernetes an Ingress and a ClusterIP Service routinely coexist so the proxy is not the only way in; and the stock nginx recipe appends to X-Forwarded-For, leaving a client-supplied entry in the left-most position — which is exactly where MinIO reads.
• The fix is a new opt-in setting, MINIO_API_TRUSTED_PROXIES, generalising a trusted-proxy mechanism this fork already built for LDAP STS rate limiting. Set to a list, forwarded headers are believed only from those peers and chains are walked right-to-left. Set to none, nothing is believed.
• The most consequential decision was one we reversed. The first implementation widened _MINIO_API_XFF_HEADER=off to suppress all three headers. That was the only part of the change that could alter an existing deployment’s behaviour, and it was backed out. The switch keeps its exact upstream semantics, and upstream’s TestXFFDisabled is retained unmodified as the proof.
• Net compatibility impact: none for any deployment that does not opt in.
• Adversarial review found four defects in the first implementation, including one that made the new setting silently ineffective for every deployment configured through an environment file.

What the address is actually used for

The value comes from a single function, handlers.GetSourceIPFromHeaders. Tracing its consumers is what turns this from a logging curiosity into a security question:

Two of these are security-relevant in different ways. A forged aws:SourceIp is a live access-control bypass: an IpAddress condition meant to confine a principal to an office CIDR is satisfied by asserting an address in that CIDR, and a NotIpAddress deny is evaded by asserting one outside it. A forged audit address is quieter and arguably worse — it corrupts the record retroactively, it applies even where no IP-based policy exists, and nobody notices until they need the logs.

Note also that the value is never validated as an IP address in the default path. Forwarded: for="_gazonk" is accepted and returned verbatim; upstream’s own test asserts it.

The setting that looked like a mitigation

internal/handlers/proxy.go gates exactly one header:

if enableXFFHeader {
    if fwd := r.Header.Get(xForwardedFor); fwd != "" {
        // ... left-most entry
    }
}
if addr == "" {
    if fwd := r.Header.Get(xRealIP); fwd != "" {
        addr = fwd                       // not gated
    } else if fwd := r.Header.Get(forwarded); fwd != "" {
        // ... first for= element        // not gated
    }
}

Setting _MINIO_API_XFF_HEADER=off costs an attacker one line: send X-Real-IP instead of X-Forwarded-For. Worse, disabling X-Forwarded-For moves the trust to a header the operator has not thought about, so the switch can leave a deployment in a state its owner has not modelled.

Where did it come from? Upstream PR minio/minio#20977, whose entire stated motivation is:

Customer request to disable all XFF header handling, ping me in Slack for more details.

No security rationale, no mention of X-Real-IP or Forwarded, no public discussion of why one header was gated and two were not. The narrowness is an oversight, not a considered scope. That mattered for the design, because it meant nobody had decided the other two should stay trusted — but as we will see, it did not end up justifying a change to the switch.

Upstream knew, in 2017

The most interesting thing found while writing this up is that none of it is news to upstream. The history is a small lesson in how a security decision decays.

August 2017. IpAddress / NotIpAddress condition support is added in PR #4736. During review the maintainer, @harshavardhana, raises exactly the concern this article is about: X-Forwarded-For is trivially spoofed, the left-most entry is the client’s own, and using it for a security decision would let a malicious client reach objects. The contributor accepts it and removes X-Forwarded-For support entirely, leaving only X-Real-IP, on the stated reasoning that a proxy sets it and a client cannot manipulate it. Merged five days later.

That reasoning is half right, and its unstated half is the whole problem: X-Real-IP is untamperable only if the proxy in front overwrites it. Nothing enforced that, and nothing told operators it was load-bearing.

Today. X-Forwarded-For is read first, ahead of X-Real-IP. The 2017 decision did not survive; it dissolved across later refactors of the condition-value plumbing rather than being reversed on purpose. There is no commit that says “we are re-admitting the spoofable header into policy decisions” — which is precisely how this class of decay happens.

August 2023. In discussion #17878 an operator reports that source IPs behind a load balancer are unreliable. The maintainer’s answer is unambiguous: without reliable source-IP visibility, IP-based restrictions are impractical, and the recommendation is to compartmentalise by tag or namespace instead. Marked working as intended.

So upstream’s own position — stated by a maintainer, in public — is do not rely on aws:SourceIp. That is a defensible engineering stance. What is missing is anywhere an operator would encounter it: it is not in the policy documentation, not in the condition-key reference, and not near the setting that appears to make it safe. An IpAddress condition is accepted without complaint and behaves as though it works.

That gap is the actual defect being fixed here, and it reframes the change. The allow-list is not overturning an upstream judgement; it is offering the mechanism that would make the 2017 concern answerable, to the deployments that want it. The documentation is doing the heavier lifting: writing down a contract that has been implicit since 2017 and contradicted by its own switch since 2025.

Why “put it behind a proxy” is not the answer

This is the standard advice, and it fails in two common, independent ways.

The proxy is not the only way in

On Kubernetes an Ingress and a ClusterIP Service routinely coexist. The Ingress sanitises headers; the Service does not, and any pod in the cluster can reach it. The security boundary is assumed to be the Ingress but is actually the pod network. The same shape appears in Pigsty deployments, where a load balancer fronts MinIO while the service ports remain reachable on the internal network.

The canonical nginx recipe preserves attacker input

The near-universal snippet is:

proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;

$proxy_add_x_forwarded_for expands to $http_x_forwarded_for, $remote_addr — it appends to whatever the client sent. A client sending X-Forwarded-For: 1.2.3.4 causes nginx to forward 1.2.3.4, <real client>. MinIO takes the left-most element, which is the attacker’s.

So a correctly-proxied, hardened, no-direct-access deployment is still forgeable, because left-most parsing and append-style proxies are mutually incompatible. Only proxy_set_header X-Forwarded-For $remote_addr; (overwrite) is safe under the default mode, and that is not what operators copy from the documentation.

This is the finding that rules out a documentation-only fix. Deployment discipline cannot close it; the chain has to be read from the other end, which requires code.

The design

Rather than inventing a mechanism, we generalised one this fork already has. MINIO_IDENTITY_LDAP_STS_TRUSTED_PROXIES — added during the LDAP STS throttling work — already implements CIDR allow-list parsing, catch-all rejection, and a right-to-left chain walk, scoped to rate-limit bucketing. The parser moved to internal/config and both paths now share it.

One setting selects the mode:

Under the allow-listed mode, X-Forwarded-For and Forwarded are read right-to-left, stepping over entries that name a listed proxy, and the first remaining address wins. Each proxy appends the peer it actually saw, so an entry the client injected sits to the left of the one its proxy wrote and the walk stops before reaching it. Appending proxies become safe.

GetSourceScheme is deliberately untouched. It feeds the Location URL in S3 responses rather than a policy decision, and suppressing it would hand http:// URLs to every deployment terminating TLS at a proxy.

Trade-offs

Whether to change the default

Changing the default to distrust forwarded headers would make every existing reverse-proxy deployment’s aws:SourceIp and audit addresses become the proxy’s address overnight. Policies could fail closed; audit continuity would break.

Not changing it leaves directly-reachable deployments exposed.

Decision: do not change it. But “do not change” is not the same as “stay silent”. The consequence is now written into the code comment and the operator documentation in as many words: under the default mode, an IpAddress condition is not access control and the audit address is not evidence. Making the cost visible so operators can choose is better than making a choice for them that detonates during a release window.

Whether to widen the existing switch

This is the decision we got wrong first and reversed, and it is the part of the story most worth recording.

The original brief asked that when an operator explicitly disables forwarded-header trust, clients must not be able to forge through an equivalent header. The obvious reading is “fix _MINIO_API_XFF_HEADER=off so it covers all three”, and that is what the first implementation did.

The case for widening was decent. Upstream’s PR title says “disable all X-Forwarded-For header handling”; its described scope is audit logs and IP-based access control, both of which are about not believing client-claimed addresses; the switch is undocumented, so its audience is small; and the failure direction is safe, since the fallback is the real TCP peer rather than an attacker-controlled value.

The case against turned out to be decisive. Widening it changes behaviour for a real population: operators whose proxy appends to X-Forwarded-For (polluted) but overwrites X-Real-IP (clean) may have discovered off as a way to get the correct address. Upstream’s TestXFFDisabled asserts exactly that behaviour — with the switch off and both headers present, X-Real-IP wins — so the behaviour is not merely incidental, it is pinned by a test. Those operators would have seen audit addresses silently flip from the real client to their proxy, and IpAddress conditions potentially start denying.

The reversal came from separating the goal from the mechanism. The goal was “a complete, enforceable way to turn this off exists”. Nothing required that the existing variable be the thing that provides it. Expressing it as MINIO_API_TRUSTED_PROXIES=none achieves the identical guarantee with zero effect on anyone who has not opted in.

Decision: leave _MINIO_API_XFF_HEADER exactly as upstream defined it. It gates X-Forwarded-For only, within whichever trust mode is in force. Upstream’s TestXFFDisabled is retained unmodified and still passes. The documentation now says plainly what the switch is not: it is a parsing switch, not a trust boundary, and a client refused one header simply sends another.

A secondary benefit: this collapses two interacting variables into one policy setting, so there is no longer a precedence rule ("off outranks the allow-list") for operators to learn and for us to get wrong.

Environment variable or config subsystem

Registering trusted_proxies as an api subsystem key would give mc admin config visibility, help text, and hot reload, matching how sts_trusted_proxies is done.

It would also introduce a window. Config subsystems load after the object layer initialises, so between process start and config application the trust policy would be empty — which under a “list is empty means trust everyone” reading is fail-open. A security boundary must not have a fail-open window. Separately, a trust boundary that can be changed at runtime is not obviously desirable.

Decision: environment variable. The cost is discoverability, and one bug described below.

Loopback is always trusted as a peer

The FTP and SFTP front-ends connect to the S3 layer over 127.0.0.1 and declare their session’s client with X-Forwarded-For (cmd/sftp-server-driver.go). An allow-list that does not exempt loopback attributes every FTP and SFTP request to the server itself.

The cost is that any process on the same host can forge. That is acceptable: an attacker who can open connections from localhost already has code execution on the host, and the threat model is lost well before this point. The FTP/SFTP regression, by contrast, would be certain and would affect everyone using those front-ends.

Decision: exempt loopback as a peer. Adversarial review then caught that the first implementation also treated loopback as a skippable chain entry, which is unnecessary for the FTP/SFTP case and actively harmful — see below. The two are now separate checks.

X-Forwarded-For versus X-Real-IP: genuinely unresolvable

Under the allow-listed mode, which header wins when both are present?

• A proxy that authors only X-Real-IP and relays the client’s X-Forwarded-For (some nginx configurations) → preferring X-Forwarded-For takes the forged value.
• A proxy that authors only X-Forwarded-For and relays the client’s X-Real-IP (AWS ALB) → preferring X-Real-IP takes the forged value.

Both are common, and the server cannot tell which situation it is in from the request. This is not an undecided question; it is undecidable without the operator telling us what their proxy authors.

Decision: prefer X-Forwarded-For. It is the only one of the two that carries a chain that can be checked against the allow-list, and this path decides access control rather than rate-limit bucketing, so the value that can be validated should win. It also keeps header precedence identical to the default mode, so switching modes does not silently change precedence as well.

This deliberately diverges from getSTSLDAPTrustedProxySourceIP, which prefers X-Real-IP. Two contradicting implementations of the same question in one codebase is a hazard in itself, so both sites now carry a comment naming the divergence and its reason, so that nobody “unifies” them without re-deciding. The operator documentation states the mitigation for both directions: strip whichever header your proxy does not author.

A broad allow-list is worse than no allow-list

This is the most counter-intuitive property and the one most likely to bite.

Entries on the list are skipped during the chain walk. So MINIO_API_TRUSTED_PROXIES=10.0.0.0/8, configured because the load balancer is 10.0.0.1, makes every client inside 10/8 skippable as well. A client at 10.5.5.5 sending X-Forwarded-For: 8.8.8.8 produces a chain of 8.8.8.8, 10.5.5.5; the walk steps over 10.5.5.5 as a “trusted hop” and returns 8.8.8.8.

A broad list therefore does not merely trust more peers — it lets those peers forge. nginx’s set_real_ip_from has the same property with real_ip_recursive.

There is no algorithmic fix: the list is doing double duty as “who may forward” and “whose address may be discarded”, and separating them would mean two lists to keep in sync. Decision: keep one list, and make the constraint prominent — a callout block in the operator documentation, and the rule stated in the code comment where the walk happens. Only /0 is rejected as a catch-all, and the documentation says explicitly that this is a guardrail rather than a proof, since 0.0.0.0/1,128.0.0.0/1 covers the same ground.

Multi-node forwarding

MinIO forwards requests between nodes for bucket-DNS routing, listing continuation, heal-by-token, batch jobs and pool decommissioning. The receiving node’s TCP peer is the forwarding node, not the client.

This is not an obscure path: a ListObjectsV2 continuation token carries the node index, so any client can cause its own request to be forwarded. Under none, that request is then evaluated with aws:SourceIp set to an internal node address — which an IpAddress condition allowing internal ranges would treat as a pass.

Decision: document it, and steer multi-node clusters to the allow-list. none cannot be corrected for this case, because it believes nothing by definition. Automatically seeding the cluster’s own addresses was considered and rejected: it needs DNS resolution at startup and re-resolution as node addresses change, which is more machinery and more failure modes than the explicit configuration it replaces.

Compatibility

The change was deliberately structured so that risk is not spread evenly across it. Every piece is either opt-in or dead code in the default configuration.

Evidence for the default path. unverifiedSourceIP is a verbatim copy of the original function body, including its quirks: the ", " separator, the fall-through when the left-most element is empty, and the acceptance of non-IP values such as _gazonk. An independent review verified behavioural parity against HEAD over 21 cases — empty X-Forwarded-For, a bare comma, a leading ", ", "," versus ", " separators, " , ", IPv4-mapped addresses, bracketed IPv6, non-IP junk, and all three Forwarded forms. Upstream’s TestGetSourceIP and TestXFFDisabled are both retained unmodified and pass.

A subtlety self-review caught. The new setting is read after MINIO_CONFIG_ENV_FILE is loaded, which is what makes it work in packaged deployments. The obvious tidiness move is to read _MINIO_API_XFF_HEADER in the same place — and that would have been a behaviour change, because upstream reads it at package initialisation, before environment files exist. An operator who wrote it into an environment file has it silently ignored today; picking it up would make an already-deployed setting suddenly start working, flipping their source addresses from the left-most X-Forwarded-For entry to X-Real-IP. The old switch therefore keeps upstream’s read timing along with upstream’s semantics, and a test pins that so nobody tidies it later. The quirk is documented instead: set it in the process environment if you want it honoured.

The defensive code that was not defending anything. Sharing the parser initially came with two extras: allow-list entries written in IPv4-mapped form were rewritten to the IPv4 prefix they denote, and the address being matched was unmapped and de-zoned. Both looked like corrections — an entry written ::ffff:192.168.1.10 is otherwise accepted and then matches nothing, which is a silent failure worth removing.

They were removed anyway, and the reason is worth recording. Both call paths reduce the address through net.ParseIP(...).String() before matching, and that already collapses ::ffff:10.0.0.1 to 10.0.0.1; a dual-stack listener reports an IPv4 peer in plain form regardless. So neither extra could be reached by a real request. Their only observable effect was on what the shared function meant for the LDAP STS allow-list that had been using it first — 18 differences that a test could see by calling the function directly and no deployment could.

Worse, one of them manufactured the fail-open described below: rewriting ::ffff:0:0/96 turned a /96 into 0.0.0.0/0. Deleting the rewrite removes the bug’s cause rather than ordering around it. What remains is pure code motion, verified identical to the previous implementation across every combination of 37 allow-list values and 21 peer addresses — zero parse differences, zero match differences. The wart it declined to fix (a mapped-form entry matches nothing) is the pre-existing behaviour, fails closed, and is now stated in the function’s own comment so the next person does not re-derive the same tempting fix.

The one residual risk worth naming. The default mode’s code path did change: there is now a switch and a function call in front of the original body. If that plumbing were wrong it would affect everyone, not just opt-in users. The parity testing above is why we believe it is not, but “verified equivalent over 21 cases” is a different claim from “provably identical”, and the honest version is the former.

What adversarial review found

An independent agent was tasked with breaking the first implementation. It found four real defects, all since fixed and covered by regression tests.

A silent fail-open, and the worst of the four. The trust policy was read in the package’s init(). But loadEnvVarsFromFiles() calls os.Setenv for everything in MINIO_CONFIG_ENV_FILE long afterwards — which is how MinIO is configured in essentially every packaged deployment. An operator putting MINIO_API_TRUSTED_PROXIES in /etc/default/minio would have got the historical trust-any-peer mode, with no error reported, and a malformed value would have been silently ignored rather than fatal. The policy is now applied in serverHandleEnvVars, which runs after the file load and before any listener.

Loopback skipped as a chain entry. Described above: the peer exemption was being reused as a hop exemption, which is a needless instance of the broad-list problem. Now two separate checks.

Two fail-closed correctness bugs. A zoned IPv6 peer (fe80::1%eth0) canonicalised to nothing, because net.ParseIP rejects zones — so such a peer could never be a trusted proxy. And an allow-list entry written in IPv4-mapped form (::ffff:192.168.1.10) was accepted at startup and then matched nothing at all, since netip.Prefix.Contains is false across differing bit widths.

Two further defects were found while writing the documentation rather than the code, which is its own small lesson:

Repeated header lines. Header.Get returns only the first header line. HAProxy’s option forwardfor adds a second X-Forwarded-For line rather than extending the first, so Get would hand back the client’s line and put the forged value right back where the right-to-left walk exists to avoid it. Now flattened across all lines with Header.Values.

The internal forwarder relayed client claims. internal/handlers/forwarder.go set X-Real-IP only when absent, so a client’s value passed between nodes unchanged. Under an allow-list that includes the cluster’s own nodes — the configuration we recommend — a client could thereby borrow a peer node’s authority. The forwarder now drops X-Real-IP and Forwarded when the incoming peer is not entitled to have set them. X-Forwarded-For needs no such handling, because Go’s ReverseProxy appends the true peer and the receiving node’s walk reaches that entry first.

A second round, after the rework

Reworking the setting warranted a second adversarial pass, which was worth running: differential testing found zero behavioural differences against HEAD across 4,745,520 source-IP resolutions and 345,600 forwarder rewrites, but it also found three more ways to fail open — one of them introduced by the first round’s own fix.

A catch-all smuggled in as an IPv4-mapped prefix. MINIO_API_TRUSTED_PROXIES=::ffff:0:0/96 is a /96 as written, so it passed the catch-all check; the rewrite that unmapped IPv4-mapped entries then turned it into 0.0.0.0/0, trusting every peer. The first fix moved the breadth check to the far side of the rewrite. The eventual fix deleted the rewrite, once it became clear it was unreachable by any real request — which removes the cause instead of guarding its output.

This is the one most worth dwelling on. The fail-open was manufactured by a fix for an unrelated fail-closed bug, and the fix for the fail-open was a reordering that left the manufacturing step in place. Two rounds of correction, both defensible, neither addressing the fact that the code should not have been there. Hardening changes deserve the same adversarial treatment as the code they harden, and “is this reachable at all?” belongs near the front of that treatment.

A deliberate value naming nobody. MINIO_API_TRUSTED_PROXIES="," parsed to an empty list and fell back to the permissive default. An empty unset variable must mean “default”, but a value the operator actually typed which names no proxy is a mistake, and answering it with trust-everyone is the one behaviour they cannot have wanted. It is now a startup error. Whitespace-only remains equivalent to unset, since that is what an empty shell variable expands to.

A remote value that could not be read. MinIO supports env:// indirection, where a variable’s value is fetched from a remote webhook. env.Get discards the error from that fetch and returns the empty string — which this code would have read as “unset”, reinstating trust-any-peer at exactly the moment the operator’s intent could not be determined. The setting is now read through env.LookupEnv so the error is surfaced and startup stops. This is a general hazard for any security-relevant setting read via env.Get, and worth remembering beyond this change.

A third pass, attacking from angles the first two shared

Both earlier passes attacked the resolver as a unit. Two things that neither could see:

Nothing had tested that the trust policy reaches a decision. Every test to that point checked what the resolver returned, and the one test at the policy layer only asserted that aws:SourceIp equalled the resolver’s output — in the default mode. So a version where the resolver was correct but the policy engine read something else would have passed everything. There is now a test that drives a forged X-Forwarded-For through getConditionValues into a real IpAddress evaluation under each mode: believed by default, ignored under none, ignored from an unlisted peer, and still honoured from the listed proxy. It passes, but it should have existed before the change was called done.

The allow-listed mode had a resource amplification the default mode does not. The chain was flattened into a slice before being walked, so a client behind a trusted proxy could turn the 1 MiB header allowance into roughly 33 MB of slice headers per request — around thirty-fold — plus a million-iteration walk. The default path never had this, because it uses strings.Index on the raw header. The walk now scans backwards over the header text in place, allocating nothing, and stops after 100 hops; real chains are a handful, the answer sits at the right-hand end, and running out of budget yields no address, which falls back to the peer. A test pins the zero-allocation property, because it is the kind of thing an innocent-looking refactor would undo.

Also corrected in this pass: the deployment contract was stated too narrowly. “The proxy must overwrite whichever headers it sets” misses the case that actually bites — a proxy that correctly authors only X-Real-IP or only Forwarded still relays the client’s X-Forwarded-For, and that is the header read first. The general rule, now stated as such, is to strip every source-address header the proxy does not itself write.

One reported finding was reviewed and not treated as a defect: the catch-all guard rejects /0 and nothing else, so 0.0.0.0/1,128.0.0.0/1 covers the same ground and is accepted. Tightening it would mean rejecting broad-but-not-/0 prefixes in the parser now shared with the LDAP allow-list, newly failing configurations that are valid today, to defend against a value no deployment realistically holds. The documentation states plainly that the check is a guardrail rather than a proof, and the callout about naming proxies instead of subnets is where the real defence lives.

Recommendations

By topology:

• Proxy you control, API port genuinely unreachable otherwise. Set nothing. But verify whether your proxy overwrites or appends: if the config says $proxy_add_x_forwarded_for, you are forgeable today. Switch to $remote_addr, or adopt the allow-list.
• Direct exposure, no proxy. MINIO_API_TRUSTED_PROXIES=none.
• Kubernetes or Pigsty, Ingress plus reachable Service. The allow-list, containing the proxy addresses and the MinIO node addresses. This is the configuration that makes an IpAddress condition mean anything.
• Any multi-node cluster. The allow-list with node addresses, not none.

Two rules apply to every allow-list deployment. Name proxies, not the subnet they occupy. And strip at the edge every source-address header your proxy does not itself write — listing a peer means believing all three headers from it, they are consulted in a fixed order, and a header your proxy leaves alone is entirely the client’s. A proxy that correctly authors only X-Real-IP, or only Forwarded, still relays the client’s X-Forwarded-For, which is read first.

What was not done

• Automatic seeding of cluster node addresses. Feasible via EndpointServerPools, but it needs DNS resolution and re-resolution on address changes. Explicit configuration was judged the smaller risk.
• Registration as an api config key. See the fail-open window above. Revisitable if observability turns out to matter more than the startup guarantee.
• ExistingObjectTag/*. The sibling defect from the condition-value hardening — it carries the request’s own tags rather than the object’s stored tags — remains open by decision, and is unaffected by any of this.

Verdict on severity

Default behaviour matches upstream, and MinIO has never documented aws:SourceIp as trustworthy on a directly-reachable deployment, so “the default is unsafe” is closer to a documentation defect than a vulnerability. One thing is squarely a defect, though: an operator set an explicit security switch and it did not do what its name and its only public description implied, and it failed silently. That is worth a record, scoped to the incompleteness of upstream’s _MINIO_API_XFF_HEADER rather than to anything the fork introduced.

minio/minio is archived, so there is no upstream to coordinate with — the same position as CVE-2026-42600.

14 - Sorted Is Not Increasing: How One Duplicate Part Number Doubled an Object

Status: Fixed on the local pgsty/minio branch as 22c1e41fd, unreleased Classification: Data correctness, not a vulnerability — see Why this is not a CVE Affected scope: All backends, any authenticated S3 client, on its own upload Tracking: pgsty/minio issue #49

One section of this article describes an unfixed process-level panic in a neighbouring code path. Hold publication until that is fixed and released.

Conclusions first

• sort.SliceIsSorted with a < predicate does not test strict increase. It tests the absence of an inversion. Equal neighbours contain no inversion, so [1,1] was accepted.
• Upload one 5 MiB part, complete with [1,1], and the server returns HTTP 200 and a 10 MiB object. The upload is then consumed: a corrected retry gets NoSuchUpload. The client cannot recover.
• Inherited from upstream, and old. The check has had this shape since 2016-08. Two refactors — 2017 and 2023 — rewrote it faithfully, because each preserved the predicate, and the predicate was never the problem.
• The fix is one loop at the handler layer. The object layer is left undefended by decision, and that IOU is written down here rather than left implicit.
• Three independent reviews found no defect in the fix. What they found was a comment that misstated why a neighbouring guard exists — and, through that comment, an unrelated node-level panic.

The verb, not the predicate

The code, as inherited:

if !sort.SliceIsSorted(complMultipartUpload.Parts, func(i, j int) bool {
	return complMultipartUpload.Parts[i].PartNumber < complMultipartUpload.Parts[j].PartNumber
}) {
	writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidPartOrder), r.URL)
	return
}

It reads as “reject unless the part numbers strictly increase.” It does not do that. IsSorted evaluates the predicate in the reversed direction only — for each neighbouring pair it asks less(i, i-1), i.e. “is this element smaller than the one before it,” and reports unsorted the moment one such inversion appears. For a pair of equal elements that question is false. No inversion, therefore sorted.

The consequence is worth stating precisely, because it is what makes the misuse survive review: no strict predicate can make IsSorted reject duplicates. The only spelling that works is the non-strict one — passing <= as the less function, so that equal neighbours register as an inversion. To ask for strictly increasing you would have to write the operator that reads as not strict. Every reviewer who checked that the predicate said < was checking the right character in the wrong function.

Our replacement drops IsSorted rather than trying to spell it correctly:

for i := 1; i < len(complMultipartUpload.Parts); i++ {
	if complMultipartUpload.Parts[i-1].PartNumber >= complMultipartUpload.Parts[i].PartNumber {
		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidPartOrder), r.URL)
		return
	}
}

The rejection-set delta is exactly one class: lists containing an adjacent equal pair. Everything previously rejected is still rejected; everything previously accepted, except duplicates, is still accepted. Non-adjacent duplicates come along for free — a strictly increasing sequence is globally distinct, so [1,2,1] and [1,3,2,3] are caught by the inversion they are forced to contain.

Two refactors preserved it faithfully

The archaeology is the most transferable part of this incident.

Both refactors were correct as refactors: they preserved behaviour exactly, which is what a refactor is supposed to do. The 2023 commit was a repository-wide cleanup with no bearing on multipart semantics at all. It carried the < across unchanged, and the < was never wrong — CompletedParts.Less needs < to be a valid sort.Interface.

The defect lived in the relationship between the predicate and the function it was handed to, and a refactor that moves the predicate cannot see that relationship. A decade, three shapes, one behaviour: an ordering check that answers a question adjacent to the one it appears to answer.

What it actually did

Measured against both erasure backends, through the real signed HTTP handler:

The ETag suffix is the part count the server believes it assembled. There is no internal disagreement to detect: the metadata, the size, and the ETag are all mutually consistent and all wrong. The object simply is not what was uploaded.

Two properties make this worse than a bad error code.

The upload is consumed. Assembly runs to completion and cleans up the multipart upload, so the corrected retry returns NoSuchUpload. A client that notices the wrong size cannot fix it by resending the right list; it has to start the whole upload over, if it still has the data.

It is reachable by accident. No adversary is required. Any client that appends a part to its completion list twice — a plausible bug in a resumable-upload wrapper, a retry path, or a list built by concatenation — silently gets a doubled part instead of a 400.

Why this is not a CVE

It belongs in this chronicle because it is a silent server-side correctness failure, and this is where we keep those. It is not a vulnerability, and we are not going to inflate it into one.

The request must carry the caller’s own credentials, address the caller’s own upload, and the damaged object is the caller’s own. There is no cross-tenant effect, no privilege change, no disclosure, and no path to another account’s data. What breaks is the guarantee that a completed multipart object equals the bytes you uploaded — serious, but a correctness guarantee, not an access-control boundary.

The entries around this one in this chronicle are authentication bypasses and path traversals. Filing this beside them under the same label would make every label in the table mean less.

The boundary decision, and what it costs

The object layer has no duplicate defence at all. erasureObjects.CompleteMultipartUpload sizes its output slice to the request (cmd/erasure-multipart.go:1249) and then resolves each requested part number against current metadata (:1255). The same number resolves twice, writes two identical ObjectPartInfo entries, and adds its size twice. AddObjectPart does deduplicate by part number, but it deduplicates the metadata slice, not the request. The 5 MiB minimum-size rule cannot help either, because the duplicated part is individually legal.

We fixed the handler and left that alone. The reasoning:

• It is the only entrance where a client-controlled list exists. The other four callers — batch, restore, decommission, rebalance — build their lists server-side from oi.Parts or 1..n, and are strictly increasing by construction.
• The required output is an S3 error code, which is an API-layer concern. The object layer’s error vocabulary maps to a different code, so intercepting lower would hand clients a less accurate diagnosis.
• Minimality. This fork ships narrow fixes, and a change in the assembly loop is not narrow.

The cost, recorded rather than implied: the uniqueness invariant now has exactly one enforcement point, and nothing enforces the enforcement. No compiler error and no test failure will greet the person who adds a fifth caller to the object layer; they will get a silently corrupted object. That is the same species of IOU the previous article recorded about getVolDir, and it is written down for the same reason: an unrecorded deliberate omission is indistinguishable from an oversight six months later.

What we deliberately did not add

Part numbers need not start at 1 and need not be consecutive. [1,3], [5,9] and [3] are all legal S3, and all still complete successfully.

This matters more than it sounds. “Also require the list to start at part 1” is a one-line addition that looks like tightening, would pass a casual review, and would break legal clients — anything that abandons a part after a failed upload and completes with what it has. The temptation is real precisely because the fix next door is about validating the same list.

So two test cases exist for no purpose other than to make that change fail. We verified they do their job by injecting the constraint and confirming that exactly those two cases went red and nothing else did. A guard rail nobody has fired once is a guess.

The one behaviour change we did not intend

A 14-input differential against the pre-fix build turned up exactly one behavioural change beyond duplicate rejection: [0,0] and [-1,-1] — lists that are both duplicated and out of range — moved from InvalidPart to InvalidPartOrder. Both are HTTP 400.

We accepted it, on the principle that a format error should outrank a state error: an ordering violation is decidable without reading any storage, while part existence is not. It also only affects requests that were going to fail regardless, so no client that previously succeeded can now fail.

On S3 fidelity itself we are making a documented inference, not a measurement. AWS defines InvalidPartOrder as the parts list not being in ascending order, and documents that part numbers may be non-consecutive; duplicates are not ascending. We did not verify this against a live AWS endpoint, and two independent reviewers reached the same conclusion by the same documentary route, which is agreement, not evidence.

Falsification, and a comment that was wrong

Two mutation experiments, in the discipline the previous article argued for — a test you have never watched fail is not yet a test.

Inject “must start at part 1.” Exactly the two gap cases went red; the four lists starting at 1 stayed green. The guard rail is targeted, not incidental.

Delete the neighbouring len(Parts) == 0 guard. The expected result was that an empty completion would produce some wrong-but-orderly error. The actual result was that the process panicked: the empty list reaches a storage decorator that indexes element zero of the part-path slice without a length check, on a goroutine that no recover can reach. The S3 face is masked by that one guard line, which has been there since 2022 and is not documented as load-bearing. It is tracked separately as an unfixed node-level defect, which is why this article is held.

And the part worth publishing at our own expense: the comment we wrote about that guard was wrong. It said dropping the length check would let an empty completion succeed — the opposite direction of the truth, and specifically the direction that understates danger. It was caught in review and corrected before the commit. A comment that misstates why a check exists is exactly how the check gets deleted three years later by someone tidying up.

Three acceptances, zero blocking findings

The change went through three independent gates before commit:

Stated plainly, because the honest version is less flattering than the table: none of the three found a defect in the fix. What review produced was the corrected comment and, through the mutation it prompted, the discovery of the unrelated panic. That is still a good return, but it is not the same as catching a bug in the patch, and the record should say which one happened.

The rebuilt-red-state detail is the one worth copying. A reviewer who reruns the author’s tests is checking the author’s arithmetic; a reviewer who reconstructs the broken state independently is checking the author’s claim.

Declined, and left open

Declined, deliberately:

• Two test additions — completing onto a pre-existing object, and giving each part distinct content so ordering is verified rather than just total size. Both are real improvements. Both were declined under a standing rule that this fork ships correctness and security fixes rather than test expansion, and the core invariant is already pinned by “the rejected request left no object and the upload still works.”
• XML root element name is not validated. A document with the wrong root but correct <Part> children is accepted. This is not a bypass — the same list still goes through the same check — it is pre-existing, and tightening it risks breaking real SDKs over namespace handling. Recorded, not fixed.

Left open, none of it in a released build as of 2026-08-03:

• Object-layer defence in depth for part uniqueness (see above).
• The empty-list panic in the storage decorator, tracked as a node-level defect.
• XML strictness, including <PartNumber>abc</PartNumber> returning 500 where 400 MalformedXML is correct.

The concurrent checksum work on completion (#46, #48, #50) was fenced off from this change entirely and shares no code with it.

Closing

The predicate was strict. The verb was not. A decade of review read the predicate — including the two commits that rewrote the line.

If only one sentence survives: check what the function does with the comparison, not just what the comparison says, and when you decide to leave the layer underneath undefended, write it down where the next person will trip over it, rather than trusting that they will re-derive your reasoning.